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

repeat review #1

Open
denizen24 opened this issue Nov 27, 2023 · 0 comments
Open

repeat review #1

denizen24 opened this issue Nov 27, 2023 · 0 comments

Comments

@denizen24
Copy link

denizen24 commented Nov 27, 2023

image
1 - Ошибки линтера
а) - по данной ошибке линтинга - не считаю ее критической так как это распространенная практика в разработке и у студентов вынести password из объекта. Кто то делает иначе кто то так. бест практис - делать так
image

  б) - то что какая то часть кода не используется, а импорт описан, это тоже не считаю критичной ошибкой и эта уведомление линтера не является критичным для проетка и для принятия работы

3 - Нет лишних файлов, которые не относятся к проекту.
не корректно называть AppService - app.service.ts лишним файлом. То что он не используется в проекте конкретного студента это на совести студента. Но данный файл создается вместе с проектом автоматически и нет такого строго требования к студентам удалять его. главное что в app.module в импортах указаны все модули для домашней работы.
4 - условие !itemsId - видимо пропустил в проверке

if (!itemsId) {
      const wishes = await this.wishRepository.findBy({
        id: In(updateWishlistDto.itemsId),
      });
      wishlist["items"] = wishes;
    }

в моих тестовых прогонах данный кейс не сработал, спасибо что заметили, впредь буду внимательней
5 - При создании и редактирование вишлиста я не проверяю, что добавляю свои подарки.
нет требования по ТЗ, поэтому это замечание отмечено как - МОЖНО ЛУЧШЕ -
image
6 - Здесь не хватает дополнительной проверки и возврат типизированной ошибки про то что - Юзер не авторизован
image

7 - Здесь при создании offer необходимо добавить доп проверки
image

if (createOfferDto.amount > (wish.price - wish.raised)) {
      throw new BadRequestException(
        'Sum is greater than the amount remaining to collect the wish',
      );
    }

    if (wish.raised === wish.price) {
      throw new BadRequestException('Sum already collected');
    }

8 - Здесь в обновлении желания не хватает доп проверки
image

async updateWishById(
    user: UserProfileResponseDto,
    wishId: number,
    updateWishDto: UpdateWishDto,
  ) {
    const wish = await this.wishRepository.findOne({
      where: { owner: { id: user.id }, id: wishId, raised: 0 },
    });
    const { affected } = await this.wishRepository.update(
      { owner: { id: user.id }, id: wishId, raised: 0 },
      updateWishDto,
    );

    if (!affected)
      throw new BadRequestException(
        "Либо это не ваш подарок, либо уже есть поддержавшие",
      );

    if (wish.raised > 0 && wish.price !== undefined) {
      throw new BadRequestException(
        'You cannot change cards for which already collecting money',
      );
    }
  }

9- в удалении removeWishById тоже нужна доп проверка -
if (wish.raised > 0 && wish.price !== undefined) и сообщение об ошибке что нельзя удалить подарок на который уже собрали деньги

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant