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

refactor(jobs): optimize code structure from previous merge #1356

Merged
merged 2 commits into from
Mar 2, 2025
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
6,492 changes: 3,210 additions & 3,282 deletions package-lock.json

Large diffs are not rendered by default.

2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
"postinstall": "npm install --platform=linux --arch=x64 sharp"
},
"dependencies": {

"@google/generative-ai": "^0.22.0",
"@nestjs/axios": "^4.0.0",
"@nestjs/bull": "^11.0.2",
Expand All @@ -47,7 +46,6 @@
"@nestjs/swagger": "^11.0.5",
"@nestjs/typeorm": "^11.0.0",
"@types/nodemailer": "^6.4.17",

"@types/speakeasy": "^2.0.10",
"@vitalets/google-translate-api": "^9.2.1",
"aws-sdk": "^2.1692.0",
Expand Down
10 changes: 3 additions & 7 deletions src/database/migrations/1740788110830-UpdatePhoneNumberType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,10 @@ import { MigrationInterface, QueryRunner } from 'typeorm';

export class UpdatePhoneNumberType1740788110830 implements MigrationInterface {
public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`ALTER TABLE contact ALTER COLUMN phone TYPE VARCHAR(20) USING phone::text`
);
await queryRunner.query(`ALTER TABLE contact ALTER COLUMN phone TYPE VARCHAR(20) USING phone::text`);
}

public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`ALTER TABLE contact ALTER COLUMN phone TYPE INTEGER USING phone::integer`
);
await queryRunner.query(`ALTER TABLE contact ALTER COLUMN phone TYPE INTEGER USING phone::integer`);
}
}
}
37 changes: 37 additions & 0 deletions src/guards/job-access.guard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { Injectable, CanActivate, ExecutionContext } from '@nestjs/common';
import { InjectRepository } from '@nestjs/typeorm';
import { Repository } from 'typeorm';
import { Job } from '../modules/jobs/entities/job.entity';
import { User } from '@modules/user/entities/user.entity';

@Injectable()
export class JobAccessGuard implements CanActivate {
constructor(
@InjectRepository(Job)
private readonly jobRepository: Repository<Job>,
@InjectRepository(User)
private readonly userRepository: Repository<User>
) {}

async canActivate(context: ExecutionContext): Promise<boolean> {
const request = context.switchToHttp().getRequest();
const jobId = request.params.id;
const userId = request.user.sub;

const user = await this.userRepository.findOne({
where: { id: userId },
});

if (user?.is_superadmin) {
return true;
}

const job = await this.jobRepository.findOne({
where: { id: jobId },
relations: ['user'],
});

if (!job) return false;
return job.user.id === userId;
}
}
2 changes: 1 addition & 1 deletion src/modules/contact-us/contact-us.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@ export class ContactUsService {
},
});
}
}
}
32 changes: 16 additions & 16 deletions src/modules/contact-us/dto/create-contact-us.dto.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
import { IsEmail, IsNotEmpty, IsOptional, IsString, Matches } from 'class-validator';

export class CreateContactDto {
@IsNotEmpty({ message: 'Name should not be empty' })
@IsString({ message: 'Name must be a string' })
name: string;
@IsNotEmpty({ message: 'Name should not be empty' })
@IsString({ message: 'Name must be a string' })
name: string;

@IsNotEmpty({ message: 'Email should not be empty' })
@IsEmail({}, { message: 'Email must be valid' })
email: string;
@IsNotEmpty({ message: 'Email should not be empty' })
@IsEmail({}, { message: 'Email must be valid' })
email: string;

@IsOptional()
@IsString({ message: 'Phone must be a string' })
@Matches(/^\+?[0-9\-\s()]{8,20}$/, {
message: 'Invalid phone number format'
})
phone: string;
@IsOptional()
@IsString({ message: 'Phone must be a string' })
@Matches(/^\+?[0-9\-\s()]{8,20}$/, {
message: 'Invalid phone number format',
})
phone: string;

@IsNotEmpty({ message: 'Message should not be empty' })
@IsString({ message: 'Message must be a string' })
message: string;
}
@IsNotEmpty({ message: 'Message should not be empty' })
@IsString({ message: 'Message must be a string' })
message: string;
}
22 changes: 11 additions & 11 deletions src/modules/contact-us/entities/contact-us.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,18 @@ import { Entity, Column } from 'typeorm';

