-
Notifications
You must be signed in to change notification settings - Fork 39
'From' name complete changes 5-8 #5747
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
Open
kr8n3r
wants to merge
10
commits into
main
Choose a base branch
from
from-name-changes
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8c4c3e1 to
6166584
Compare
35202fe to
378206a
Compare
Trello ticket: https://trello.com/c/jwWpGx0o/1355-from-name-5-make-from-and-reply-to-settings-options-always-visible-if-email-is-set-to-off The email from name and reply-to address row should always be visible. The other email rows should behave like they currently do and only be visible if a service can send emails.
Account for the change of making "email from' and 'reply to' settings visible even if email is set to off
We want to tell services they haven't set 'From name' if they haven't actively made a choice between the default (derived from the service name) or a custom one. So we don't confuse older services, created before we added the `confirmed_email_sender_name` field (who will have that field set to null), we just show them the default for this. Note: services created after the field was added have it set to False.
We need a method for live services to create a ‘from’ name and ‘reply-to’ address if users decide to turn emails ON after making their service live. This updates /service-settings/set-email view to show: - Task list if ‘from’ and ‘reply to’ are not set or if both are set and turning emails back on - The radio toggle if turning emails off On/off toggle works as before (no change). For tasklist complete/incomplete states, we use `has_email_reply_to_address` and `confirmed_email_sender_name` (which is true when either "use 'name' of a service" or "custom name" are saved) properties on the service object. To turn emails ON in the tasklist view, users click the 'Start sending emails' button, which POSTS the form to `/set-email/on` enpoint, where service permissions are updated if all tasks are completed.
To avoid messing with the shared on/off Class used to turn on/off emails (and other two channel), that is a POST only and is hit when users see the tasklist view on the /set-email page and want to turn email on from there. If both required properties are not set (sender name and reply-to address), we redirect back to the page and show an error message (same pattern and message that we use on 'make' your service live'). If both proeprties are set, we update the service with email permissions - same way the on/off switch does. We then redirect the user to the /settings page.
This adds two tests: - test_set_email_page_markup - test_switch_email_on_from_tasklist_form `test_set_email_page_markup` tests if the page displays correct markup given service permissions and properties set `test_switch_email_on_from_tasklist_form` tests if the endpoint to which the form submits does the right thing: - updates and redirects to the right place or - redirects back to teh same page and shows the error message
It's used for for set-email form when a tasklist is present. That form post to this endpoint and updates service email permissions. This i so avoid clashing with teh on/off form on the page
If trial service show they haven't been using email notifications (by having no email templates), and that they don't intend to send any in future (in the projected volumes for email), this will turn emails off when we make the service live as a platform admin, through service settings. We want to push services to actively involve themselves in setting up their email notifications, by choosing a 'From name' and reply-to address. They will be funnelled down a route that makes them do this when emails are turned on, by changing their default emails state to 'off'.
The same logic we applied to normal go-lives should apply to services made live by approval from an org member. If they don't have email templates and don't expect to send emails, we should turn emails off. This is to push services towards a flow where they are forced to choose a 'From name' and reply-to address, which starts when you go to turn emails on.
We now want whether you have email templates to be the main deciding factor, with any expected email volumes being the fallback.
58a8688 to
503704c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contains
which are the following stories: