Patch bug around DETECTOR_VERSION managing new record creation #115
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
16 tasks
JPrevost
approved these changes
Oct 4, 2024
** Why are these changes being introduced: We initially tried populating the detector_version value in the Detection and Categorization records within those models themselves via a before_create lifecycle hook. That proved unworkable because the app ends up calling .find_by_or_create, and the "find" part of that pathway has no relevant lifecycle hook to pick up the current value. This caused a bug that would have prevented the anticipated creation of new records when we bump the DETECTOR_VERSION env variable. ** Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/tco-90 ** How does this address that need: This abandons the attempt to populate detector_version via a lifecycle hook, and moves that responsibility to the models which initiate the creation of these records (where they are already defining the other needed fields). Because these values are now being defined upstream, the lifecycle hook is no longer needed, so we remove it. Removing this lifecycle hook causes some other tests to fail. Some of those tests need to be updated to _also_ define detector_version, while a test about that auto-population feature is just deleted as no longer relevant. ** Document any side effects to this change: No side effects other than our code is a little less sparse than I was hoping for. Such is life.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patches a bug in the Detection and Categorization workflow, where new records don't adequately respond to changes in the DETECTOR_VERSION value. The solution is to be more explicit about assigning that value from ENV, doing so one step farther upstream rather than inside the Detection and Categorization models themselves.
Developer
Ticket(s)
https://mitlibraries.atlassian.net/browse/TCO-90
Accessibility
all issues introduced by these changes have been resolved or opened
as new issues (link to those issues in the Pull Request details above)
Documentation
ENV
Stakeholders
Dependencies and migrations
NO dependencies are updated
NO migrations are included
Reviewer
Code
added technical debt.
Documentation
(not just this pull request message).
Testing