Skip to content

Fb dataset update rewrite #6552

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

Merged
merged 19 commits into from
May 16, 2025
Merged

Fb dataset update rewrite #6552

merged 19 commits into from
May 16, 2025

Conversation

labkey-klum
Copy link
Contributor

@labkey-klum labkey-klum commented Apr 10, 2025

Rationale

Dataset updateRows had previously been implemented as a single row delete followed by an insert row. This PR attempts to rewrite this code to look like more typical QueryUpdateService.updateRow implementations.

This work was focused on a few main areas and tasks:

  • Refactor the helpers in DatasetDataIteratorBuilder so they could be used outside of a DataIterator context. These helpers were responsible for : LSID, subject ID, sequence number, and participant sequence number generation.
  • Add unit tests and selenium tests to validate the integrity of insert and update operations around these special fields
  • Swap in the new helpers into DatasetDataIteratorBuilder
  • Rewrite DatasetUpdateService.updateRow to use the new helpers as well as Table.update to update the row.

Copy link

WARNING: This PR appears to have the default title generated by GitHub. Please use something more descriptive.

@labkey-klum labkey-klum requested a review from labkey-adam April 16, 2025 16:47
@labkey-klum labkey-klum marked this pull request as ready for review April 16, 2025 16:47
Copy link

WARNING: This PR appears to have the default title generated by GitHub. Please use something more descriptive.

@labkey-klum labkey-klum requested a review from XingY April 16, 2025 17:45
@labkey-klum labkey-klum removed the request for review from XingY May 8, 2025 16:57
@labkey-klum labkey-klum requested a review from labkey-chrisj May 12, 2025 19:31
Copy link
Contributor

@labkey-chrisj labkey-chrisj left a comment

Choose a reason for hiding this comment

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

Tests look good to go

@labkey-klum labkey-klum merged commit bb6eb11 into develop May 16, 2025
6 checks passed
@labkey-klum labkey-klum deleted the fb_dataset_update_rewrite branch May 16, 2025 15:28
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.

4 participants