Skip to content

Commit

Permalink
SIMSBIOHUB-10: pending review status for related datasets (#205)
Browse files Browse the repository at this point in the history
* Added supplementary data that reflects if the related dataset has documents pending review

* Front end changes to show pending review status when applicable
  • Loading branch information
anissa-agahchen authored Jul 1, 2023
1 parent 7190ce0 commit 8b1bcc8
Show file tree
Hide file tree
Showing 9 changed files with 223 additions and 27 deletions.
14 changes: 10 additions & 4 deletions api/src/paths/dwc/submission/{datasetId}/artifacts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import sinonChai from 'sinon-chai';
import * as db from '../../../../database/db';
import { HTTPError } from '../../../../errors/http-error';
import { Artifact } from '../../../../repositories/artifact-repository';
import { SECURITY_APPLIED_STATUS } from '../../../../repositories/security-repository';
import { ArtifactService } from '../../../../services/artifact-service';
import { SecurityService } from '../../../../services/security-service';
import { UserService } from '../../../../services/user-service';
import { getMockDBConnection, getRequestHandlerMocks } from '../../../../__mocks__/db';
import { GET, getArtifactsByDatasetId } from './artifacts';

Expand Down Expand Up @@ -219,9 +221,11 @@ describe('getArtifactsByDatasetId', () => {
{ artifact_id: 2, security_review_timestamp: new Date('1970-01-01T00:00:00.000Z') }
] as Artifact[]);

const securityServiceStub = sinon
.stub(SecurityService.prototype, 'getPersecutionAndHarmRulesByArtifactId')
.resolves([]);
const isSystemUserAdminStub = sinon.stub(UserService.prototype, 'isSystemUserAdmin').resolves(true);

const getArtifactSupplementaryDataStub = sinon
.stub(SecurityService.prototype, 'getArtifactSupplementaryData')
.resolves({ persecutionAndHarmRules: [], persecutionAndHarmStatus: SECURITY_APPLIED_STATUS.UNSECURED });

const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

Expand All @@ -235,7 +239,6 @@ describe('getArtifactsByDatasetId', () => {

expect(mockRes.statusValue).to.equal(200);
expect(artifactServiceStub).to.be.calledWith('abcd');
expect(securityServiceStub).to.be.calledTwice;
expect(mockRes.jsonValue).to.eql([
{
artifact_id: 1,
Expand All @@ -254,13 +257,16 @@ describe('getArtifactsByDatasetId', () => {
}
}
]);
expect(isSystemUserAdminStub).to.be.calledOnce;
expect(getArtifactSupplementaryDataStub).to.be.calledTwice;
});

it('catches and re-throws an error', async () => {
const dbConnectionObj = getMockDBConnection();
sinon.stub(db, 'getDBConnection').returns(dbConnectionObj);

sinon.stub(ArtifactService.prototype, 'getArtifactsByDatasetId').rejects(new Error('a test error'));
sinon.stub(UserService.prototype, 'isSystemUserAdmin').resolves(true);

const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

Expand Down
21 changes: 5 additions & 16 deletions api/src/paths/dwc/submission/{datasetId}/artifacts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { RequestHandler } from 'express';
import { Operation } from 'express-openapi';
import { getAPIUserDBConnection, getDBConnection } from '../../../../database/db';
import { defaultErrorResponses } from '../../../../openapi/schemas/http-responses';
import { SECURITY_APPLIED_STATUS } from '../../../../repositories/security-repository';
import { ArtifactService } from '../../../../services/artifact-service';
import { SecurityService } from '../../../../services/security-service';
import { UserService } from '../../../../services/user-service';
import { getLogger } from '../../../../utils/logger';

const defaultLog = getLogger('paths/dwc/submission/{datasetId}/artifacts');
Expand Down Expand Up @@ -145,30 +145,19 @@ export function getArtifactsByDatasetId(): RequestHandler {

try {
await connection.open();

const userService = new UserService(connection);
const artifactService = new ArtifactService(connection);
const securityService = new SecurityService(connection);

const isAdmin = await userService.isSystemUserAdmin();
const artifacts = await artifactService.getArtifactsByDatasetId(datasetId);

const artifactsWithRules = await Promise.all(
artifacts.map(async (artifact) => {
const persecutionAndHarmRules = await securityService.getPersecutionAndHarmRulesByArtifactId(
artifact.artifact_id
);
let persecutionAndHarmStatus: SECURITY_APPLIED_STATUS = SECURITY_APPLIED_STATUS.PENDING;

if (artifact.security_review_timestamp) {
persecutionAndHarmStatus =
persecutionAndHarmRules.length > 0 ? SECURITY_APPLIED_STATUS.SECURED : SECURITY_APPLIED_STATUS.UNSECURED;
}

const supplementaryData = await securityService.getArtifactSupplementaryData(artifact.artifact_id, isAdmin);
return {
...artifact,
supplementaryData: {
persecutionAndHarmRules,
persecutionAndHarmStatus
}
supplementaryData: supplementaryData
};
})
);
Expand Down
30 changes: 28 additions & 2 deletions api/src/paths/dwc/submission/{datasetId}/related.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import sinon from 'sinon';
import sinonChai from 'sinon-chai';
import * as db from '../../../../database/db';
import { HTTPError } from '../../../../errors/http-error';
import { SecurityService } from '../../../../services/security-service';
import { RelatedDataset, SubmissionService } from '../../../../services/submission-service';
import { UserService } from '../../../../services/user-service';
import { getMockDBConnection, getRequestHandlerMocks } from '../../../../__mocks__/db';
import { GET, getRelatedDatasetsByDatasetId } from './related';

Expand Down Expand Up @@ -67,7 +69,10 @@ describe('getRelatedDatasetsByDatasetId', () => {
const mockRelatedDataset = {
datasetId: '374c4d6a-3a04-405b-af6d-b6497800a691',
title: 'Test-Related-Dataset',
url: 'https://example.com/374c4d6a-3a04-405b-af6d-b6497800a691'
url: 'https://example.com/374c4d6a-3a04-405b-af6d-b6497800a691',
supplementaryData: {
isPendingReview: false
}
};

describe('should throw an error when', () => {
Expand Down Expand Up @@ -281,6 +286,12 @@ describe('getRelatedDatasetsByDatasetId', () => {
.stub(SubmissionService.prototype, 'findRelatedDatasetsByDatasetId')
.resolves([{ datasetId: 'aaa' }, { datasetId: 'bbb' }] as RelatedDataset[]);

const isSystemUserAdminStub = sinon.stub(UserService.prototype, 'isSystemUserAdmin').resolves(true);

const securityStub = sinon.stub(SecurityService.prototype, 'isDatasetPendingReview');
securityStub.onFirstCall().resolves(true);
securityStub.onSecondCall().resolves(false);

const { mockReq, mockRes, mockNext } = getRequestHandlerMocks();

mockReq.params = {
Expand All @@ -294,13 +305,28 @@ describe('getRelatedDatasetsByDatasetId', () => {
expect(mockRes.statusValue).to.equal(200);
expect(submissionServiceStub).to.be.calledWith('abcd');
expect(mockRes.jsonValue).to.eql({
datasets: [{ datasetId: 'aaa' }, { datasetId: 'bbb' }]
datasetsWithSupplementaryData: [
{
datasetId: 'aaa',
supplementaryData: {
isPendingReview: true
}
},
{
datasetId: 'bbb',
supplementaryData: {
isPendingReview: false
}
}
]
});
expect(isSystemUserAdminStub).to.be.calledOnce;
});

it('catches and re-throws an error', async () => {
const dbConnectionObj = getMockDBConnection();
sinon.stub(db, 'getDBConnection').returns(dbConnectionObj);
sinon.stub(UserService.prototype, 'isSystemUserAdmin').resolves(true);

sinon.stub(SubmissionService.prototype, 'findRelatedDatasetsByDatasetId').rejects(new Error('a test error'));

Expand Down
35 changes: 33 additions & 2 deletions api/src/paths/dwc/submission/{datasetId}/related.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import { RequestHandler } from 'express';
import { Operation } from 'express-openapi';
import { getAPIUserDBConnection, getDBConnection } from '../../../../database/db';
import { defaultErrorResponses } from '../../../../openapi/schemas/http-responses';
import { SecurityService } from '../../../../services/security-service';
import { SubmissionService } from '../../../../services/submission-service';
import { UserService } from '../../../../services/user-service';
import { getLogger } from '../../../../utils/logger';

const defaultLog = getLogger('paths/dwc/submission/{datasetId}/related');
Expand Down Expand Up @@ -41,7 +43,7 @@ GET.apiDoc = {
type: 'array',
items: {
type: 'object',
required: ['datasetId', 'title', 'url'],
required: ['datasetId', 'title', 'url', 'supplementaryData'],
properties: {
datasetId: {
type: 'string',
Expand All @@ -52,6 +54,14 @@ GET.apiDoc = {
},
url: {
type: 'string'
},
supplementaryData: {
type: 'object',
properties: {
isPendingReview: {
type: 'boolean'
}
}
}
}
}
Expand Down Expand Up @@ -80,12 +90,33 @@ export function getRelatedDatasetsByDatasetId(): RequestHandler {
await connection.open();

const submissionService = new SubmissionService(connection);
const securityService = new SecurityService(connection);
const userService = new UserService(connection);

const isAdmin = await userService.isSystemUserAdmin();
const datasets = await submissionService.findRelatedDatasetsByDatasetId(datasetId);

const datasetsWithSupplementaryData = await Promise.all(
datasets.map(async (dataset) => {
if (!isAdmin) {
return {
...dataset,
supplementaryData: {}
};
}

const isDatasetPendingReview = await securityService.isDatasetPendingReview(dataset.datasetId);

return {
...dataset,
supplementaryData: { isPendingReview: isDatasetPendingReview }
};
})
);

await connection.commit();

res.status(200).json({ datasets });
res.status(200).json({ datasetsWithSupplementaryData });
} catch (error) {
defaultLog.error({ label: 'getRelatedDatasetsByDatasetId', message: 'error', error });
await connection.rollback();
Expand Down
2 changes: 1 addition & 1 deletion api/src/services/artifact-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export class ArtifactService extends DBService {
* @memberof ArtifactService
*/
async updateArtifactSecurityReviewTimestamp(artifactId: number): Promise<void> {
defaultLog.debug({ label: 'removeAllSecurityRulesFromArtifact' });
defaultLog.debug({ label: 'updateArtifactSecurityReviewTimestamp' });

await this.artifactRepository.updateArtifactSecurityReviewTimestamp(artifactId);
}
Expand Down
66 changes: 66 additions & 0 deletions api/src/services/security-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,4 +494,70 @@ describe('SecurityService', () => {
expect(result).to.eql(false);
});
});

describe('isDatasetPendingReview', () => {
afterEach(() => {
sinon.restore();
});

it('should return true if the any artifact in the dataset is pending review', async () => {
const mockDBConnection = getMockDBConnection();
const securityService = new SecurityService(mockDBConnection);

const getArtifactsByDatasetIdStub = sinon.stub(ArtifactService.prototype, 'getArtifactsByDatasetId').resolves([
{
artifact_id: 1,
key: 'secured-string-a',
security_review_timestamp: null
} as Artifact,
{
artifact_id: 2,
key: 'secured-string-b',
security_review_timestamp: null
} as Artifact
]);

sinon
.stub(SecurityService.prototype, 'isArtifactPendingReview')
.onFirstCall()
.resolves(true)
.onSecondCall()
.resolves(false);

const result = await securityService.isDatasetPendingReview('datasetId');

expect(getArtifactsByDatasetIdStub).to.be.calledWith('datasetId');
expect(result).to.eql(true);
});

it('should return false if the no artifact in the dataset is pending review', async () => {
const mockDBConnection = getMockDBConnection();
const securityService = new SecurityService(mockDBConnection);

const getArtifactsByDatasetIdStub = sinon.stub(ArtifactService.prototype, 'getArtifactsByDatasetId').resolves([
{
artifact_id: 1,
key: 'secured-string-a',
security_review_timestamp: null
} as Artifact,
{
artifact_id: 2,
key: 'secured-string-b',
security_review_timestamp: null
} as Artifact
]);

sinon
.stub(SecurityService.prototype, 'isArtifactPendingReview')
.onFirstCall()
.resolves(false)
.onSecondCall()
.resolves(false);

const result = await securityService.isDatasetPendingReview('datasetId');

expect(getArtifactsByDatasetIdStub).to.be.calledWith('datasetId');
expect(result).to.eql(false);
});
});
});
52 changes: 52 additions & 0 deletions api/src/services/security-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,39 @@ export class SecurityService extends DBService {
return this.securityRepository.getPersecutionAndHarmRulesByArtifactId(artifactId);
}

/**
* Get Artifact Supplementary Data.
*
* @param {number} artifactId
* @param {boolean} isAdmin
* @return {*} {Promise<{ persecutionAndHarmRules: ArtifactPersecution[]; persecutionAndHarmStatus: SECURITY_APPLIED_STATUS }>}
* @memberof SecurityService
*/
async getArtifactSupplementaryData(
artifactId: number,
isAdmin: boolean
): Promise<{ persecutionAndHarmRules: ArtifactPersecution[]; persecutionAndHarmStatus: SECURITY_APPLIED_STATUS }> {
defaultLog.debug({ label: 'getArtifactSupplementaryData' });

let persecutionAndHarmRules: ArtifactPersecution[] = [];

//If user is Admin, get all rules
if (isAdmin) {
persecutionAndHarmRules = await this.getPersecutionAndHarmRulesByArtifactId(artifactId);
}

let persecutionAndHarmStatus = await this.getSecurityAppliedStatus(artifactId);
//If user is not Admin and status is pending, set to secured
if (!isAdmin && persecutionAndHarmStatus === SECURITY_APPLIED_STATUS.PENDING) {
persecutionAndHarmStatus = SECURITY_APPLIED_STATUS.SECURED;
}

return {
persecutionAndHarmRules: persecutionAndHarmRules,
persecutionAndHarmStatus: persecutionAndHarmStatus
};
}

/**
* Apply security rules to all selected artifacts.
*
Expand Down Expand Up @@ -272,4 +305,23 @@ export class SecurityService extends DBService {
const artifact = await this.artifactService.getArtifactById(artifactId);
return artifact.security_review_timestamp ? false : true;
}

/**
* Returns true is any artifacts in the dataset are pending review
*
* @param {string} datasetId
* @return {*} {Promise<boolean>}
* @memberof SecurityService
*/
async isDatasetPendingReview(datasetId: string): Promise<boolean> {
const artifactIds = (await this.artifactService.getArtifactsByDatasetId(datasetId)).map((item) => item.artifact_id);

const artifactSecurityRules = await Promise.all(
artifactIds.map(async (artifactId) => await this.isArtifactPendingReview(artifactId))
);

const isPendingReview = artifactSecurityRules.includes(true);

return isPendingReview;
}
}
Loading

0 comments on commit 8b1bcc8

Please sign in to comment.