Fix question form errors not being displayed #3046
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I'll be using proper form builder helpers for the inputs of these forms so that proper error messages can be displayed around each one. In order to do that, each template must have different base input names to be able to distinguish which question each answer option belongs to. So we render a separate template next to each "add field" button, and we can locate it from the input button itself without needing any ids.
In later commits nested templates could go through several interpolations and the placeholder won't always be in the middle, so stop caring about its position. This allows us to extract an utility method since the replacement is consistently done in the same way.
For consistency with the `blank_question` method.
I think this is "more unique" and it also generates an integer, which will be handy later.
So that the generate unique key is actually an integer. This will be useful later when will use this id as the id for our rectify form.
Instead, create a different virtual attribute at the form level and do proper mapping in there.
I think it reads slightly better like this.
We now use proper form builder methods that wrap actual objects, so we get full error messages, required attribute detection, and so on.
ghost
assigned 
mrcasals
approved these changes
Mar 20, 2018
Codecov Report
@@ Coverage Diff @@
## master #3046 +/- ##
==========================================
- Coverage 98.67% 98.62% -0.06%
==========================================
Files 1699 1699
Lines 40584 40602 +18
==========================================
- Hits 40048 40042 -6
- Misses 536 560 +24 |
oriolgual
approved these changes
Mar 20, 2018
rbngzlv
added a commit
that referenced
this pull request
Mar 21, 2018
* master: [RFC] Use cells for meeting m cards (#3022) Do not force Postgresql user to be admin when enabling trigram extension (#3053) Make organization reference_prefix required (#3056) admin can duplicate/copy meetings (#3051) Fix question form errors not being displayed (#3046) Erb whitespace cutting (#3047) Show debates statistics on space show and homepage (#3016) Fix broken translated field after form errors (#3026) Move decidim executable to "exe" folder (#3028) Friendlier buttons (#3027) Feedback needed after Endorsing when user has no user_groups (#2998) Fix seeding error on generator specs (#3021) fix spelling error in threshold (#3019) Migration plus seeds (#2933)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🎩 What? Why?
The errors happening on a question form where not being properly displayed making it hard to figure out what was wrong. Now we use proper form builder methods that wrap actual models, so we get proper error messages, required attribute detection and so on.
After this PR,
decidimform helper methods (to build inputs not associated to models) are no longer used anywhere in the code base. It'd be nice to get rid of them since they are quite complex and almost fully duplicate the logic of their form builder counterparts. However, I guess they can be considered public API and might be used by external plugins, so we might want to deprecate them first or not remove them at all... 🤷♂️surveyswere first implemented. But it was never used.📌 Related Issues
None.
📋 Subtasks
CHANGELOGentry📷 Screenshots (optional)
Before
After