Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

M2-6212: Allow direct linking to activity #431

Merged
merged 9 commits into from
Apr 11, 2024

Conversation

sultanofcardio
Copy link
Contributor

@sultanofcardio sultanofcardio commented Apr 5, 2024

📝 Description

🔗 Jira Ticket M2-6212

This change allows directly linking to an activity by wrapping the < AppletDetailsPage /> component in an <AuthorizationGuard />. I also created a new reusable component called <LoginWithRedirect /> that, in combination with the <AuthorizationGuard />, automatically takes the user to the login page if they try to access a page while unauthenticated, and then redirects them to wherever they originally tried to access.

The URL takes the format https://localhost:5173/protected/applets/{appletId}?startActivityOrFlow={activityOrFlowId}

This lands the user on the page of a specific applet, and immediately starts the activity/flow specified by the ID.

I contemplated making this <LoginWithRedirect /> component the default fallback for the <AuthorizationGuard />, as that made sense to me but I opted not to do that at this time. Let me know if you think that's something worth doing.

After initial review, I ultimately decided to make the <LoginWithRedirect /> component the default fallback for the <AuthorizationGuard />

I went back and forth on this but eventually decided to leave the <AuthorizationGuard /> component without a default fallback, and avoid increasing the scope of the PR.

I also removed the isInvitationFlow parameter from the useOnLogin hook, and chose instead to rely on the existence of the backRedirectPath parameter, since it seems that's what isInvitationFlow was meant to do anyway.

📸 Screenshots

Before this change, if you tried to directly access an activity by URL, you would be taken to the applets screen after authenticating

before-2.mov

Now, you'll be redirected to the activity, provided the URL parameters you passed are valid

after-2.mov

🪤 Peer Testing

Warning

This won't work using the preview environment due to the way it currently handles client side routing. You must test this locally

First authenticate, and navigate to an activity. Then copy the URL from the browser, log out, paste the URL again and go. You should be asked to log in, after which you are taken back to the activity

@sultanofcardio sultanofcardio self-assigned this Apr 5, 2024
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-431.d15zn9do8xbzga.amplifyapp.com

@sultanofcardio sultanofcardio changed the title M2-6175: Allow hotlinking to activity M2-6175: Allow direct linking to activity Apr 5, 2024
Copy link
Contributor

@farmerpaul farmerpaul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @sultanofcardio, I like your changes. I made one suggestion and had a minor question.

Just a reminder to update the JIRA task description & AC to align with the new requirements before passing it over to QA (I'd probably just do it myself rather than wait for Suzanne, but your call).

src/widgets/ActivityDetails/ui/ActivityDetails.tsx Outdated Show resolved Hide resolved
src/entities/user/model/hooks/useOnLogin.ts Show resolved Hide resolved
src/pages/ActivityDetails/index.tsx Outdated Show resolved Hide resolved
@sultanofcardio sultanofcardio changed the title M2-6175: Allow direct linking to activity M2-6212: Allow direct linking to activity Apr 8, 2024
@sultanofcardio sultanofcardio force-pushed the feature/M2-6175-activity-hotlinking branch from 30821de to 52bb4e9 Compare April 8, 2024 21:54
@sultanofcardio sultanofcardio marked this pull request as ready for review April 8, 2024 22:35
Copy link
Contributor

@moiskillnadne moiskillnadne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All current changes look good. But when I tested it locally I found one bug. The root of the issue is when we go directly to activity we skip the step where we set the activity group in the store. The activity group data contains info about flow, timestamps and so on. When we want to submit our answers we use some data from the activity group state. So we should fix it somehow because without this data we can`t send answers.

The issue
Screenshot 2024-04-10 at 23 52 50

The slice in the store
Screenshot 2024-04-10 at 23 53 27

The place in the code where we use this data. [/src/widgets/ActivityDetails/model/hooks/useAnswers.ts]
Screenshot 2024-04-10 at 23 53 51

@sultanofcardio
Copy link
Contributor Author

@moiskillnadne thanks for the review. I believe I've fixed the error you identified. Please review again

Copy link
Contributor

@moiskillnadne moiskillnadne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. Now it`s working. I left one comment about the console log but generally approve. Thank you!

@sultanofcardio sultanofcardio merged commit 1fcd4d7 into dev Apr 11, 2024
6 checks passed
@sultanofcardio sultanofcardio deleted the feature/M2-6175-activity-hotlinking branch April 11, 2024 12:12
sultanofcardio added a commit that referenced this pull request Apr 25, 2024
This PR builds on #431 and introduces validation of the following query parameters:

- `startActivityOrFlow` - An activity or Flow ID
- `respondentId` - The respondent taking the assessment
- `subjectId` - The subject who the assessment is about

If one of these parameter is omitted, then no validation takes place and the app behaves as it does today with one exception: the activity or flow is not started. Starting the activity/flow automatically now requires the provision of all three parameters.

If they are all provided, then the following validation is performed:
- `startActivityOrFlow` identifies a valid activity
- `respondentId` matches the currently authenticated user
- `subjectId` corresponds to a real subject
- The currently authenticated user has one of these roles in the applet
  - Super Admin
  - Owner
  - Manager

If all the validation passes, then the activity/flow is started automatically and the context is stored in a multi-informant redux store.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants