-
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
III-6365 - Add ownerships managements page #956
base: main
Are you sure you want to change the base?
Conversation
🔍 Existing Issues For ReviewYour pull request is modifying functions with the following pre-existing issues: 📄 File: src/pages/organizers/[organizerId]/ownerships/index.page.tsx
Did you find this useful? React with a 👍 or 👎 |
# Conflicts: # src/hooks/api/ownerships.ts # src/pages/organizers/[organizerId]/ownerships/OwnershipsTable.tsx # src/pages/organizers/[organizerId]/ownerships/index.page.tsx
# Conflicts: # src/hooks/api/ownerships.ts # src/pages/organizers/[organizerId]/ownerships/OwnershipsTable.tsx # src/pages/organizers/[organizerId]/ownerships/index.page.tsx
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 reabstraction of the logic and nice cleanup on the types as well 👏 I think this should be the last major chunk? so that's nice that the hooks and stuff will get a bit more stable now
...configuration, | ||
enabled: !!name && configuration.enabled === 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.
Should this maybe account for when enabled
is not passed as part of configuration
? I'm worried it might make enabled
always false then 🤔
const searchParams = new URLSearchParams(); | ||
if (paginationOptions) { | ||
searchParams.set('limit', `${paginationOptions.limit}`); | ||
searchParams.set('offset', `${paginationOptions.start}`); |
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.
Are the template strings to cast to string or leftover maybe?
Added
Changed
Removed
Ticket: https://jira.uitdatabank.be/browse/III-6365