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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 Fixed sending multiple support email verification emails #15044

Merged

Conversation

SimonBackx
Copy link
Contributor

refs https://github.com/TryGhost/Team/issues/1686

  • When the settings are updated with the members_support_address key present, it would always send a verification email
  • Root cause is that the service failed to check if the email was changed or not. Due to a bug it always thought the email was changed, triggering the verification flow.
  • The admin app will always send all the settings keys when changing some other value. This causes a lot of email verification emails.
  • Added tests and email count checks in tests

refs https://github.com/TryGhost/Team/issues/1686

- When the settings are updated with the `members_support_address` key present, it would always send a verification email
- Root cause is that the service failed to check if the email was changed or not. Due to a bug it always thought the email was changed, triggering the verification flow.
- The admin app will always send all the settings keys when changing some other value. This causes a lot of email verification emails.
- Added tests and email count checks in tests
@codecov
Copy link

codecov bot commented Jul 19, 2022

Codecov Report

Merging #15044 (73e66d7) into main (d1f2040) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main   #15044   +/-   ##
=======================================
  Coverage   61.48%   61.48%           
=======================================
  Files         597      597           
  Lines       47944    47944           
  Branches     4325     4325           
=======================================
  Hits        29476    29476           
  Misses      18419    18419           
  Partials       49       49           
Impacted Files Coverage 螖
...server/services/settings/settings-bread-service.js 73.93% <100.00%> (酶)

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update d1f2040...73e66d7. Read the comment docs.

@SimonBackx SimonBackx marked this pull request as ready for review July 19, 2022 11:36
Copy link
Contributor

@matthanley matthanley left a comment

Choose a reason for hiding this comment

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

Tested locally, lgtm 馃憤馃徏

@SimonBackx SimonBackx merged commit 2e1a756 into TryGhost:main Jul 19, 2022
@SimonBackx SimonBackx deleted the fix-duplicate-support-email-verifications branch July 19, 2022 13:13
daniellockyer pushed a commit that referenced this pull request Jul 19, 2022
refs TryGhost/Product#1686

- When the settings are updated with the `members_support_address` key present, it would always send a verification email
- Root cause is that the service failed to check if the email was changed or not. Due to a bug it always thought the email was changed, triggering the verification flow.
- The admin app will always send all the settings keys when changing some other value. This causes a lot of email verification emails.
- Added tests and email count checks in tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants