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

Add more text options for email alerts #986

Merged
merged 1 commit into from Jan 18, 2017

Conversation

rubenarakelyan
Copy link
Contributor

@rubenarakelyan rubenarakelyan commented Jan 17, 2017

This commit adds fields to most finders to define a separate email alert list title text which will be used when the normally generated string is too long (> 255 characters). It will be appended with a string of the form “x items”, where “x” is the number of selected topics and “items” is either the singular or plural string defined as email_filter_name.

Also reverts #984 since this is a more comprehensive solution.

Trello: https://trello.com/c/KTHRa4a0/422-fix-email-alert-api-errors-when-the-email-alert-short-name-is-longer-than-255-characters

Deployment checklist:

  • Deploy specialist-publisher
  • Run the rake task publishing_api:publish_finders to re-publish all the finders

This commit adds fields to most finders to define a separate email alert list title text which will be used when the normally generated string is too long (> 255 characters). It will be appended with a string of the form “x items”, where “x” is the number of selected topics and “items” is either the singular or plural string defined as `email_filter_name`.
@jennyd
Copy link
Contributor

jennyd commented Jan 17, 2017

I'm not sure if this approach will work with GovDelivery, because it sounds like we would generate identical names for multiple topics. I've just tried to manually create a topic with the same name as an existing one (in our Integration account) and it errors, saying the name needs to be unique 😢 The API docs don't mention that constraint though.

We use the generated title as both name and short-name - it looks like only name needs to be unique, whereas the length error was on short-name, so there might be a way around those constraints. It would be simpler for our future plans to keep those two fields identical if possible, though. I can help you test on integration this afternoon if you like.

@rubenarakelyan
Copy link
Contributor Author

Discussed with @jennyd: following this PR I will raise another PR for finder-frontend that splits the logic for title and short_name to be separate so that we can continue to set title as we do now and use new logic for short_name that limits it to 255 characters. This PR is ready for review.

Copy link
Contributor

@Davidslv Davidslv left a comment

Choose a reason for hiding this comment

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

🌻 🍯

@Davidslv Davidslv merged commit 48af7a2 into master Jan 18, 2017
@Davidslv Davidslv deleted the add-email-alert-text-for-long-strings branch January 18, 2017 10:10
rubenarakelyan pushed a commit that referenced this pull request Jan 19, 2017
#986 added a new `email_filter_name` field to the various finder signup page schemas. This commit changes the relevant presenter to send this new field to the publishing-api whenever the content items are re-published.
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.

None yet

3 participants