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.

Sign up for GitHub

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

Jump to bottom

Use org slug in place of UUIDs in app URLs #1277

Merged
merged 22 commits into from
Oct 18, 2023
Merged

Use org slug in place of UUIDs in app URLs #1277

merged 22 commits into from
Oct 18, 2023

Conversation

SuaYoo
Copy link
Member

@SuaYoo SuaYoo commented Oct 12, 2023 •
edited
Loading

Resolves #1258
Follows #1279

Changes

Breaking change: Replaces org UUID in URL/browser location bar with org slug.
Refactor: Adds shared app state utility using lit-shared-state to access org data from deep descendants.

Manual testing

Requires regression testing all features as both superadmin and regular user.

Follow-ups

Now that shared app state is implemented, we could refactor other properties on App to be properties of AppState, rather than the LitElement.

@SuaYoo SuaYoo force-pushed the frontend-org-slug-url branch from c4a459d to d85116d Compare October 16, 2023 17:48
@SuaYoo SuaYoo force-pushed the frontend-org-slug-url branch from 156da96 to 3e5a7ab Compare October 17, 2023 19:40
@SuaYoo SuaYoo marked this pull request as ready for review October 17, 2023 20:12
@SuaYoo SuaYoo requested review from ikreymer, Shrinks99 and tw4l October 17, 2023 20:12
@tw4l
Copy link
Member

tw4l commented Oct 17, 2023

After changing an org slug in Org Settings, the user is taken to a blank page with the old org slug:

Screen Shot 2023-10-17 at 5 47 03 PM

Probably we want to redirect them to the new slug.

@tw4l
Copy link
Member

tw4l commented Oct 17, 2023

I'm also unable to create a new Browser Profile, seemingly because we're passing in the org slug instead of oid in the request.

Request:

Screen Shot 2023-10-17 at 5 51 07 PM

Backend logs:

ERROR:    Exception in ASGI application
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/uvicorn/protocols/http/h11_impl.py", line 408, in run_asgi
    result = await app(  # type: ignore[func-returns-value]
  File "/usr/local/lib/python3.10/site-packages/uvicorn/middleware/proxy_headers.py", line 84, in __call__
    return await self.app(scope, receive, send)
  File "/usr/local/lib/python3.10/site-packages/fastapi/applications.py", line 208, in __call__
    await super().__call__(scope, receive, send)
  File "/usr/local/lib/python3.10/site-packages/starlette/applications.py", line 112, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/usr/local/lib/python3.10/site-packages/starlette/middleware/errors.py", line 181, in __call__
    raise exc
  File "/usr/local/lib/python3.10/site-packages/starlette/middleware/errors.py", line 159, in __call__
    await self.app(scope, receive, _send)
  File "/usr/local/lib/python3.10/site-packages/starlette/exceptions.py", line 82, in __call__
    raise exc
  File "/usr/local/lib/python3.10/site-packages/starlette/exceptions.py", line 71, in __call__
    await self.app(scope, receive, sender)
  File "/usr/local/lib/python3.10/site-packages/starlette/routing.py", line 656, in __call__
    await route.handle(scope, receive, send)
  File "/usr/local/lib/python3.10/site-packages/starlette/routing.py", line 259, in handle
    await self.app(scope, receive, send)
  File "/usr/local/lib/python3.10/site-packages/starlette/routing.py", line 61, in app
    response = await func(request)
  File "/usr/local/lib/python3.10/site-packages/fastapi/routing.py", line 216, in app
    solved_result = await solve_dependencies(
  File "/usr/local/lib/python3.10/site-packages/fastapi/dependencies/utils.py", line 525, in solve_dependencies
    solved = await call(**sub_values)
  File "/app/btrixcloud/orgs.py", line 434, in org_dep
    org = await ops.get_org_for_user_by_id(uuid.UUID(oid), user)
  File "/usr/local/lib/python3.10/uuid.py", line 177, in __init__
    raise ValueError('badly formed hexadecimal UUID string')
ValueError: badly formed hexadecimal UUID string

@ikreymer ikreymer mentioned this pull request Oct 17, 2023
Closed
@SuaYoo
Copy link
Member Author

SuaYoo commented Oct 17, 2023

After changing an org slug in Org Settings, the user is taken to a blank page with the old org slug
I'm also unable to create a new Browser Profile

Fixed!

@ikreymer
Copy link
Member

Looks good! Added a redirect from old orgId -> org slugs: 53a3ba9

ikreymer
ikreymer approved these changes Oct 18, 2023
View reviewed changes
Copy link
Member

@ikreymer ikreymer left a comment

Choose a reason for hiding this comment

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

I think this should be good to go! (will do some more testing while others review as well)
Definitely makes the app more user-friendly with nicer URLs!

Shrinks99
Shrinks99 approved these changes Oct 18, 2023
View reviewed changes
Copy link
Member

@Shrinks99 Shrinks99 left a comment

Choose a reason for hiding this comment

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

Really nice to have the org name in URLs!

@ikreymer ikreymer merged commit 4610d95 into main Oct 18, 2023
5 checks passed
@ikreymer ikreymer deleted the frontend-org-slug-url branch October 18, 2023 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tw4l tw4l tw4l approved these changes

@Shrinks99 Shrinks99 Shrinks99 approved these changes

@ikreymer ikreymer ikreymer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use org URIs in place of UUIDs in app URLs
4 participants
@SuaYoo @tw4l @ikreymer @Shrinks99