add findAll(GET) method in the campaign-application.service(second try) #652
Conversation
if any of the 3 agreements for creating campaign are not checked -> throw a HttpException with status 405
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
created findAll functionality in campaign-application.service and add testing for the method
new the mocked campaign-applications in the tests have the right schema(schema of the campaign-application)
This reverts commit cb14fdf.
|
✅ Tests will run for this PR. Once they succeed it can be merged. |
| // But actually provide the service that implements the interface | ||
| useClass: SendGridNotificationsProvider, | ||
| }, | ||
| CampaignService, |
There was a problem hiding this comment.
Do we need all these providers? CampaignService? Vault Service.... and all the rest?
It looks like CampaignApplicationService is the only requirement for this PR's scope
There was a problem hiding this comment.
Keep in mind that all of these would need to be instantiated with all of their dependencies for each test!!!
Instead use mocking (see other tests or look for autoSpy usage)
| // But actually provide the service that implements the interface | ||
| useClass: SendGridNotificationsProvider, | ||
| }, | ||
| CampaignService, |
There was a problem hiding this comment.
Keep in mind that all of these would need to be instantiated with all of their dependencies for each test!!!
Instead use mocking (see other tests or look for autoSpy usage)
| @@ -20,7 +22,7 @@ export class CampaignApplicationService { | |||
| } | |||
|
|
|||
| findAll() { | |||
| return `This action returns all campaignApplication` | |||
| return this.prisma.campaignApplication.findMany() | |||
There was a problem hiding this comment.
The controller would need to be updated to check for authenticated user and allow getting only for admin/operator roles.
remove unused providers in campaign-application.spec.ts add validation if user is admin in findAll method in campaign-application.controller.ts add tests for findAll method
There was a problem hiding this comment.
Please remove commented out code
There was a problem hiding this comment.
Sure. I have missed it.
sashko9807
added
the
run tests
github-actions
bot
removed
the
run tests
|
Трябва ли да правя някакви промени? |
| @@ -19,7 +19,10 @@ export class CampaignApplicationController { | |||
| } | |||
|
|
|||
| @Get('list') | |||
| findAll() { | |||
| findAll(@AuthenticatedUser() user: KeycloakTokenParsed) { | |||
There was a problem hiding this comment.
For further information you could also use the @Roles decorator to guard specific endpoint. You can see its usage from other modules. Specifically for administrators you should look for ViewSupporters.role
Closes #{issue number}
Motivation and context
Testing
Steps to test
New endpoints
Environment
New environment variables:
NEW_ENV_VAR: env var detailsNew or updated dependencies:
dependency/namev1.0.0v2.0.0