Clicking 'add metadata' when files are uploading breaks the metadata #341

StefanBabukov · 2021-06-17T14:08:19Z

Background

Link to issue

https://biomage.atlassian.net/secure/RapidBoard.jspa?rapidView=1&projectKey=BIOMAGE&modal=detail&selectedIssue=BIOMAGE-1048

Link to staging deployment URL

https://ui-stefan-ui341.scp-staging.biomage.net/

Links to any Pull Requests related to this

Anything else the reviewers should know about the changes here

Changes

Code changes

Definition of DONE

Your changes will be ready for merging after each of the steps below have been completed:

Testing

Unit tests written
Tested locally with Inframock (with latest production data downloaded with biomage experiment pull)
Deployed to staging

To set up easy local testing with inframock, follow the instructions here: https://github.com/biomage-ltd/inframock
To deploy to the staging environment, follow the instructions here: https://github.com/biomage-ltd/biomage-utils

Documentation updates

Is all relevant documentation updated to reflect the proposed changes in this PR?

Relevant Github READMEs updated
Relevant wiki pages created/updated

Approvers

Approved by a member of the core engineering team
(UX changes) Approved by vickymorrison (this is her username, tag her if you need approval)

Just before merging:

After the PR is approved, the unstage script in here: https://github.com/biomage-ltd/biomage-utils is executed. This script cleans up your deployment to staging

Optional

Photo of a cute animal attached to this PR

aerlaut

Code looks good to me. You can merge in after removing the unused lodash import. Tested and it works. Can you explain how this fix works?

aerlaut · 2021-06-18T06:26:58Z

src/components/data-management/ProjectDetails.jsx

@@ -1,4 +1,5 @@
 import React, { useState, useEffect, useRef } from 'react';
+import _ from 'lodash';


I don't see where _ is used. I think you can remove this.

When clicking add metadata a new column is added to the table. But when uploading a file the table is updated all the time and you can see that the old changes were setting the table columns to the initial ones every time which removes the metadata popover.

vickymorrison · 2021-06-18T07:41:37Z

I can't test if it's fixed in the staging link because this error happens when trying to upload files to a new project. Is this expected?

vickymorrison · 2021-06-18T08:19:34Z

You can ignore my comment above ^^ - it's an issue with using Firefox (I'll make a ticket for it). Your fix works in Chrome.

…341) * one liner fix * remove unused library

