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.

Sign up for GitHub

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

Showing curator info on edit page #72

Merged
merged 2 commits into from
Mar 25, 2024
Merged

Showing curator info on edit page #72

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
52f1a51
Showing curator info on edit page
jim-sheldon Mar 22, 2024
b5738a8
Fix issues blocking tests
stanislaw-zakrzewski Mar 25, 2024
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
117 changes: 65 additions & 52 deletions data-serving/data-service/src/controllers/case.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
CuratorsDocument,
} from '../model/day0-case';
import caseFields from '../model/fields.json';
import { CaseStatus } from '../types/enums';
import { CaseStatus, Role } from '../types/enums';
import { Error, Query } from 'mongoose';
import { ObjectId } from 'mongodb';
import { GeocodeOptions, Geocoder, Resolution } from '../geocoding/geocoder';
Expand Down Expand Up @@ -59,14 +59,24 @@ const caseFromDTO = async (receivedCase: CaseDTO) => {

const user = await User.findOne({ email: receivedCase.curator?.email });
if (user) {
if (user.roles.includes('junior curator')) {
logger.info(`User: ${JSON.stringify(user)}`)
if (user.roles.includes(Role.JuniorCurator)) {
aCase.curators = {
createdBy: user,
createdBy: {
name: user.name || '',
email: user.email
},
};
} else {
} else if (user.roles.includes(Role.Curator) || user.roles.includes(Role.Admin)) {
aCase.curators = {
createdBy: user,
verifiedBy: user,
createdBy: {
name: user.name || '',
email: user.email
},
verifiedBy: {
name: user.name || '',
email: user.email
},
};
}
}
Expand All @@ -88,40 +98,43 @@ const dtoFromCase = async (storedCase: CaseDocument) => {
});
}

if (ageRange && creator) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This was the main culprit causing issues with tests. this if SHOULD only contain age range and NOT the creator. Latter was added because of eslint which required check for a variable that based on the api requirements MUST be present, but due to the other issue in the tests was not. Decoupling this if into two separate ones resolves the issue.

if (verifier) {
dto = {
...dto,
demographics: {
...dto.demographics!,
ageRange,
},
curators: {
createdBy: {
email: creator.email,
name: creator.name,
},
verifiedBy: {
email: verifier.email,
name: verifier.name,
},
} as CuratorsDocument,
};
} else {
dto = {
...dto,
demographics: {
...dto.demographics!,
ageRange,
},
curators: {
createdBy: {
email: creator.email,
name: creator.name,
},
} as CuratorsDocument,
};
if (ageRange) {
if(creator) {
if (verifier) {
dto = {
...dto,
curators: {
createdBy: {
email: creator.email,
name: creator.name,
},
verifiedBy: {
email: verifier.email,
name: verifier.name,
},
} as CuratorsDocument,
};
} else {
dto = {
...dto,
curators: {
createdBy: {
email: creator.email,
name: creator.name,
},
} as CuratorsDocument,
};
}
}
dto = {
...dto,
demographics: {
...dto.demographics!,
ageRange,
}
};



// although the type system can't see it, there's an ageBuckets property on the demographics DTO now
delete ((dto as unknown) as {
Expand Down Expand Up @@ -242,7 +255,7 @@ export class CasesController {
return;
}

logger.info(
logger.debug(
`Streaming case data in format ${req.body.format} matching query ${req.body.query} for correlation ID ${req.body.correlationId}`,
);

Expand All @@ -255,7 +268,7 @@ export class CasesController {
// Stream from mongoose directly into response
// Use provided query and limit, if provided
if (req.body.query) {
logger.info('Request body with query');
logger.debug('Request body with query');
cursor = casesMatchingSearchQuery({
searchQuery: req.body.query as string,
count: false,
Expand All @@ -264,7 +277,7 @@ export class CasesController {
.collation({ locale: 'en_US', strength: 2 })
.cursor();
} else if (req.body.caseIds && queryLimit) {
logger.info('Request body with case IDs and limit');
logger.debug('Request body with case IDs and limit');
cursor = Day0Case.find({
_id: { $in: req.body.caseIds },
})
Expand All @@ -276,7 +289,7 @@ export class CasesController {
})
.cursor();
} else if (req.body.caseIds) {
logger.info('Request body with case IDs and no limit');
logger.debug('Request body with case IDs and no limit');
cursor = Day0Case.find({
_id: { $in: req.body.caseIds },
})
Expand All @@ -287,7 +300,7 @@ export class CasesController {
})
.cursor();
} else if (queryLimit) {
logger.info('Request body with limit and no case IDs');
logger.debug('Request body with limit and no case IDs');
cursor = Day0Case.find()
.lean()
.limit(queryLimit)
Expand All @@ -297,7 +310,7 @@ export class CasesController {
})
.cursor();
} else {
logger.info('Request body with no query, limit, or case IDs');
logger.debug('Request body with no query, limit, or case IDs');
cursor = Day0Case.find()
.lean()
.collation({
Expand Down Expand Up @@ -392,7 +405,7 @@ export class CasesController {
* Handles HTTP GET /api/cases.
*/
list = async (req: Request, res: Response): Promise<void> => {
logger.info('List method entrypoint');
logger.debug('List method entrypoint');
const page = Number(req.query.page) || 1;
const limit = Number(req.query.limit) || 10;
const countLimit = Number(req.query.count_limit) || 10000;
Expand All @@ -401,7 +414,7 @@ export class CasesController {
const verificationStatus =
Boolean(req.query.verification_status) || undefined;

logger.info('Got query params');
logger.debug('Got query params');

if (page < 1) {
res.status(422).json({ message: 'page must be > 0' });
Expand All @@ -420,7 +433,7 @@ export class CasesController {
return;
}

logger.info('Got past 422s');
logger.debug('Got past 422s');
try {
const casesQuery = casesMatchingSearchQuery({
searchQuery: req.query.q || '',
Expand Down Expand Up @@ -457,7 +470,7 @@ export class CasesController {
]);

const dtos = await Promise.all(docs.map(dtoFromCase));
logger.info('got results');
logger.debug('got results');
// total is actually stored in a count index in mongo, so the query is fast.
// however to maintain existing behaviour, only return the count limit
const reportedTotal = Math.min(total, countLimit);
Expand All @@ -469,11 +482,11 @@ export class CasesController {
nextPage: page + 1,
total: reportedTotal,
});
logger.info('Got multiple pages of results');
logger.debug('Got multiple pages of results');
return;
}
// If we fetched all available data, just return it.
logger.info('Got one page of results');
logger.debug('Got one page of results');
res.json({ cases: dtos, total: reportedTotal });
} catch (e) {
if (e instanceof ParsingError) {
Expand All @@ -495,7 +508,7 @@ export class CasesController {
* Handles HTTP GET /api/cases/countryData.
*/
countryData = async (req: Request, res: Response): Promise<void> => {
logger.info('Get cases by country method entrypoint');
logger.debug('Get cases by country method entrypoint');

try {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down
5 changes: 3 additions & 2 deletions data-serving/data-service/src/model/day0-case.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
RevisionMetadataDocument,
revisionMetadataSchema,
} from './revision-metadata';
import { userSchema } from '../model/user';

/*
* There are separate types for case for data storage (the mongoose document) and
Expand Down Expand Up @@ -60,8 +61,8 @@ export const sourceSchema = new mongoose.Schema({

export const curatorsSchema = new mongoose.Schema(
{
verifiedBy: { type: mongoose.Schema.Types.ObjectId, ref: 'user' },
createdBy: { type: mongoose.Schema.Types.ObjectId, ref: 'user' },
verifiedBy: { type: userSchema },
createdBy: { type: userSchema },
},
{ _id: false },
);
Expand Down
1 change: 1 addition & 0 deletions data-serving/data-service/src/model/user.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export type UserDocument = mongoose.Document &
};

export const userSchema = new mongoose.Schema({
name: String,
email: String,
roles: [String],
});
Expand Down
6 changes: 6 additions & 0 deletions data-serving/data-service/src/types/enums.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,9 @@ export enum YesNo {
N = 'N',
NA = 'NA',
}

export enum Role {
Admin = 'admin',
Curator = 'curator',
JuniorCurator = 'junior curator',
}
22 changes: 18 additions & 4 deletions data-serving/data-service/test/controllers/case.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,18 @@ import {
import fs from 'fs';
import { AgeBucket } from '../../src/model/age-bucket';
import { User, UserDocument } from '../../src/model/user';
import { Role } from '../../src/types/enums';

let mongoServer: MongoMemoryServer;

const curatorName = 'Casey Curatorio';
const curatorUserEmail = 'case_curator@global.health';
// const curatorMetadata = {
// curator: {
// name: curatorName,
// email: curatorUserEmail
// }
// };
const curatorMetadata = { curator: { email: curatorUserEmail } };

const minimalRequest = {
Expand Down Expand Up @@ -69,10 +77,16 @@ beforeAll(async () => {
mockLocationServer.listen();
mongoServer = new MongoMemoryServer();
await createAgeBuckets();
curator = await User.create({ email: curatorUserEmail });
curator = await User.create(
{
name: curatorName,
email: curatorUserEmail,
roles: [Role.Curator]
}
);
minimalDay0CaseData = {
...minimalDay0CaseData,
...{ curators: { createdBy: curator._id } },
...{ curators: { createdBy: curatorMetadata.curator } },
Comment on lines -75 to +89
Copy link
Collaborator

@stanislaw-zakrzewski stanislaw-zakrzewski Mar 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I updated it (and also one in the full case) from:

...{ curators: curatorMetadata.curator },

to:

...{ curators: { createdBy: curatorMetadata.curator } },

this triggered the hidden if coupling described above.

...{
revisionMetadata: {
revisionNumber: 0,
Expand All @@ -84,7 +98,7 @@ beforeAll(async () => {
};
fullDay0CaseData = {
...fullCase,
...{ curators: { createdBy: curator._id } },
...{ curators: { createdBy: curatorMetadata.curator } },
...{
revisionMetadata: {
revisionNumber: 0,
Expand Down Expand Up @@ -1000,7 +1014,7 @@ describe('POST', () => {
.post('/api/cases/batchUpsert')
.send({
cases: [
{ ...minimalCase, curators: { createdBy: curator._id } },
minimalDay0CaseData,
invalidRequest,
],
...curatorMetadata,
Expand Down
Loading