[BI-1720] - Export Multiple Environment Datasets as Zip by mlm483 · Pull Request #263 · Breeding-Insight/bi-api

mlm483 · 2023-06-13T16:29:07Z

Description

Story: https://breedinginsight.atlassian.net/browse/BI-1720

See details of UI changes on the bi-web PR: Breeding-Insight/bi-web#320.

The behavior of the /${micronaut.bi.api.version}/programs/{programId}/experiments/{experimentId}/export{?queryParams*} endpoint was extended to handle the case when one or more environmentIds are sent in the environments query param. When multiple separate files are required to satisfy a request, they are returned as a zip file.

environments selection (UI):	`environments` query param:	expected output file:
"All Environments"	not sent	CSV/Excel with all environments
an individual environment	contains 1 environmentId	CSV/Excel with selected environment
multiple individual environments	contains multiple environmentIds	Zip containing CSV/Excel, one for each selected environment

I implemented a GET controller method for Study (StudyController::getStudies) which returns studies with the Program Key stripped from Trial, Study and Location names, as this was needed to correctly display the Study (aka Environment) names on the frontend.

I stripped the Program Key and any characters that may cause issues on different operating systems from the filenames generated for the file export.

Dependencies

Accompanying UI changes are in the bi-web feature/BI-1720 branch.

Testing

Try uploading experiments with multiple locations, with or without phenotypic data. Then, download experiment data with different options selected for "Environments", "File Format", and "With Timestamps?". Make sure the output file is the expected type (CSV/Excel/Zip) and the contents are consistent with what was uploaded. Note that ordering of rows in the output is not yet expected to be preserved, same goes for the ordering of phenotypic columns https://breedinginsight.atlassian.net/browse/BI-1819.

Checklist:

I have performed a self-review of my own code
I have tested my code and ensured it meets the acceptance criteria of the story
I have tested that my code works with both the brapi-java-server and BreedBase
I have create/modified unit tests to cover this change
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to documentation
I have run TAF: https://github.com/Breeding-Insight/taf/actions/runs/5592771430

…tudy BrAPI Study - strip program key

…tudy Cleanup

dmeidlin · 2023-07-19T20:49:38Z

src/main/java/org/breedinginsight/brapi/v2/StudyController.java

+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.MediaType;
+import io.micronaut.http.annotation.*;
+import io.micronaut.http.server.exceptions.InternalServerException;


this import is unused and can be removed

Done, 47af603.

dmeidlin · 2023-07-19T20:50:13Z

src/main/java/org/breedinginsight/brapi/v2/StudyController.java

+import org.breedinginsight.brapi.v1.controller.BrapiVersion;
+import org.breedinginsight.brapi.v2.model.request.query.StudyQuery;
+import org.breedinginsight.brapi.v2.services.BrAPIStudyService;
+import org.breedinginsight.services.exceptions.DoesNotExistException;


this import can also be removed

Done, 47af603.

dmeidlin · 2023-07-19T20:51:36Z

src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java

+import org.breedinginsight.daos.cache.ProgramCacheProvider;
+import org.breedinginsight.model.Program;
+import org.breedinginsight.services.brapi.BrAPIEndpointProvider;
+import org.breedinginsight.services.exceptions.DoesNotExistException;


import is unused and can be removed

Done, 47af603.

dmeidlin · 2023-07-19T21:03:28Z

src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java

