-
Notifications
You must be signed in to change notification settings - Fork 33
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
Added session feedback controller and service with tests for error an… #520
Changes from all commits
0786b4f
1353ee2
cefb184
6c6a24e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import { ApiProperty } from '@nestjs/swagger'; | ||
import { IsEnum, IsNotEmpty, IsString } from 'class-validator'; | ||
import { FEEDBACK_TAGS_ENUM } from 'src/utils/constants'; | ||
|
||
export class SessionFeedbackDto { | ||
@IsNotEmpty() | ||
@IsString() | ||
@ApiProperty({ type: String }) | ||
sessionFeedbackId: string; | ||
|
||
@IsNotEmpty() | ||
@IsString() | ||
@ApiProperty({ type: String }) | ||
sessionId: string; | ||
|
||
@IsNotEmpty() | ||
@IsEnum(FEEDBACK_TAGS_ENUM) | ||
@ApiProperty({ | ||
enum: FEEDBACK_TAGS_ENUM, | ||
type: String, | ||
example: Object.values(FEEDBACK_TAGS_ENUM), | ||
}) | ||
feedbackTags: FEEDBACK_TAGS_ENUM; | ||
|
||
@IsNotEmpty() | ||
@IsString() | ||
@ApiProperty({ type: String }) | ||
feedbackDescription: string; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import { Body, Controller, Post, UseGuards } from '@nestjs/common'; | ||
import { ApiBearerAuth, ApiOperation, ApiTags } from '@nestjs/swagger'; | ||
import { ControllerDecorator } from 'src/utils/controller.decorator'; | ||
import { FirebaseAuthGuard } from '../firebase/firebase-auth.guard'; | ||
import { SessionFeedbackDto } from './dtos/session-feedback.dto'; | ||
import { SessionFeedbackService } from './session-feedback.service'; | ||
|
||
@ApiTags('Session Feedback') | ||
@ControllerDecorator() | ||
@Controller('/v1/session-feedback') | ||
export class SessionFeedbackController { | ||
constructor(private readonly sessionFeedbackService: SessionFeedbackService) {} | ||
|
||
@Post() | ||
@ApiBearerAuth('access-token') | ||
@ApiOperation({ | ||
description: 'Stores feedback from a user', | ||
}) | ||
@UseGuards(FirebaseAuthGuard) | ||
async storeUserFeedback( | ||
@Body() sessionFeedbackDto: SessionFeedbackDto, | ||
): Promise<SessionFeedbackDto> { | ||
return await this.sessionFeedbackService.createSessionFeedback(sessionFeedbackDto); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
import { Module } from '@nestjs/common'; | ||
import { TypeOrmModule } from '@nestjs/typeorm'; | ||
import { SlackMessageClient } from 'src/api/slack/slack-api'; | ||
import { ZapierWebhookClient } from 'src/api/zapier/zapier-webhook-client'; | ||
import { PartnerAccessEntity } from 'src/entities/partner-access.entity'; | ||
import { PartnerEntity } from 'src/entities/partner.entity'; | ||
import { SessionFeedbackEntity } from 'src/entities/session-feedback.entity'; | ||
import { SessionEntity } from 'src/entities/session.entity'; | ||
import { SubscriptionUserEntity } from 'src/entities/subscription-user.entity'; | ||
import { SubscriptionEntity } from 'src/entities/subscription.entity'; | ||
import { TherapySessionEntity } from 'src/entities/therapy-session.entity'; | ||
import { UserEntity } from 'src/entities/user.entity'; | ||
import { PartnerAccessService } from 'src/partner-access/partner-access.service'; | ||
import { SessionService } from 'src/session/session.service'; | ||
import { SubscriptionUserService } from 'src/subscription-user/subscription-user.service'; | ||
import { SubscriptionService } from 'src/subscription/subscription.service'; | ||
import { TherapySessionService } from 'src/therapy-session/therapy-session.service'; | ||
import { UserService } from 'src/user/user.service'; | ||
import { SessionFeedbackController } from './session-feedback.controller'; | ||
import { SessionFeedbackService } from './session-feedback.service'; | ||
|
||
@Module({ | ||
imports: [ | ||
TypeOrmModule.forFeature([ | ||
SessionFeedbackEntity, | ||
SessionEntity, | ||
UserEntity, | ||
PartnerAccessEntity, | ||
PartnerEntity, | ||
SubscriptionUserEntity, | ||
SubscriptionEntity, | ||
TherapySessionEntity, | ||
]), | ||
], | ||
controllers: [SessionFeedbackController], | ||
providers: [ | ||
SessionFeedbackService, | ||
SessionService, | ||
UserService, | ||
SubscriptionUserService, | ||
SubscriptionService, | ||
PartnerAccessService, | ||
TherapySessionService, | ||
ZapierWebhookClient, | ||
SlackMessageClient, | ||
], | ||
}) | ||
export class SessionFeedbackModule {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
import { createMock, DeepMocked } from '@golevelup/ts-jest'; | ||
import { HttpException, HttpStatus } from '@nestjs/common'; | ||
import { Test, TestingModule } from '@nestjs/testing'; | ||
import { getRepositoryToken } from '@nestjs/typeorm'; | ||
import { PartnerAccessEntity } from 'src/entities/partner-access.entity'; | ||
import { PartnerEntity } from 'src/entities/partner.entity'; | ||
import { SessionFeedbackEntity } from 'src/entities/session-feedback.entity'; | ||
import { SessionEntity } from 'src/entities/session.entity'; | ||
import { SubscriptionUserEntity } from 'src/entities/subscription-user.entity'; | ||
import { SubscriptionEntity } from 'src/entities/subscription.entity'; | ||
import { TherapySessionEntity } from 'src/entities/therapy-session.entity'; | ||
import { UserEntity } from 'src/entities/user.entity'; | ||
import { SessionFeedbackService } from 'src/session-feedback/session-feedback.service'; | ||
import { SessionService } from 'src/session/session.service'; | ||
import { FEEDBACK_TAGS_ENUM } from 'src/utils/constants'; | ||
import { mockSessionEntity } from 'test/utils/mockData'; | ||
import { Repository } from 'typeorm'; | ||
|
||
const sessionFeedbackDto = { | ||
sessionFeedbackId: 'session-feedback-id', | ||
sessionId: mockSessionEntity.id, | ||
feedbackTags: FEEDBACK_TAGS_ENUM.INSPIRING, | ||
feedbackDescription: 'feedback-description', | ||
}; | ||
|
||
describe('SessionFeedbackService', () => { | ||
let service: SessionFeedbackService; | ||
let mockPartnerAccessRepository: DeepMocked<Repository<PartnerAccessEntity>>; | ||
let mockPartnerRepository: DeepMocked<Repository<PartnerEntity>>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment/question as below about all the transitive dependencies for this service |
||
let mockUserRepository: DeepMocked<Repository<UserEntity>>; | ||
let mockSessionRepository: DeepMocked<Repository<SessionEntity>>; | ||
let mockSubscriptionUserRepository: DeepMocked<Repository<SubscriptionUserEntity>>; | ||
let mockSubscriptionRepository: DeepMocked<Repository<SubscriptionEntity>>; | ||
let mockTherapySessionRepository: DeepMocked<Repository<TherapySessionEntity>>; | ||
let mockSessionFeedbackRepository: DeepMocked<Repository<SessionFeedbackEntity>>; | ||
let mockSessionService: DeepMocked<SessionService>; | ||
|
||
beforeEach(async () => { | ||
mockPartnerAccessRepository = createMock<Repository<PartnerAccessEntity>>(); | ||
mockPartnerRepository = createMock<Repository<PartnerEntity>>(); | ||
mockUserRepository = createMock<Repository<UserEntity>>(); | ||
mockSessionRepository = createMock<Repository<SessionEntity>>(); | ||
mockSubscriptionUserRepository = createMock<Repository<SubscriptionUserEntity>>(); | ||
mockSubscriptionRepository = createMock<Repository<SubscriptionEntity>>(); | ||
mockTherapySessionRepository = createMock<Repository<TherapySessionEntity>>(); | ||
mockSessionFeedbackRepository = createMock<Repository<SessionFeedbackEntity>>(); | ||
mockSessionService = createMock<SessionService>(); | ||
|
||
const module: TestingModule = await Test.createTestingModule({ | ||
providers: [ | ||
SessionFeedbackService, | ||
{ | ||
provide: getRepositoryToken(PartnerAccessEntity), | ||
useValue: mockPartnerAccessRepository, | ||
}, | ||
{ | ||
provide: getRepositoryToken(PartnerEntity), | ||
useValue: mockPartnerRepository, | ||
}, | ||
{ | ||
provide: getRepositoryToken(UserEntity), | ||
useValue: mockUserRepository, | ||
}, | ||
{ | ||
provide: getRepositoryToken(SessionEntity), | ||
useValue: mockSessionRepository, | ||
}, | ||
{ | ||
provide: getRepositoryToken(SubscriptionUserEntity), | ||
useValue: mockSubscriptionUserRepository, | ||
}, | ||
{ | ||
provide: getRepositoryToken(SubscriptionEntity), | ||
useValue: mockSubscriptionRepository, | ||
}, | ||
{ | ||
provide: getRepositoryToken(TherapySessionEntity), | ||
useValue: mockTherapySessionRepository, | ||
}, | ||
{ | ||
provide: getRepositoryToken(SessionFeedbackEntity), | ||
useValue: mockSessionFeedbackRepository, | ||
}, | ||
{ | ||
provide: SessionService, | ||
useValue: mockSessionService, | ||
}, | ||
], | ||
}).compile(); | ||
|
||
service = module.get<SessionFeedbackService>(SessionFeedbackService); | ||
}); | ||
|
||
it('should be defined', () => { | ||
expect(service).toBeDefined(); | ||
}); | ||
describe('createSessionFeedback', () => { | ||
it('when session id exists should return dto', async () => { | ||
jest.spyOn(mockSessionService, 'getSession').mockResolvedValueOnce(mockSessionEntity); | ||
const response = await service.createSessionFeedback(sessionFeedbackDto); | ||
|
||
expect(response).toMatchObject(sessionFeedbackDto); | ||
}); | ||
it('when session id does not exist should throw exception', async () => { | ||
jest.spyOn(mockSessionService, 'getSession').mockResolvedValueOnce(null); | ||
|
||
await expect(service.createSessionFeedback(sessionFeedbackDto)).rejects.toThrow( | ||
new HttpException('SESSION NOT FOUND', HttpStatus.NOT_FOUND), | ||
); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
import { HttpException, HttpStatus, Injectable } from '@nestjs/common'; | ||
import { InjectRepository } from '@nestjs/typeorm'; | ||
import { PartnerAccessEntity } from 'src/entities/partner-access.entity'; | ||
import { PartnerEntity } from 'src/entities/partner.entity'; | ||
import { SessionFeedbackEntity } from 'src/entities/session-feedback.entity'; | ||
import { SessionEntity } from 'src/entities/session.entity'; | ||
import { SubscriptionUserEntity } from 'src/entities/subscription-user.entity'; | ||
import { SubscriptionEntity } from 'src/entities/subscription.entity'; | ||
import { TherapySessionEntity } from 'src/entities/therapy-session.entity'; | ||
import { UserEntity } from 'src/entities/user.entity'; | ||
import { Repository } from 'typeorm'; | ||
import { SessionService } from '../session/session.service'; | ||
import { SessionFeedbackDto } from './dtos/session-feedback.dto'; | ||
|
||
@Injectable() | ||
export class SessionFeedbackService { | ||
constructor( | ||
@InjectRepository(PartnerAccessEntity) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not exactly sure why all these repos are needed, as it does not seem to be the case for other services in this repo, but I had to add all of these to get rid of any errors created. The only other service that this service directly depends on is the session service - the rest of the dependencies are just transitive There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a great question and you are right to pick it up. It is something we are slowly trying to fix. The issue is the function getUserByFirebaseId in the userService. It does waayyyyyy too much. It's unfortunately used in the AuthGuard. The FirebaseAuthGuard is used across all the services. What we are trying to do is start to divide up how we fetch the data and ensure everything is fetched separately. See this issue. It's going to be a slow process. |
||
private partnerAccessRepository: Repository<PartnerAccessEntity>, | ||
@InjectRepository(PartnerEntity) | ||
private partnerRepository: Repository<PartnerEntity>, | ||
@InjectRepository(UserEntity) | ||
private userRepository: Repository<UserEntity>, | ||
@InjectRepository(SessionEntity) | ||
private sessionRepository: Repository<SessionEntity>, | ||
@InjectRepository(SubscriptionUserEntity) | ||
private subscriptionUserRepository: Repository<SubscriptionUserEntity>, | ||
@InjectRepository(SubscriptionEntity) | ||
private subscriptionRepository: Repository<SubscriptionEntity>, | ||
@InjectRepository(TherapySessionEntity) | ||
private therapySessionRepository: Repository<TherapySessionEntity>, | ||
@InjectRepository(SessionFeedbackEntity) | ||
private sessionFeedbackRepository: Repository<SessionFeedbackEntity>, | ||
private readonly sessionService: SessionService, | ||
) {} | ||
|
||
public async createSessionFeedback( | ||
sessionFeedbackDto: SessionFeedbackDto, | ||
): Promise<SessionFeedbackDto> { | ||
const session = await this.sessionService.getSession(sessionFeedbackDto.sessionId); | ||
|
||
if (!session) { | ||
throw new HttpException('SESSION NOT FOUND', HttpStatus.NOT_FOUND); | ||
} | ||
|
||
const sessionFeedbackObject = this.sessionFeedbackRepository.create(sessionFeedbackDto); | ||
await this.sessionFeedbackRepository.save(sessionFeedbackObject); | ||
|
||
return sessionFeedbackDto; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will show all the enum values in the swagger "Example Value" section in a list - not sure if this is more or less convenient than it just saying "string"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!