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

Finish Interview Improvements #82

Merged
merged 11 commits into from
Feb 23, 2024
Merged

Conversation

buckhalt
Copy link
Member

@buckhalt buckhalt commented Feb 20, 2024

This PR implements several improvements to the Finish Interview functionality:

Confirm dialog UI/UX Improvements

  • Refactored to use Interview dialog system
  • Loading spinner while finish is processing added

Event tracking for InterviewCompleted

  • nodeCount and edgeCount metadata added

Option to limit participants to one interview per protocol

  • limitInterviews setting added to appSettings. Toggleable from app setup and settings page on the dashboard
  • Limiting achieved using cookies. Cookie is set when interview is completed, with the protocolId as the key. There are checks for the cookie in the /interview and /onboard routes. If cookie is present and limitInterviews is true, user is redirected to the thank you for participating page.

adds to appSettings, toggle setting on settings page
uses cookies-next to set a cookie with protocolId on complete interview. check for cookie in interview shell. if app setting limitInterviews and cookie is present, redirect to finished interview screen
Copy link

vercel bot commented Feb 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
fresco ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 23, 2024 0:43am

allows us to use built in cookies from next/headers. also puts check/redirect in onboard route so that no interviews are created if condition is met.
Copy link
Contributor

@mrkarimoff mrkarimoff left a comment

Choose a reason for hiding this comment

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

Really great work, man 👏 . I asked for a minor change above. If you think this isn't necessary, feel free to mark it as resolved

Comment on lines +18 to +25
const appSettings = await api.appSettings.get.query();

// if limitInterviews is enabled
// Check cookies for interview already completed for this user for this protocol
// and redirect to finished page
if (appSettings?.limitInterviews && cookies().get(protocolId)) {
redirect('/interview/finished');
}
Copy link
Member

Choose a reason for hiding this comment

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

In future, think about making this a single function in a separate file, that the route handler and the page can both import. That way we can update the behaviour later in a simpler way.

@jthrilly jthrilly merged commit 83a3e1d into main Feb 23, 2024
2 of 3 checks passed
@jthrilly jthrilly deleted the feature/finish-interview-improvements branch March 28, 2024 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants