Please specify in the help text at users interface that form_id is regards to value and not id
Thanks
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | 633938_wrong_form_id_suggestion_04.patch | 4.72 KB | soxofaan |
| #8 | 633938-3.patch | 2.51 KB | cafuego |
| #6 | 633938_wrong_form_id_suggestion_02.patch | 2.2 KB | soxofaan |
| #3 | 633938.patch | 2.08 KB | cafuego |
Comments
Comment #1
soxofaan commentedI assume you tried to enter a from ID as "foo-bar-form", while it should be "foo_bar_form".
I'm not sure we should put this information in the description text of the admin UI. It would take a fair amount of text to explain (view source, search for form, get id attribute, ...). Moreover, viewing in the markup is not the only way to get a form ID, so the description would only cover one use case.
I would take another approach. Now, when you enter "foo-bar-form", you get in that case a rather spartan message "Illegal form_id".
I would suggest to change this error message to make it more descriptive and helpfull, e.g.:
any thoughts?
Comment #2
treksler commented+1
Comment #3
cafuego commentedI totally concur, it had me stumped for ages too, to the point of writing a blog about it when I finally got it working. Someone just pinged me after finding my blog and solving their issue and suggested I write a patch.
The patch in question is attached.
It adds a single line to the admin form, explaining that a form_id must be only alphanumeric characters and underscores. The validation function can now detects alphanumeric characters with hyphens and in that case will suggest a form_id with hyphens replaced with underscores.
The default error message is now less spartan and also re-iterates the need for only alphanumeric characters and underscores.
Comment #4
cafuego commentedComment #6
soxofaan commentedGreat, thanks cafuego!
There seemed to be a syntax error in the path, which triggered to testbot to complain.
Fixed patch in attachment.
About the wording: English is not my mother tongue, but I would say "A form_id must only consist of ..." instead of "A form_id must consist of only ...". Any thoughts on this?
I think this feature definitely needs some test coverage too. E.g. things to test:
"foo_bar" -> accept
"foo-bar" -> suggest "foo_bar"
"foo_bar-baz" -> suggest "foo_bar_baz"
"FOO_BAR-BAZ" -> suggest "foo_bar_baz"
"foo$bar" -> deny
Comment #8
cafuego commentedI think the test bot is on crack.
It wasn't wrong per sé, but it wasn't very good english words ;-) Now reworded now as "A form_id may only contain lowercase alphanumeric characters and underscores."
I added support to test for uppercase characters and mixed hyphen/underscore use. The error message will show the entered form-id and the suggested form_id.
I've not looked at test coverage yet, though I did test the pregs used:
Comment #9
cafuego commentedActually, it's not. I was missing a } in my if / else block.
Comment #10
soxofaan commentedworked a bit on error handling code and added tests
Comment #13
wundo commentedComment #14
wundo commented