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

[JH] Restrict Route Access #52

Merged
merged 3 commits into from
Mar 22, 2024
Merged

Conversation

hsiangj
Copy link
Collaborator

@hsiangj hsiangj commented Mar 21, 2024

Description

This code restricts route access to "List" and "Manage List" when user is not signed in. Using the <Navigate> component, user is redirected to "Home" or "/" route path to sign in.

Note: The route path to "Home" currently renders information about user's lists. This may be changed to the landing page in a future issue.

Related Issue

Closes #51
Sub-issue of #14

Acceptance Criteria

  • Users who are not signed in won't be able to navigate to "List" and "Manage List" pages via navbar or directly through the URL.

Type of Changes

Use one or more labels to help your team understand the nature of the change(s) you’re proposing. E.g., bug fix or enhancement are common ones.

Updates

Before

Successfully navigated to "Manage List" page without signing in.
Screen Shot 2024-03-21 at 2 17 33 PM

After

Page redirected to "Home" when clicking on "Manage List."
Screen Shot 2024-03-21 at 2 20 35 PM

Testing Steps / QA Criteria

  1. Without signing in, try navigate to "List" page using the navbar or direct URL (/list). Page should redirect to "Home."
  2. Repeat for "Manage List."

@hsiangj hsiangj added enhancement New feature or request suggestion a suggestion for a new issue design sprint design sprint from issue 14 labels Mar 21, 2024
Copy link

github-actions bot commented Mar 21, 2024

Visit the preview URL for this PR (updated for commit 1ad7245):

https://tcl-69-smart-shopping-list--pr52-jh-restrict-signout-cbhhbtp7.web.app

(expires Fri, 29 Mar 2024 22:36:56 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 93172cc46147b7d365c2b1b8239b61e2efb07a80

@hsiangj
Copy link
Collaborator Author

hsiangj commented Mar 21, 2024

The original idea was to implement useContext to share current user info. However, there is already a current user object from the useAuth hook set up in App.jsx.

@hsiangj hsiangj self-assigned this Mar 21, 2024
Copy link
Collaborator

@krsnamara krsnamara left a comment

Choose a reason for hiding this comment

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

Curious why you chose not to useContext?
I think the way you accomplished this was super clean and to the point.
Great work!

Copy link
Collaborator

@piecanoe piecanoe 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! Works as it should.

Copy link
Collaborator

@amalyam amalyam left a comment

Choose a reason for hiding this comment

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

Nice, works as expected!

@hsiangj hsiangj merged commit 6311467 into main Mar 22, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design sprint design sprint from issue 14 enhancement New feature or request suggestion a suggestion for a new issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restrict Route Access
4 participants