Skip to content
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

Nv 2460 - Update Tenant Endpoint #3638

Merged

Conversation

djabarovgeorge
Copy link
Contributor

What change does this PR introduce?

Adds updated tenant metadata.

Why was this change needed?

So the user's tenants will be updated.

Other information (Screenshots)

@linear
Copy link

linear bot commented Jun 21, 2023

);

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return (await this.tenantRepository.findOne({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can not be null because we fetched it in the beginning.

Base automatically changed from NV-2459-get-tenant-by-id-endpoint to nv-2402-tenancy-crud-api June 21, 2023 16:43
…V-2460-update-tenant-endpoint

# Conflicts:
#	apps/api/src/app/tenant/tenant.controller.ts
apps/api/src/app/tenant/dtos/update-tenant-request.dto.ts Outdated Show resolved Hide resolved
apps/api/src/app/tenant/dtos/update-tenant-request.dto.ts Outdated Show resolved Hide resolved
apps/api/src/app/tenant/dtos/update-tenant-response.dto.ts Outdated Show resolved Hide resolved
updatePayload.data = command.data;
}

if (command.newIdentifier != null && command.newIdentifier !== command.identifier) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (command.newIdentifier != null && command.newIdentifier !== command.identifier) {
if (command?.newIdentifier !== command.identifier) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we need to make sure that command.newIdentifier is not undefined as well, and then check if they are not equal, because of the following case:
command?.newIdentifier !== command.identifier could be undefine !== 'current_identifier' and the if will the true but i am not sure we will want to update it with undefined

Copy link
Contributor

@LetItRock LetItRock Jun 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This TS statement command?.newIdentifier !== command.identifier is compiled to JS like (command === null || command === void 0 ? void 0 : command.newIdentifier) !== command.identifier in short it's like command.newIdentifier && command.newIdentifier !== command.identifier.

So for the undefined case it will be like undefined && command.newIdentifier !== command.identifier, in the result false.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a dedicated test to check the null/undefined updates just in case.

@djabarovgeorge djabarovgeorge merged commit db00449 into nv-2402-tenancy-crud-api Jun 28, 2023
16 of 17 checks passed
@djabarovgeorge djabarovgeorge deleted the NV-2460-update-tenant-endpoint branch June 28, 2023 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants