feat: Implement rename organization #2634
Conversation
👇 Click on the image for a new way to code review
Legend |
Abobos
force-pushed
the
feature/update-organisation
branch
2 times, most recently
from
b58bace
to
22f4c49
Compare
Abobos
changed the title
Abobos
force-pushed
the
feature/update-organisation
branch
from
22f4c49
to
d485e76
Compare
| @IsDefined() | ||
| public readonly id: string; | ||
|
|
||
| name: string; |
There was a problem hiding this comment.
@Abobos due you think we should add here some maxLen for the organization name? Would use something permissive here, but maybe providing some limit will be a good idea to not break UI and etc...
There was a problem hiding this comment.
Could you throw more perspective?.. How would this break the UI if we don't limit it?
There was a problem hiding this comment.
For example everywhere we show the organization name it can overflow (the settings dropdown, organization select) and maybe other places. This is quite and edge case to be honest. Really not a must.
There was a problem hiding this comment.
@scopsy yes. this makes sense. done
| @@ -154,4 +157,15 @@ export class OrganizationController { | |||
| }) | |||
| ); | |||
| } | |||
|
|
|||
| @Put('/') | |||
There was a problem hiding this comment.
Can we add the member role enum here:
@Roles(MemberRoleEnum.ADMIN)
There was a problem hiding this comment.
Yes. we want to restrict the update to only members of an organisation with role admin?
- Add update organization usecase, command - Create a PUT endpoint to /organizations - Add e2e test for updating the organization name
Abobos
force-pushed
the
feature/update-organisation
branch
from
d485e76
to
a5ada4e
Compare
- Restrict organization update to admin only - Add a max length to the name field
Abobos
force-pushed
the
feature/update-organisation
branch
from
a5ada4e
to
b9370b6
Compare
- Change method name - Change HTTP verb from PUT to PATCH
Abobos
changed the title
Abobos
force-pushed
the
feature/update-organisation
branch
from
9355ebe
to
60eb79a
Compare
| @IsDefined() | ||
| public readonly id: string; | ||
|
|
||
| @MaxLength(50) |
There was a problem hiding this comment.
Just noticed this. I think it is better to not limit the max length because in the creation of organization we don't limit the length and it would be a misalignment. It is a good thing to have done that, but for now we can skip it until we commit to a decision to limit org name and implement it everywhere, also at database level. 👍
There was a problem hiding this comment.
I tend to aggree that it should be better implemented across the board, was initially thinking we can only implement it here. Let me revert that rule
What change does this PR introduce?
This PR introduce an endpoint to update the name of an organization
Why was this change needed?
It is the API endpoint needed to implement Rename Organization
Other information (Screenshots)