-
Notifications
You must be signed in to change notification settings - Fork 26
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
Bug/4810 changing textcase on form variable update #4837
Bug/4810 changing textcase on form variable update #4837
Conversation
16bda26
to
cb900c3
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4837 +/- ##
=======================================
Coverage 96.58% 96.58%
=======================================
Files 748 748
Lines 25476 25477 +1
Branches 3368 3368
=======================================
+ Hits 24605 24607 +2
Misses 608 608
+ Partials 263 262 -1 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
b2d86fa
to
ab29881
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks nice to me, only a couple of remarks (changes are only needed for the type checking if you agree as well)
For future bug fixes, it's nice to have a first commit with the regression test (which should fail) and then a second with the real fix of the bug.
ab29881
to
a2c84db
Compare
Closes #4810
Changes
The JSON parser on the form variables endpoint now ignores the
initial_value
field. This ensures that the form variable initial value, and the default value of the component, stay exactly the same.Checklist
Check off the items that are completed or not relevant.
Impact on features
Release management
I have updated the translations assets (you do NOT need to provide translations)
./bin/makemessages_js.sh
./bin/compilemessages_js.sh
Commit hygiene