feat(HistoryRecords): prev and next attributes #365
Conversation
Codecov Report
@@ Coverage Diff @@
## master #365 +/- ##
==========================================
+ Coverage 97.45% 97.47% +0.02%
==========================================
Files 15 15
Lines 590 595 +5
Branches 73 73
==========================================
+ Hits 575 580 +5
Misses 9 9
Partials 6 6
Continue to review full report at Codecov.
|
simple_history/models.py
Outdated
| """ | ||
| Get the previous history record for the instance. `None` if first. | ||
| """ | ||
| entry = self.instance |
There was a problem hiding this comment.
I don't love going from historical record to instance back to historical record. Ideally we'd avoid this. Maybe look to see what they do in cleanerversion? https://github.com/rossmechanic/cleanerversion/blob/master/versions/models.py#L83
I think something similar to that on our historical tables would be a much cleaner solution
SpainTrain
force-pushed
the
230-get-prev-next-record
branch
3 times, most recently
from
351352c
to
a2096b5
Compare
| """ | ||
| Get the next history record for the instance. `None` if last. | ||
| """ | ||
| return self.instance.history.filter( |
There was a problem hiding this comment.
Good feedback. This forced me to implement these in a much more efficient and clean way. Thanks!
|
@rossmechanic should be good for another review. If reasonable, I will add relevant docs. Thanks! |
|
@SpainTrain need to pass the |
SpainTrain
force-pushed
the
230-get-prev-next-record
branch
from
a2096b5
to
2c254ea
Compare
|
derp ✅ |
SpainTrain
force-pushed
the
230-get-prev-next-record
branch
from
2c254ea
to
9bfb2ed
Compare
|
docs added |
|
@SpainTrain Thanks. This looks way better to me at first glance. Going to spend some time playing around with it and get you comments by end of week. |
|
Mind adding your name to |
simple_history/models.py
Outdated
| """ | ||
| return self.instance.history.filter( | ||
| Q(history_date__lt=self.history_date) | ||
| ).order_by('history_date').first() |
There was a problem hiding this comment.
This should reverse order by history date or take .last() with the existing order. We should also use say 3 records in test_get_prev_record / test_get_next_record and get the previous and next records of the last / first records, respectively, to ensure we catch this type of bug in the unit tests.
There was a problem hiding this comment.
very good catch, thanks!
- New attributes on history model retrieve previous and next records Closes jazzband#230
SpainTrain
force-pushed
the
230-get-prev-next-record
branch
from
9bfb2ed
to
1597816
Compare
simple_history/models.py
Outdated
| """ | ||
| return self.instance.history.filter( | ||
| Q(history_date__lt=self.history_date) | ||
| ).order_by('history_date').first() |
There was a problem hiding this comment.
very good catch, thanks!
TODO
Changes
Closes #230
This change is