-
-
Couldn't load subscription status.
- Fork 10.3k
auth report improvements #39013
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
auth report improvements #39013
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Studio E2E Results
Artifacts: https://github.com/supabase/supabase/actions/runs/18325185338 Last updated: Tuesday 7, October, 2025 20:39:28 (UTC) |
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 don't know if this is intended behavior, but I personally find it kinda odd that this shows 0 when nothing is hovered, I would expect it to show the cumulative number
CleanShot.2025-10-02.at.17.48.55.mp4
| url += `&f={"product":{"auth":true},"status_code":{"error":true,"warning":true}}` | ||
| } | ||
|
|
||
| url += `&f={"product":{"auth":true}}` |
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.
This gets added twice for the chartId?.includes('errors') case, is that intentional?
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.
fixed thx
| )} | ||
| </div> | ||
| </> | ||
| <div className="w-full flex items-center gap-2 flex-wrap"></div> |
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.
Is this div intentional?
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.
fixed
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.
Can we centralize these? Docs uses this file which we can move to a common package: https://github.com/supabase/supabase/blob/master/apps/docs/content/errorCodes/authErrorCodes.toml
And we adapt our webpack config to read it:
supabase/apps/docs/next.config.mjs
Lines 42 to 48 in 7eb70e1
| config.module.rules.push({ | |
| test: /\.toml$/, | |
| type: 'json', | |
| parser: { | |
| parse: parseToml, | |
| }, | |
| }) |
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.
moved to separate file, will make the changes to docs in a separate PR tho.
| return whereClauses.length > 0 ? `WHERE ${whereClauses.join(' AND ')}` : '' | ||
| } | ||
|
|
||
| export const AUTH_ERROR_CODE_VALUES: string[] = [ |
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.
Can we derive this from a central definition of auth error codes? (See comment on auth.utils.ts)
| })) | ||
|
|
||
| // Reuse the status-code pivot util by category name | ||
| const { transformCategoricalCountData } = await import( |
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.
Why does this one have to be a dynamic import?
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.
ai slop that i missed 🫠
Co-authored-by: Charis <26616127+charislam@users.noreply.github.com>
Yep, that's planned but I'd like to ship it for all charts in a different PR, could get quite big. |
To test