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

Refactor commodity repository and update database schema #158

Conversation

pveeckhout
Copy link
Contributor

Updated MybatisCommodityRepository, replacing several methods with a createOrUpdateOnConflict method and added new pertinent fields to the database table "commodity". This includes adding and renaming columns, updating the "validated_commodity_view" and adding an index for the "is_rare" column. In "build.gradle", the backendMessageProcessorLibVersion has been updated.

closes #157

Updated MybatisCommodityRepository, replacing several methods with a createOrUpdateOnConflict method and added new pertinent fields to the database table "commodity". This includes adding and renaming columns, updating the "validated_commodity_view" and adding an index for the "is_rare" column. In "build.gradle", the backendMessageProcessorLibVersion has been updated.
@pveeckhout pveeckhout self-assigned this Mar 24, 2024
@pveeckhout pveeckhout requested a review from a team as a code owner March 24, 2024 00:39
In the 'ReceiveCommodityMessageUseCaseTest.java', two new null parameters were added to the 'CommodityMessage.V3.Payload()' invocation. This aligns with recent schema updates to the Commodity Message functionality.
Copy link
Collaborator

@Daniel-J-Mason Daniel-J-Mason left a comment

Choose a reason for hiding this comment

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

validated_commodity may be useful to query, but in the spirit of merging these entities I think the entire ValidatedCommodity application class should be removed from the backend stack. The Commodity class should be updated to include the full field set and be the sole reference so cut down on confusion. We havent separated any of our other classes based on completeness either so this would be a more consistent approach

Copy link
Collaborator

Choose a reason for hiding this comment

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

This breaks liquibase changelog checksum, have to recreate in a new file. maybe something like update_validated_commodity_view.xml

May come up with a standard naming system if we have multiple updates

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should not break the checksum as it should always do a recreate upon start.

The settings of the validated_commodity_view have been updated to run on change, in addition to running always. The 'create_validated_commodity_view' changeset in 'create_validated_commodity_view.xml' now includes 'runOnChange' making the database changes more responsive to modifications.
Copy link
Collaborator

@Daniel-J-Mason Daniel-J-Mason left a comment

Choose a reason for hiding this comment

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

View corrected

maintaining ValidatedCommodity architecture for the time being as its the only domain object we have intentionally null data to distinguish validity

@pveeckhout pveeckhout merged commit 6177548 into development Mar 25, 2024
4 checks passed
@pveeckhout pveeckhout deleted the 157-merge-commodity-and-commodity_whitelist-tables-to-improve-query-performance branch March 25, 2024 00:42
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.

Merge commodity and commodity_whitelist Tables to Improve Query Performance
2 participants