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

Fix non-alphanumeric characters in flag names #78

Merged
merged 2 commits into from
Jul 5, 2024
Merged

Conversation

willbarton
Copy link
Member

As @chosak notes in #73, the admin URL patterns prevented creating flags that didn't match [\w\-]. This change fixes that by switching from re_path to path for those patterns, simplifying them and making them more readable.

Fixes #73

As @chosak notes in #73, the admin URL patterns prevented creating flags that didn't match `[\w\-]`. This change fixes that by switching from `re_path` to `path` for those patterns, simplifying them and making them more readable.

Fixes #73
@willbarton willbarton requested a review from chosak July 5, 2024 17:40
@chosak
Copy link
Member

chosak commented Jul 5, 2024

At long last my dream of creating a flag named 🎏 has come true...

One issue - with this change you can create a flag with a / in it, like foo/bar, but then when you get redirected to the index view you get

django.urls.exceptions.NoReverseMatch: Reverse for 'flag_index' with arguments '('foo/bar',)' not found. 1 pattern(s) tried: ['admin/flags/(?P[^/]+)/\Z']

Granted, this issue exists on main as well. What about changing the flag name form here to use a Django SlugField? Sadly this would prevent the use of emoji in flag names but would ensure that the slug-name-based URL patterns never break weirdly.

Copy link
Member

@chosak chosak left a comment

Choose a reason for hiding this comment

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

Thankfully one can still get creative:

image

@willbarton willbarton merged commit 819c9c9 into main Jul 5, 2024
16 checks passed
@willbarton willbarton deleted the fix/flag_names branch July 5, 2024 18:52
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.

Creating a flag with non-alphanumeric characters breaks the UI
2 participants