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

Cpfm 246 #113

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Cpfm 246 #113

wants to merge 11 commits into from

Conversation

SeanTCasey
Copy link
Collaborator

Description
The DB Poller orders results by a timestamp (like updated_at, created_at, etc.), but if a join was done between multiple tables that share these timestamps, the order query becomes ambiguous, resulting in a crash. This fix sets the owner column for the timestamp to prevent this.

Fixes #112 (issue)

Type of change
Please delete options that are not relevant.

[ X ] Bug fix (non-breaking change which fixes an issue)
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Existing tests pass successfully. This is a bit of an edge case with my own project, so probably doesn't warrant a full test around it on Deimos. It's just making the ordering a bit more explicit. The service that uses this logic, Content Sieve, has tests that pass with this fix.

[ X ] My code follows the style guidelines of this project
[ X ] I have performed a self-review of my own code
[ X ]I have added a line in the CHANGELOG describing this change, under the UNRELEASED heading
[ X ] My changes generate no new warnings
[ X ] New and existing unit tests pass locally with my changes

@dorner
Copy link
Member

dorner commented Jan 22, 2021

@SeanTCasey looks good but:

  • lint is failing
  • needs rebase
  • can we add a spec where we do this via join?

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

Successfully merging this pull request may close these issues.

DB Poller should order results by record class timestamps, to prevent ambiguity from joins
2 participants