-
Notifications
You must be signed in to change notification settings - Fork 12
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
refactor(dashboard): tenant cookie -> dynamic route slug #1139
Conversation
There is a warning on the server about RocksDB if you have `earliestStart` undefined & `latestStart` as `Date.now()`
@sauljabin @coltmcnealy-lh This will help us with detecting metrics for each tenant. |
really good catch, yes |
1ae1c7b
to
0eaa35e
Compare
0eaa35e
to
bd5d3b4
Compare
61ce019
to
e047bdc
Compare
e047bdc
to
8983d4b
Compare
8983d4b
to
a241471
Compare
c8f7e18
to
7cb08ee
Compare
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.
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
import getWhoAmI from './getWhoami' | ||
|
||
export default async function Home() { | ||
const whoAmI = await getWhoAmI() |
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.
I was thinking why not do this in middleware but yes Next.js doesn't support node apis in middleware.ts
resolves #1137
[tenantId]
dynamic routetenantId
to the passedhref
prop.default
tenantId
withinnext.config.mjs
redirect
&href
is accounted for with the new route to maintain functionality