-
Notifications
You must be signed in to change notification settings - Fork 4
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
Refactor test/integration/patients/patient/dashboard.js
tests for jsonapi
#1245
Conversation
This pull request has been linked to Shortcut Story #49091: Refactor |
Passing run #6385 ↗︎
Details:
Review all test suite changes for PR #1245 ↗︎ |
Pull Request Test Coverage Report for Build a3c1413d-5cca-4ec7-b546-802f35b4f802Details
💛 - Coveralls |
fx.data[4].attributes.archived_at = testTs(); | ||
fx.data[4].relationships['program-actions'] = { data: [] }; | ||
fx.data[4].relationships['program-flows'] = { data: [] }; | ||
fx.data = [ |
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.
this test setup would be best to create the elements at the top using the uuids that come back.. however there is a circular relationship so some of the getFoo
s will need to be passed uuid
s since they need to be used in a relationship prior to their instantiation.
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.
I made some updates to this test. But probably more that can be done here.
relationships: { | ||
'program-flows': getRelationship([{ id: '4' }], 'flows'), | ||
'program-actions': getRelationship( | ||
[{ id: '1' }, { id: '4' }, { id: '5' }, { id: '6' }], |
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.
all of the ids and the ones below should be able to be done by using only getProgram
, getProgramAction
and getProgramFlow
except that you'll need uuid
s to setup the circular ids.. should be like on patient-flow or program-flow tests as well.
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.
I've got a solution for this. But it feels overly complicated. Open to any better ideas.
@@ -405,7 +405,7 @@ context('patient dashboard page', function() { | |||
.should('contain', `patient-action/${ testAction.id }/form/${ testForm.id }`); | |||
}); | |||
|
|||
specify('add action and flow', function() { | |||
specify.only('add action and flow', function() { |
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.
Forgot the .only
here 🤦, but it's removed in the next commit.
const currentClinican = getCurrentClinician({ | ||
relationships: { | ||
role: getRelationship(roleEmployee), | ||
}, | ||
}); | ||
|
||
const testProgramActionIds = [uuid(), uuid(), uuid(), uuid(), uuid(), uuid()]; | ||
const testProgramFlowIds = [uuid(), uuid(), uuid(), uuid(), uuid(), uuid()]; |
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.
I needed to set these all at the top. And then use them throughout.
Otherwise using e.g. testProgramActions[0].id
or testProgramFlows[0].id
in testPrograms = []
would cause errors. Since we'd be trying to use data/variables that weren't instantiated yet.
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.
I think if you flipped it and made programIds with uuid then you could define the flows/actions first and use the ids generated from there for relations on the program.
You could also use _.times
with uuid
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.
Think that works well 👍 , and _.times
is also much better
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.
nearly there
const currentClinican = getCurrentClinician({ | ||
relationships: { | ||
role: getRelationship(roleEmployee), | ||
}, | ||
}); | ||
|
||
const testProgramActionIds = [uuid(), uuid(), uuid(), uuid(), uuid(), uuid()]; | ||
const testProgramFlowIds = [uuid(), uuid(), uuid(), uuid(), uuid(), uuid()]; |
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.
I think if you flipped it and made programIds with uuid then you could define the flows/actions first and use the ids generated from there for relations on the program.
You could also use _.times
with uuid
archived_at: null, | ||
}, | ||
relationships: { | ||
program: getRelationship(testProgramIds[1], 'programs'), |
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.
The test still passes if these are removed... 🤔
Shortcut Story ID: [sc-49091]