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

[kbss-cvut/record-manager-ui#71] Reduce the size of PatientRecordDto data #45

Merged
merged 7 commits into from
Feb 1, 2024

Conversation

ledsoft
Copy link

@ledsoft ledsoft commented Jan 24, 2024

  • Also update code to the new OWL2Java constant naming strategy.
  • Update dependencies to work with the latest JOPA 2.0.0-SNAPSHOT build.

kbss-cvut/record-manager-ui#71.

…data.

Also update code to the new OWL2Java constant naming strategy.
@ledsoft ledsoft requested a review from blcham January 24, 2024 14:38
@ledsoft
Copy link
Author

ledsoft commented Jan 31, 2024

Note that the changes regarding kbss-cvut/record-manager-ui#71 are backwards compatible with the frontend, so the PR can be merged without waiting for frontend implementation.

@ledsoft
Copy link
Author

ledsoft commented Jan 31, 2024

@blcham

Copy link

@blcham blcham left a comment

Choose a reason for hiding this comment

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

LGTM, just have a look at my comment.

@OWLObjectProperty(iri = Vocabulary.s_p_has_author, fetch = FetchType.EAGER)
private User author;
@OWLObjectProperty(iri = Vocabulary.s_p_has_author)
private URI author;
Copy link

Choose a reason for hiding this comment

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

Not sure that we won't be using it to show provenance information before the record is fully loaded:
image

I guess we do not need to optimize PatientRecordDto too much anymore since implementing on the backend, so I was wondering if it is not useful to leave there User author instead (at least developer friendly:). I guess the most important argument to consider is that we do it somehow consistently across the application.

Copy link

Choose a reason for hiding this comment

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

I just tested it and the current frontend works well with this backend, so picture above is not the issue :)

Copy link

Choose a reason for hiding this comment

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

image

@OWLObjectProperty(iri = Vocabulary.s_p_has_last_editor, fetch = FetchType.EAGER)
private User lastModifiedBy;
@OWLObjectProperty(iri = Vocabulary.s_p_has_last_editor)
private URI lastModifiedBy;
Copy link

Choose a reason for hiding this comment

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

See comment above.

@blcham blcham merged commit d08164a into kbss-cvut:main Feb 1, 2024
2 checks passed
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.

Add sorting/filtering/paginating of records based on AVA scenarios
2 participants