Skip to content

Uncrispyfy DeviceGroupForm in seeddb#2994

Merged
podliashanyk merged 5 commits intomasterfrom
uncrispyfy/device-group-form
Sep 20, 2024
Merged

Uncrispyfy DeviceGroupForm in seeddb#2994
podliashanyk merged 5 commits intomasterfrom
uncrispyfy/device-group-form

Conversation

@podliashanyk
Copy link
Copy Markdown
Contributor

@podliashanyk podliashanyk commented Sep 19, 2024

Closes #2988

Introduces extra pattern - uncrispyfying of the required fields.

@podliashanyk podliashanyk requested a review from a team September 19, 2024 16:03
@podliashanyk podliashanyk self-assigned this Sep 19, 2024
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Sep 19, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 19, 2024

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON black 987 0 23.75s
✅ PYTHON flake8 987 0 447.87s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 19, 2024

Test results

    9 files      9 suites   8m 57s ⏱️
3 330 tests 3 330 ✅ 0 💤 0 ❌
6 399 runs  6 399 ✅ 0 💤 0 ❌

Results for commit 7fe2f74.

♻️ This comment has been updated with latest results.

@codecov
Copy link
Copy Markdown

codecov bot commented Sep 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.59%. Comparing base (807deb4) to head (7fe2f74).
Report is 552 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2994      +/-   ##
==========================================
+ Coverage   56.30%   56.59%   +0.28%     
==========================================
  Files         602      602              
  Lines       43728    43728              
  Branches       48       48              
==========================================
+ Hits        24622    24746     +124     
+ Misses      19094    18970     -124     
  Partials       12       12              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@podliashanyk
Copy link
Copy Markdown
Contributor Author

Seems like edit- and add-forms in seeddb/* use crispy_forms (what to me seems like) implicitly. They are not mentioned in #2794, or am I missing some django/nav logic where those will be fixed by fixing some other templates mentioned in #2794?

Anyway, with _form_fields.html most of fields in all add/edit forms in /seeddb can be fixed in a separate 1 liner PR.
Very few fields will not be fixed universaly and will require some extra logic or tweaks. I so far noticed only select2 fields not being universaly converted with _form_fields.html. Will investigate and report more tomorrow.

@podliashanyk podliashanyk marked this pull request as ready for review September 19, 2024 16:17
@podliashanyk podliashanyk changed the title Uncrispyfy/device group form Uncrispyfy DeviceGroupForm Sep 19, 2024
@podliashanyk podliashanyk changed the title Uncrispyfy DeviceGroupForm Uncrispyfy DeviceGroupForm in seeddb Sep 19, 2024
Is mostly universal, yet fields that require extra classes (like f.e. select2) need extra logic.
Meaning that with few tweaks this template has a potential to fix all add/edit templates in /seeddb.
Will finish and showcase this in a separate PR

fixup
@podliashanyk podliashanyk force-pushed the uncrispyfy/device-group-form branch from 6af5ca9 to 25ce6c1 Compare September 20, 2024 07:27
queryset=Netbox.objects.all(), required=False
)
netboxes.widget.attrs.update({"class": "select2"})
no_crispy = True
Copy link
Copy Markdown
Contributor

@hmpf hmpf Sep 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When crispy is gone we will no longer need "no_crsipy" for choosing which form to show. It is still useful to hold extra form attributes though, like with the filter forms.

Copy link
Copy Markdown
Contributor Author

@podliashanyk podliashanyk Sep 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can remove no_crispy = True completely once all edit/add forms i seeddb are converted. Because then we can safely remove the {% if form.no_crispy %}-block from the seeddb/edit.html that was added in this PR and render {% include "seeddb/_form_fields.html" %} by default there

@hmpf
Copy link
Copy Markdown
Contributor

hmpf commented Sep 20, 2024

Are you considerimg adding django-widget-tweaks for the special casing?

@podliashanyk
Copy link
Copy Markdown
Contributor Author

podliashanyk commented Sep 20, 2024

Are you considerimg adding django-widget-tweaks for the special casing?

I was really tempted to add it while working on this one and investigating the situation with other edit/add forms in seeddb, but there was not overwhelmingly definite need so far. But will see how it goes with conversion of other fields, might happen that django-widget-tweaks will be necessary there

Copy link
Copy Markdown
Contributor

@johannaengland johannaengland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only a small cosmetic suggestion

Co-authored-by: Johanna England <johanna.england@sikt.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Uncrispyfy python/nav/web/seeddb/forms/__init__.py:DeviceGroupForm

4 participants