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

Display sandbox layout to sandbox users #830

Merged
merged 9 commits into from
Jan 17, 2024
Merged

Conversation

daniellemaxwell
Copy link
Contributor

@daniellemaxwell daniellemaxwell commented Jan 16, 2024

Scope of changes

  • Adds a hook to return the user's account type.
  • Displays the SandboxLayout to sandbox users who have completed onboarding.
  • Adds a fallback to getInitials function to resolve an error experienced during development.
  • Removes duplicate getInitials function.
  • Resolves error where the org name didn't appear in the Sidebar after completing onboarding by invalidating the org list and org detail queries.

Note: A test will be added in a separate PR.

Fixes SC-23123, SC-23125, & SC-23347

Type of change

  • new feature
  • bug fix
  • documentation
  • testing
  • technical debt
  • other (describe)

Acceptance criteria

https://www.awesomescreenshot.com/video/24094232?key=a812e61e398631659e307c58a6e24836

Definition of Done

  • I have manually tested the change running it locally (having rebuilt all containers) or via unit tests
  • I have added unit and/or integration tests that cover my changes
  • I have added new test fixtures as needed to support added tests
  • I have updated the dependencies list if necessary (including updating yarn.lock and/or go.sum)
  • I have recompiled and included new protocol buffers to reflect changes I made if necessary
  • Check this box if a reviewer can merge this pull request after approval (leave it unchecked if you want to do it yourself)
  • I have notified the reviewer via Shortcut or Slack that this is ready for review
  • Front-end: Checked sm, md, lg screen resolutions for effective responsiveness
  • Backend-end: Documented service configuration changes or created related devops stories

Reviewer(s) checklist

  • Front-end: I've reviewed the Figma design and confirmed that changes match the spec.
  • Any new user-facing content that has been added for this PR has been QA'ed to ensure correct grammar, spelling, and understandability.
  • Are there any TODOs in this PR that should be turned into stories?

Copy link

This pull request has been linked to Shortcut Story #23123: Create hook to check the user's account type.

.toUpperCase();
}
export const getInitials = (name: string) => {
const nameArray = name?.split(' ') || [''];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While toggling between onboarded and not onboarded user states, I kept getting an undefined error related to this function. Using optional chaining (? after name) didn't completely resolve it, so [''] was added as a fallback.

Copy link
Contributor

Choose a reason for hiding this comment

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

the updated function is easier to follow as well - thanks for making this fix!

@@ -16,6 +16,8 @@ export function useUpdateProfile(): ProfileUpdateMutation {
queryClient.invalidateQueries({ queryKey: [RQK.MEMBER_LIST] });
queryClient.invalidateQueries({ queryKey: [RQK.MEMBER_DETAIL] });
queryClient.invalidateQueries({ queryKey: [RQK.PROFILE] });
queryClient.invalidateQueries({ queryKey: [RQK.ORG_DETAIL] });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change informs React Query that the data for these queries is now stale, due to a user completing the onboarding process, and should be re-fetched.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for noticing the issue and making the fix!

@daniellemaxwell daniellemaxwell marked this pull request as ready for review January 16, 2024 21:12
@pdamodaran pdamodaran self-requested a review January 17, 2024 16:14
Copy link
Contributor

@pdamodaran pdamodaran left a comment

Choose a reason for hiding this comment

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

looks good @daniellemaxwell!

.toUpperCase();
}
export const getInitials = (name: string) => {
const nameArray = name?.split(' ') || [''];
Copy link
Contributor

Choose a reason for hiding this comment

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

the updated function is easier to follow as well - thanks for making this fix!

@@ -16,6 +16,8 @@ export function useUpdateProfile(): ProfileUpdateMutation {
queryClient.invalidateQueries({ queryKey: [RQK.MEMBER_LIST] });
queryClient.invalidateQueries({ queryKey: [RQK.MEMBER_DETAIL] });
queryClient.invalidateQueries({ queryKey: [RQK.PROFILE] });
queryClient.invalidateQueries({ queryKey: [RQK.ORG_DETAIL] });
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for noticing the issue and making the fix!

@daniellemaxwell daniellemaxwell merged commit 83638d3 into develop Jan 17, 2024
12 checks passed
@daniellemaxwell daniellemaxwell deleted the sc-23123/acct-hook branch January 17, 2024 16:32
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.

None yet

2 participants