Flags with question marks are routed incorrectly

[tompave]

Flags with question marks are routed incorrectly

[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. 🙂

[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?

fun_with_flags_ui/lib/fun_with_flags/ui/templates.ex

Lines 64 to 66 in d3513e0

	def path(conn, path) do
	Path.join(conn.assigns[:namespace], path)
	end

[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.