[ENG-6669] Institutional Access project Request Modal #2433

Johnetordoff · 2024-12-17T20:48:36Z

⚠️ Needs feature/instituional_access branch and migrations to fully work.

Ticket: [https://openscience.atlassian.net/browse/ENG-6669]
Feature flag: n/a

https://openscience.atlassian.net/browse/ENG-6669

Purpose

Allows institutional admins to request access to projects from the institutional dashboard projects tab. Also allows them to message thier institutional users.

Summary of Changes

add node-request model
add node-request adapter
add two tabbed model with Send Message and Request Access tabs
add css for style
standardize casing and verbage for BccSender and MessageRecipent

Screenshot(s)

Request access disabled behavior

Screen.Recording.2024-12-17.at.10.06.33.AM.mov

Side Effects

QA Notes

I used mirage to simulate a SHARE locally here's a gist with all the unstaged files I needed to run this code:
https://gist.github.com/Johnetordoff/d32d8b6772508d7130e88d154fcd4fd7

…al dashboard

…terForOpenScience/ember-osf-web into institutional-access-project-request-modal * 'feature/institutional-access' of https://github.com/CenterForOpenScience/ember-osf-web: use proper Ember input tags, remove extra close modal command make message types an enum update language and fix modal closing bug. update to fit Product language and styling add checkboxes for reply-to and ccing Add type annotations for belongs to update text area for Ember type area update sendMessage to task fix label for message box add aira-label. fix en-us typo Add user messaging modal to user's tab on institutional dashboard

* [ENG-6823] Explicitly list columns for SHARE download (CenterForOpenScience#2450) - Ticket: [ENG-6823] - Feature flag: n/a ## Purpose - Tell SHARE what information to download when downloading institutional dashboard data ## Summary of Changes - Add a new `propertyPathKey` property to the object-list columns - this is used to tell SHARE the format of each column - Remove `sortKey` property from column, and replace it with `isSortable` boolean property since `sortKey` is the same as `propertyPathKey` * Use proper key for DOI field download (CenterForOpenScience#2455)

Update request params for SHARE download

* [ENG-6823] Explicitly list columns for SHARE download (CenterForOpenScience#2450) - Ticket: [ENG-6823] - Feature flag: n/a ## Purpose - Tell SHARE what information to download when downloading institutional dashboard data ## Summary of Changes - Add a new `propertyPathKey` property to the object-list columns - this is used to tell SHARE the format of each column - Remove `sortKey` property from column, and replace it with `isSortable` boolean property since `sortKey` is the same as `propertyPathKey` * Use proper key for DOI field download (CenterForOpenScience#2455)

…er-osf-web into institutional-access-project-request-modal * 'develop' of https://github.com/CenterForOpenScience/ember-osf-web: Bump version no. Add CHANGELOG. Feature/share download integration (CenterForOpenScience#2457)

…er-osf-web into institutional-access-project-request-modal * 'develop' of https://github.com/CenterForOpenScience/ember-osf-web: Update CHANGELOG, bump version Fix funder column propertyPathKey Bump version no. Add CHANGELOG. Feature/share download integration (CenterForOpenScience#2457)

futa-ikeda

Looks good over all! Just some minor things here and there

app/adapters/node-request.ts

futa-ikeda · 2025-01-08T18:47:18Z

app/institutions/dashboard/-components/object-list/component.ts

+    handleBackToSendMessage() {
+        this.activeTab = 'send-message';
+        this.showSendMessagePrompt = false;
+        setTimeout(() => {


Is there a reason why we have a setTimeout here? I'd like to avoid any unneeded timeouts if possible

Yes, I'll leave a comment here, I clarified this but only later on line 243, and not very well. The reason for the timeout is that ember bans having two dialogue modals open at the same time and the modal closing animation happens too slowly, so just

modal1.close() modal2.open()

opens modal2 before the modal1 close animation is complete. Causing the framework to produce an error because there are technically two open modals. Product wanted a modal on this, because the toast message is not enough.

futa-ikeda · 2025-01-08T20:01:07Z

app/institutions/dashboard/-components/object-list/component.ts

+        } catch (error) {
+            const errorDetail = error?.errors?.[0]?.detail.user || error?.errors?.[0]?.detail || '';
+            const errorCode = parseInt(error?.errors?.[0]?.status, 10);
+
+            if (errorCode === 400 && errorDetail.includes('does not have Access Requests enabled')) {
+                setTimeout(() => {
+                    this.showSendMessagePrompt = true; // Timeout to allow the modal to exit
+                }, 200);
+            } else if ([409, 400, 403].includes(errorCode)) {
+                // Handle specific errors
+                this.toast.error(errorDetail);
+            } else if (errorDetail.includes('Request was throttled')) {
+                this.toast.error(errorDetail);
+            } else if (errorDetail === 'You cannot request access to a node you contribute to.') {
+                this.toast.error(errorDetail);
+            } else {
+                this.toast.error(
+                    this.intl.t('institutions.dashboard.object-list.request-project-message-modal.message_sent_failed'),
+                );
+            }
+        } finally {
+            this.projectRequestModalShown = false; // Close the main modal
+        }


It seems like there's some redundant conditions here in this catch block as most of these seem to just result in the same toast message. Some of the conditions may be a bit fragile as well if they are doing a String.includes
and if the language returned from the API changes, these could break as well. I would recommend using getApiErrorMessage(error) and this should just find the appropriate error message from the API. Then at the end, you could just call this.toast.error(getApiErrorMessage(error), someOptionalToastTitle)

Yes this is because there isn't standardization of error messages, so some http response codes are in the payload and some are simply not there. the 429s and some 409s are not standard, however looking at this now and I think I can improve, so that it reads the response header instead of the payload.

futa-ikeda · 2025-01-08T20:06:29Z

app/institutions/dashboard/-components/object-list/component.ts

+                setTimeout(() => {
+                    this.showSendMessagePrompt = true; // Timeout to allow the modal to exit
+                }, 200);


Similar to a comment above, I would avoid using setTimeout to set page states if possible. In this case, I think you can just add this bit of logic in to the finally block like:

} finally { this.projectRequestModalShown = false; // Close the main modal if (relevantCondition) { this.showSendMessagePrompt = true; } }

As mentioned in the comment above this actual will temporarily cause where both modals are open simultaneously.

app/institutions/dashboard/-components/object-list/component.ts

futa-ikeda · 2025-01-08T20:49:33Z

app/institutions/dashboard/-components/object-list/contributors-field/styles.scss

We typically discourage the use of !important in css files, as these could lead to weird one-off styles. If you can achieve the same styles without the !important (which at first glance, I don't see anything that needs it), that would be appreciated. If it is needed, I usually leave a little comment to indicate what it is overriding or why it's needed

I keep having issues with Ember "Uppercase" component's css, especially <Button> button's need !important to override the no border rule, because (I believe) buttons had an accessibility requirement to have boarders at some point. I'll try to fix it, but I'll leave a long comment if I can't fix it.

futa-ikeda · 2025-01-08T20:52:12Z

app/institutions/dashboard/-components/object-list/styles.scss

Similarly, on this CSS file, there's a couple "mystery colors" floating around. We typically try to use color variables like $color-border-gray instead of say #ddd. You might find colors that match in app/styles/_variables.scss, so I would encourage the use of the color variables in there

app/models/node-request.ts

translations/en-us.yml

…github.com/CenterForOpenScience/ember-osf-web into institutional-access-project-request-modal * 'institutional-access-project-request-modal' of https://github.com/CenterForOpenScience/ember-osf-web: integrations fixes and refactors Update CHANGELOG, bump version Fix funder column propertyPathKey Bump version no. Add CHANGELOG. Feature/share download integration (CenterForOpenScience#2457) Bump version no. Add CHANGELOG. Feature/share download integration (CenterForOpenScience#2457) add Project request modal with user messaging tab to the institutional dashboard # Conflicts: # app/institutions/dashboard/-components/object-list/component.ts # app/institutions/dashboard/-components/object-list/template.hbs # app/models/node-request.ts

futa-ikeda

Just one translation key that I noticed. Looks good otherwise

app/institutions/dashboard/-components/object-list/contributors-field/template.hbs

translations/en-us.yml

futa-ikeda

🐍

mfraezz added 2 commits November 21, 2024 13:34

Merge branch 'hotfix/24.09.1'

51fd954

Merge branch 'release/24.10.0'

3a29a83

Johnetordoff force-pushed the institutional-access-project-request-modal branch from fa41842 to e80b7f9 Compare December 17, 2024 20:54

add Project request modal with user messaging tab to the institution…

5a7fe94

…al dashboard

Johnetordoff force-pushed the institutional-access-project-request-modal branch from e80b7f9 to 5a7fe94 Compare December 18, 2024 14:25

Johnetordoff changed the title ~~[ENG-6669] Institutional Access project Request Modal~~ [ENG-6669][WIP] Institutional Access project Request Modal Dec 18, 2024

John Tordoff and others added 13 commits December 18, 2024 09:52

Bump version no. Add CHANGELOG.

660cee2

Merge branch 'release/25.01.0'

2fe05c7

Merge tag '25.01.0' into develop

5300c97

Update request params for SHARE download

Bump version no. Add CHANGELOG.

bf8ab2c

Fix funder column propertyPathKey

d54a71d

Update CHANGELOG, bump version

dae4684

Merge branch 'hotfix/25.01.1' into develop

6fb0bee

integrations fixes and refactors

524eef0

Johnetordoff force-pushed the institutional-access-project-request-modal branch from 524eef0 to 16a4476 Compare January 7, 2025 19:39

Johnetordoff marked this pull request as ready for review January 7, 2025 19:51

add project request and messaging modal to institution dashboard

4057df3

Johnetordoff force-pushed the institutional-access-project-request-modal branch from 16a4476 to 4057df3 Compare January 7, 2025 20:20

Johnetordoff changed the title ~~[ENG-6669][WIP] Institutional Access project Request Modal~~ [ENG-6669] Institutional Access project Request Modal Jan 7, 2025

Johnetordoff requested a review from futa-ikeda January 7, 2025 20:21

futa-ikeda requested changes Jan 8, 2025

View reviewed changes

John Tordoff added 4 commits January 9, 2025 07:37

Code review cleanup and typos

4b74341

clean-up user message pane and make tasks

983ac69

use variables for colors

e7f569d

John Tordoff added 2 commits January 9, 2025 11:38

clean-up exception handling and include notes

715bee8

add translation to default value for unknown contributor

a92edbf

futa-ikeda requested changes Jan 10, 2025

View reviewed changes

app/institutions/dashboard/-components/object-list/contributors-field/template.hbs Outdated Show resolved Hide resolved

translations/en-us.yml Outdated Show resolved Hide resolved

clean-up translation keys

9a3e056

Johnetordoff force-pushed the institutional-access-project-request-modal branch from bee035c to 9a3e056 Compare January 10, 2025 20:50

futa-ikeda approved these changes Jan 10, 2025

View reviewed changes

Johnetordoff merged commit 497b301 into CenterForOpenScience:feature/institutional-access Jan 10, 2025
9 checks passed

futa-ikeda added this to the 25.02.0 milestone Jan 22, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ENG-6669] Institutional Access project Request Modal #2433

[ENG-6669] Institutional Access project Request Modal #2433

Johnetordoff commented Dec 17, 2024 •

edited

Loading

futa-ikeda left a comment

futa-ikeda Jan 8, 2025

Johnetordoff Jan 9, 2025 •

edited

Loading

futa-ikeda Jan 8, 2025

Johnetordoff Jan 9, 2025

futa-ikeda Jan 8, 2025

Johnetordoff Jan 9, 2025

futa-ikeda Jan 8, 2025

Johnetordoff Jan 9, 2025

futa-ikeda Jan 8, 2025

futa-ikeda left a comment

futa-ikeda left a comment

[ENG-6669] Institutional Access project Request Modal #2433

[ENG-6669] Institutional Access project Request Modal #2433

Conversation

Johnetordoff commented Dec 17, 2024 • edited Loading

Purpose

Summary of Changes

Screenshot(s)

Side Effects

QA Notes

futa-ikeda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Johnetordoff Jan 9, 2025 • edited Loading

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

Choose a reason for hiding this comment

futa-ikeda left a comment

Choose a reason for hiding this comment

futa-ikeda left a comment

Choose a reason for hiding this comment

Johnetordoff commented Dec 17, 2024 •

edited

Loading

Johnetordoff Jan 9, 2025 •

edited

Loading