Skip to content

Fix for sending welcome emails for new participants#10991

Merged
alecslupu merged 2 commits intodevelopfrom
fix/force-welcome-emails
Jul 2, 2023
Merged

Fix for sending welcome emails for new participants#10991
alecslupu merged 2 commits intodevelopfrom
fix/force-welcome-emails

Conversation

@andreslucena
Copy link
Copy Markdown
Member

🎩 What? Why?

Reviewing old PRs, I found that the welcome email feature wasn't working with the introduction of digest emails (see #8486). @arusa explained it really well in #10470:

10 Months ago there was a check added, that only allows notification mails to be sent to users with notifications_sending_frequency == "real_time"

The problem with that is, that in decidim 0.27 new users don't get any welcome notification mails anymore, because new users by default have notifications_sending_frequency set as "daily".

I just commented out the line that breaks welcome mails for my decidim installation.

A better fix would probably be to make an exception for the event "decidim.events.core.welcome_notification".

Then @ahukkanen explained how to approach this exception change in a comment, and as its a PR that's without much movement, it's an ongoing bug on the last release, and we already have the code, I went ahead and do it 😄

Thanks both of you for detecting and telling how to fix this! 👏🏽 👏🏽

📌 Related Issues

Testing

  1. Create a new user
  2. Confirm their account on /letter_opener
  3. See the welcome email

📷 Screenshots

Screenshot of the welcome email in letter opener

♥️ Thank you!

@alecslupu alecslupu added type: fix PRs that implement a fix for a bug and removed type: bug labels Jun 12, 2023
@alecslupu
Copy link
Copy Markdown
Contributor

@andreslucena I changed the label from bug to fix :) I guess you want to introduce a bug fix :D

@andreslucena
Copy link
Copy Markdown
Member Author

I guess you want to introduce a bug fix :D

Yes, of course!! I went too fast and in automatic pilot 😅

@andreslucena andreslucena requested a review from a team June 14, 2023 04:43
Copy link
Copy Markdown
Contributor

@alecslupu alecslupu left a comment

Choose a reason for hiding this comment

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

LGTM.

Tested, and with this patch the welcome notification is being sent.

@alecslupu alecslupu merged commit e2e2424 into develop Jul 2, 2023
@alecslupu alecslupu deleted the fix/force-welcome-emails branch July 2, 2023 11:31
entantoencuanto added a commit that referenced this pull request Jul 7, 2023
* feature/redesign:
  Redesign: replace cells with redesigned versions if present and update references (#11123)
  Remove duplicated constant
  Simplify logic
  Enable specs
  Fix leaflet
  fix failing specs
  Fix issues with overriding maps and loading Leaflet (#11105)
  Update decidim-proposals/app/views/decidim/proposals/proposals/new.html.erb
  Update decidim-comments/lib/decidim/comments/comments_helper.rb
  Update decidim-assemblies/spec/system/filter_assemblies_spec.rb
  Update decidim-assemblies/spec/system/filter_assemblies_spec.rb
  Document how to work locally with Elections/Votings (#10870)
  Fix Admin dashboard disappear if you are in Trustee Zone (#11111)
  Fix Shakapacker upgrade does not work for existing instances (#10814)
  Fix for sending welcome emails for new participants (#10991)
  Fix seeded trustees (#10964)
andreslucena added a commit that referenced this pull request Jul 18, 2023
* Fix for sending welcome emails for new participants

* Refactor spec to shared examples
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

2 participants