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

fix: hide content except login when not authenticated to prevent errors #8398

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from

Conversation

Abhi1992002
Copy link
Contributor

Fixes #8112

I did this to stop errors when users aren't logged in and try to access restricted content. If they go to another page, they'll be redirected to "/login" for a smooth experience.

Changes 🏗️

  • I hid the navbar if the user is not logged in.
  • If the user visits a page without being logged in, they will be redirected to "/login".
login.flow.mp4

@Abhi1992002 Abhi1992002 requested a review from a team as a code owner October 22, 2024 16:35
@Abhi1992002 Abhi1992002 requested review from Bentlybro and aarushik93 and removed request for a team October 22, 2024 16:35
@github-actions github-actions bot added platform/frontend AutoGPT Platform - Front end platform/backend AutoGPT Platform - Back end size/l labels Oct 22, 2024
Copy link

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

8112 - Partially compliant

Fully compliant requirements:

  • Hide content for non-authenticated users
  • Prevent errors when accessing restricted content

Not compliant requirements:

  • Specific handling of Monitor, Build, and Marketplace is not evident
  • Login box visibility is not explicitly addressed
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Conditional Rendering
The entire NavBar component is now conditionally rendered based on user authentication. This might cause layout shifts or unexpected behavior for non-authenticated users.

Error Handling
The condition for redirecting to login page has been modified. Verify if this change covers all necessary scenarios for authentication failures.

Copy link

netlify bot commented Oct 22, 2024

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit 42fcca6
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/673187c87c5678000892e3d5

autogpt_platform/supabase Outdated Show resolved Hide resolved
Copy link
Member

@ntindle ntindle left a comment

Choose a reason for hiding this comment

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

Revert the supabase change

@ntindle ntindle changed the base branch from master to dev October 22, 2024 19:53
@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Oct 23, 2024
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Oct 23, 2024
@Abhi1992002
Copy link
Contributor Author

Abhi1992002 commented Oct 23, 2024

@ntindle I've removed the supabase folder.

@ntindle ntindle self-requested a review October 23, 2024 21:19
@ntindle
Copy link
Member

ntindle commented Oct 23, 2024

Sorry for the lack of clarity on my request. I meant remove your changes to it so its unaltered rather than removed

@Abhi1992002
Copy link
Contributor Author

Sorry for the mix-up—I’ve undone my changes and restored the original document.

ntindle
ntindle previously approved these changes Oct 29, 2024
Copy link
Member

@ntindle ntindle left a comment

Choose a reason for hiding this comment

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

only q for @aarushik93

.gitignore Outdated Show resolved Hide resolved
aarushik93
aarushik93 previously approved these changes Nov 6, 2024
Copy link
Contributor

@aarushik93 aarushik93 left a comment

Choose a reason for hiding this comment

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

sorry for the delay in review, this looks good, let's gooo

@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Nov 7, 2024
Copy link
Contributor

github-actions bot commented Nov 7, 2024

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflicts Automatically applied to PRs with merge conflicts platform/backend AutoGPT Platform - Back end platform/frontend AutoGPT Platform - Front end Review effort [1-5]: 2 size/m
Projects
Status: 👍🏼 Mergeable
Development

Successfully merging this pull request may close these issues.

Hide Monitor, Build, and possible Marketplace on a non-authenticated user & server auth enabled
3 participants