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

resolve duplicate oai results #1582

Closed
wants to merge 5 commits into from

Conversation

rferris
Copy link
Contributor

@rferris rferris commented Apr 14, 2023

Resolve duplicate OAI results when updated_at dates are the same. Issue #1581.

dependabot bot and others added 5 commits January 11, 2023 14:15
Bumps [json5](https://github.com/json5/json5) from 2.2.1 to 2.2.3.
- [Release notes](https://github.com/json5/json5/releases)
- [Changelog](https://github.com/json5/json5/blob/main/CHANGELOG.md)
- [Commits](json5/json5@v2.2.1...v2.2.3)

---
updated-dependencies:
- dependency-name: json5
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [loader-utils](https://github.com/webpack/loader-utils) from 2.0.2 to 2.0.4.
- [Release notes](https://github.com/webpack/loader-utils/releases)
- [Changelog](https://github.com/webpack/loader-utils/blob/v2.0.4/CHANGELOG.md)
- [Commits](webpack/loader-utils@v2.0.2...v2.0.4)

---
updated-dependencies:
- dependency-name: loader-utils
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Include dist folder in tarball. Required for Bootstrap 5 themes.
@anvit anvit changed the base branch from stable/2.7.x to qa/2.x April 25, 2023 18:12
anvit pushed a commit that referenced this pull request May 2, 2023
Add a secondary column to sort by to avoid the problem of duplicate
records during OAI harvest
@@ -437,6 +437,8 @@ public static function getUpdatedRecords($options = [])
}

$criteria->addAscendingOrderByColumn(QubitObject::UPDATED_AT);
// Secondarily filter by ID in case UPDATED_AT has duplicate entries.
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @rferris! Thanks so much for your contribution! I just finished looking at your changes and this definitely makes sense to me, and should fix the issue!

I have a few super quick changes I'd like to request before we can accept this PR —

  1. Could you rebase this to the branch qa/2.x instead of stable/2.7. We use that branch for any bug fixes or features intended for the next release, and as you might've noticed here there's a few extra changes it picked up that we don't need, when we changed the base to qa/2.x here on the PR.
  2. Line #440 seems to be using tabs for indentation which is making PHP Coding Standards fixer throw an error in the tests. Could you make sure this uses spaces instead?
  3. Small nitpick, but this repo uses uppercase first letter for the commit message.

Let us know if you have the time to make these changes. If not, we're happy to do these and accept this fix. Thanks again!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Anvit,

Can do!

Rohan

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome! Thanks Rohan! Really appreciate it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Closed this PR and reissued over at #1593

@rferris rferris closed this May 3, 2023
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.

None yet

3 participants