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.

Sign up for GitHub

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

Jump to bottom

Feature: Add respondent and subject labels to DataViz responses page (M2-7855) #1964

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open

Feature: Add respondent and subject labels to DataViz responses page (M2-7855) #1964

wants to merge 12 commits into from

Conversation

LashaunnaS
Copy link
Contributor

@LashaunnaS LashaunnaS commented Oct 29, 2024 •
edited
Loading

  • Tests for the changes have been added
  • Delivered the fix or feature branches into develop or release branches via Squash and Merge (to keep clean history)

📝 Description

🔗 Jira Ticket M2-7855

Changes include:

  • Update to the useRespondenLabel hook to add appropriate labels for subject, respondent, or the users credentials without the label.
  • This also adds additional formatted translations for "subject" and "respondent" both with colons.
  • I've also added a column for displaying the Respondents credential on the DataViz response page. (tbd)

📸 Screenshots

Before (Optional) After
Screenshot 2024-11-04 at 13 35 28 Screenshot 2024-11-04 at 13 36 17

🪤 Peer Testing

  1. navigate to a users DataViz screen and select the Responses tab.
  2. confirm that the left Menu displays the Subject with the name and id.
  3. confirm that in the activity summary header, there's a "Respondent" column with the respondent for the activity listed below it.
  4. do the same for a flow summary.

@LashaunnaS
Add label for Subject as the user.
d9c41de
@LashaunnaS
Update useRespondentLabel hook to account for when a label needs to b…
9ae2153 
…e hidden as well as when we may want to to label the result as a respondent or subject. Add coloumn to display the respondent on the DataViz screen in RespondentSummary.
@aws-amplify-us-east-1 AWS Amplify (us-east-1)
Copy link

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

Access this pull request here: https://pr-1964.d19gtpld8yi51u.amplifyapp.com

@LashaunnaS LashaunnaS added the In Progress Some work in progress label Oct 30, 2024
@LashaunnaS LashaunnaS self-assigned this Oct 30, 2024
@LashaunnaS
Get the source subject for the activity or flow and to be used in the…
22a8c1c 
… useResposeLabel to denote who submitted the response.
@LashaunnaS LashaunnaS removed the In Progress Some work in progress label Nov 4, 2024
@LashaunnaS LashaunnaS marked this pull request as ready for review November 4, 2024 18:38
@farmerpaul farmerpaul self-requested a review November 4, 2024 19:49
sultanofcardio
sultanofcardio approved these changes Nov 5, 2024
View reviewed changes
Copy link
Contributor

@sultanofcardio sultanofcardio left a comment

Choose a reason for hiding this comment

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

Nice work @LashaunnaS. This PR nails the functionality, so I'm happy to approve it. I've left you some suggestions for how I think things could be improved

src/modules/Dashboard/features/RespondentData/RespondentDataReview/RespondentDataReview.tsx Show resolved Hide resolved
...oard/features/RespondentData/RespondentDataReview/ResponsesSummary/ResponsesSummary.hooks.ts Outdated Show resolved Hide resolved
src/shared/hooks/useRespondentLabel.ts Outdated Show resolved Hide resolved
farmerpaul
farmerpaul approved these changes Nov 6, 2024
View reviewed changes
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.

Great work @LashaunnaS! I just identified a couple nits, and a failing unit test that you might already be working on.

src/resources/app-fr.json Outdated Show resolved Hide resolved
src/modules/Dashboard/features/RespondentData/RespondentDataReview/RespondentDataReview.tsx Outdated
@@ -296,8 +305,12 @@ export const RespondentDataReview = () => {
/>
{!isLoading && (
<>
{selectedAnswer && responsesSummary && !error && (
<ResponsesSummary {...responsesSummary} data-testid={dataTestid} />
{selectedAnswer && responsesSummary && !!sourceSubject && !error && (
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a new test is failing in RespondentDataReview.test.tsx, maybe you're already on top of it. I'm guessing it's because the data for sourceSubject needs to get mocked into the response?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@farmerpaul I removed that additional check because I assume the sourceSubject will always be returned from the answer object. I don't believe the whole summary should not return if that fails . Is that a correct assumption?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah correct. Yes if for whatever reason the sourceSubject is missing we should still render the component. Does this fix that broken test?

...oard/features/RespondentData/RespondentDataReview/ResponsesSummary/ResponsesSummary.hooks.ts Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@farmerpaul farmerpaul farmerpaul approved these changes

@sultanofcardio sultanofcardio sultanofcardio approved these changes

Assignees

@LashaunnaS LashaunnaS

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@LashaunnaS @farmerpaul @sultanofcardio