-
Notifications
You must be signed in to change notification settings - Fork 0
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
936 change id to UUID #939
Conversation
046ac6b
to
b3fc3ae
Compare
72ff2b4
to
9a89e98
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.
Nice! Looks good to me. Just one suggestion that can be tackled later.
I did some basic testing locally on the main workflow & didn't see any issues:
- Request operator access
- Approve operator access
- Update operator
- Request BORO ID
@@ -66,6 +66,7 @@ export default async function Page() { | |||
]; | |||
|
|||
const statusRows: GridRowsProp = userOperatorStatuses.map((uOS) => ({ | |||
id: uOS.id, // This unique ID is needed for DataGrid to work properly |
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.
👍
We should also remove lines 14-20 above, showing the guid in the table is not helpful
ISSUE 936
AND
ISSUE 822
AND
An issue with the Admin Requests Table which was not showing up because of removing
id
attribute in recent changes.NOTE:
orjson
package as it doesn't support 128-bit integers so we can't get UUIDs to work.(128-bit integers ijl/orjson#116 (comment))