Skip to content

Commit

Permalink
Merge pull request #1499 from SciCatProject/SWAP-4298-parent-proposal
Browse files Browse the repository at this point in the history
feat: add parent proposal to the proposal document
  • Loading branch information
martin-trajanovski authored Nov 15, 2024
2 parents 367f38b + 56b0450 commit 15ee6aa
Show file tree
Hide file tree
Showing 10 changed files with 145 additions and 57 deletions.
9 changes: 9 additions & 0 deletions src/proposals/dto/update-proposal.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,15 @@ export class UpdateProposalDto extends OwnableDto {
@IsOptional()
@IsObject()
readonly metadata?: Record<string, unknown>;

@ApiProperty({
type: String,
required: false,
description: "Parent proposal id.",
})
@IsOptional()
@IsString()
readonly parentProposalId?: string;
}

export class PartialUpdateProposalDto extends PartialType(UpdateProposalDto) {}
42 changes: 19 additions & 23 deletions src/proposals/proposals.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
Req,
ForbiddenException,
ConflictException,
BadRequestException,
Logger,
InternalServerErrorException,
} from "@nestjs/common";
Expand Down Expand Up @@ -89,14 +88,17 @@ export class ProposalsController {
return proposalInstance;
}

private async permissionChecker(
private permissionChecker(
group: Action,
proposal: ProposalClass | CreateProposalDto,
proposal: ProposalClass | CreateProposalDto | null,
request: Request,
) {
const proposalInstance = this.generateProposalInstanceForPermissions(
proposal as ProposalClass,
);
if (!proposal) {
return false;
}

const proposalInstance =
this.generateProposalInstanceForPermissions(proposal);

const user: JWTUser = request.user as JWTUser;
const ability = this.caslAbilityFactory.proposalsInstanceAccess(user);
Expand Down Expand Up @@ -172,34 +174,29 @@ export class ProposalsController {
proposalId: id,
});

if (proposal) {
const canDoAction = await this.permissionChecker(
group,
proposal,
request,
);
const canDoAction = this.permissionChecker(group, proposal, request);

if (!canDoAction) {
throw new ForbiddenException("Unauthorized to this proposal");
}
if (!canDoAction) {
throw new ForbiddenException("Unauthorized to this proposal");
}

return proposal;
}

private async checkPermissionsForProposalCreate(
private checkPermissionsForProposalCreate(
request: Request,
proposal: CreateProposalDto,
group: Action,
) {
if (!proposal) {
throw new BadRequestException("Not able to create this proposal");
}
const canDoAction = await this.permissionChecker(group, proposal, request);
const canDoAction = this.permissionChecker(group, proposal, request);

if (!canDoAction) {
throw new ForbiddenException("Unauthorized to create this proposal");
}

return proposal;
}

updateFiltersForList(
request: Request,
mergedFilters: IFilters<ProposalDocument, IProposalFields>,
Expand Down Expand Up @@ -272,7 +269,7 @@ export class ProposalsController {
@Req() request: Request,
@Body() createProposalDto: CreateProposalDto,
): Promise<ProposalClass> {
const proposalDTO = await this.checkPermissionsForProposalCreate(
const proposalDTO = this.checkPermissionsForProposalCreate(
request,
createProposalDto,
Action.ProposalsCreate,
Expand Down Expand Up @@ -578,9 +575,8 @@ export class ProposalsController {
const proposal = await this.proposalsService.findOne({
proposalId,
});
if (!proposal) return { canAccess: false };

const canAccess = await this.permissionChecker(
const canAccess = this.permissionChecker(
Action.ProposalsRead,
proposal,
request,
Expand Down
15 changes: 15 additions & 0 deletions src/proposals/schemas/proposal.schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,21 @@ export class ProposalClass extends OwnableClass {
})
@Prop({ type: Object, required: false, default: {} })
metadata?: Record<string, unknown>;

@ApiProperty({
type: String,
required: false,
description: "Parent proposal id",
default: null,
nullable: true,
})
@Prop({
type: String,
required: false,
default: null,
ref: "Proposal",
})
parentProposalId: string;
}

export const ProposalSchema = SchemaFactory.createForClass(ProposalClass);
Expand Down
39 changes: 21 additions & 18 deletions src/samples/samples.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,17 @@ export class SamplesController {

return sampleInstance;
}
private async permissionChecker(

private permissionChecker(
group: Action,
sample: SampleClass | CreateSampleDto,
sample: SampleClass | CreateSampleDto | null,
request: Request,
) {
const sampleInstance = this.generateSampleInstanceForPermissions(
sample as SampleClass,
);
if (!sample) {
return false;
}

const sampleInstance = this.generateSampleInstanceForPermissions(sample);

const user: JWTUser = request.user as JWTUser;
const ability = this.caslAbilityFactory.samplesInstanceAccess(user);
Expand Down Expand Up @@ -164,28 +167,26 @@ export class SamplesController {
sampleId: id,
});

if (sample) {
const canDoAction = await this.permissionChecker(group, sample, request);
if (!canDoAction) {
throw new ForbiddenException("Unauthorized to this sample");
}
const canDoAction = this.permissionChecker(group, sample, request);

if (!canDoAction) {
throw new ForbiddenException("Unauthorized to this sample");
}

return sample;
}

private async checkPermissionsForSampleCreate(
private checkPermissionsForSampleCreate(
request: Request,
sample: CreateSampleDto,
group: Action,
) {
if (!sample) {
throw new BadRequestException("Not able to create this sample");
}
const canDoAction = await this.permissionChecker(group, sample, request);
const canDoAction = this.permissionChecker(group, sample, request);

if (!canDoAction) {
throw new ForbiddenException("Unauthorized to create this sample");
}

return sample;
}

Expand Down Expand Up @@ -271,11 +272,12 @@ export class SamplesController {
@Req() request: Request,
@Body() createSampleDto: CreateSampleDto,
): Promise<SampleClass> {
const sampleDTO = await this.checkPermissionsForSampleCreate(
const sampleDTO = this.checkPermissionsForSampleCreate(
request,
createSampleDto,
Action.SampleCreate,
);

return this.samplesService.create(sampleDTO);
}

Expand Down Expand Up @@ -551,6 +553,7 @@ export class SamplesController {
id,
Action.SampleRead,
);

return sample;
}

Expand Down Expand Up @@ -583,8 +586,8 @@ export class SamplesController {
const sample = await this.samplesService.findOne({
sampleId: id,
});
if (!sample) return { canAccess: false };
const canAccess = await this.permissionChecker(

const canAccess = this.permissionChecker(
Action.SampleRead,
sample,
request,
Expand Down
2 changes: 1 addition & 1 deletion src/users/users.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class UsersServiceMock {
return { id };
}

async findByIdUserSettings(userId: string) {
async findByIdUserSettings() {
return mockUserSettings;
}

Expand Down
2 changes: 1 addition & 1 deletion src/users/users.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export class UsersController {
description:
"This endpoint is deprecated and will be removed soon. Use /auth/login instead",
})
async login(@Req() req: Record<string, unknown>): Promise<null> {
async login(): Promise<null> {
return null;
}

Expand Down
51 changes: 46 additions & 5 deletions test/Proposal.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
/* eslint-disable @typescript-eslint/no-var-requires */
"use strict";

const { faker } = require("@faker-js/faker");
const utils = require("./LoginUtils");
const { TestData } = require("./TestData");

let accessTokenProposalIngestor = null,
accessTokenAdminIngestor = null,
accessTokenArchiveManager = null,
defaultProposalId = null,
minimalProposalId = null,
proposalId = null,
proposalWithParentId = null,
attachmentId = null;

describe("1500: Proposal: Simple Proposal", () => {
before(() => {
db.collection("Proposal").deleteMany({});
});
beforeEach(async() => {
beforeEach(async () => {
accessTokenProposalIngestor = await utils.getToken(appUrl, {
username: "proposalIngestor",
password: TestData.Accounts["proposalIngestor"]["password"],
Expand Down Expand Up @@ -89,7 +91,7 @@ describe("1500: Proposal: Simple Proposal", () => {
res.body.should.have.property("ownerGroup").and.be.string;
res.body.should.have.property("proposalId").and.be.string;
defaultProposalId = res.body["proposalId"];
proposalId = encodeURIComponent(res.body["proposalId"]);
minimalProposalId = encodeURIComponent(res.body["proposalId"]);
});
});

Expand Down Expand Up @@ -216,7 +218,46 @@ describe("1500: Proposal: Simple Proposal", () => {
});
});

it("0120: should delete this proposal attachment", async () => {
it("0115: adds a new proposal with parent proposal", async () => {
const proposalWithParentProposal = {
...TestData.ProposalCorrectComplete,
proposalId: faker.string.numeric(8),
parentProposalId: proposalId,
};

return request(appUrl)
.post("/api/v3/Proposals")
.send(proposalWithParentProposal)
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenProposalIngestor}` })
.expect(TestData.EntryCreatedStatusCode)
.expect("Content-Type", /json/)
.then((res) => {
res.body.should.have.property("ownerGroup").and.be.string;
res.body.should.have.property("proposalId").and.be.string;
proposalWithParentId = res.body.proposalId;
res.body.should.have.property("parentProposalId").and.be.string;
res.body.parentProposalId.should.be.equal(proposalId);
});
});

it("0120: updates a proposal with a new parent proposal", async () => {
return request(appUrl)
.patch("/api/v3/Proposals/" + proposalWithParentId)
.send({ parentProposalId: minimalProposalId })
.set("Accept", "application/json")
.set({ Authorization: `Bearer ${accessTokenAdminIngestor}` })
.expect(TestData.SuccessfulPatchStatusCode)
.expect("Content-Type", /json/)
.then((res) => {
res.body.should.have.property("ownerGroup").and.be.string;
res.body.should.have.property("proposalId").and.be.string;
res.body.should.have.property("parentProposalId").and.be.string;
res.body.parentProposalId.should.be.equal(minimalProposalId);
});
});

it("0130: should delete this proposal attachment", async () => {
return request(appUrl)
.delete(
"/api/v3/Proposals/" + proposalId + "/attachments/" + attachmentId,
Expand All @@ -226,7 +267,7 @@ describe("1500: Proposal: Simple Proposal", () => {
.expect(TestData.SuccessfulDeleteStatusCode);
});

it("0130: admin can remove all existing proposals", async () => {
it("0140: admin can remove all existing proposals", async () => {
return await request(appUrl)
.get("/api/v3/Proposals")
.set("Accept", "application/json")
Expand Down
9 changes: 5 additions & 4 deletions test/ProposalAuthorization.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-disable @typescript-eslint/no-var-requires */
"use strict";

const { faker } = require("@faker-js/faker");
const utils = require("./LoginUtils");
const { TestData } = require("./TestData");
const sandbox = require("sinon").createSandbox();
Expand All @@ -22,21 +23,21 @@ let proposalPid1 = null,

const proposal1 = {
...TestData.ProposalCorrectMin,
proposalId: "20170268",
proposalId: faker.string.numeric(8),
ownerGroup: "group4",
accessGroups: ["group5"],
};

const proposal2 = {
...TestData.ProposalCorrectComplete,
proposalId: "20170269",
proposalId: faker.string.numeric(8),
ownerGroup: "group1",
accessGroups: ["group3"],
};

const proposal3 = {
...TestData.ProposalCorrectMin,
proposalId: "20170270",
proposalId: faker.string.numeric(8),
ownerGroup: "group2",
accessGroups: ["group3"],
};
Expand All @@ -53,7 +54,7 @@ describe("1400: ProposalAuthorization: Test access to proposal", () => {
before(() => {
db.collection("Proposal").deleteMany({});
});
beforeEach(async() => {
beforeEach(async () => {
accessTokenAdminIngestor = await utils.getToken(appUrl, {
username: "adminIngestor",
password: TestData.Accounts["adminIngestor"]["password"],
Expand Down
Loading

0 comments on commit 15ee6aa

Please sign in to comment.