[DUOS-1903][risk=low] Modify Chair Manage DAR Collections to use Summary Endpoint #1699

lu-c · 2022-07-21T19:21:54Z

Modified chair manage DAR collections to use summary endpoint
This will break the other pages that use the DAR collection table object

... sorry :(

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

connorlbark

Looks great and functioned well when I tested it! If we decide to keep the separate actions classes, then I think it's ready to have stuff merged in.

src/components/dar_collection_table/ChairActions.js

connorlbark · 2022-07-25T14:13:21Z

src/components/dar_collection_table/DarCollectionTableCellData.js

+      actionComponent = h(ChairActions, {
+        collection, showConfirmationModal, goToVote, relevantDatasets,
+        openEnabled: actions.includes('Open'),
+        cancelEnabled: actions.includes('Cancel'),
+        voteEnabled: actions.includes('Vote')
+      });


I personally don't have a strong opinion, but since we're being told what actions are acceptable from the backend, would it make sense to consolidate AdminActions, ChairActions, etc., into one Actions class which displays the actions the backend says are available? That way the frontend is more separate from the backend.

I considered doing that initially, but the ChairActions object was really complex, with further logic that controls what the state of the button should be (based on failure of request). I wasn't sure if the other Action components would also have a lot of specific logic to their state, but assumed they had to since they were split out in this way in the first place 🤔

lu-c · 2022-07-25T15:46:31Z

Looks great and functioned well when I tested it! If we decide to keep the separate actions classes, then I think it's ready to have stuff merged in.

What would you think about adding the combined actions as a separate function? I think it needs more investigation than the single ticket for conversion of the summary endpoint requires, and is a slight bit of scope creep from the original ask

connorlbark · 2022-07-25T15:48:32Z

Looks great and functioned well when I tested it! If we decide to keep the separate actions classes, then I think it's ready to have stuff merged in.

What would you think about adding the combined actions as a separate function? I think it needs more investigation than the single ticket for conversion of the summary endpoint requires, and is a slight bit of scope creep from the original ask

I definitely agree this might be getting into some scope creep! @rushtong what do you think?

rushtong · 2022-07-25T16:24:37Z

Looks great and functioned well when I tested it! If we decide to keep the separate actions classes, then I think it's ready to have stuff merged in.

What would you think about adding the combined actions as a separate function? I think it needs more investigation than the single ticket for conversion of the summary endpoint requires, and is a slight bit of scope creep from the original ask

I definitely agree this might be getting into some scope creep! @rushtong what do you think?

Agreed on the scope issue. I suggest we address refactoring the actions in a separate PR/ticket, depending on what seems the most practical.

…Summary on Console pages (#1715) * DUOS-1992 updated search bar filter function to handle DARCollectionSummary * DUOS-1992 updated util tests for search function

* 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>

…ction summaries endpoint (#1709) * Switch endpoint * Remove unused prop Co-authored-by: Shae Marks <smarks.dev1@gmail.com>

…1903

* 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>

…min 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>

…t 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>

JVThomas

Tested locally, consoles are loading the summaries quickly. Actions go through successfully and re-renders the table with the updated summary model. Search bar looks to be working as intended.

shaemarks

Dar Collection Table appearance and functionality looks great! I think DarDraftTable wasn't meant to be added here, and I left some minor suggestions for using the USER_ROLES constants more consistently, but overall 👍

shaemarks · 2022-08-11T14:35:42Z

src/components/dar_drafts_table/DarDraftTable.js

+  }
+};
+
+export default function DarDraftTable(props) {


I think this file was deleted while we worked on this feature branch and adding it back in here is unintentional

shaemarks · 2022-08-11T14:42:32Z

src/pages/AdminManageDarCollections.js

@@ -26,7 +26,7 @@ export default function AdminManageDarCollections() {
  useEffect(() => {
    const init = async() => {
      try {
-        const collectionsResp = await Collections.getCollectionsByRoleName('admin');
+        const collectionsResp = await Collections.getCollectionSummariesByRoleName('admin');


Suggested change

const collectionsResp = await Collections.getCollectionSummariesByRoleName('admin');

const collectionsResp = await Collections.getCollectionSummariesByRoleName(USER_ROLES.admin);

nit: use existing user role constant

shaemarks · 2022-08-11T14:45:17Z

src/pages/ChairConsole.js

@@ -29,7 +29,7 @@ export default function ChairConsole(props) {
    const init = async() => {
      try {
        const [collections, datasets] = await Promise.all([
-          Collections.getCollectionsByRoleName('chairperson'),
+          Collections.getCollectionSummariesByRoleName('chairperson'),


Suggested change

Collections.getCollectionSummariesByRoleName('chairperson'),

Collections.getCollectionSummariesByRoleName(USER_ROLES.chairperson),

shaemarks · 2022-08-11T14:45:46Z

src/pages/MemberConsole.js

@@ -27,7 +27,7 @@ export default function MemberConsole(props) {
    const init = async () => {
      try {
        const [collections, datasets] = await Promise.all([
-          Collections.getCollectionsByRoleName('member'),
+          Collections.getCollectionSummariesByRoleName('member'),


Suggested change

Collections.getCollectionSummariesByRoleName('member'),

Collections.getCollectionSummariesByRoleName(USER_ROLES.member),

shaemarks · 2022-08-11T14:46:20Z

src/pages/ResearcherConsole.js

-      }
-      setResearcherCollections(collectionArray);
-      setFilteredList(collectionArray);
+      const collections = await Collections.getCollectionSummariesByRoleName('Researcher');


Suggested change

const collections = await Collections.getCollectionSummariesByRoleName('Researcher');

const collections = await Collections.getCollectionSummariesByRoleName(USER_ROLES.researcher);

shaemarks · 2022-08-11T14:50:40Z

src/components/dar_collection_table/DarCollectionTableCellData.js

  return {
-    data: datasets.length > 0 ? datasets.length : '- -',
+    data: datasetIds.length > 0 ? datasetIds.length : '- -',


optional nit: dar collection summaries have a datasetCount field that could be used instead

Currently updating the branch, doesn't make sense to review myself

shaemarks

Looks good to me! Tested the DarCollectionTables on all consoles and actions.

JVThomas

Tested locally, feature branch looks to be in good working condition

[DUOS-1903] Updated dar collection table for chair console view

569a211

lu-c requested a review from a team as a code owner July 21, 2022 19:21

lu-c marked this pull request as draft July 22, 2022 13:43

[DUOS-1903] Updated unit tests

9b2756e

lu-c marked this pull request as ready for review July 22, 2022 17:32

connorlbark reviewed Jul 25, 2022

View reviewed changes

[DUOS-1903] added votel abel back

6b2ee9c

lu-c requested a review from connorlbark July 25, 2022 15:46

lu-c and others added 9 commits July 26, 2022 19:35

merged with develop, fixed merge issue

dfadfc9

DUOS-1992[risk=no] Update search bar function to handle DARCollection…

cb4ce35

…Summary on Console pages (#1715) * DUOS-1992 updated search bar filter function to handle DARCollectionSummary * DUOS-1992 updated util tests for search function

[DIOS-1906][risk=no] modify signing official console to use new colle…

d93c266

…ction summaries endpoint (#1709) * Switch endpoint * Remove unused prop Co-authored-by: Shae Marks <smarks.dev1@gmail.com>

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

c510532

…1903

DUOS-1903 resolved merge conflicts

63763f2

JVThomas previously approved these changes Aug 11, 2022

View reviewed changes

shaemarks reviewed Aug 11, 2022

View reviewed changes

DUOS-1903 pr feedback

c48a0d4

JVThomas requested a review from shaemarks August 12, 2022 13:55

shaemarks approved these changes Aug 12, 2022

View reviewed changes

JVThomas approved these changes Aug 15, 2022

View reviewed changes

JVThomas merged commit 7bf04bd into develop Aug 15, 2022

JVThomas deleted the DUOS-1903 branch August 15, 2022 11:58

JVThomas mentioned this pull request Aug 15, 2022

DUOS-1912-draft #1748

Closed

This was referenced Aug 15, 2022

[DUOS-1902][risk=no] Use DAR Collection on Admin Console Page #1712

Closed

[DUOS-1905][risk=no] Update Researcher Console to use Summary Endpoint #1708

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DUOS-1903][risk=low] Modify Chair Manage DAR Collections to use Summary Endpoint #1699

[DUOS-1903][risk=low] Modify Chair Manage DAR Collections to use Summary Endpoint #1699

lu-c commented Jul 21, 2022 •

edited

Loading

connorlbark left a comment

connorlbark Jul 25, 2022

lu-c Jul 25, 2022

lu-c commented Jul 25, 2022

connorlbark commented Jul 25, 2022

rushtong commented Jul 25, 2022

JVThomas left a comment

shaemarks left a comment

shaemarks Aug 11, 2022

shaemarks Aug 11, 2022

shaemarks Aug 11, 2022

shaemarks Aug 11, 2022

shaemarks Aug 11, 2022

shaemarks Aug 11, 2022

shaemarks left a comment

JVThomas left a comment

	const collectionsResp = await Collections.getCollectionSummariesByRoleName('admin');
	const collectionsResp = await Collections.getCollectionSummariesByRoleName(USER_ROLES.admin);

	Collections.getCollectionSummariesByRoleName('chairperson'),
	Collections.getCollectionSummariesByRoleName(USER_ROLES.chairperson),

	const collections = await Collections.getCollectionSummariesByRoleName('Researcher');
	const collections = await Collections.getCollectionSummariesByRoleName(USER_ROLES.researcher);

[DUOS-1903][risk=low] Modify Chair Manage DAR Collections to use Summary Endpoint #1699

[DUOS-1903][risk=low] Modify Chair Manage DAR Collections to use Summary Endpoint #1699

Conversation

lu-c commented Jul 21, 2022 • edited Loading

connorlbark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lu-c commented Jul 25, 2022

connorlbark commented Jul 25, 2022

rushtong commented Jul 25, 2022

JVThomas left a comment

Choose a reason for hiding this comment

shaemarks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shaemarks left a comment

Choose a reason for hiding this comment

JVThomas left a comment

Choose a reason for hiding this comment

lu-c commented Jul 21, 2022 •

edited

Loading