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

Added session feedback controller and service with tests for error an… #520

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { PartnerAccessModule } from './partner-access/partner-access.module';
import { PartnerAdminModule } from './partner-admin/partner-admin.module';
import { PartnerFeatureModule } from './partner-feature/partner-feature.module';
import { PartnerModule } from './partner/partner.module';
import { SessionFeedbackModule } from './session-feedback/session-feedback.module';
import { SessionUserModule } from './session-user/session-user.module';
import { SessionModule } from './session/session.module';
import { SubscriptionUserModule } from './subscription-user/subscription-user.module';
Expand All @@ -35,6 +36,7 @@ import { WebhooksModule } from './webhooks/webhooks.module';
CourseModule,
CourseUserModule,
SessionUserModule,
SessionFeedbackModule,
CoursePartnerModule,
SubscriptionUserModule,
FeatureModule,
Expand Down
29 changes: 29 additions & 0 deletions src/session-feedback/dtos/session-feedback.dto.ts
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),
Copy link
Contributor Author

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"

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

})
feedbackTags: FEEDBACK_TAGS_ENUM;

@IsNotEmpty()
@IsString()
@ApiProperty({ type: String })
feedbackDescription: string;
}
25 changes: 25 additions & 0 deletions src/session-feedback/session-feedback.controller.ts
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);
}
}
48 changes: 48 additions & 0 deletions src/session-feedback/session-feedback.module.ts
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 {}
112 changes: 112 additions & 0 deletions src/session-feedback/session-feedback.service.spec.ts
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>>;
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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),
);
});
});
});
51 changes: 51 additions & 0 deletions src/session-feedback/session-feedback.service.ts
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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

The 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;
}
}
Loading