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.

Sign up for GitHub

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

Jump to bottom

Fix for metadata uploader overwriting existing data #1489

Merged
merged 7 commits into from
May 9, 2023
Merged

Fix for metadata uploader overwriting existing data #1489

merged 7 commits into from
May 9, 2023

Conversation

ksierks
Copy link
Contributor

@ksierks ksierks commented Apr 26, 2023 •
edited
Loading

Description of changes

Fixing a bug where using the metadata uploader deleted exiting sample metadata.

Related issue

Fixes #1488.

Checklist

Things for the developer to confirm they've done before the PR should be accepted:

  • CHANGELOG.md (and UPGRADING.md if necessary) updated with information for new change.
  • Tests added (or description of how to test) for any new features.
  • [ ] User documentation updated for UI or technical changes.

@ksierks ksierks changed the base branch from development to main April 26, 2023 18:55
@ksierks ksierks marked this pull request as ready for review April 27, 2023 19:04
@ksierks ksierks requested a review from ericenns April 27, 2023 19:04
joshsadam
joshsadam requested changes May 1, 2023
View reviewed changes
Copy link
Contributor

@joshsadam joshsadam left a comment

Choose a reason for hiding this comment

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

This definitely works as advertised.  My only request is that you add a test to confirm the number of metadata fields on a sample pre and post file upload.

@ksierks
Copy link
Contributor Author

ksierks commented May 1, 2023 •
edited
Loading

This definitely works as advertised.  My only request is that you add a test to confirm the number of metadata fields on a sample pre and post file upload.

Please see 8911b5d.

@ksierks ksierks requested a review from joshsadam May 1, 2023 21:42
joshsadam
joshsadam reviewed May 2, 2023
View reviewed changes
Copy link
Contributor

@joshsadam joshsadam left a comment

Choose a reason for hiding this comment

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

This looks great. Double-check the failing test.

...acility/bioinformatics/irida/ria/integration/projects/ProjectSampleMetadataImportPageIT.java Show resolved Hide resolved
joshsadam
joshsadam approved these changes May 2, 2023
View reviewed changes
Copy link
Contributor

@joshsadam joshsadam left a comment

Choose a reason for hiding this comment

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

Thanks for updating the test.

ericenns
ericenns approved these changes May 9, 2023
View reviewed changes
Copy link
Member

@ericenns ericenns left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@ericenns ericenns merged commit a9cd759 into main May 9, 2023
@ericenns ericenns deleted the fix-metadata-uploader-removing-existing-data branch May 9, 2023 13:59
@apetkau apetkau mentioned this pull request Jun 2, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshsadam joshsadam joshsadam approved these changes

@ericenns ericenns ericenns approved these changes

Assignees

@ericenns ericenns

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Uploading metadata from a file removes pipeline-derived metadata from a project
3 participants
@ksierks @ericenns @joshsadam