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

refactor(dashboard): tenant cookie -> dynamic route slug #1139

Merged

Conversation

bryson-g
Copy link
Contributor

@bryson-g bryson-g commented Nov 18, 2024

resolves #1137

  • Added [tenantId] dynamic route
  • Moved file directories
  • Updated broken imports
  • Modified Navigation component to prepend tenantId to the passed href prop.
    • This was done to make the component less tedious and easier to use, but I think we may want to force the href to include the slug anyways to avoid potential confusion. LMK.
  • Support root page redirect to default tenantId within next.config.mjs
  • Ensuring every redirect & href is accounted for with the new route to maintain functionality

@hazimoarafa
Copy link
Member

@sauljabin @coltmcnealy-lh This will help us with detecting metrics for each tenant.

@sauljabin
Copy link
Member

@sauljabin @coltmcnealy-lh This will help us with detecting metrics for each tenant.

really good catch, yes

@bryson-g bryson-g marked this pull request as ready for review November 26, 2024 18:33
@sauljabin sauljabin requested a review from mijailr November 26, 2024 18:44
@hazimoarafa hazimoarafa self-requested a review November 26, 2024 18:58
@hazimoarafa hazimoarafa force-pushed the refactor-tenant-slug branch 3 times, most recently from 61ce019 to e047bdc Compare November 26, 2024 19:32
Copy link
Member

@hazimoarafa hazimoarafa left a comment

Choose a reason for hiding this comment

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

You had some missing hrefs that needed to be updated with the new [tenantId] url so I went ahead and fixed everything.

Also for future reference tenantId when being used in the code should be coming from the URL not from whoamI context or the cookie. I have already fixed that as well

dashboard/src/contexts/WhoAmIContext.tsx Outdated Show resolved Hide resolved
import getWhoAmI from './getWhoami'

export default async function Home() {
const whoAmI = await getWhoAmI()
Copy link
Member

Choose a reason for hiding this comment

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

I was thinking why not do this in middleware but yes Next.js doesn't support node apis in middleware.ts

@hazimoarafa hazimoarafa merged commit 3e67fa7 into littlehorse-enterprises:master Nov 27, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move Tenant Information to the URL
3 participants