finish create functionality and testing on it #656
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.
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
improved controller testing add the create functionality add create method testing
update CampaignApplication model - organizerEmail now is not unique update create method testing now created campaig-application data is persisted to db
|
❌ Not all tests have run for this PR. Please add the |
| toEntity: new CreateCampaignApplicationDto().toEntity, | ||
| } as CreateCampaignApplicationDto | ||
|
|
||
| const mockUser = { |
There was a problem hiding this comment.
mockUser and mockAdminUser are great things to have, but they are related more to the auth module.
Consider creating a folder __mocks__ inside, src/auth, and export mockUser, and mockUserAdmin from there.
|
|
||
| describe('CampaignApplicationService', () => { | ||
| let service: CampaignApplicationService | ||
|
|
||
| const mockPerson = { | ||
| id: '3ae36b22-2c46-4038-aa17-5427e2c082e5', |
There was a problem hiding this comment.
You can import personMock from `person/mock/personMock.ts. If it is not sufficient, due to missing relation or data, you can either add your mockPerson there or update the personMock accordingly.
| controllers: [CampaignApplicationController], | ||
| providers: [CampaignApplicationService], | ||
| providers: [CampaignApplicationService, OrganizerService, PersonService], |
There was a problem hiding this comment.
Prefer importing the modules, instead of the providers. E.g. imports:[PrismaModule, OrganizerModule, PersonModule]. Reason for that is because we create new instance for the used provider each time it is used within a module.
For example if we have 5 modules, that inject the OrganizerService, 5 instances of it will be created. Where as if we import OrganizerModule to 5 modules, only a single instance is created.
| category: CampaignTypeCategory.medical, | ||
| } | ||
|
|
||
| const mockCampaigns = [ |
There was a problem hiding this comment.
Can you create __mocks__ folder and have one campaignMocks file in it, and export mockCampaigns from it?
| constructor(private readonly campaignApplicationService: CampaignApplicationService) {} | ||
| constructor( | ||
| private readonly campaignApplicationService: CampaignApplicationService, | ||
| @Inject(forwardRef(() => PersonService)) private readonly personService: PersonService, |
There was a problem hiding this comment.
@Inject won't be needed if we import the PersonModule, to the CampaignApplicationModule. Please refer to one of the other comments from more information.
| organizer = await this.organizerService.create({ | ||
| personId: person.id, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Not sure if we should create a person here. I'd say - if the organizer is missing - no new campaignApplication.
There was a problem hiding this comment.
Here I am not creating person. Initially there are no organizers, so when a person creates a new campaign Application he becomes an organizer => creating organizer with the person's id
|
|
||
| return newCampaignApplication | ||
| } catch (error) { | ||
| console.error('Error in create():', error) |
There was a problem hiding this comment.
It looks like Logger (imported from 'nestjs`) is the standard logging used in other services. Let's use that.
@sashko9807 since I'm kinda new in nestjs and looking at for example CampaignService.createCampaign - there is no explicit logging if create fails. So the question is - do we need to explicitly log or the Logger will log [class][method] Error message, error stack automatically if a prisma error appears?
|
I committed to the wrong branch, that updated the PR with wrong changes. So I closed the PR and made a new one. Sorry, I am still learning. |
update CampaignApplication model - organizerEmail now is not unique
update create method testing(cotroller and service file)
add create method functionality - now created campaign-application data is persisted to db
update createCampaignApplicationDto - add @isnotempty decorator for properties that are required
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