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

Move methods of creating multiple manifests from API to manifest generator #1333

Merged
merged 9 commits into from
Dec 13, 2023

Conversation

linglp
Copy link
Contributor

@linglp linglp commented Nov 29, 2023

Context

Move some methods that were previously in routes.py to ManifestGenerator without going through refactor. Related to FDS-1350.

Changelog

  • Move create_single_manifest function that was previously nested under get_manifest_route in routes.py to an independent function in ManifestGenerator
  • Move the logic of previously generating multiple manifests that was previously under get_manifest_route to ManifestGenerator.
  • Now, regardless the number of data_type and dataset_ids, the API could just call mg.create_manifests to generate multiple or single manifests
  • Added type hinting to create_single_manifest and create_manifests and reorganize some of the parameters

Test

  1. Generate new Patient manifest as a google sheet
  • Example data model
  • data_type: Patient
  • output_format: google_sheet
    Got expected output: a new patient manifest gets generated. The return contains one google sheet link.
  1. Generate multiple new manifests as google sheets
  • Example data model
  • data_type: Patient
  • output_format: google_sheet
    Got expected output: three manifests gets generated. The return contains three google sheet link.
  1. Generate an existing manifest as a google sheet
  1. Generate an existing manifest as an Excel sheet
  • Example data model
  • data_type: Patient
  • dataset_id: syn51547836
  • output_format: excel
    Got expected output: An excel spreadsheet gets returned and then content of the manifest is the same as the one above
  1. Generate multiple existing manifests as google sheets
  • Example data model
  • data_type: Patient, Biospecimen
  • dataset_id: syn51547836, syn51547837
  • asset_view: syn51547844
  • output_format: google_sheet
    Got expected output: Two google sheet urls got returned. (See here and here)
  1. Make sure use_annotations parameter is working
  1. Ran all the tests in test_api.py related to generating a manifest

@linglp linglp marked this pull request as ready for review December 6, 2023 20:44
@mialy-defelice
Copy link
Contributor

@andrewelamb do you think this PR is ready?

@andrewelamb
Copy link
Contributor

@mialy-defelice I have some unresolved comments from last week.

@andrewelamb andrewelamb self-requested a review December 13, 2023 20:34
@linglp linglp merged commit 6ac7fe8 into develop Dec 13, 2023
3 checks passed
@linglp linglp deleted the develop-fds-1350 branch December 13, 2023 20:38
@linglp linglp restored the develop-fds-1350 branch December 13, 2023 22:47
@linglp linglp deleted the develop-fds-1350 branch December 14, 2023 15:48
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