DUOS-2014[risk=no] Updated Researcher Actions functionality to account for DarCollectionSummary #1744

JVThomas · 2022-08-10T13:11:35Z

Addresses DUOS-2014

PR updates the functions around Researcher actions to account for the new DarCollectionSummary format. PR also includes import corrections to applyHoverStyle in utils

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

Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com>

…S-1904

Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com>

…button)

Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com>

…S-1904

…to use new summary endpoint, removed formatDraft call for revise

… updated url reference in summary fetch method

lu-c · 2022-08-10T13:34:20Z

src/libs/utils.js

@@ -1,7 +1,7 @@
 import Noty from 'noty';
 import 'noty/lib/noty.css';
 import 'noty/lib/themes/bootstrap-v3.css';
-import {map as nonFPMap} from 'lodash';
+import {map as nonFPMap, forEach as nonFPForEach} from 'lodash';


would you feel okay with leaving these as the original names? It seems less confusing that way

So there's two forEach's being used in this file, one from 'lodash/fp' and one from 'lodash'. The reason being is that with the fp version, the function is forEach(collection, (value) => //code here) where as in the regular version its forEach(value, key) => //insert code here)(collection).

As for why we need two versions, remember the error Shae pointed out here: #1718 (review)

It's being caused by this function in utils.js

export const applyHoverEffects = (e, style) => { nonFPForEach((key, value) => { e.target.style[key] = value; })(style); };

What had happened was that the normal forEach that was used here was replaced with the fp variant. The difference in the iterating function causes the function to error out.

So in short, we need both functions, but they can't be named the same variable, hence the nonFPForEach.

That said, I still need to update the iterating function to match the non-fp version.

Ok! Didnt realize it was because both lodash and lodash/fp were beign used.... That being said, I still think the name is confusing (just because it wasn't immediately obvious to me what it was "nonFP" was supposed to mean. Don't know what to do about that though

maybe it could be lodashForEach and lodashFpForEach ? that way they mirror each other

Another way to solve this is to import a prefix for either the non-fp or the fp version. In the past, I've used import _ from 'lodash'; and import * as fp from 'lodash/fp';, for example

Yeah but that would cause Webpack to import the entire lodash library on file bundling, and lodash is a pretty big library. Even if the code is minified, I'd rather not import methods that are never used for a particular file.

maybe it could be lodashForEach and lodashFpForEach ? that way they mirror each other

That works for me

…n-fp forEach arguments

shaemarks

Actions are all functioning as expected 👍

I think the header of the CollectionConfirmationModal needs to be updated as well - currently it always says undefined as the project title

connorlbark

worked well in ui! other than the weird naming of the forEach & modal issue, 👍

… on applyStyleOnEnter, updated aliases for forEach methods in utils.js

shaemarks

Changes look good to me!

shaemarks · 2022-08-10T16:01:08Z

src/components/dar_collection_table/CollectionConfirmationModal.js

@@ -7,7 +7,7 @@ import {getProjectTitle} from './DarCollectionTable';
 export default function CollectionConfirmationModal(props) {
  const {collection, showConfirmation, setShowConfirmation, cancelCollection, reviseCollection, openCollection, consoleAction, deleteDraft} = props;

-  const getModalHeader = () => {
+  const getModalHeader = (collection) => {
    if(!isNil(collection)) {
      return `${collection.darCode} - ${getProjectTitle(collection)}`;


Optional nit: I don't see getProjectTitle used anywhere but here, so maybe inline collection.name here and delete the function

connorlbark

👍 changes since i last reviewed look good to me

…ary Endpoint (#1699) * [DUOS-1903] Updated dar collection table for chair console view * [DUOS-1903] Updated unit tests * [DUOS-1903] added votel abel back * DUOS-1992[risk=no] Update search bar function to handle DARCollectionSummary on Console pages (#1715) * DUOS-1992 updated search bar filter function to handle DARCollectionSummary * DUOS-1992 updated util tests for search function * [DUOS-1904] Added uniform Actions component (#1710) * fixed merge issue * ???? Testing new actions component. * updated unit tests? * Fixed eslint * fix admin console * fix tests * Fixing unit tests * Apply suggestions from code review Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> * added unit test for Update button (used to be combined with the vote button) * Update cypress/component/DarCollectionTable/admin_actions.spec.js Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> * fix codacy * Fixed unit tests * Fixed unit tests? * merged with DUOS-1902, deleted unnecessary files Co-authored-by: Connor Barker <connorlbark@gmail.com> Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> * [DIOS-1906][risk=no] modify signing official console to use new collection summaries endpoint (#1709) * Switch endpoint * Remove unused prop Co-authored-by: Shae Marks <smarks.dev1@gmail.com> * [DUOS-1904] Added researcher actions to Actions component (#1718) * update researcher console dar table + actions * naming nit * fix tests * Apply suggestions from code review Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> * Update cypress/component/DarCollectionTable/researcher_actions.spec.js Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> * fixed merge issue * ???? Testing new actions component. * updated unit tests? * Fixed eslint * fix name * fix admin console * fix tests * Fixing unit tests * Apply suggestions from code review Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> * added unit test for Update button (used to be combined with the vote button) * Update cypress/component/DarCollectionTable/admin_actions.spec.js Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> * fix codacy * Fixed unit tests * Fixed unit tests? * merged with DUOS-1902, deleted unnecessary files * Merged with 1905 * Fixing unit tests? * Fixing eslint * Fixing unit test, but dont think this will sovle the problem * Fixing unit test, but dont think this will sovle the problem * Fixing unit test, but dont think this will sovle the problem * feeling more confident now that the problem will be solved haha * Updated variable names, due to merge conflict discrepancies! * Updated variable names, due to merge conflict discrepancies! * testing cypress tests on github * Checking original researcher unit test. Getting mildly frustrated! * Converted hard coded researcher to console type * fixing? * fix test * removed unnecessary line * re-added actions (sorry connor, i deleted it) * [DUOS-1904] updated deleteDrafts to delete a draft with multiple referenceIds (previously only had one) * DUOS-setup-proxy initial setup of local proxy * [DUOS-1904] fixed eslint * DUOS-setup-proxy added status urls to proxy * DUOS-setup-proxy updated ontologyService url fetch * DUOS-setup-proxy fixed method reference in getOntologyUrl * DUOS-setup-proxy added comments, commented out notification proxy for now * [DUOS-1904] reverted referenceIds so we just delete the first one * DUOS-2011 [risk=no] Local Proxy setup for npm start (#1735) * DUOS-setup-proxy initial setup of local proxy * DUOS-setup-proxy added status urls to proxy * DUOS-setup-proxy updated ontologyService url fetch * DUOS-setup-proxy fixed method reference in getOntologyUrl * DUOS-setup-proxy added comments, commented out notification proxy for now * [DUOS-1973][risk=no]Change research purpose to Research Use Statement (Narrative) (#1738) * Change research purpose to Research Use Statement (Narrative) * Fix test Co-authored-by: Shae Marks <smarks.dev1@gmail.com> * added "breathing" margins on buttons * [DUOS-1904] Cleaning up more. * [DUOS-1904] fixed the weird bug with applyign style bug Co-authored-by: Connor Barker <connorlbark@gmail.com> Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> Co-authored-by: JVThomas <jthomas0209@gmail.com> Co-authored-by: Justin Variath Thomas <JVThomas@users.noreply.github.com> Co-authored-by: shaemarks <81024249+shaemarks@users.noreply.github.com> Co-authored-by: Shae Marks <smarks.dev1@gmail.com> * DUOS-1997[risk=no] Updated Open and Close Collection functions for Admin and Chair Consoles (#1723) * update researcher console dar table + actions * naming nit * fix tests * Apply suggestions from code review Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> * Update cypress/component/DarCollectionTable/researcher_actions.spec.js Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> * fixed merge issue * ???? Testing new actions component. * updated unit tests? * Fixed eslint * fix name * fix admin console * fix tests * Fixing unit tests * Apply suggestions from code review Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> * added unit test for Update button (used to be combined with the vote button) * Update cypress/component/DarCollectionTable/admin_actions.spec.js Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> * fix codacy * Fixed unit tests * Fixed unit tests? * merged with DUOS-1902, deleted unnecessary files * Merged with 1905 * Fixing unit tests? * Fixing eslint * Fixing unit test, but dont think this will sovle the problem * Fixing unit test, but dont think this will sovle the problem * Fixing unit test, but dont think this will sovle the problem * feeling more confident now that the problem will be solved haha * DUOS-1997 updated open and close collection functions for Admin and Chair pages * DUOS-1997 updated integration tests for collection callback functions * Updated variable names, due to merge conflict discrepancies! * Updated variable names, due to merge conflict discrepancies! * testing cypress tests on github * Checking original researcher unit test. Getting mildly frustrated! * Converted hard coded researcher to console type * fixing? * fix test * removed unnecessary line * re-added actions (sorry connor, i deleted it) * DUOS-1997 used USER_ROLES instead of explicit string values * DUOS-1997 updated spec to use USER_ROLES instead of hard coded strings Co-authored-by: Connor Barker <connorlbark@gmail.com> Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> Co-authored-by: clu@asymmetrik.com <clu@asymmetrik.com> * DUOS-2014[risk=no] Updated Researcher Actions functionality to account for DarCollectionSummary (#1744) * update researcher console dar table + actions * naming nit * fix tests * Apply suggestions from code review Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> * Update cypress/component/DarCollectionTable/researcher_actions.spec.js Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> * fixed merge issue * ???? Testing new actions component. * updated unit tests? * Fixed eslint * fix name * fix admin console * fix tests * Fixing unit tests * Apply suggestions from code review Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> * added unit test for Update button (used to be combined with the vote button) * Update cypress/component/DarCollectionTable/admin_actions.spec.js Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> * fix codacy * Fixed unit tests * Fixed unit tests? * merged with DUOS-1902, deleted unnecessary files * Merged with 1905 * Fixing unit tests? * Fixing eslint * Fixing unit test, but dont think this will sovle the problem * Fixing unit test, but dont think this will sovle the problem * Fixing unit test, but dont think this will sovle the problem * feeling more confident now that the problem will be solved haha * DUOS-1997 updated open and close collection functions for Admin and Chair pages * DUOS-1997 updated integration tests for collection callback functions * Updated variable names, due to merge conflict discrepancies! * Updated variable names, due to merge conflict discrepancies! * testing cypress tests on github * Checking original researcher unit test. Getting mildly frustrated! * Converted hard coded researcher to console type * fixing? * fix test * removed unnecessary line * re-added actions (sorry connor, i deleted it) * DUOS-1997 used USER_ROLES instead of explicit string values * DUOS-1997 updated spec to use USER_ROLES instead of hard coded strings * DUOS-2014 fixed missing forEach immport from lodash, updated actions to use new summary endpoint, removed formatDraft call for revise * DUOS-2014 adjusted tooltip on cancel action, removed unused function, updated url reference in summary fetch method * DUOS-2014 updated iterating function on applyHoverEffects to match non-fp forEach arguments * DUOS-2014 updated getProjectTitle helper method, updated style update on applyStyleOnEnter, updated aliases for forEach methods in utils.js * DUOS-2014 removed getProjectTitle, inlined collection name in string Co-authored-by: Connor Barker <connorlbark@gmail.com> Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> Co-authored-by: clu@asymmetrik.com <clu@asymmetrik.com> * DUOS-1903 pr feedback Co-authored-by: Justin Variath Thomas <JVThomas@users.noreply.github.com> Co-authored-by: Connor Barker <connorlbark@gmail.com> Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com> Co-authored-by: shaemarks <81024249+shaemarks@users.noreply.github.com> Co-authored-by: Shae Marks <smarks.dev1@gmail.com> Co-authored-by: JVThomas <jthomas0209@gmail.com>

connorlbark and others added 30 commits July 25, 2022 15:40

Merge branch 'develop' into DUOS-1905

78cad95

update researcher console dar table + actions

92a91e3

naming nit

30bd225

fix tests

2b00ec3

Apply suggestions from code review

feb667d

Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com>

Update cypress/component/DarCollectionTable/researcher_actions.spec.js

6d527a7

Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com>

merged with develop

abae302

fixed merge issue

207b559

???? Testing new actions component.

738971b

updated unit tests?

049cbb0

Fixed eslint

139bd50

Merge branch 'DUOS-1903' of github.com:DataBiosphere/duos-ui into DUO…

3e57b6a

…S-1904

Merge branch 'DUOS-1903' into DUOS-1905

b4ba695

fix name

40ddc16

fix admin console

b0038ac

fix tests

608715a

Fixing unit tests

dd377b2

Apply suggestions from code review

18b85e0

Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com>

added unit test for Update button (used to be combined with the vote …

9fe3206

…button)

Update cypress/component/DarCollectionTable/admin_actions.spec.js

b69adb8

Co-authored-by: Gregory Rushton <rushtong@users.noreply.github.com>

fix codacy

df99fb2

Fixed unit tests

6138a59

Fixed unit tests?

b5d84dc

Merge branch 'DUOS-1902' of github.com:DataBiosphere/duos-ui into DUO…

fc96fd7

…S-1904

merged with DUOS-1902, deleted unnecessary files

60fea9b

Merge branch 'DUOS-1905' of github.com:DataBiosphere/duos-ui into DUO…

7e8e79c

…S-1904

Merged with 1905

3fe5316

Fixing unit tests?

aa1bbf2

Fixing eslint

e825982

Fixing unit test, but dont think this will sovle the problem

fb8c1e1

lu-c and others added 9 commits August 1, 2022 16:46

fixing?

e55aeb1

fix test

0f7e151

removed unnecessary line

635734b

re-added actions (sorry connor, i deleted it)

d997737

Merge branch 'DUOS-1904-2' into DUOS-1997

fc1c330

DUOS-1997 used USER_ROLES instead of explicit string values

b96e81d

DUOS-1997 updated spec to use USER_ROLES instead of hard coded strings

8db6702

DUOS-1997 resolved merge conflicts

9d834bd

DUOS-2014 fixed missing forEach immport from lodash, updated actions …

5eb5e35

…to use new summary endpoint, removed formatDraft call for revise

JVThomas changed the base branch from develop to DUOS-1903 August 10, 2022 13:11

JVThomas added 2 commits August 10, 2022 09:25

DUOS-2014 adjusted tooltip on cancel action, removed unused function,…

6108db9

… updated url reference in summary fetch method

DUOS-2014 resolved merge conflicts

03bad5a

lu-c reviewed Aug 10, 2022

View reviewed changes

DUOS-2014 updated iterating function on applyHoverEffects to match no…

85008d0

…n-fp forEach arguments

JVThomas marked this pull request as ready for review August 10, 2022 14:14

JVThomas requested a review from a team as a code owner August 10, 2022 14:14

shaemarks reviewed Aug 10, 2022

View reviewed changes

connorlbark approved these changes Aug 10, 2022

View reviewed changes

DUOS-2014 updated getProjectTitle helper method, updated style update…

fe13e7a

… on applyStyleOnEnter, updated aliases for forEach methods in utils.js

JVThomas requested review from shaemarks, lu-c, rushtong and connorlbark August 10, 2022 15:44

shaemarks approved these changes Aug 10, 2022

View reviewed changes

DUOS-2014 removed getProjectTitle, inlined collection name in string

de0ce7a

connorlbark approved these changes Aug 10, 2022

View reviewed changes

lu-c approved these changes Aug 10, 2022

View reviewed changes

JVThomas merged commit 9c91328 into DUOS-1903 Aug 10, 2022

JVThomas deleted the DUOS-2014 branch August 10, 2022 20:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DUOS-2014[risk=no] Updated Researcher Actions functionality to account for DarCollectionSummary #1744

DUOS-2014[risk=no] Updated Researcher Actions functionality to account for DarCollectionSummary #1744

JVThomas commented Aug 10, 2022

lu-c Aug 10, 2022

JVThomas Aug 10, 2022 •

edited

Loading

lu-c Aug 10, 2022

connorlbark Aug 10, 2022

rushtong Aug 10, 2022

JVThomas Aug 10, 2022

JVThomas Aug 10, 2022

shaemarks left a comment

connorlbark left a comment •

edited

Loading

shaemarks left a comment

shaemarks Aug 10, 2022

connorlbark left a comment

DUOS-2014[risk=no] Updated Researcher Actions functionality to account for DarCollectionSummary #1744

DUOS-2014[risk=no] Updated Researcher Actions functionality to account for DarCollectionSummary #1744

Conversation

JVThomas commented Aug 10, 2022

lu-c Aug 10, 2022

Choose a reason for hiding this comment

JVThomas Aug 10, 2022 • edited Loading

Choose a reason for hiding this comment

lu-c Aug 10, 2022

Choose a reason for hiding this comment

connorlbark Aug 10, 2022

Choose a reason for hiding this comment

rushtong Aug 10, 2022

Choose a reason for hiding this comment

JVThomas Aug 10, 2022

Choose a reason for hiding this comment

JVThomas Aug 10, 2022

Choose a reason for hiding this comment

shaemarks left a comment

Choose a reason for hiding this comment

connorlbark left a comment • edited Loading

Choose a reason for hiding this comment

shaemarks left a comment

Choose a reason for hiding this comment

shaemarks Aug 10, 2022

Choose a reason for hiding this comment

connorlbark left a comment

Choose a reason for hiding this comment

JVThomas Aug 10, 2022 •

edited

Loading

connorlbark left a comment •

edited

Loading