Conversation
Klakurka
left a comment
Klakurka
left a comment
There was a problem hiding this comment.
Can we apply the loading spinner to the change address/name buttons like we have across the rest of the site?
utils/validators.ts
Outdated
| export const parseUpdateOrganizationPUTRequest = function (params: UpdateOrganizationPUTParameters): UpdateOrganizationInput { | ||
| if (params.userId === '' || params.userId === undefined) throw new Error(RESPONSE_MESSAGES.USER_ID_NOT_PROVIDED_400.message) | ||
| if (params.name === '' || params.name === undefined) throw new Error(RESPONSE_MESSAGES.ORGANIZATION_NAME_NOT_PROVIDED_400.message) | ||
| if ((params.name === '' || params.name === undefined) && (params.address === '' || params.address === undefined)) throw new Error(RESPONSE_MESSAGES.ORGANIZATION_NAME_NOT_PROVIDED_400.message) |
There was a problem hiding this comment.
should be in another if bracket with the appropriate error
- What's going on w/ this cancel button?...
|
lissavxo
force-pushed
the
feat/add-org-address
branch
from
9954feb to
7d44264
Compare
Fixed |
Klakurka
left a comment
Klakurka
left a comment
There was a problem hiding this comment.
Did you forget to push something?
- Still have the address input showing on the initial creation form.
- The Cancel button is still weird. The Update and Cancel buttons should be sized and positioned like we do on the dialogs.
Yup, I am sure I addressed these in my last commit |
prisma/schema.prisma
Outdated
| createdAt DateTime @default(now()) | ||
| updatedAt DateTime @updatedAt | ||
| creatorId String @unique | ||
| address String? |
There was a problem hiding this comment.
Should not be nullable. We never let a string be nullable, because the empty string can fulfill that role (unless it is a foreign key, in which case we have to do that to indicate its an optional related field).
services/organizationService.ts
Outdated
| where: { id: organization.id }, | ||
| data: { name } | ||
| data: { | ||
| ...(name !== undefined || name === '' ? { name } : {}), |
There was a problem hiding this comment.
Very hard to read, and I think that's why it's wrong — should be name !== '', not name === ''. Same thing for the address in the line below. In any case, there are more readable ways to write this logic, such as:
const updateData: Partial<Organization> = {}
if (name !== undefined && name !== '') updateData.name = name
if (address !== undefined && address !== '') updateData.address = address
return await prisma.organization.update({
where: { id: organization.id },
data: updateData,
})
utils/validators.ts
Outdated
| export const parseUpdateOrganizationPUTRequest = function (params: UpdateOrganizationPUTParameters): UpdateOrganizationInput { | ||
| if (params.userId === '' || params.userId === undefined) throw new Error(RESPONSE_MESSAGES.USER_ID_NOT_PROVIDED_400.message) | ||
| if (params.name === '' || params.name === undefined) throw new Error(RESPONSE_MESSAGES.ORGANIZATION_NAME_NOT_PROVIDED_400.message) | ||
| if ((params.name === '' || params.name === undefined) && (params.address === '' || params.address === undefined)) throw new Error(RESPONSE_MESSAGES.ORGANIZATION_MISSING_PARAMS_400.message) |
There was a problem hiding this comment.
Just a nitpick, but this error name "ORGANIZATION_MISSING_PARAMS_400" leads one to believe that the organization itself is missing these, better if it was something like MISSING_PARAMS_TO_UPDATE_ORGANIZATION_400
3 participants
Related to #1015
Description
Added address to organization
Test plan
Open the dashboard, go to account, check address config