django-extra-settings icon indicating copy to clipboard operation
django-extra-settings copied to clipboard

Reset settings and fix for AppConfig.ready

Open ar0ne opened this issue 1 year ago • 4 comments
trafficstars


name: Remove db calls from app ready hook about: Submit a pull request for this project assignees: fabiocaccamo


Describe your changes First of all, Django "doesn't recommend" to run db calls in app ready hook (https://django.readthedocs.io/en/latest/ref/applications.html#django.apps.AppConfig.ready)

Instead, I'd like to explicitly ask users to set default values. For that they could use new dj-command:

python manage.py reset_extra_settings

Additionally, I added a button to "changelist" admin page.

Related issue 110 121

Checklist before requesting a review

  • [x] I have performed a self-review of my code.
  • [x] I have added tests for the proposed changes.
  • [x] I have run the tests and there are not errors.

ar0ne avatar Jan 11 '24 12:01 ar0ne

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.45%. Comparing base (fd891da) to head (7608c7e). Report is 13 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #128      +/-   ##
==========================================
- Coverage   85.51%   85.45%   -0.07%     
==========================================
  Files          21       21              
  Lines         435      433       -2     
==========================================
- Hits          372      370       -2     
  Misses         63       63              
Flag Coverage Δ
unittests 85.45% <100.00%> (-0.07%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 11 '24 12:01 codecov[bot]

@ar0ne thank you for this PR, could you please split it into two smaller PRs, one for the fix and the other one for the admin integration (this one with a separate test module)?

fabiocaccamo avatar Feb 15 '24 17:02 fabiocaccamo

@ar0ne if you're still interested in working on this PR, please rebase it.

fabiocaccamo avatar Feb 27 '24 17:02 fabiocaccamo

@fabiocaccamo hi, sure i hope to find some time on the weekend to address you comments

ar0ne avatar Feb 28 '24 06:02 ar0ne