Skip to content

Commit

Permalink
data,curator: remove notes #2673
Browse files Browse the repository at this point in the history
  • Loading branch information
abhidg committed Apr 27, 2022
1 parent dda0830 commit dee6881
Show file tree
Hide file tree
Showing 11 changed files with 36 additions and 80 deletions.
4 changes: 0 additions & 4 deletions data-serving/data-service/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -816,10 +816,6 @@ components:
type: integer
format: int32
minimum: 0
notes:
type: string
restrictedNotes:
type: string
revisionMetadata:
type: object
properties:
Expand Down
9 changes: 4 additions & 5 deletions data-serving/data-service/src/controllers/case.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ const dtoFromCase = async (storedCase: LeanDocument<CaseDocument>) => {
delete (dto as unknown as { demographics: { ageBuckets?: [ObjectId] }}).demographics.ageBuckets;
}
delete dto.restrictedNotes;
delete dto.notes;

return dto;
}
Expand Down Expand Up @@ -117,9 +118,10 @@ export class CasesController {
return;
}

// don't export restricted notes
// don't export any note
c.forEach((aCase: LeanDocument<CaseDocument>) => {
delete aCase.restrictedNotes;
delete aCase.notes;
});

res.json(await Promise.all(c.map(aCase => dtoFromCase(aCase))));
Expand Down Expand Up @@ -242,10 +244,8 @@ export class CasesController {
doc = await cursor.next();
while (doc != null) {
delete doc.restrictedNotes;
delete doc.notes;
const normalizedDoc = denormalizeFields(doc);
if (!doc.hasOwnProperty('notes')) {
normalizedDoc.notes = '';
}
if (!doc.hasOwnProperty('SGTF')) {
normalizedDoc.SGTF = 'NA';
}
Expand Down Expand Up @@ -340,7 +340,6 @@ export class CasesController {
]);

const dtos = await Promise.all(docs.map(dtoFromCase));

logger.info('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
Expand Down
4 changes: 1 addition & 3 deletions data-serving/data-service/src/model/case.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ caseSchema.methods.equalsJSON = function (jsonCase: any): boolean {
_.isEqual(thisJson.exclusionData, other.exclusionData) &&
_.isEqual(thisJson.genomeSequences, other.genomeSequences) &&
_.isEqual(thisJson.location, other.location) &&
_.isEqual(thisJson.notes, other.notes) &&
_.isEqual(thisJson.restrictedNotes, other.restrictedNotes) &&
_.isEqual(thisJson.pathogens, other.pathogens) &&
_.isEqual(
thisJson.preexistingConditions,
Expand All @@ -145,7 +143,7 @@ export type ICase = {
importedCase: unknown;
location: LocationDocument;
revisionMetadata: RevisionMetadataDocument;
notes: string;
notes?: string;
restrictedNotes?: string;
pathogens: [PathogenDocument];
list: boolean;
Expand Down
2 changes: 1 addition & 1 deletion data-serving/data-service/src/model/fields.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
["_id","caseReference.additionalSources","caseReference.sourceEntryId","caseReference.sourceId","caseReference.sourceUrl","caseReference.uploadIds","caseReference.verificationStatus","demographics.ageRange.end","demographics.ageRange.start","demographics.ethnicity","demographics.gender","demographics.nationalities","demographics.occupation","events","location.administrativeAreaLevel1","location.administrativeAreaLevel2","location.administrativeAreaLevel3","location.country","location.geoResolution","location.geometry.latitude","location.geometry.longitude","location.name","location.place","location.query","notes","pathogens","preexistingConditions.hasPreexistingConditions","preexistingConditions.values","revisionMetadata.creationMetadata.curator","revisionMetadata.creationMetadata.date","revisionMetadata.creationMetadata.notes","revisionMetadata.editMetadata.curator","revisionMetadata.editMetadata.date","revisionMetadata.editMetadata.notes","revisionMetadata.revisionNumber", "SGTF", "symptoms.status","symptoms.values","transmission.linkedCaseIds","transmission.places","transmission.routes","travelHistory.travel.dateRange.end","travelHistory.travel.dateRange.start","travelHistory.travel.location.name","travelHistory.travel.methods","travelHistory.travel.purpose","travelHistory.traveledPrior30Days","vaccines.0.name","vaccines.0.batch","vaccines.0.date","vaccines.0.sideEffects","vaccines.1.name","vaccines.1.batch","vaccines.1.date","vaccines.1.sideEffects","vaccines.2.name","vaccines.2.batch","vaccines.2.date","vaccines.2.sideEffects","vaccines.3.name","vaccines.3.batch","vaccines.3.date","vaccines.3.sideEffects",""]
["_id","caseReference.additionalSources","caseReference.sourceEntryId","caseReference.sourceId","caseReference.sourceUrl","caseReference.uploadIds","caseReference.verificationStatus","demographics.ageRange.end","demographics.ageRange.start","demographics.ethnicity","demographics.gender","demographics.nationalities","demographics.occupation","events","location.administrativeAreaLevel1","location.administrativeAreaLevel2","location.administrativeAreaLevel3","location.country","location.geoResolution","location.geometry.latitude","location.geometry.longitude","location.name","location.place","location.query","pathogens","preexistingConditions.hasPreexistingConditions","preexistingConditions.values","revisionMetadata.creationMetadata.curator","revisionMetadata.creationMetadata.date","revisionMetadata.creationMetadata.notes","revisionMetadata.editMetadata.curator","revisionMetadata.editMetadata.date","revisionMetadata.editMetadata.notes","revisionMetadata.revisionNumber", "SGTF", "symptoms.status","symptoms.values","transmission.linkedCaseIds","transmission.places","transmission.routes","travelHistory.travel.dateRange.end","travelHistory.travel.dateRange.start","travelHistory.travel.location.name","travelHistory.travel.methods","travelHistory.travel.purpose","travelHistory.traveledPrior30Days","vaccines.0.name","vaccines.0.batch","vaccines.0.date","vaccines.0.sideEffects","vaccines.1.name","vaccines.1.batch","vaccines.1.date","vaccines.1.sideEffects","vaccines.2.name","vaccines.2.batch","vaccines.2.date","vaccines.2.sideEffects","vaccines.3.name","vaccines.3.batch","vaccines.3.date","vaccines.3.sideEffects",""]
55 changes: 25 additions & 30 deletions data-serving/data-service/test/controllers/case.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,13 @@ describe('GET', () => {
const res = await request(app).get(`/api/cases/${c._id}`).expect(200);
expect(res.body[0].restrictedNotes).toBeUndefined();
});
it('should not show the notes for a case', async () => {
const c = new Case(minimalCase);
c.notes = 'I want to tell you a secret';
await c.save();
const res = await request(app).get(`/api/cases/${c._id}`).expect(200);
expect(res.body[0].notes).toBeUndefined();
});
it('should convert age bucket to age range', async () => {
const c = new Case(minimalCase);
const bucket = await AgeBucket.findOne({});
Expand Down Expand Up @@ -200,41 +207,21 @@ describe('GET', () => {
.expect(200);
expect(res.body.cases).toHaveLength(1);
});
it('should query results', async () => {
// Simulate index creation used in unit tests, in production they are
// setup by the migrations and such indexes are not present by
// default in the in memory mongo spawned by unit tests.
await mongoose.connection.collection('cases').createIndex({
notes: 'text',
});

const c = new Case(minimalCase);
c.notes = 'got it at work';
await c.save();
// Search for non-matching notes.
const res = await request(app)
.get('/api/cases?page=1&limit=10&q=home')
.expect(200)
.expect('Content-Type', /json/);
expect(res.body.cases).toHaveLength(0);
expect(res.body.total).toEqual(0);
// Search for matching notes.
await request(app)
.get(`/api/cases?page=1&limit=10&q=${encodeURI('at work')}`)
.expect(200, /got it at work/)
.expect('Content-Type', /json/);
});
it('should use age buckets in results', async () => {
const c = new Case(minimalCase);
const aBucket = await AgeBucket.findOne({});
c.demographics.ageBuckets = [aBucket!._id];
await c.save();
const res = await request(app)
.get(`/api/cases?page=1&limit=10`)
.get('/api/cases?page=1&limit=10')
.expect(200)
.expect('Content-Type', /json/);
expect(res.body.cases[0].demographics.ageRange.start).toEqual(aBucket!.start);
expect(res.body.cases[0].demographics.ageRange.end).toEqual(aBucket!.end);
expect(res.body.cases[0].demographics.ageRange.start).toEqual(
aBucket!.start,
);
expect(res.body.cases[0].demographics.ageRange.end).toEqual(
aBucket!.end,
);
});
it('should ignore the restricted collection', async () => {
const r = new RestrictedCase(minimalCase);
Expand Down Expand Up @@ -265,6 +252,14 @@ describe('GET', () => {
expect(res.body.cases).toHaveLength(1);
expect(res.body.cases[0].restrictedNotes).toBeUndefined();
});
it('should strip out notes', async () => {
const c = new Case(minimalCase);
c.notes = 'Can you keep a secret?';
await c.save();
const res = await request(app).get('/api/cases').expect(200);
expect(res.body.cases).toHaveLength(1);
expect(res.body.cases[0].notes).toBeUndefined();
});
describe('keywords', () => {
beforeEach(async () => {
const c = new Case(minimalCase);
Expand Down Expand Up @@ -504,7 +499,7 @@ describe('POST', () => {
const theCase = await Case.findOne({});
// case has range 40-50, should be bucketed into 36-40, 41-45, 46-50
expect(theCase!.demographics.ageBuckets).toHaveLength(3);
})
});
it('GETting the POSTed case should return an age range', async () => {
const theCase = await request(app)
.post('/api/cases')
Expand Down Expand Up @@ -1596,11 +1591,11 @@ describe('PUT', () => {
.send({
...curatorMetadata,
cases: [
{
{
_id: c._id,
demographics: {
ageRange,
}
},
},
],
})
Expand Down
8 changes: 4 additions & 4 deletions data-serving/data-service/test/util/case.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ describe('Case', () => {
'demographics.gender','demographics.nationalities','demographics.occupation','events','genomeSequences',
'location.administrativeAreaLevel1','location.administrativeAreaLevel2','location.administrativeAreaLevel3',
'location.country','location.geoResolution','location.geometry.latitude','location.geometry.longitude',
'location.name','location.place','location.query','notes','pathogens',
'location.name','location.place','location.query','pathogens',
'preexistingConditions.hasPreexistingConditions','preexistingConditions.values',
'revisionMetadata.creationMetadata.curator','revisionMetadata.creationMetadata.date',
'revisionMetadata.creationMetadata.notes','revisionMetadata.editMetadata.curator',
Expand All @@ -114,7 +114,7 @@ describe('Case', () => {
'events.outcome.value','events.icuAdmission.value', 'genomeSequences','location.administrativeAreaLevel1',
'location.administrativeAreaLevel2','location.administrativeAreaLevel3',
'location.country','location.geoResolution','location.geometry.latitude','location.geometry.longitude',
'location.name','location.place','location.query','notes','pathogens',
'location.name','location.place','location.query','pathogens',
'preexistingConditions.hasPreexistingConditions','preexistingConditions.values',
'revisionMetadata.creationMetadata.curator','revisionMetadata.creationMetadata.date',
'revisionMetadata.creationMetadata.notes','revisionMetadata.editMetadata.curator',
Expand All @@ -139,7 +139,7 @@ describe('Case', () => {
'events.selfIsolation.value', 'genomeSequences','location.administrativeAreaLevel1',
'location.administrativeAreaLevel2','location.administrativeAreaLevel3',
'location.country','location.geoResolution','location.geometry.latitude','location.geometry.longitude',
'location.name','location.place','location.query','notes','pathogens',
'location.name','location.place','location.query','pathogens',
'preexistingConditions.hasPreexistingConditions','preexistingConditions.values',
'revisionMetadata.creationMetadata.curator','revisionMetadata.creationMetadata.date',
'revisionMetadata.creationMetadata.notes','revisionMetadata.editMetadata.curator',
Expand All @@ -163,7 +163,7 @@ describe('Case', () => {
'events.selfIsolation.value', 'genomeSequences','location.administrativeAreaLevel1',
'location.administrativeAreaLevel2','location.administrativeAreaLevel3',
'location.country','location.geoResolution','location.geometry.latitude','location.geometry.longitude',
'location.name','location.place','location.query','notes','pathogens',
'location.name','location.place','location.query','pathogens',
'preexistingConditions.hasPreexistingConditions','preexistingConditions.values',
'revisionMetadata.creationMetadata.curator','revisionMetadata.creationMetadata.date',
'revisionMetadata.creationMetadata.notes','revisionMetadata.editMetadata.curator',
Expand Down
8 changes: 0 additions & 8 deletions verification/curator-service/api/openapi/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1590,14 +1590,6 @@ components:
type: integer
format: int32
minimum: 0
notes:
type: string
description: >
Anything that does not fit in a schema but worth mentioning about
the case
example: >
Case travelled to meet their family in their hometown, got infected
at a party there.
revisionMetadata:
type: object
description: Various revisions of this case over time
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ describe('View case', function () {
cy.visit('/');
cy.visit(`cases/view/${resp.body.cases[0]._id}`);
cy.contains('France');
cy.contains('some notes');
cy.should('not.contain', 'some notes');
cy.contains('www.example.com');
cy.contains('PCR test');
cy.contains('French');
Expand Down
1 change: 0 additions & 1 deletion verification/curator-service/ui/src/components/Case.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ export interface Case {
travelHistory: TravelHistory;
genomeSequences: GenomeSequence[];
pathogens: Pathogen[];
notes: string;
revisionMetadata: RevisionMetadata;
exclusionData?: ExclusionData;
variant?: Variant;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ it('loads and displays case', async () => {
expect(getByText('2020-01-20')).toBeInTheDocument();
expect(getByText('xyz789')).toBeInTheDocument();
expect(getByText('2020-04-13')).toBeInTheDocument();
expect(
getByText('Contact of a confirmed case at work.'),
).toBeInTheDocument();
// Demographics.
expect(getByText('Non-binary/Third gender')).toBeInTheDocument();
expect(getByText('50-59')).toBeInTheDocument();
Expand Down
20 changes: 0 additions & 20 deletions verification/curator-service/ui/src/components/ViewCase.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -235,12 +235,6 @@ function CaseDetails(props: CaseDetailsProps): JSX.Element {
>
vaccines
</Button>
<Button
variant="text"
onClick={(): void => scrollTo('notes')}
>
notes
</Button>
</nav>
)}
<div
Expand Down Expand Up @@ -722,20 +716,6 @@ function CaseDetails(props: CaseDetailsProps): JSX.Element {
</Grid>
</Scroll.Element>
</Paper>
<Paper className={classes.paper} variant="outlined" square>
<Scroll.Element name="notes" className={classes.casebox}>
<Typography
className={classes.sectionTitle}
variant="overline"
>
Notes
</Typography>
<Grid container className={classes.grid}>
<RowHeader title="Notes" />
<RowContent content={props.c.notes} />
</Grid>
</Scroll.Element>
</Paper>
</div>
</>
);
Expand Down

0 comments on commit dee6881

Please sign in to comment.