* Deprecated check in ci.yaml * using also k8s env when retrieving identity pool * [BIOMAGE-1118] - Fix normalization option (#339) * fix normalization option * disable scTransform until implemented * [BIOMAGE-1049]Implement functionality to upload files for incomplete samples (#325) * uploading individual files * fix tests * fixing * remove comments and stuff * forgot about this * change * start workflow * refactor * changes * fixes * fixes * fix * fixes * change * enable harmony integration (#331) * enable harmony * move harmony to top * fix quibles (#340) * Retrieve errors thrown by pipeline from the state machine and display on UI (#338) * changes * fix tests * fix * [BIOMAGE-1104] Remove unneeded calls to cell clustering in qc data integ and conf embed (#335) * Remove unneeded calls to cell clustering in qc data integ and conf embed * Clean up code and replace missing plot figure to show the user * Rename data update to worker specific type * Update clustering requests to work for all clients viewing the experiment * Fix tests Co-authored-by: ogibson <[email protected]> * Clicking 'add metadata' when files are uploading breaks the metadata (#341) * one liner fix * remove unused library * Drag and dropping in Firefox fixed (#343) * small fix * change to endswith * Remove console logs (#342) Co-authored-by: ogibson <[email protected]> * [BIOMAGE-1101] Fix warnings during compilation (#344) * Move static inside public * Fix blueprintjs warnings * [BIOMAGE-1105] Tweak Data Processing step dropdown (#346) * Add step number to Data Processing dropdown * Fit Select width to longest item * Fix * Fix dropdown width (#348) * removing louvain grouping fixed (#347) * [Data Processing UX] Allow user to change settings of multiple filters without running QC pipeline (#345) * pushing changes * changes * fix * fix tests * change * run pipeline on last step only if other filters are also changed * fixing tests again * added tests * fix test * fix mt fraction plot color (#353) * improve disabled filter message (#351) * add link in message and fix plot axis (#352) * fix doublet score filter coloring (#354) * [BIOMAGE-1089] Fix launch button disabled on analysis modal (#350) * Fix Launch button disabled After editing both name and description Co-authored-by: Martin Fosco <[email protected]> * Less hacky fix * Do not re-sort clusters in categorical embedding (#356) The worker sends them in the right order * Add Select Data panel to categorical embedding (#360) * [BIOMAGE-687] - Update tooltips (#357) * change hide button tooltip * rename Tools to Genes * update gene table tooltip content * show cellset information in embedding * change ci use https instead of ssh * remove sorter tooltip from columns * [BIOMAGE-938] Improve UI responsiveness by eliminating unnecessary renders (#359) * Fix UI responsiveness problems * fix * fix * fix * fix * Restore default filter settings in Data Processing when Automatic is selected (#358) * Move reloadPlotData dispatches to the parent component in ContinuousEmbeddingPlot * Add immer * Add custom set step enabled action and replace in all relevant uses of updateProcessingSettings * Fix setQCStep * Add copy settings to all samples reducer/action * Renaming of some properties * Migrate to immerjs updateSampleFilterSettings * Rename and update many experimentSettings reducers * Fix minor bug in updateSampleFilterSettings * Update to immer processing config actions and remove default sample usage where possible * Replace updateExperimentInfo * Fix updateProcessingSettings * Fix again * Remove references to default filterSettings because they are no longe rnecessary * Fix tests * save default settings in dynamo and revert when auto is pressed * fixing tests * fix * remove console log * fix Co-authored-by: ogibson <[email protected]> * [BIOMAGE-1161] Fix slow response of frequency plot (#362) * fix-frequency * fix * disable-debug * add new illustrations for steps * fix * fix paths * Reorder to define key before usages of it (#363) Co-authored-by: Martin Fosco <[email protected]> * Test build failure notifications * Repeat testing of build failure notifications * Enable Slack build failure notifications * Remove test failure breakpoint * Change skeleton image to loading (#368) * mocking backend success response when runnning locally and having not_created pipelines (#367) * Unlog y-axis (#371) * update cellsizedistribution plot axis * switch plot numbers to improve readability * Replace qc name for real one (#374) Co-authored-by: Martin Fosco <[email protected]> * Change default selected plot to colored by cell set (#370) * Change selected plot to cellSets louvain * Fix tests Co-authored-by: Martin Fosco <[email protected]> * Show popup on changes (#364) * Move save button right and add discard button * Add alert wrapper and fix spacing * Reorder to define key before usages of it * Economize some horizontal space * Minor refactoring for increased legibility * Fix tests * Fix error run buttons width * Make marginRight more generic Co-authored-by: ogibson <[email protected]> Co-authored-by: Martin Fosco <[email protected]> * [BIOMAGE-663] Allow x-axis label rotation (#372) * Add axes setting for label rotation to frequency plot * Add for violin plot * Add axes setting for remainder of plots * Update snapshot * Set default of right property * Fix possible RuntimeError when loading frequency * Add for elbow plot * Fix axes setting titles * Introduce immerjs to experiment settings (#355) * Move reloadPlotData dispatches to the parent component in ContinuousEmbeddingPlot * Add immer * Add custom set step enabled action and replace in all relevant uses of updateProcessingSettings * Fix setQCStep * Add copy settings to all samples reducer/action * Renaming of some properties * Migrate to immerjs updateSampleFilterSettings * Rename and update many experimentSettings reducers * Fix minor bug in updateSampleFilterSettings * Update to immer processing config actions and remove default sample usage where possible * Replace updateExperimentInfo * Fix updateProcessingSettings * Fix again * Remove references to default filterSettings because they are no longe rnecessary * Fix tests * Move files to folders * Add restricions to usages in updateProcessingSettings and change name to better reflect its usage * Add original * Fix tests and update action names * Fix * Fix merge conflicts * Fix tests * Fix * Set tests back to original form * Delete duplicated reducers * Delete files * Fix accessing qc status with wrong key * Fix errors * Add error back on Co-authored-by: ogibson <[email protected]> Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Martin Fosco <[email protected]> * removed local handling because it has been moved to the api (#376) * [BIOMAGE-977] Add user details to feedback (#377) * Add user details to feedback * Simplify * Re-add React import Not sure how that happened * Handle user not being logged in * Fix several issues with frequency plot (#375) * Fix possible RuntimeError when loading frequency * Fix frequency plot not updating when config changes * Restore useEffect * [BIOMAGE-1159] Add sourcemaps to production build for easier debugging (#373) * fix * fix * fix * fix * fix * fix * fix * add sourcemap to nextjs config instead of webpack * fix * fix * fix * add-redux-debug * Enable codesigning * fix * fix * fix * fix * fix * Add reminder on navigate away (#369) * Move save button right and add discard button * Add alert wrapper and fix spacing * Reorder to define key before usages of it * Economize some horizontal space * Minor refactoring for increased legibility * Fix tests * Fix error run buttons width * Make marginRight more generic * Refactor changedFilters into redux to allow for easier access and simplify code and use it in ChangesNotAppliedModal event handlers * Add incomplete redux action for discard and minor fixes * Fix most tests * Fix more tests and minor bug in conf embedding * Tests working * Add steps to message * UX changes based on agis input * Fix calculation config of configure embedding and some tests * Fix another test * Add tests for qc pipeline * Remove commented thing * Minor changes to modal format Co-authored-by: ogibson <[email protected]> Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Martin Fosco <[email protected]> * [BIOMAGE-1125][Data Processing UX] Create tooltips and modal for Run and Discard buttons (#381) * tooltips and popup added * fix test * fix * fix * change variable name * Fixed default values (#365) * Fixed default values * fix Co-authored-by: Oliver Gibson <[email protected]> * Implement discard settings changes (#378) * Move save button right and add discard button * Add alert wrapper and fix spacing * Reorder to define key before usages of it * Economize some horizontal space * Minor refactoring for increased legibility * Fix tests * Fix error run buttons width * Make marginRight more generic * Refactor changedFilters into redux to allow for easier access and simplify code and use it in ChangesNotAppliedModal event handlers * Add incomplete redux action for discard and minor fixes * Fix most tests * Fix more tests and minor bug in conf embedding * Tests working * Fix merge conflicts * Add originalProcessing entry used to replace new changes when qc changes are discarded * Discard changes action disaptched on clicking discard button * Fix merge conflicts * Fix * Add discard button action and integrate enable/disable step * Fix * Fixed qc processing config updates * Fix tests * Remove isALocalChange variable from where it is no longe rused * Avoid overwriting defaultFilterSettings on copyFilterSettings * Minor fix * Fix copy filter settings Co-authored-by: ogibson <[email protected]> Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Martin Fosco <[email protected]> * Use switch for x-axis rotation setting (#388) * Random fixes (#349) * Fix upload select going off modal * Use EditableField for project description * Fix typo in CCA * Revert "Use EditableField for project description" This reverts commit e535878 * Change upload divider text * Make ui compatible with new gene expression (#386) * Gene expression working with new worker result * Data exploration embedding working with truncated coloring,dat exp and ps&tb heatmap, continuous embedding and violin plot working but not wiht truncated * Data exploration heatmap coloring with truncated values and displaying not truncated * Continuous embedding with truncated * Remove temporal struct change because it is no longer necessary * Fix tests * Remove line * Move zScore in test Co-authored-by: Martin Fosco <[email protected]> * Fixing README.md (#380) * Fixing README.md * Adding a link to localhost:5000 * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md Co-authored-by: Adam Kurkiewicz <[email protected]> Co-authored-by: Pol Alvarez <[email protected]> Co-authored-by: James Robinson <[email protected]> * [BIOMAGE-1155] [BIOMAGE-1164] Fix volcano plot issues (#387) * Get y-axis to show up What on earth * Get title to show up * Fix properly * Remove trailing space in y-axis title * Remove insignificant and unknown statuses Requested by @ogibson-biomage * Make sliders slightly functional * Support "auto" (==0) range * Remove xMax calculation It was always too small as a maximum, around the same as "auto" results in anyway and expensive to calculate. If the new value being a constant is bad then then revert and use the original value multiplied by 10 or something. * Increase maximum y-axis range So it's bigger than "auto" * Fix merge conflict * Remove default value for single-sample basis This fixes the problem where "All" cannot be selected. It additionally fixes the validation problem in #382. I can't yet work out yet to keep the default and properly fix these two issues. * Set diff expression default sample again * Don't use range for colour * Fix title size config reference * Edit y-axis label * [BIOMAGE-1183] Modify the expression embedding colours (#390) * Implement new colour scheme for Vega plot * Implement new colour scheme for Vitessce plot * Neaten hash assembly * Programmatically generate gradient legend * Fix import formatting * Fix test * Make hexToRgb faster * Make plots readable * Remove 'Red-Blue' scheme from continuous plot * [BIOMAGE-1189] added matomo tracking (#385) * added matomo tracking * removed console.logs * cleaning up * [BIOMAGE-1179] - Refine uuid validation when getting experiment data (#395) * init classifier kneeplot (#384) * init classifier kneeplot * add keep/discard calc * Basic style fixes * Various refactoring changes based on stefan's comment * Remove unneeded plotType * Refactor ClassifierKneePlot rendering * Fix plots rendering order * Fix crash on no data * Changes base don comments Co-authored-by: Martin Fosco <[email protected]> * easy fix (#392) * [BIOMAGE-1183] Modify the expression embedding colours (#390) * Implement new colour scheme for Vega plot * Implement new colour scheme for Vitessce plot * Neaten hash assembly * Programmatically generate gradient legend * Fix import formatting * Fix test * Make hexToRgb faster * Make plots readable * Remove 'Red-Blue' scheme from continuous plot * adapt uuid to delete prefix * add return * fix uuid length Co-authored-by: Alex Pickering <[email protected]> Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: StefanBabukov <[email protected]> Co-authored-by: James Robinson <[email protected]> Co-authored-by: Anugerah Erlaut <[email protected]> Co-authored-by: StefanBabukov <[email protected]> Co-authored-by: Alex Pickering <[email protected]> Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: ogibson <[email protected]> Co-authored-by: James Robinson <[email protected]> Co-authored-by: Marcell Pék <[email protected]> Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Marcell Pek <[email protected]> Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: ogibson-biomage <[email protected]> Co-authored-by: Oliver Gibson <[email protected]> Co-authored-by: Adam Kurkiewicz <[email protected]> Co-authored-by: Adam Kurkiewicz <[email protected]>

one liner fix

ca0e524

StefanBabukov changed the title ~~[draft]Clicking 'add metadata' when files are uploading breaks the metadata~~ Clicking 'add metadata' when files are uploading breaks the metadata Jun 17, 2021

aerlaut approved these changes Jun 18, 2021

View reviewed changes

remove unused library

f6169f8

vickymorrison approved these changes Jun 18, 2021

View reviewed changes

StefanBabukov merged commit bbb461a into develop Jun 18, 2021

StefanBabukov deleted the add-metadata-while-uploading branch June 18, 2021 09:16

ivababukova pushed a commit that referenced this pull request Jul 19, 2021

Clicking 'add metadata' when files are uploading breaks the metadata (#…

2107b51

…341) * one liner fix * remove unused library

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clicking 'add metadata' when files are uploading breaks the metadata #341

Clicking 'add metadata' when files are uploading breaks the metadata #341

StefanBabukov commented Jun 17, 2021 •

edited

Loading

aerlaut left a comment

aerlaut Jun 18, 2021

StefanBabukov Jun 18, 2021

vickymorrison commented Jun 18, 2021

vickymorrison commented Jun 18, 2021

		@@ -1,4 +1,5 @@
		import React, { useState, useEffect, useRef } from 'react';
		import _ from 'lodash';

Clicking 'add metadata' when files are uploading breaks the metadata #341

Clicking 'add metadata' when files are uploading breaks the metadata #341

Conversation

StefanBabukov commented Jun 17, 2021 • edited Loading

Background

Link to issue

Link to staging deployment URL

Links to any Pull Requests related to this

Anything else the reviewers should know about the changes here

Changes

Code changes

Definition of DONE

Testing

Documentation updates

Approvers

Just before merging:

Optional

aerlaut left a comment

Choose a reason for hiding this comment

aerlaut Jun 18, 2021

Choose a reason for hiding this comment

StefanBabukov Jun 18, 2021

Choose a reason for hiding this comment

vickymorrison commented Jun 18, 2021

vickymorrison commented Jun 18, 2021

StefanBabukov commented Jun 17, 2021 •

edited

Loading