Add more tests for instructor permissions to fetch ddahs #599
Conversation
alex-kozin
changed the title
alex-kozin
force-pushed
the
feat/test-fetch-instructor-ddahs
branch
from
0a942d9
to
2c27c54
Compare
alex-kozin
changed the title
alex-kozin
changed the title
alex-kozin
changed the title
alex-kozin
force-pushed
the
feat/test-fetch-instructor-ddahs
branch
from
3339cde
to
4697c51
Compare
alex-kozin
changed the title
| expect(resp).toHaveStatus("success"); | ||
|
|
||
| // Only one DDAH is intially seeded for this instructor | ||
| expect(resp.payload).toHaveLength(1); |
There was a problem hiding this comment.
Even though this implementation should be fine, I think you should not assume that only one DDAH is initially seeded for the instructor unless you are seeding the instructor within the test. This is because someone may need to seed more DDAH for this particular instructor in the future and might run into problems with this test.
There was a problem hiding this comment.
I think you should fetch all DDAHs as admin and filter that list to contain only those for that instructor. Then check the list the instructor retrieves is the same.
There was a problem hiding this comment.
It's a great idea! For the implementation though, we don't have an endpoint to get all DDAHs per instructor? That means we will have to fetch all positions, filter them per instructor, then get all assignments for these positions and then get their respective DDAHs? That would complicate the test a lot imo, even though that will yield the most predictable outcome regardless of the seeded data.
That also conflicts with the idea that we "know the seeded data" and can rely on it unless I am missing something of course
| // Cleanup | ||
| for (const assignment of seeded.assignments) { | ||
| if (assignment._temp_id) { | ||
| const { temp_id, ...cleanAssignment } = assignment; |
There was a problem hiding this comment.
@alex-kozin Fix this - delete all properties with an underscore
More tests for fetching DDAHs for instructor. Related to issue #597