-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
fix(context): Redirect the user to the right page when a project was renamed #106643
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
base: master
Are you sure you want to change the base?
fix(context): Redirect the user to the right page when a project was renamed #106643
Conversation
…er to the right page on the frontend
| } | ||
| )} | ||
| </Text> | ||
| <Flex direction="column" gap="lg"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just aesthetic changes here, no changes to content. See screen recording for what it looks like.
| if (project?.slug && project.slug !== projectSlug) { | ||
| redirectToProject(project.slug); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an alternative to actually initiating this redirect, I'm also fine with just showing the project as not found. Let me know if anyone has opinions about which experience is better / which one we should provide.
I guess the redirect is good for backwards compatibility with old links, but I don't think it's a huge benefit.
| // Project exists but wasn't in store - this shouldn't normally happen | ||
| // but handle gracefully by showing not found error | ||
| this.setState({ | ||
| loading: false, | ||
| error: true, | ||
| errorType: ErrorTypes.PROJECT_NOT_FOUND, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The code incorrectly shows a "Project Not Found" error after successfully fetching a valid project from the API that was not in the local store.
Severity: HIGH
Suggested Fix
After a successful API fetch where the project slug matches, instead of setting an error state, update the component state with the fetched project data. This would likely involve calling this.setState with the new project information and setting loading to false, ensuring the project is rendered correctly.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: static/app/views/projects/projectContext.tsx#L230-L236
Potential issue: When a project is not found in the local store, an API request is made
to fetch it. If this request is successful and the returned project's slug matches the
requested slug, the code incorrectly proceeds to set an error state with `error: true`
and `errorType: ErrorTypes.PROJECT_NOT_FOUND`. This means that when a user navigates to
a valid project that isn't yet loaded in the `ProjectsStore`, they will be shown a
"Project Not Found" error instead of the project's content. The correct behavior would
be to update the state with the successfully fetched project data.
Did we get this right? 👍 / 👎 to inform future reviews.
Fixes ID-1231.
We were seeing a bug where the ownership rules page could show an infinite loading spinner when the old slug for a project that was renamed is provided. The API actually follows the redirect and gives us the up-to-date project slug, but we weren't following it. This PR updates the
ProjectContextProvidercomponent to redirect the user to the right page path in this case.Here's a screen recording of the redirect experience.
Image of the redirect modal:
