-
Notifications
You must be signed in to change notification settings - Fork 39
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
Uncrispify management profile forms in seedDB #3105
Uncrispify management profile forms in seedDB #3105
Conversation
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports |
Test results 8 files 8 suites 8m 21s ⏱️ Results for commit 1c396cf. ♻️ This comment has been updated with latest results. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3105 +/- ##
=======================================
Coverage 60.44% 60.44%
=======================================
Files 605 605
Lines 43757 43757
Branches 48 48
=======================================
Hits 26451 26451
Misses 17294 17294
Partials 12 12 ☔ View full report in Codecov by Sentry. |
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.
This is an interesting form to refactor. Those forms are quite simple and both self.attrs = set_flat_form_attributes()
in forms.py
are redundant if combined with suggested changes in the templates (see below).
I originally added the {% if form.attrs %}...{% else %}{{ form }}{% endif %}
logic to non-crispy templates purely as a failsafe - if we miss something out, at least basic (no styling and no customization in tag structure) form fields will be rendered. It does add redundancy. So we might need to consider if and when to drop those if else
blocks in uncrispyfyed templates.. In this PR there was no crispy helper defined so we use set_flat_form_attributes()
just for if else
blocks in the template. So if we are going to drop extra failsaving, this PR is a good place to start this discussion
I have removed all of these if-else blocks in #3128. But we can also already do it now bit by bit, I don't really have a preference. |
841378f
to
0ce9772
Compare
0ce9772
to
1c396cf
Compare
Quality Gate passedIssues Measures |
Closes #3104.
Urls:
http://localhost/seeddb/management-profile/edit/<id>/
http://localhost/seeddb/management-profile/add/