Flags with question marks are routed incorrectly

[sascha-wolf]

We have a check_email_verified? flag in our system which gets presented properly in the UI. But selecting it routes to feature-flags/flag/check_email_verified? so that the ? is interpreted as beginning of the query string.

This results in the flag not being found.

Current

Selecting a flag with a question mark (?) displays a "not found" details page instead of allowing to change the settings for the flag.

Expected

It's possible to see the detail page for flags with question marks (?) when selecting them on the index page.

[sascha-wolf]

I'm happy to submit a PR with a fix. Can you point me to the general direction where this needs to be handled? Maybe here?

https://github.com/tompave/fun_with_flags_ui/blob/d3513e0e74e22b061f58f4ed148afd183279e8e3/lib/fun_with_flags/ui/templates.ex#L64-L66

[tompave]

Hi, thank you for trying the library.

That limitation is actually there by design, as explained in the readme:

For this reason this library enforces some stricter rules when creating flags and groups. Blank values are not allowed, ? neither, and flag names must match /^w+$/. (https://github.com/tompave/fun_with_flags_ui#caveats)

I'd rather not change it because I don't want to add too many escaping rules to the routing. The reason is that technically a flag name could contain anything:

iex(1)> is_atom :"foo?=[aa]🍕nope   😬"
true

And allowing the same variety of chars in an URL is a rabbit hole I'd rather stay away from.

[tompave]

I'm afraid that the solution is to just not use ? in your flag names.

[tompave]

That's also why this web dashboard will prevent you from creating flags with those invalid characters: https://github.com/tompave/fun_with_flags_ui/blob/v0.7.1/lib/fun_with_flags/ui/utils.ex#L128-L139

I imagine that you created the flag check_email_verified? directly in code. On the other hand, if you did manage to create it using the dashboard, then I'd treat that as a bug and I'd be happy to accept a PR that fixes the validation.

[sascha-wolf]

Fair enough. Yes, we created it in code. We have a task which sets some flags with defaults on startup, basically migration like.

[sascha-wolf]

I will close this then. Thank you again. :)

[tompave]

No worries, I'm glad that the workaround makes sense. 👍

Fair enough. Yes, we created it in code. We have a task which sets some flags with defaults on startup, basically migration like.

@Zeeker I see. I'd just suggest caution with this approach though, because you risk to override some manual flag change when a new process starts. For example, an engineer could manually disable a flag because they have a good reason, but then the flag could get automatically re-enabled if a new application process/container/instance (re)starts somewhere.

[sascha-wolf]

Thanks for the heads up, the task checks if the flag is set and does nothing if that's the case, exactly to avoid it. It really only is for initial setup. 🙂