flaeppe commented 7 years ago

Added an initial implementation for introducing the status PENDING on a for product transactions and purchases, also implementing so that the state history for these entries are kept on an continuing timeline where their history is read only.

I'm leaving it here for reviewal, to make sure we find any skew or missing functionality/testing/choices/situations quite early and before any views apply the changes fully.

Another natural next step could be to make sure Wallet transactions also apply this concept.

codecov[bot] commented 7 years ago

Codecov Report

Merging #32 into develop will increase coverage by 1.48%. The diff coverage is 98.33%.

@@             Coverage Diff             @@
##           develop      #32      +/-   ##
===========================================
+ Coverage    90.27%   91.76%   +1.48%     
===========================================
  Files           76       84       +8     
  Lines         2868     3520     +652     
  Branches       155      190      +35     
===========================================
+ Hits          2589     3230     +641     
- Misses         252      261       +9     
- Partials        27       29       +2

Impacted Files	Coverage Δ
src/foobar/rest/views/wallet.py	`100% <100%> (ø)`	:arrow_up:
src/foobar/tests/factories.py	`100% <100%> (ø)`	:arrow_up:
src/utils/factories.py	`100% <100%> (ø)`
src/wallet/tests/factories.py	`100% <100%> (ø)`	:arrow_up:
src/wallet/admin.py	`91.11% <100%> (-0.2%)`	:arrow_down:
src/shop/tests/test_api.py	`100% <100%> (ø)`	:arrow_up:
src/wallet/signals/handlers.py	`100% <100%> (ø)`	:arrow_up:
src/wallet/signals/signals.py	`100% <100%> (ø)`
src/utils/exceptions.py	`100% <100%> (ø)`
src/foobar/tests/rest/test_purchase.py	`99.4% <100%> (+1.04%)`	:arrow_up:
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update be2efec...f9adc14. Read the comment docs.

flaeppe commented 7 years ago

Me too!

However, even though I've tried running the migrations with existing data locally, I'm a little bit worried that there might be some missed edge case that can break the migration.

Just make sure appropriate backups are in place before running any migration, if something strange would happen.

kjagiello commented 7 years ago

We should simply try to migrate locally using the latest database dump and see what happens. You have implemented data integrity checks in your migrations so I feel like we should be pretty fine.

kjagiello commented 7 years ago

I've just run migrations on the latest database snapshot and everything seemed to be ok.

flaeppe commented 7 years ago

Ship it 🚢

kjagiello commented 7 years ago

Not so fast, sailor! It seems that we have still some issues to resolve before we can merge.

uppsaladatavetare / foobar-api

Refactoring for introducing a timeline based history #32

Codecov Report