Skip to content

Fix performance issue in notification settings page#8155

Merged
leio10 merged 5 commits intodevelopfrom
fix/improve-notification-settings-queries
Jul 20, 2021
Merged

Fix performance issue in notification settings page#8155
leio10 merged 5 commits intodevelopfrom
fix/improve-notification-settings-queries

Conversation

@leio10
Copy link
Copy Markdown
Contributor

@leio10 leio10 commented Jun 24, 2021

🎩 What? Why?

If you visit https://www.decidim.barcelona/notifications_settings with a non admin user you will get a server error page. This occurs because the page needs to know if the current user is a moderator or not to show an additional setting, and that is done browsing all the participatory spaces and checking if the user is in the moderators list. This was implemented in #7328.

This PR adds to all participatory spaces the ability to ask for all the moderators for an organization. With that, the queries needed to know if a user is a moderator or not decreases dramatically.

📌 Related Issues

Testing

Describe the best way to test or validate your PR.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

📷 Screenshots

Please add screenshots of the changes you're proposing
Description

♥️ Thank you!

@leio10 leio10 force-pushed the fix/improve-notification-settings-queries branch from 09f7883 to ffd05d0 Compare June 24, 2021 11:15
@leio10 leio10 added module: core type: fix PRs that implement a fix for a bug labels Jun 24, 2021
@leio10 leio10 marked this pull request as draft July 2, 2021 09:06
@leio10 leio10 marked this pull request as ready for review July 20, 2021 10:52
@leio10 leio10 force-pushed the fix/improve-notification-settings-queries branch 2 times, most recently from db6438b to 72dc343 Compare July 20, 2021 11:11
@leio10 leio10 merged commit 1787935 into develop Jul 20, 2021
@leio10 leio10 deleted the fix/improve-notification-settings-queries branch July 20, 2021 14:24
entantoencuanto added a commit that referenced this pull request Jul 26, 2021
* develop: (32 commits)
  Remove obsolete rake webpack task (#8237)
  Active storage migrations service (#7902)
  Fix content type delegation to blank attachments (#8230)
  Evote bug fixing (#8220)
  Fix the proposal data migration for proposals without authors or organization (#8015)
  Bump addressable version because security issues (#8229)
  Online meetings iframe visibility with time (#8097)
  Meetings iframe and iframe URL (#8096)
  Remove flaky test on meetings (#8226)
  Fix broken tests after problematic PRs (#8224)
  Apply permissions system to comments (#8035)
  Set current_component as commentable when commentable is a participatory space (#8189)
  Fix don't require inactive authorization handlers (#8122)
  Improve metrics calculations performance (#8215)
  Fix performance issue in notification settings page (#8155)
  Active storage migration (#7598)
  Update manual installation guide in documentation (#8217)
  Load JS configuration in elections focus mode layout (#8213)
  Fix user activity pagination when there are hidden items (#8202)
  Make it possible to define SCSS settings overrides from modules (#8198)
  ...
roxanaopr pushed a commit to i-need-another-coffee/decidim that referenced this pull request Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in-review module: core type: fix PRs that implement a fix for a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant