Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[issue-983] querying history, convert to instance, chase foreign key on history_date #984

Conversation

jeking3
Copy link
Contributor

@jeking3 jeking3 commented May 7, 2022

Description

This fixes an issue whereby:

  1. User queries history directly.
  2. User converts Historical<> to instance with instance property.
  3. Original query (from 3.1) was not with history.as_of so it did not have _as_of property
  4. instance failed

The fix:

When _history._as_of is not present use the history.history_date field instead
when chasing foreign keys.

This closes #983

In the end, if you did not query history with as_of, and you end up with a historical instance, if you call .instance on it, the HistoricalForeignKey chasers will use a fallback of the history_date of the record that made the instance when chasing relations! This was an easy fix and an easy win, and makes django-simple-history more powerful.

@jeking3 jeking3 added the bug Issues related to confirmed bugs label May 7, 2022
@jeking3 jeking3 self-assigned this May 7, 2022
@codecov
Copy link

codecov bot commented May 7, 2022

Codecov Report

Merging #984 (c806622) into master (720fd8a) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #984   +/-   ##
=======================================
  Coverage   97.64%   97.64%           
=======================================
  Files          23       23           
  Lines        1147     1147           
  Branches      222      222           
=======================================
  Hits         1120     1120           
  Misses         12       12           
  Partials       15       15           
Impacted Files Coverage Δ
simple_history/models.py 97.77% <100.00%> (ø)

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 720fd8a...c806622. Read the comment docs.

@jeking3 jeking3 force-pushed the issue-983/historical-instance-foreign-key-lookup-fixup branch from fd220ec to c806622 Compare May 7, 2022 01:11
@jeking3 jeking3 requested a review from tim-schilling May 7, 2022 02:54
Copy link
Contributor

@tim-schilling tim-schilling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me. Good catch!

@jeking3 jeking3 merged commit a947655 into jazzband:master May 8, 2022
@jeking3 jeking3 deleted the issue-983/historical-instance-foreign-key-lookup-fixup branch May 8, 2022 11:43
@LuxsSoft
Copy link

LuxsSoft commented Feb 24, 2023

This seems not fully resolved.

I had the mentioned error before version 3.2.0, now i can get a record with something like historicalOrgInstance.history.latest().instance and also historicalOrgInstance.history.latest().prev_record.instance. However getting the instance that way always returns an empty set of historical foreign key models.

It only works by getting the instance through an as_of() query, then the reverse relationships models show up through the related name eg.
historicalOrgInstance.historical_participants.all()

@LuxsSoft
Copy link

LuxsSoft commented Feb 24, 2023

More specifically here is what i am trying to do

t1: org with participant 1

org = HistoricalOrg(name="original")
part1 = HistoricalParticipant.objects.create(name="p1", org=org)
before_mod = timezone.now()

t2: org with participant 2

org.name="modified"
org.save()
org.participants.all().delete()
part2 = HistoricalParticipant.objects.create(name="p2", org=org)

now what works as expected:

org.as_of(before_mod).participants.all() # returns part1
org.as_of(before_mod).name # returns "original"
org.as_of(timezone.now().participants.all() # returns part2
org.as_of(timezone.now()).name # returns "modified"
org.history.most_recent().participants.all() # returns part2
org.history.most_recent() # returns "modified"

what is weird when i want to work with the helpers:

org.history.first().instance.name # returns "modified" as expected
org.history.first().instance.participants.all() returns part1 BUT part2 is expected
org.history.first().prev_record.instance.name # returns "original" as expected
org.history.first().prev_record.instance.participants.all() # returns empty HistoricalQuerySet [] BUT part1 is expected

@tim-schilling
Copy link
Contributor

@LuxsSoft it would be best to open up a new issue and write a draft PR with the tests that show the problem.

@LuxsSoft
Copy link

@LuxsSoft it would be best to open up a new issue and write a draft PR with the tests that show the problem.

ok i tried to make the case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues related to confirmed bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Select from Model.history, then use .instance, then chase a foreign key is broken
3 participants