DON"T MERGE 1367 multiple env select #1372
Conversation
marySalvi
force-pushed
the
1367-multiple-env-select
branch
from
3650877
to
b170e6d
Compare
There was a problem hiding this comment.
I tested this branch a bit locally and found a few issues:
- I created a new submission. Once I got to the
/templatespage, I saw that "soil" was pre-checked, as expected. However I was able to uncheck this box and click the "go to next step" button. I expected that button to be disabled if no checkboxes are checked. -
- I created a new submission and selected the "soil" and "water" extensions. On the
/samplespage I see a "soil" tab and a "water" tab as expected. - In the "sample name" column I entered 1, 2, and 3 in the first three rows, respectively, of the "soil" tab.
- I clicked on the "water" tab and entered 4, 5, and 6 into the "sample name" column of the first three rows, respectively.
- I clicked on the "soil" tab again and saw the first three rows filled out as expected.
- I clicked on the "water" tab again and saw the first three rows were blank. I expected to see 4, 5, and 6 in the first column.
- I created a new submission and selected the "soil" and "water" extensions. On the
-
- I created a new submission and on the
/contextpage I selected "No" for "Have data already been generated for your study?" and "JGI" for "Are you submitting metadata for samples that will be sent to a DOE user facility?" and "CSP" for "What kind of project have you been awarded?". - On the
/multiomicspage I selected "Metagenome" in the JGI section and entered "123456" as a proposal ID. - On the
/templatespage I selected "soil" and "water". - On the
/samplespage I entered 1, 2, and 3 in the first three rows, respectively, of the "sample name" column and selected "metagenomics" for the first three rows in the "analysis/data type" column. - I clicked on the "water" tab and entered 4, 5, and 6 into the first three rows, respectively, of the "sample name" column. I selected "metagenomics" for the first three rows in the "analysis/data type" column.
- I clicked on the "JGI MG" tab and saw no rows. I think would have expected to see six rows with 1 through 6 in the first column. I say "think" because I'm not sure the behavior in this case has been clearly defined.
- I created a new submission and on the
This is mostly a note to myself, but this change will need corresponding changes in the Field Notes app. This will be a good test case for releasing coordinating changes to the backend and the app.
@mslarae13 @aclum Thoughts on correct behavior in this instance? Are there others I should loop in? |
|
you are correct, there should be 6 rows with 1-6 |
|
just looking at this but yes. the current behavior with 1 sample type for JGI or EMSL metadata & sample submission (data not generated yet) is to have samples of specific types appear in the respective user facilities sheet. |
|
@pkalita-lbl I think I have addressed all of the issues/bugs you found. Do you mind checking again? |
There was a problem hiding this comment.
I believe everything is working as expected now! I made one minor comment, but I'll mark this as approved so that we can get this on dev soon and other folks can test it.
There's a good chance I might request one or two minor backend tweaks to help ease the transition from the Field Notes perspective. But I think it'll be easier if I just open a separate PR for that later.
| for submission in submissions: | ||
| metadata_submission = submission.metadata_submission | ||
| package_name = metadata_submission.get("packageName") |
There was a problem hiding this comment.
| for submission in submissions: | |
| metadata_submission = submission.metadata_submission | |
| package_name = metadata_submission.get("packageName") | |
| for submission in submissions: | |
| metadata_submission = submission.metadata_submission | |
| package_name = metadata_submission.get("packageName", None) |
I don't see any submissions in dev that are missing the packageName, but it might be safer to add a default here just in case.
| // if we're not on the first tab, the common columns should be read-only | ||
| if (activeTemplateKey.value !== templateList.value[0]) { | ||
|
|
||
| // If the environment tab selected is a misin it should be readonly |
There was a problem hiding this comment.
| // If the environment tab selected is a misin it should be readonly | |
| // If the environment tab selected is a mixin it should be readonly |
|
@pkalita-lbl status on getting this for the app so we can merge? |
@mslarae13 I will coordinate with Patrick my other changes to runtime to make sure this can get merged either today or tomorrow. |
|
Checking in on this since the last comment was that this would be merged shortly and that was 2 weeks ago. FYI there is currently a conflict in nmdc_server/schemas_submission.py |
This is ready for a review but not ready to be merged yet.
closes #1367
Allows the ability to select multiple enviroment extensions f/k/a environment packages.
Testing:
TODO:
Before this can be merged the corresponding runtime code needs to be updated.