Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix group mentions in notifications #8598

Merged
merged 3 commits into from Dec 16, 2021
Merged

Fix group mentions in notifications #8598

merged 3 commits into from Dec 16, 2021

Conversation

quinHD
Copy link
Contributor

@quinHD quinHD commented Dec 10, 2021

🎩 What? Why?

This PR display fix the group mention in notifications. It's deeply explained in the issue #8485 .

πŸ“Œ Related Issues

Link your PR to an issue

Testing

Acceptance criteria
  • Given that I'm a participant with a new user group mention notification,
    When I go to my notifications page
    Then I can see which group I was mentioned.
Step by Step

You need a user_group_mentioned notification. First of all, you have to belong to a group. To do that, log in with an alternative user and go to /profiles/<user_nickname>/groups. There, go to a group you administrate and invite your original user.
Then, log in with your original user an accept the group invitation from that you'll find in /profiles/<user_nickname>/groups.
Once you belong to a group, you are ready to receive the notification.

From the alternative user, create a comment in a proposal, in that comment, you have to mention the group the original user belongs to.
After that, go to /notifications with the original user logged and there, you''ll see a notification with the group mentioned.

Screenshots

staging example

πŸ“‹ 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
example

Staging:

staging example

β™₯️ Thank you!

@quinHD quinHD linked an issue Dec 10, 2021 that may be closed by this pull request
1 task
@andreslucena andreslucena added module: core contract: online-meetings Barcelona City Council contract labels Dec 10, 2021
@quinHD quinHD changed the title Fix group mention in notifications Add group mention in notifications Dec 10, 2021
@quinHD quinHD marked this pull request as ready for review December 13, 2021 08:41
@quinHD
Copy link
Contributor Author

quinHD commented Dec 13, 2021

@decidim/product This feature is ready and uploaded in staging.

@andreslucena andreslucena added contract: PWA Barcelona City Council contract and removed contract: online-meetings Barcelona City Council contract labels Dec 13, 2021
@quinHD quinHD changed the base branch from develop to feature/8484 December 14, 2021 12:32
ferblape
ferblape previously approved these changes Dec 15, 2021
@quinHD quinHD changed the base branch from feature/8484 to develop December 15, 2021 09:44
@quinHD quinHD dismissed ferblape’s stale review December 15, 2021 09:44

The base branch was changed.

@andreslucena andreslucena changed the title Add group mention in notifications Fix group mentions in notifications Dec 15, 2021
@andreslucena
Copy link
Member

While reviewing this issue, I found a couple of things:

  1. This was initially implemented in Be able to mention groupsΒ #5763
  2. There was a reported bug by me a couple of weeks ago with the exception that we have now in develop in Exception when mentioning UserGroups (stack level too deep)Β #8459. I found it in Metadecidim's Sentry, but I actually forgot about it πŸ˜… . This PR will fix that also

Copy link
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a couple of suggestions and one general comment, can you check it please @quinHD?

decidim-core/lib/decidim/events/user_group_event.rb Outdated Show resolved Hide resolved
decidim-core/lib/decidim/core/test/factories.rb Outdated Show resolved Hide resolved
Copy link
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ‘πŸ½

@andreslucena andreslucena merged commit 45f3bce into develop Dec 16, 2021
@andreslucena andreslucena deleted the feature/8485 branch December 16, 2021 12:43
@andreslucena andreslucena added the type: fix PRs that implement a fix for a bug label Dec 22, 2021
@alecslupu alecslupu added this to the 0.26.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contract: PWA Barcelona City Council contract 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.

Fix groups mentions in Notifications
4 participants