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

add create campaign agreements validation #649

Merged
merged 3 commits into from
Jun 25, 2024

Conversation

Martbul
Copy link
Contributor

@Martbul Martbul commented Jun 25, 2024

if any of the 3 agreements for creating campaign are not checked -> throw a HttpException with status 405

add validation if the 3 agreements are checked(true)
add test
update create-campaign-application.dto(acceptTermsAndConditions, transparencyTermsAccepted,personalInformationProcessingAccepted are boolean)

Motivation and context

Testing

Steps to test

New endpoints

Environment

New environment variables:

  • NEW_ENV_VAR: env var details

New or updated dependencies:

Dependency name Previous version Updated version Details
dependency/name v1.0.0 v2.0.0

if any of the 3 agreements for creating campaign are not checked -> throw a HttpException with status 405
Copy link

github-actions bot commented Jun 25, 2024

✅ Tests will run for this PR. Once they succeed it can be merged.

!createCampaignApplicationDto.transparencyTermsAccepted ||
!createCampaignApplicationDto.personalInformationProcessingAccepted
) {
throw new HttpException('All agreements must be checked', HttpStatus.METHOD_NOT_ALLOWED)
Copy link
Member

Choose a reason for hiding this comment

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

Method not allowed (405) is not the appropriate http response to respond with in such scenarios.
We've established some practice to respond with Bad Request (400) for invalid DTO content, but I suppose you could also respond with Unprocessable Content (422). Either of them are semantically fine.

http error code is now 400 in validation for all 3 agreements  in create method
changed the error from HttpException to BadRequestException for the agreements validation in create method in campaign-application.service.ts
@Martbul Martbul requested a review from sashko9807 June 25, 2024 19:51
@sashko9807 sashko9807 added the run tests Allows running the tests workflows for forked repos label Jun 25, 2024
@github-actions github-actions bot removed the run tests Allows running the tests workflows for forked repos label Jun 25, 2024
@sashko9807 sashko9807 merged commit 544c19d into podkrepi-bg:master Jun 25, 2024
10 of 11 checks passed
gparlakov pushed a commit to gparlakov/podkrepi-bg-api that referenced this pull request Jun 30, 2024
* add create campaign agreements  validation

if any of the 3 agreements for creating campaign are not checked -> throw a HttpException with status 405

* fix http error status code in campaign-application.service

http error code is now 400 in validation for all 3 agreements  in create method

* fix throw new BadRequestException

changed the error from HttpException to BadRequestException for the agreements validation in create method in campaign-application.service.ts
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

Successfully merging this pull request may close these issues.

2 participants