Add check to handle empty form attributes in general non-crispy template

[podliashanyk]

Originally mentioned in #3105 (review)

@lunkwill42 har suggested IRL to add the check to the general template.

A general failsafe for #2794

[github-actions]

🦙 MegaLinter status: ✅ SUCCESS

Descriptor	Linter	Files	Fixed	Errors	Elapsed time
✅ PYTHON	black	992		0	11.08s
✅ PYTHON	ruff	987		0	0.1s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by

[github-actions]

Test results

    9 files      9 suites   8m 30s ⏱️
2 135 tests 2 135 ✅ 0 💤 0 ❌
4 009 runs  4 009 ✅ 0 💤 0 ❌

Results for commit 626cc3e.

♻️ This comment has been updated with latest results.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.42%. Comparing base (1e9d859) to head (626cc3e).
Report is 21 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3140      +/-   ##
==========================================
- Coverage   60.44%   60.42%   -0.02%     
==========================================
  Files         605      605              
  Lines       43749    43745       -4     
  Branches       48       48              
==========================================
- Hits        26443    26435       -8     
- Misses      17294    17298       +4     
  Partials       12       12              

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

[johannaengland]

The tests fail since we have not added attrs to the management profile form in #3105.

And generally I thought we were moving away from having the catch-all {{ form }} since we will uncrispify all of nav.

If we don't want to have to add attrs to all forms this tactic is not ideal, since it shows that the tests fail even with the fallback.

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[johannaengland]

Does it make any difference now that {% include 'foundation-5/errors.html' %} will only be rendered if forms.attrs is set? Because before if this template was used even without setting form.attrs it would include that template

[podliashanyk]

Does it make any difference now that {% include 'foundation-5/errors.html' %} will only be rendered if forms.attrs is set? Because before if this template was used even without setting form.attrs it would include that template

Good point. Furthermore upon further inspection of the diff I am actually inclined to close this one without merging. After inspecting Django code, it seems to me that having the original implementation of _form_content.html brings us closer to https://github.com/django/django/blob/main/django/forms/templates/django/forms/div.html (the one that is rendered when calling {{ form }}). Which means that the original is a better failsafe already. I will add an issue to fix rendering of CSRF token that works when _form_content.html is to be reused by templates that were non-crispy to begin with. Also we can now drop the failsafe {% if form.attrs %}...{% else %}{{ form }}{% endif %} everywhere when uncrispyfying crispy (if reusing _form_content.html).

[johannaengland]

Furthermore upon further inspection of the diff I am actually inclined to close this one without merging. After inspecting Django code, it seems to me that having the original implementation of _form_content.html brings us closer to https://github.com/django/django/blob/main/django/forms/templates/django/forms/div.html (the one that is rendered when calling {{ form }}). Which means that the original is a better failsafe already.

I agree with you on that. Let's close this one