-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Jamkase/workspace slug #8734
Jamkase/workspace slug #8734
Conversation
I'm having trouble running this at all on docker-compose OSS. When I started with an old installation (just rebuilt images) I got the following: I could see the sidebar, just going to http://localhost:8000/ redirected me to http://localhost:8000/, not an actual browsable page. If I go to http://localhost:8000//connections or other pages like that it works though. However, if I start from scratch (not the old docker volumes) I still get redirected from http://localhost:8000/ to http://localhost:8000/ but I don't even see the sidebar: It also doesn't allow me to view anything at all and keeps on showing this page even on http://localhost:8000/36da2c49-861f-4fb4-8bb8-27d0c5a83afa/connections I assume this is just because the initial setup wizard isn't set up for this. My initial playing around looked good on my old Docker volumes except for these issues. |
@jrhizor indeed there was a problem with redirects: fixed it. Also fixed unit tests for ServiceForm that were skipped for a long time. |
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.
Looks good. Think it's mergable as is.
It would be nice if http://localhost:8000/doesnotexist
showed as a 404 instead of "Unknown error occurred".
Also, some of the setting pages may end up on different routes eventually, since a lot of the settings are instance-wide and not workspace-specific. However I think it's worth it to just merge as is and figure that out later.
-> It would be nice if http://localhost:8000/doesnotexist showed as a 404 instead of "Unknown error occurred". I think it's actually not necessary. e.g. github doesn't modify URL. I suppose seeing something like |
* Workspace slug refactoring * Migrate to react-router v6 * Update cloud routes * Redirect after auth to initally visited route * Remove redundant changes * Add error boundaries for unfound resources * Fix ServiceForm tests * Fix redirects * Fix e2e tests
What
Upgrade react-router to v6
Add redirect to the visited path in cloud version
Support workspace Slugs ( currently disabled and used ids instead )
Add ErrorBoundaries for absent resources
Comment indicators for connector update in cloud version
Closes #5227
Closes #7643
Closes #7721
Closes #6807
How
StartOverErrorView
with ResourceNotFoundErrorBoundary.