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

[DUOS-1764][risk=no] Loop over all dataset ids instead of assuming the first #1677

Merged
merged 14 commits into from
Jul 21, 2022

Conversation

rushtong
Copy link
Contributor

@rushtong rushtong commented Jul 14, 2022

Addresses

Partially addresses https://broadworkbench.atlassian.net/browse/DUOS-1764

See also: DataBiosphere/consent#1648

This PR refactors the outdated assumption that a submitted DAR will always have a single dataset id. In all cases where we looked for the first element of the datasetIds array in data, first look in the dar itself. This PR tries to maintain the same logical conditions but for the whole list instead of just the head of the array.


Have you read Terra's Contributing Guide lately? If not, do that first.

  • Label PR with a Jira ticket number and include a link to the ticket
  • Label PR with a security risk modifier [no, low, medium, high]
  • PR describes scope of changes
  • Get a minimum of one thumbs worth of review, preferably two if enough team members are available
  • Get PO sign-off for all non-trivial UI or workflow changes
  • Verify all tests go green
  • Test this change deployed correctly and works on dev environment after deployment

@rushtong rushtong changed the title WIP: [DUOS-1764][risk=no] Loop over all dataset ids instead of assuming the first [DUOS-1764][risk=no] Loop over all dataset ids instead of assuming the first Jul 19, 2022
@rushtong rushtong marked this pull request as ready for review July 19, 2022 18:17
@rushtong rushtong requested a review from a team as a code owner July 19, 2022 18:17
@shaemarks
Copy link
Contributor

When I submited a collection with two datasets, inside of the collection there were two dars. For each of these dars, datasetIds was an array with 1 dataset id, but data.datasetIds and data.datasets inside of each dar contained both datasets. Is this what is expected?

dar datasetIds (2)

dar data datasetIds

@rushtong
Copy link
Contributor Author

When I submited a collection with two datasets, inside of the collection there were two dars. For each of these dars, datasetIds was an array with 1 dataset id, but data.datasetIds and data.datasets inside of each dar contained both datasets. Is this what is expected?

In short, yes, I think this is expected.

Longer answer ... when we submit a DAR (on the server side), we still split that DAR into one DAR per dataset requested. Each individual DAR should then have a single dataset id associated with it at the DAR reference id level. At the data level of the dar, I don't think we do anything specific so it keeps in place whatever was initially created when the DAR was still a draft. However, this all happens on the server side so this PR can't have any effect on the final result of whatever we do in consent.

In a future PR, when we stop splitting the DAR up on submission, then the datasetIds on both the DAR and the data should be in alignment.

Copy link
Contributor

@shaemarks shaemarks 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!

Copy link
Contributor

@JVThomas JVThomas left a comment

Choose a reason for hiding this comment

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

Changes looks good to me

@rushtong rushtong merged commit ae44dfe into develop Jul 21, 2022
@rushtong rushtong deleted the DUOS-1764-dataset-ids branch July 21, 2022 13:56
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.

3 participants