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

Remove unnecessary peer_review_id foreign key #6397

Merged

Conversation

mishaschwartz
Copy link
Contributor

@mishaschwartz mishaschwartz commented Dec 20, 2022

Motivation and Context

Results have many peer reviews so the foreign key that associates these two tables should be (and it already is) on the peer_reviews table. The extra foreign key on the results table wasn't really being used as any association, it was just being used as a flag that this result is associated with a peer review.

This set up causes a circular association (peer reviews need to reference results which need to reference peer reviews) which is unnecessary and complicates other functionality like archiving.

This PR removes the foreign key from the results table and updates the code to use use the has_many association that already exists when looking for peer reviews that are associated with results.

Your Changes

Description:

Type of change (select all that apply):

  • Refactoring (internal change to codebase, without changing functionality)

Testing

Questions and Comments (if applicable)

Checklist

  • I have performed a self-review of my own code.
  • I have verified that the pre-commit.ci checks have passed.
  • I have verified that the CI tests have passed.
  • I have reviewed the test coverage changes reported on Coveralls.

Pull request to make documentation changes (if applicable)

@david-yz-liu david-yz-liu merged commit cb4432b into MarkUsProject:master Jan 1, 2023
@mishaschwartz mishaschwartz deleted the remove-peer-review-id branch January 2, 2023 13:23
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

2 participants