Skip to content

Conversation

@jrcastro2
Copy link

email = (
recipient.data.get("email")
or recipient.data.get("email_hidden")
or recipient.data.get("name")
Copy link
Contributor

Choose a reason for hiding this comment

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

this part will be only true for CERN, no? we can't always assume group name == email can we? also where do we get the email domain from?

Copy link
Author

Choose a reason for hiding this comment

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

This would be true to anyone using groups with emails. That's right we cannot assume that the group name will always be the email. I guess it makes sense that we do add the email field to the group but be aware that we will need to also update the the group sync in CDS. Please confirm that we want to update the data model for the groups and I will do it.

Copy link
Contributor

Choose a reason for hiding this comment

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

ping @ntarocco @slint @zzacharo for opinions

Copy link
Contributor

Choose a reason for hiding this comment

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

I am wondering, how did you test that email is sent for CDS? sorry I am a bit lost on this assumption that e-group name == e-group email address

Copy link

Choose a reason for hiding this comment

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

Looks like that in both Google Workspace and MS the name and the e-mail are the same.
For simplicity, we could start with that assumption.

Copy link
Contributor

Choose a reason for hiding this comment

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

if we start with this assumption, we are still missing the part which makes it work for CERN use case (adding @ is not present to properly send the email)

content = self.render_template(notification, recipient)

# Groups use the name as email if no email is set
email = (
Copy link

Choose a reason for hiding this comment

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

Could it be None in any possible way? Should it be handled?

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.

1 - Enhance e-mail notifications

3 participants