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

feat(files_sharing): add send mail toggle #50064

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nfebe
Copy link
Contributor

@nfebe nfebe commented Jan 7, 2025

Summary

For new internal shares, show the user a toggle for them configure if they want to send the share receiver a notification mail.

This is subject to whether the configured user has an email send up

Screenshots

Inactive Active
inactive-sendmail active-sendmail

TODO

  • Ensure no failure if activating toggle does not lead to errors for users without configured emails

@nfebe nfebe force-pushed the fix/49954/add-send-mail-toggle branch from f178f55 to 4372d7a Compare January 7, 2025 11:02
@nfebe nfebe marked this pull request as ready for review January 7, 2025 11:02
@skjnldsv
Copy link
Member

skjnldsv commented Jan 7, 2025

Why not put it to true straight away?

@nfebe
Copy link
Contributor Author

nfebe commented Jan 7, 2025

Why not put it to true straight away?

This sounds okay, but should not require another review cycle?

cc: @skjnldsv

For new internal shares, show the user a toggle for them configure if they want
to send the share receiver a notification mail.

This is subject to wether the configured user has an email send up

Signed-off-by: nfebe <[email protected]>
@nfebe nfebe force-pushed the fix/49954/add-send-mail-toggle branch from 4372d7a to c43bbd2 Compare January 7, 2025 16:08
Copy link
Contributor Author

@nfebe nfebe left a comment

Choose a reason for hiding this comment

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

Addressed.

@@ -329,8 +335,8 @@ export default {
data() {
return {
writeNoteToRecipientIsChecked: false,
sendNotificationMailOnCreation: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
sendNotificationMailOnCreation: true,
sendNotificationMailOnCreation: true,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🏗️ In progress
Development

Successfully merging this pull request may close these issues.

[Bug]: New Share to Local User Does Not Trigger Email Notification (sharebyemail)
2 participants