@Entity()
export class ContactUs extends AbstractBaseEntity {
@Column('varchar', { length: 20, nullable: true })
phone: string;
@Column('varchar', { length: 20, nullable: true })
phone: string;

@Column('varchar', { nullable: false })
name: string;
@Column('varchar', { nullable: false })
name: string;

@Column('varchar', { nullable: false })
email: string;
@Column('varchar', { nullable: false })
email: string;

@Column('text', { nullable: false })
message: string;
@Column('text', { nullable: false })
message: string;

@Column({ type: 'timestamp', default: () => 'CURRENT_TIMESTAMP' })
createdAt: Date;
}
@Column({ type: 'timestamp', default: () => 'CURRENT_TIMESTAMP' })
createdAt: Date;
}
1 change: 1 addition & 0 deletions src/modules/invite/mocks/mockUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export const mockUser: User = {
first_name: 'John',
last_name: 'Doe',
is_active: true,
is_superadmin: false,
phone: '+1234567890',
status: 'Hello from the children of planet Earth',
id: 'some-uuid-value-here',
Expand Down
4 changes: 2 additions & 2 deletions src/modules/jobs/jobs.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import { JobSearchDto } from './dto/jobSearch.dto';
import { UpdateJobDto } from './dto/update-job.dto';
import { JobOwnerGuard } from '../../guards/job-owner.guard';
import { AuthGuard } from '../../guards/auth.guard';
import { JobAccessGuard } from '@guards/job-access.guard';

@ApiTags('Jobs')
@Controller('jobs')
Expand Down Expand Up @@ -124,8 +125,7 @@ export class JobsController {
}

@Patch('/:id')
@UseGuards(AuthGuard)
@UseGuards(SuperAdminGuard, JobOwnerGuard)
@UseGuards(AuthGuard, JobAccessGuard)
@ApiBearerAuth()
@ApiOperation({ summary: 'Update a job posting' })
@ApiResponse({ status: 200, description: 'Job updated successfully' })
Expand Down
6 changes: 3 additions & 3 deletions src/modules/jobs/jobs.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ import { OrganisationUserRole } from '@modules/role/entities/organisation-user-r
import { Profile } from '@modules/profile/entities/profile.entity';
import { Role } from '@modules/role/entities/role.entity';
import { UserModule } from '@modules/user/user.module';
import { JobOwnerGuard } from '@guards/job-owner.guard';
import { AuthGuard } from '@guards/auth.guard';
import { SuperAdminGuard } from '@guards/super-admin.guard';
import { JobAccessGuard } from '@guards/job-access.guard';
import { S3Service } from '@modules/s3/s3.service';

@Module({
imports: [
TypeOrmModule.forFeature([Job, User, JobApplication, Organisation, OrganisationUserRole, Profile, Role]),
UserModule,
],
providers: [JobsService, JobOwnerGuard, AuthGuard, SuperAdminGuard, S3Service],
providers: [JobsService, JobAccessGuard, AuthGuard, S3Service],
controllers: [JobsController],
})
export class JobsModule {}
17 changes: 11 additions & 6 deletions src/modules/jobs/jobs.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export class JobsService {
...others,
applicant_name,
resume: resumeUrl,
job: job.data
job: job.data,
});

await this.jobApplicationRepository.save(createJobApplication);
Expand Down Expand Up @@ -181,16 +181,21 @@ export class JobsService {
this.validateUserId(userId);
this.validateUpdateData(updateJobDto);

const job = await this.jobRepository.findOne({
where: { id },
relations: ['user'],
});
const [job, user] = await Promise.all([
this.jobRepository.findOne({
where: { id },
relations: ['user'],
}),
this.userRepository.findOne({
where: { id: userId },
}),
]);

if (!job) {
throw new CustomHttpException('Job not found', HttpStatus.NOT_FOUND);
}

if (job.user.id !== userId) {
if (job.user.id !== userId && !user?.is_superadmin) {
throw new CustomHttpException('Unauthorized to update this job', HttpStatus.FORBIDDEN);
}

Expand Down
58 changes: 57 additions & 1 deletion src/modules/jobs/tests/jobs.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ describe('JobsService', () => {
new CustomHttpException('Duplicate application', HttpStatus.CONFLICT)
);
});

});

describe('searchJobs', () => {
Expand Down Expand Up @@ -386,7 +385,12 @@ describe('JobsService', () => {
user: { ...mockUser, id: 'different_user_id' } as User,
} as Job;

// Mock both repository calls
jest.spyOn(jobRepository, 'findOne').mockResolvedValue(mockJob);
jest.spyOn(userRepository, 'findOne').mockResolvedValue({
...mockUser,
is_superadmin: false,
} as User);

await expect(service.update('job-id', updateDto, 'user_id')).rejects.toThrow(
new CustomHttpException('Unauthorized to update this job', HttpStatus.FORBIDDEN)
Expand Down Expand Up @@ -435,7 +439,12 @@ describe('JobsService', () => {
user: { ...mockUser, id: 'different_user_id' } as User,
} as Job;

// Mock both repository calls
jest.spyOn(jobRepository, 'findOne').mockResolvedValue(mockJob);
jest.spyOn(userRepository, 'findOne').mockResolvedValue({
...mockUser,
is_superadmin: false,
} as User);

await expect(service.update('job-id', updateDto, 'user_id')).rejects.toThrow(
new CustomHttpException('Unauthorized to update this job', HttpStatus.FORBIDDEN)
Expand Down Expand Up @@ -498,5 +507,52 @@ describe('JobsService', () => {
})
);
});

it('should allow super admin to update any job', async () => {
const superAdminUser = {
...mockUser,
is_superadmin: true,
id: 'super_admin_id',
};

const jobOwnedByOtherUser = {
...jobsMock[0],
user: { ...mockUser, id: 'other_user_id' } as User,
} as Job;

jest.spyOn(jobRepository, 'findOne').mockResolvedValue(jobOwnedByOtherUser);
jest.spyOn(userRepository, 'findOne').mockResolvedValue(superAdminUser as User);
jest.spyOn(jobRepository, 'save').mockResolvedValue({
...jobOwnedByOtherUser,
...updateDto,
} as Job);

const result = await service.update('job-id', updateDto, 'super_admin_id');

expect(result.status).toBe('success');
expect(result.status_code).toBe(200);
expect((result.data as Job).title).toBe(updateDto.title);
});

it('should not allow non-owner non-admin user to update job', async () => {
const regularUser = {
...mockUser,
is_superadmin: false,
id: 'regular_user_id',
};

const jobOwnedByOtherUser = {
...jobsMock[0],
user: { ...mockUser, id: 'other_user_id' } as User,
} as Job;

// Mock both repository calls
jest.spyOn(jobRepository, 'findOne').mockResolvedValue(jobOwnedByOtherUser);
jest.spyOn(userRepository, 'findOne').mockResolvedValue(regularUser as User);

await expect(service.update('job-id', updateDto, 'regular_user_id')).rejects.toThrow(
new CustomHttpException('Unauthorized to update this job', HttpStatus.FORBIDDEN)
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export const mockUser: User = {
status: 'Hello from the children of planet Earth',
hashPassword: async () => {},
is_active: true,
is_superadmin: false,
attempts_left: 3,
time_left: 3600,
owned_organisations: [],
Expand Down
1 change: 1 addition & 0 deletions src/modules/organisations/tests/mocks/organisation.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export const createMockOrganisation = (): Organisation => {
phone: '+1234567890',
hashPassword: async () => {},
is_active: true,
is_superadmin: false,
attempts_left: 3,
time_left: 3600,
owned_organisations: [],
Expand Down
1 change: 1 addition & 0 deletions src/modules/organisations/tests/mocks/user.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,5 @@ export const mockUser = {
comments: null,
cart: [],
organisations: null,
is_superadmin: false,
};
1 change: 1 addition & 0 deletions src/modules/profile/mocks/mockUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export const mockUserWithProfile: User = {
first_name: 'John',
last_name: 'Doe',
is_active: true,
is_superadmin: false,
phone: '+1234567891',
id: 'some-uuid-value-here',
attempts_left: 2,
Expand Down
3 changes: 3 additions & 0 deletions src/modules/user/entities/user.entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ export class User extends AbstractBaseEntity {
@Column({ default: false })
is_2fa_enabled: boolean;

@Column({ default: false })
is_superadmin: boolean;

@DeleteDateColumn({ nullable: true })
deletedAt?: Date;

Expand Down
1 change: 1 addition & 0 deletions src/modules/user/tests/mocks/user.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@ export const mockUser: User = {
hashPassword: () => null,
cart: [],
organisations: null,
is_superadmin: false,
};