Test/global actions#7955
Conversation
david-yz-liu
left a comment
david-yz-liu
left a comment
There was a problem hiding this comment.
@akarki2005 in addition to the inline comments I left, I see that the pre-commit bot made some updates to your branch. This indicates that you haven't set up the pre-commit hooks properly in your local repository, so please make sure to do so before making another commit.
| describe '#check_for_groupings' do | ||
| let(:grouping) { create(:grouping_with_inviter) } | ||
|
|
||
| it 'should return a bad request when no grouping is selected.' do |
There was a problem hiding this comment.
This test is good, but should be grouped with the other validate_grouping test. #check_for_groupings is a helper method, so this test is really checking the behaviour of the main controller method (global_actions), and the tests there are subdivided into the different action types.
| assignment_id: grouping.assignment.id, | ||
| groupings: [], | ||
| global_actions: 'valid' } | ||
| expect(response).to have_http_status(:bad_request) |
There was a problem hiding this comment.
This is good, but also check the contents of the flash error message (look at how other tests do this).
This applies to the other tests you've added below.
| end | ||
| end | ||
|
|
||
| describe '#add_member' do |
There was a problem hiding this comment.
Similarly to my above comment, you don't need to add a new describe block; it's okay to just add these test cases to the existing block
| groupings: [grouping], | ||
| students: [student1.id], | ||
| global_actions: 'assign' } | ||
| expect(assigns(:warning_grace_day)).to be_present |
There was a problem hiding this comment.
This one is interesting. I looked into this and the @warning_grace_day instance variable isn't actually used anywhere.
Please modify the actual controller code to instead flash a warning message, and then make sure to test this in the UI. The point is that this warning message should appear to the instructor when this situation happens (but the student can still be assigned---verify this in the test case too).
2 participants
Proposed Changes
This PR adds missing test coverage for three private helper methods called through
GroupsController#global_actions:check_for_groupings: raises when no groupings are selectedadd_members: raises when 2+ groupings are selected, no students are selected, or group_max would be exceededadd_member: sets:inviterstatus when grouping is empty, raises when student is already grouped, raises when student cannot be invited (different course), and sets@warning_grace_daywhen student has insufficient grace creditsType of Change
(Write an
Xor a brief description next to the type or types that best describe your changes.)Checklist
(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the
[ ]into a[x]in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)Before opening your pull request:
After opening your pull request: