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

[#169703944] check isEmailValidated / isEmailEnabled flags before sending email #32

Merged
merged 1 commit into from
Mar 8, 2020

Conversation

gunzip
Copy link
Contributor

@gunzip gunzip commented Mar 8, 2020

Since not every user profile stored into CosmosDB has email flags defined (so they are neither true nor false), we use strict equality and assume isEmailEnabled = true and isEmailValidated = true when they're undefined.

@BurnedMarshal
Copy link
Contributor

Codecov Report

Merging #32 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #32   +/-   ##
=======================================
  Coverage   83.97%   83.97%           
=======================================
  Files           6        6           
  Lines         337      337           
  Branches       43       43           
=======================================
  Hits          283      283           
  Misses         53       53           
  Partials        1        1           

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 9331cfa...305c1d0. Read the comment docs.

@@ -154,8 +154,12 @@ export const getCreateNotificationActivityHandler = (

// whether email notifications are enabled in this user profile - this is
// true by default, it's false only for users that have isEmailEnabled = false
// in their profile
const isEmailEnabledInProfile = profile.isEmailEnabled;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

false when undefined

// in their profile
const isEmailEnabledInProfile = profile.isEmailEnabled;
// in their profile. We assume it's true when not defined in user's profile.
const isEmailEnabledInProfile = profile.isEmailEnabled !== false;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

true when undefined

@gunzip gunzip changed the title [#169703944] fix default for undefined email flags [#169703944] check isEmailValidated / isEmailEnabled flags before sending email Mar 8, 2020
@gunzip gunzip requested a review from Undermaken March 8, 2020 20:48

// Check if the email in the user profile is validated.
// we assume it's true when not defined in user's profile.
const isEmailValidatedInProfile = profile.isEmailValidated !== false;
Copy link
Contributor

Choose a reason for hiding this comment

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

@gunzip Could you confirm the field isEmailValidated is set to false on CIE first authentication?
Because this assumption is valid only for SPID users

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this fix regards existing (SPID) users that never had this field set up (so it's undefined in their profile)

@gunzip gunzip merged commit 9c33151 into master Mar 8, 2020
@gunzip gunzip deleted the 169703944-defaut-boolean-flags branch March 8, 2020 21:01
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.

3 participants