+        Map<String, BrAPIStudy> programStudyByFullName = new HashMap<>();
+        for (BrAPIStudy study: programStudy) {
+            programStudyByFullName.put(study.getStudyName(), study);


programStudyByFullName isn't used after storing study refs and updating the studies. Can it be removed?

Done, 47af603.

dmeidlin · 2023-07-19T21:37:49Z

src/main/java/org/breedinginsight/brapi/v2/services/BrAPITrialService.java

+        return downloadFile;
+    }
+
+    private StreamedFile writeToStreamedFile(List<Column> columns, List<Map<String, Object>> data, FileType extension, String sheetName) throws IOException {


The only value passed in for sheetName is "Experiment Data'. Can this arg be removed from the method signature?

I think it's worth keeping the sheetName parameter for extensibility.

dmeidlin · 2023-07-19T21:41:42Z

src/main/java/org/breedinginsight/utilities/response/mappers/StudyQueryMapper.java

+
+    private final String defaultSortField = "studyName";
+    private final SortOrder defaultSortOrder = SortOrder.ASC;
+    private Map<String, Function<BrAPIStudy, ?>> fields;


The field can be final.

Done, 47af603.

timparsons

If I created a new environment in a second upload, that environment didn't show up to choose on the experiment download modal until I cleared the cache. Once you merge the BrAPIStudyDAO you created with the existing one (org.breedinginsight.brapps.importer.daos.BrAPIStudyDAO), then you should update the createBrAPIStudies method to create the records via the cache (example implementation is in BrAPITrialDAOImpl#createBrAPITrials).

timparsons · 2023-07-26T20:05:10Z

src/main/java/org/breedinginsight/brapi/v2/model/request/query/StudyQuery.java

+
+@Getter
+@Introspected
+public class StudyQuery extends BrapiQuery {


I think externalReferenceId and externalReferenceSource should be added to the query param, and probably change the frontend to send the request using those params instead of trialDbId

timparsons · 2023-07-26T20:27:11Z

src/main/java/org/breedinginsight/brapi/v2/dao/BrAPIStudyDAO.java

There is already a BrAPIStudyDAO in the org.breedinginsight.brapps.importer.daos package. I think this code should be merged into that class.

Done, 024f4e6.

changed when externalReferenceSource was made a different type, wasn't changed back when it was made a String again

mlm483 · 2023-07-27T21:28:24Z

If I created a new environment in a second upload, that environment didn't show up to choose on the experiment download modal until I cleared the cache. Once you merge the BrAPIStudyDAO you created with the existing one (org.breedinginsight.brapps.importer.daos.BrAPIStudyDAO), then you should update the createBrAPIStudies method to create the records via the cache (example implementation is in BrAPITrialDAOImpl#createBrAPITrials).

Thanks for this! Implemented, 88a4cd5.

mlm483 · 2023-07-27T21:35:17Z

@timparsons I updated the naming to mirror BrAPITrialDAOImpl::experimentById more closely in 6aa7cc8, what do you think of the use of Environment vs. Study?

mlm483 force-pushed the feature/BI-1720 branch from c79d080 to a46217d Compare June 30, 2023 20:12

mlm483 mentioned this pull request Jul 5, 2023

[BI-1720] - Export Multiple Environment Datasets as Zip Breeding-Insight/bi-web#320

Merged

6 tasks

mlm483 added 8 commits July 5, 2023 15:56

[BI-1720] - initial proof of concept

48bc5b9

[BI-1720] - comments

f7d5a19

[BI-1720] - refactoring in progress

867e4c9

[BI-1720] - cleaned up and fixed bugs

809eaa7

[BI-1720] - expanded unit test (WIP)

a4653dd

[BI-1720] - completed unit test

2ed7b63

[BI-1720] - updated comment

e48da59

[BI-1720] - removed unused imports

c6a6cbf

mlm483 force-pushed the feature/BI-1720 branch from e1c5fa5 to c6a6cbf Compare July 5, 2023 19:56

mlm483 added 3 commits July 5, 2023 15:59

[BI-1720] - fixed media type

cf113c7

[BI-1720] - cleaned up ExperimentExportQuery.java

a95e95f

[BI-1720] - simplified TestUtils::unzipFile

4eb2c6f

mlm483 marked this pull request as ready for review July 5, 2023 20:12

mlm483 requested review from a team, dmeidlin and timparsons and removed request for a team July 5, 2023 20:19

mlm483 assigned timparsons Jul 5, 2023

This comment was marked as outdated.

Sign in to view

mlm483 added 9 commits July 16, 2023 16:13

[BI-1720] - added BrAPI Study specific code

65675f7

[BI-1720] - TODO

7d0ea06

[BI-1720] - DRY

11172c8

[BI-1720] - study GET endpoint filtering working

84f7f54

[BI-1720] - removed unused controller method

6d3040e

[BI-1720] - updated TODO

6e0b36f

Merge pull request #274 from Breeding-Insight/feature/BI-1720-brapi-s…

18ce98f

…tudy BrAPI Study - strip program key

[BI-1720] - renamed method

be73324

[BI-1720] - removed unused methods

0342789

mlm483 added 2 commits July 18, 2023 18:02

Merge pull request #275 from Breeding-Insight/feature/BI-1720-brapi-s…

8bebb8b

…tudy Cleanup

[BI-1720] - pulled in latest from develop

cf1727d

dmeidlin reviewed Jul 19, 2023

View reviewed changes

dmeidlin approved these changes Jul 19, 2023

View reviewed changes

timparsons requested changes Jul 26, 2023

View reviewed changes

mlm483 added 5 commits July 27, 2023 12:00

[BI-1720] - clean up

47af603

[BI-1720] - reverted ListQuery change

fdb4c4d

changed when externalReferenceSource was made a different type, wasn't changed back when it was made a String again

[BI-1720] - added exRef source and ID to study query

7c0b3f7

[BI-1720] - merged redundant BrAPIStudyDAO

024f4e6

[BI-1720] - cache studies on create

88a4cd5

mlm483 requested a review from timparsons July 27, 2023 21:28

[BI-1720] - made naming more consistent

6aa7cc8

timparsons approved these changes Jul 28, 2023

View reviewed changes

Merge branch 'develop' into feature/BI-1720

f975aa2

mlm483 merged commit c2aa223 into develop Jul 28, 2023

mlm483 deleted the feature/BI-1720 branch July 28, 2023 16:44

Conversation

mlm483 commented Jun 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Dependencies

Testing

Checklist:

Uh oh!

This comment was marked as outdated.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timparsons left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlm483 commented Jul 27, 2023

Uh oh!

mlm483 commented Jul 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mlm483 commented Jun 13, 2023 •

edited

Loading

mlm483 commented Jul 27, 2023 •

edited

Loading