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

Remove double underscores / hyphens from generated ids #316

Merged
merged 3 commits into from
Apr 29, 2024

Conversation

phillipbarron
Copy link
Contributor

@phillipbarron phillipbarron commented Apr 25, 2024

What does this change?

Titles may contain non-alpha-numeric characters however, the ids what we derive from these may not. When hyphens, colons etc are used in a title, the auto-generated ids fail validation, and it is not possible to launch the newsletter.

This PR updates the deriveNewsletterFieldsFromName function to strip out non-alpha-numeric characters and also, removes double underscores or hyphens (which also break validation rules for identifiers)

The result is that the title is not constrained by the id validation rules.

How to test

On main, try creating a newsletter with a title containing non-numeric characters. EG:

foo ::: bar ------ Baz
The Stakes - US Election Edition

The above will fail at the point you attempt to launch. Now try again on this branch

How can we measure success?

The newsletter can be launched

Have we considered potential risks?

Up will now, it was only possible to use a-Z, 0-9 in titles - this enables use to use anything else. That anything else will be returned from the API. The API response is used in:

  • Embeds
  • Manage My Account
  • All Newsletters Page

Is this OK?

@phillipbarron phillipbarron requested a review from a team as a code owner April 25, 2024 16:35
@phillipbarron phillipbarron merged commit 66699cc into main Apr 29, 2024
2 checks passed
@phillipbarron phillipbarron deleted the pb/id-generation-sanitisation branch April 29, 2024 08:44
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.

2 participants