-
Notifications
You must be signed in to change notification settings - Fork 0
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
Update getUser and create getApplication endpoint #15
Changes from 15 commits
b41671b
465e126
14fac06
083355e
22d9f2b
2973f85
17d14ec
5053508
53ab655
5563bfd
c71b24f
cbf3515
a909dce
4304d8a
74c6467
553fd36
8b1d9f0
5e7622f
391ad28
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,39 @@ | ||
import { | ||
IsArray, | ||
IsDateString, | ||
IsEnum, | ||
IsObject, | ||
IsPositive, | ||
} from 'class-validator'; | ||
import { Entity, Column } from 'typeorm'; | ||
import { Response, Note, ApplicationStatus } from './types'; | ||
import { Cycle } from './dto/cycle.dto'; | ||
|
||
@Entity() | ||
export class Application { | ||
@Column({ primary: true }) | ||
@IsPositive() | ||
id: number; | ||
|
||
@Column() | ||
@IsDateString() | ||
createdAt: Date; | ||
|
||
@Column() | ||
@IsObject() | ||
cycle: Cycle; | ||
chromium-52 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
@Column() | ||
@IsEnum(ApplicationStatus) | ||
status: ApplicationStatus; | ||
|
||
@Column() | ||
@IsArray() | ||
@IsObject({ each: true }) | ||
application: Response[]; | ||
|
||
@Column() | ||
@IsArray() | ||
@IsObject({ each: true }) | ||
notes: Note[]; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import { Controller, Get, ParseIntPipe, Param } from '@nestjs/common'; | ||
import { ApplicationsService } from './applications.service'; | ||
|
||
@Controller('apps') | ||
export class ApplicationsController { | ||
constructor(private readonly applicationsService: ApplicationsService) {} | ||
|
||
@Get('/:userId') | ||
getApplication(@Param('userId', ParseIntPipe) userId: number) { | ||
return this.applicationsService.findOne(userId); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import { Module } from '@nestjs/common'; | ||
import { TypeOrmModule } from '@nestjs/typeorm'; | ||
import { ApplicationsService } from './applications.service'; | ||
import { ApplicationsController } from './applications.controller'; | ||
import { Application } from './application.entity'; | ||
import { UsersService } from '../users/users.service'; | ||
import { User } from '../users/user.entity'; | ||
|
||
@Module({ | ||
imports: [TypeOrmModule.forFeature([Application, User])], | ||
providers: [ApplicationsService, UsersService], | ||
controllers: [ApplicationsController], | ||
}) | ||
export class ApplicationsModule {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
import { | ||
BadRequestException, | ||
Injectable, | ||
UnauthorizedException, | ||
} from '@nestjs/common'; | ||
import { InjectRepository } from '@nestjs/typeorm'; | ||
import { MongoRepository } from 'typeorm'; | ||
import { UserStatus } from '../users/types'; | ||
import { UsersService } from '../users/users.service'; | ||
import { getCurrentUser } from '../users/utils'; | ||
import { Application } from './application.entity'; | ||
import { getCurrentCycle } from './utils'; | ||
chromium-52 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import { Cycle } from './dto/cycle.dto'; | ||
import { plainToClass } from 'class-transformer'; | ||
|
||
@Injectable() | ||
export class ApplicationsService { | ||
constructor( | ||
@InjectRepository(Application) | ||
private applicationsRepository: MongoRepository<Application>, | ||
private readonly usersService: UsersService, | ||
) {} | ||
|
||
async findOne(userId: number): Promise<Application> { | ||
const currentUser = getCurrentUser(); | ||
const currentStatus = currentUser.status; | ||
switch (currentStatus) { | ||
case UserStatus.ADMIN: | ||
case UserStatus.RECRUITER: | ||
break; | ||
default: | ||
if (currentUser.userId !== userId) { | ||
throw new UnauthorizedException('User not found'); | ||
} | ||
break; | ||
} | ||
|
||
const applicant = await this.usersService.findOne(userId); | ||
const application = applicant.applications[0]; | ||
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. ❓ Are we assuming that the application at index 0 will always be the latest application for a particular user? We should probably write something more robust so that we don't have to remember this assumption in the long-term. 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. Hmm yeah we do something like this for SFTT and it's been fine so I don't think it's too big of an issue but any given user won't have more than a couple of apps (so doing more complex operations for this won't affect its runtime to a noticeable extent) and this is a common operation so I think this is a good idea! I did change the logic so that it actually gets the app for the current cycle instead of the latest app because we shouldn't need to look at apps for past cycles often. Maybe in a future iteration we can modify this endpoint so that it takes in a cycle year and semester so that we can also get an app for an arbitrary cycle |
||
if (application == null) { | ||
throw new BadRequestException('Application not found'); | ||
} | ||
chromium-52 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
const cycle = plainToClass(Cycle, application.cycle); | ||
chromium-52 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
//the user with the given userId has not applied in the current recruitment cycle | ||
if (!cycle.isCurrentCycle(getCurrentCycle())) { | ||
throw new BadRequestException( | ||
"Applicant hasn't applied in the current cycle", | ||
); | ||
} | ||
|
||
return application; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import { IsEnum, IsPositive } from 'class-validator'; | ||
import { Semester } from '../types'; | ||
|
||
export class Cycle { | ||
@IsPositive() | ||
chromium-52 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
year: number; | ||
|
||
@IsEnum(Semester) | ||
semester: Semester; | ||
|
||
constructor(year: number, semester: Semester) { | ||
this.year = year; | ||
this.semester = semester; | ||
} | ||
|
||
public isCurrentCycle(cycle: Cycle): boolean { | ||
return this.year === cycle.year && this.semester === cycle.semester; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
export type Response = { | ||
question: string; | ||
answer: string; | ||
}; | ||
|
||
export type Note = { | ||
userId: number; | ||
note: string; | ||
}; | ||
|
||
export enum Semester { | ||
FALL = 'FALL', | ||
SPRING = 'SPRING', | ||
} | ||
|
||
export enum ApplicationStatus { | ||
SUBMITTED = 'SUBMITTED', | ||
REVIEWED = 'REVIEWED', | ||
ACCEPTED = 'ACCEPTED', | ||
REJECTED = 'REJECTED', | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
import { Cycle } from './dto/cycle.dto'; | ||
import { Semester } from './types'; | ||
|
||
chromium-52 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
export const getCurrentCycle = () => new Cycle(2023, Semester.FALL); | ||
chromium-52 marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
import { | ||
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. ❓ What is the purpose of this dto? 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. The purpose of this dto is to define the response schema for the |
||
IsArray, | ||
IsEmail, | ||
IsEnum, | ||
IsNumber, | ||
IsString, | ||
IsUrl, | ||
} from 'class-validator'; | ||
import { Role, Team, UserStatus } from '../types'; | ||
|
||
export class GetUserResponseDto { | ||
@IsNumber() | ||
userId: number; | ||
|
||
@IsEnum(UserStatus) | ||
status: UserStatus; | ||
|
||
@IsString() | ||
firstName: string; | ||
|
||
@IsString() | ||
lastName: string; | ||
|
||
@IsEmail() | ||
email: string; | ||
|
||
// TODO make custom decorator for @IsUrl()s | ||
@IsUrl({ | ||
protocols: ['https'], | ||
require_protocol: true, | ||
}) | ||
profilePicture: string | null; | ||
|
||
@IsUrl({ | ||
protocols: ['https'], | ||
require_protocol: true, | ||
host_whitelist: ['www.linkedin.com'], | ||
}) | ||
linkedin: string | null; | ||
|
||
@IsUrl({ | ||
protocols: ['https'], | ||
require_protocol: true, | ||
host_whitelist: ['github.com'], | ||
}) | ||
github: string | null; | ||
|
||
@IsEnum(Team) | ||
team: Team | null; | ||
|
||
@IsArray() | ||
@IsEnum(Role, { each: true }) | ||
role: Role[] | null; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,35 +1,72 @@ | ||
import { | ||
IsArray, | ||
IsEmail, | ||
IsEnum, | ||
IsObject, | ||
IsPositive, | ||
IsString, | ||
IsUrl, | ||
} from 'class-validator'; | ||
import { Entity, Column } from 'typeorm'; | ||
import { Status, Role, Team } from './types'; | ||
import { Application } from '../applications/application.entity'; | ||
import { Role, Team, UserStatus } from './types'; | ||
|
||
@Entity() | ||
export class User { | ||
@Column({ primary: true }) | ||
@IsPositive() | ||
userId: number; | ||
|
||
@Column() | ||
status: Status; | ||
@IsEnum(UserStatus) | ||
status: UserStatus; | ||
|
||
@Column() | ||
@IsString() | ||
firstName: string; | ||
|
||
@Column() | ||
@IsString() | ||
lastName: string; | ||
|
||
@Column() | ||
@IsEmail() | ||
email: string; | ||
|
||
@Column() | ||
@IsUrl({ | ||
protocols: ['https'], | ||
require_protocol: true, | ||
}) | ||
profilePicture: string | null; | ||
|
||
@Column() | ||
@IsUrl({ | ||
protocols: ['https'], | ||
require_protocol: true, | ||
host_whitelist: ['www.linkedin.com'], | ||
}) | ||
linkedin: string | null; | ||
|
||
@Column() | ||
@IsUrl({ | ||
protocols: ['https'], | ||
require_protocol: true, | ||
host_whitelist: ['github.com'], | ||
}) | ||
github: string | null; | ||
|
||
@Column() | ||
@IsEnum(Team) | ||
team: Team | null; | ||
|
||
@Column() | ||
@IsArray() | ||
@IsEnum(Role, { each: true }) | ||
role: Role[] | null; | ||
|
||
@Column() | ||
@IsArray() | ||
@IsObject({ each: true }) | ||
applications: Application[]; | ||
} |
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.
❓ I noticed that the application schema you posted has a
lastUpdatedAt
field. I'm assuming that thelastUpdatedAt
field is now obsolete/no longer necessary, and the current application entity I'm looking at is the most recent application schema?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.
Yup, the
lastUpdatedAt
field isn't needed anymore bc we made the decision to not allow applicants to update their app after they submit it (which is what we do atm)