-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat(authority-storage): extend authority with additional fields #317
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Please create Jira ticket for the change (link is in README) and describe requirements and why it needed. It's needed for history purposes and to be able to have knowledge about why the change was made.
- Update NEWS.md
- Update Module descriptor by bumping minor version of authority-storage interface (as you made change to API)
- Please cover your change with tests.
- Please update description of the PR using provided template.
src/main/resources/db/changelog/changes/v4.0/add-additional-fields-to-authority-archive.xml
Outdated
Show resolved
Hide resolved
<addColumn tableName="authority_archive"> | ||
<column name="additional_headings" type="jsonb"/> | ||
</addColumn> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why it was decided to have only one jsonb field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to minimize number of new fields/columns in database
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What benefits will we gain from this approach? I want to ensure that it won't require us to undertake refactoring in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to add other additional fields in future we don't have to add new columns to the table in db, we just need to extend a single "additinal_fields" jsonb column. It required minor change comparing to column-per-fieldType approach.
src/test/java/org/folio/entlinks/service/authority/AuthoritiesBulkContextTest.java
Outdated
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
Purpose
Describe the purpose of this pull request. Why is this change necessary? What problem does it solve?
Approach
How does this change fulfill the purpose? Provide a high-level overview of the technical approach taken to address the problem.
Changes Checklist
Related Issues
List any Jira issues related to this pull request.
Learning and Resources (if applicable)
Discuss any research conducted during the development of this pull request. Include links to relevant blog posts, patterns, libraries, or addons that were used to solve the problem.
Screenshots (if applicable)
If this pull request involves any visual changes or new features, consider including screenshots or GIFs to illustrate the changes.