Skip to content
This repository has been archived by the owner on Feb 7, 2019. It is now read-only.

Filter on related objects #113

Merged
merged 5 commits into from
Jan 6, 2016
Merged

Conversation

brki
Copy link
Contributor

@brki brki commented Dec 24, 2015

Allow using object for filtering, clarify behaviour in docs, adjusted one existing test to use explicit ids.

…e) filter

When this test was added, this returned a Player object:
- Player.objects.as_of(t1).get(team=team_current, name='p1.v1')
  (note: team_current is not the version of the team associated with this player at t1)

But these did not:
- Player.objects.as_of(t1).get(team=team_at_t1, name='p1.v1')
  (note: team_at_t1 *is* the version of the team associated with this player at t1)

- Player.objects.as_of(t1).get(team=team_at_t2, name='p1.v1')
  (note: team_at_t2 is not the version of the team associated with this player at t1)

Providing an object in the filter is standard practice, and it
seems like it would be nice to allow this to work, without having
to write something like:
- Player.objects.as_of(t1).get(team__identity=team_at_tx.identity, name='p1.v1')

Although the foreign key actually points to the id column of the foreign object,
when we ask for objects at a point in time, we need to look for related objects
that a) have the identity == foreign key value, and b) are valid at
the given point in time.
@brki
Copy link
Contributor Author

brki commented Jan 5, 2016

Also, see the long commit message for brki@be92df9 , which provides some explanation.

@maennel
Copy link
Contributor

maennel commented Jan 5, 2016

Hey @brki, I agree! Thanks for implementing this!
Just a small remark: Could you add the .id property also to the test filters at this place and this place for consistency?

maennel added a commit that referenced this pull request Jan 6, 2016
Filter on related versionable objects using objects as values for lookups, which is the Django standard behaviour.
@maennel maennel merged commit e09b37f into swisscom:master Jan 6, 2016
@maennel
Copy link
Contributor

maennel commented Jan 6, 2016

Thanks @brki !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants