-
Notifications
You must be signed in to change notification settings - Fork 7
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
Remove notes field, fixes #2673 #2680
Changes from 5 commits
dda0830
dee6881
3ccce55
a4a738e
2201142
46194bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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",""] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this would be easier to diff if we put each element on its own line :) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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({}); | ||
|
@@ -200,41 +207,21 @@ describe('GET', () => { | |
.expect(200); | ||
expect(res.body.cases).toHaveLength(1); | ||
}); | ||
it('should query results', async () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any text that the query should work over instead of notes, or is there any free-text search facility at all in the platform now? If not we can also drop the text index, which will speed up ingestion and save 13.5G in the prod cluster (the main collection is only 10.1G!) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, in #2687 |
||
// 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); | ||
|
@@ -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); | ||
|
@@ -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') | ||
|
@@ -1596,11 +1591,11 @@ describe('PUT', () => { | |
.send({ | ||
...curatorMetadata, | ||
cases: [ | ||
{ | ||
{ | ||
_id: c._id, | ||
demographics: { | ||
ageRange, | ||
} | ||
}, | ||
}, | ||
], | ||
}) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this is a "not yet" task but we could migrate
restrictedNotes
back over tonotes
, as all notes are now restricted. Then we would be able to build a single approach to exposing notes later.