fix(emailNotification): Add Multiple Users | Invite email didnt receive by User(s)

[tfjanjua]

Description

User(s) didnt receive invite emails when invited via Add Multiple Users. Users has been created but invite email does not sent out to users.

Why

Recently bulk uploaded users were not getting email notifications

Issue

Ref: #920

Checklist

Please delete options that are not relevant.

• I have followed the contributing guidelines
• I have performed a self-review of my own code
• I have successfully tested my changes locally
• I have added tests that prove my changes work
• I have checked that new and existing tests pass locally with my changes
• I have commented my code, particularly in hard-to-understand areas

[Phil91]

My suggestion would be to move the mail process creation into the call of _userProvisioningServiceCreateOwnCompanyIdpUsersAsync and adjust the usages accordingly.

[tfjanjua]

My suggestion would be to move the mail process creation into the call of _userProvisioningServiceCreateOwnCompanyIdpUsersAsync and adjust the usages accordingly.

@Phil91 thanks for your suggestion, I see _userProvisioningService.CreateOwnCompanyIdpUsersAsync method and I found that we are calling this method from 8 different places (except test cases, otherwise its 24 places) and a few place has different email template and params implementation, wherever this method is been called.

e.g. in UserBusinessLogic.cs and UserUploadBusinessLogic.cs files, there are mostly 3 email templates have been used after calling the _userProvisioningService.CreateOwnCompanyIdpUsersAsync method from different places:

1. NewUserTemplate
2. NewUserPasswordTemplate
3. NewUserExternalIdpTemplate

and in InvitationProcessService.cs file, there are 2 different email templates have been used after calling the _userProvisioningService.CreateOwnCompanyIdpUsersAsync method:

1. RegistrationTemplate
2. PasswordForRegistrationTemplate

and in RegistrationBusinessLogic.cs file, there are 2 different email templates have been used after calling the _userProvisioningService.CreateOwnCompanyIdpUsersAsync method:

1. invite or inviteWithMessage
2. password

So I feel like, if I move the mail process creation for UserBusinessLogic.cs and UserUploadBusinessLogic.cs files into the call of _userProvisioningService.CreateOwnCompanyIdpUsersAsync so, this would be adjustable because here we have IsSharedIdp bit to check and make decision upon email templates but I am wondering how the identification of email templates (params are more or less same) would work for InvitationProcessService.cs and RegistrationBusinessLogic.cs files? would it possible to add another param in _userProvisioningService.CreateOwnCompanyIdpUsersAsync method to check from where this method is being called and make decision of email templates? or am I missing any already placed variable with different values?

[Phil91]

My suggestion would be to move the mail process creation into the call of _userProvisioningServiceCreateOwnCompanyIdpUsersAsync and adjust the usages accordingly.

@Phil91 thanks for your suggestion, I see _userProvisioningService.CreateOwnCompanyIdpUsersAsync method and I found that we are calling this method from 8 different places (except test cases, otherwise its 24 places) and a few place has different email template and params implementation, wherever this method is been called.

e.g. in UserBusinessLogic.cs and UserUploadBusinessLogic.cs files, there are mostly 3 email templates have been used after calling the _userProvisioningService.CreateOwnCompanyIdpUsersAsync method from different places:

1. NewUserTemplate
2. NewUserPasswordTemplate
3. NewUserExternalIdpTemplate

and in InvitationProcessService.cs file, there are 2 different email templates have been used after calling the _userProvisioningService.CreateOwnCompanyIdpUsersAsync method:

1. RegistrationTemplate
2. PasswordForRegistrationTemplate

and in RegistrationBusinessLogic.cs file, there are 2 different email templates have been used after calling the _userProvisioningService.CreateOwnCompanyIdpUsersAsync method:

1. invite or inviteWithMessage
2. password

So I feel like, if I move the mail process creation for UserBusinessLogic.cs and UserUploadBusinessLogic.cs files into the call of _userProvisioningService.CreateOwnCompanyIdpUsersAsync so, this would be adjustable because here we have IsSharedIdp bit to check and make decision upon email templates but I am wondering how the identification of email templates (params are more or less same) would work for InvitationProcessService.cs and RegistrationBusinessLogic.cs files? would it possible to add another param in _userProvisioningService.CreateOwnCompanyIdpUsersAsync method to check from where this method is being called and make decision of email templates? or am I missing any already placed variable with different values?

@tfjanjua sorry for the late response, didn't saw the additional usages. I like the idea, instead of the parameter to define where the call came from I'd suggest to just add a parameter to pass an enumerable of the mail templates?

[ntruchsess]

I'm not sure this might be causing threading issues. I need to check what the specification says about synchronization and visibility of changes done from an inner Task writing to some outer scoped address that is not defined as AtomicXXX (could be a no-issue, but I'm not 100% sure, I did not yet find information about whether anything else than Task.Result that is not explicitly synchronized and being written to within the Tasks execution is guaranteed to be visible outside)
Also this relies on CreateOwnCompanyIdpUserAsync iterating userCreationInfos sequentially. If someone decides to add parallelism it would break this code.

[ntruchsess]

ok, I just notice this new method is a copy of the existing method CreateOwnCompanyIdpUsersWithEmailAsync with email-generation adjusted to the SharedIdp-usecase. I'll do some reusable refactoring to accomplish this in a less fragile way.

[ntruchsess]

I did refactor this adding a new onSuccess parameter that allows to pass a lambda that is going to be executed after each user-creation. Any db-changes applied within that lambda are being saved within the same transaction as the creation of the respective user now.
(I had to do a force-push after rebasing on latest main to resolve the conflicts)

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud