adamcharnock / django-hordak

Double entry accounting in Django
http://django-hordak.readthedocs.io
MIT License
228 stars 55 forks source link

Stable migrations #95

Closed PetrDlouhy closed 1 year ago

PetrDlouhy commented 1 year ago

@nitsujri @Joshun When I am testing the new code I am often struggling with the migrations changing all the time. I have tried to mitigate that with this PR. Part of it is taken from #90.

I will have to fix the tests in Django 3.2.

Can you please, can you check if this is correct?

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 :tada:

Comparison is base (7f83ecf) 92.90% compared to head (352c683) 92.92%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #95 +/- ## ========================================== + Coverage 92.90% 92.92% +0.02% ========================================== Files 59 59 Lines 3734 3747 +13 Branches 233 236 +3 ========================================== + Hits 3469 3482 +13 Misses 223 223 Partials 42 42 ``` | [Impacted Files](https://app.codecov.io/gh/adamcharnock/django-hordak/pull/95?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Adam+Charnock) | Coverage Δ | | |---|---|---| | [hordak/defaults.py](https://app.codecov.io/gh/adamcharnock/django-hordak/pull/95?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Adam+Charnock#diff-aG9yZGFrL2RlZmF1bHRzLnB5) | `100.00% <100.00%> (ø)` | | | [hordak/models/core.py](https://app.codecov.io/gh/adamcharnock/django-hordak/pull/95?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Adam+Charnock#diff-aG9yZGFrL21vZGVscy9jb3JlLnB5) | `98.23% <100.00%> (+0.01%)` | :arrow_up: | | [hordak/tests/models/test\_core.py](https://app.codecov.io/gh/adamcharnock/django-hordak/pull/95?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Adam+Charnock#diff-aG9yZGFrL3Rlc3RzL21vZGVscy90ZXN0X2NvcmUucHk=) | `100.00% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

PetrDlouhy commented 1 year ago

Looks good to me! I tested the migration locally works just fine.

Yeah that's weird that 3.2 thinks there is a missing migration where the other versions don't?

(answering here on official PR, the https://github.com/PetrDlouhy/django-hordak/pull/1 was my testing PR)

The Django 3.2 was creating new migrations when get_internal_currency was a function. When I changed it to:

get_internal_currency = defaults.INTERNAL_CURRENCY

everything works for all versions.