View source for CSC/ECE 517 Fall 2021 - E2161. Merge code for role based reviewing with code for topic specific rubrics

This wiki page contains description of changes designed and implemented for [https://docs.google.com/document/d/1slx4HPIbgTH-psIKMSCF-HDF9brxf-FuYhzVT9ZiIrM/edit#heading=h.8s6o9jnnxj0p E2161. Merge code for role-based reviewing with code for topic-specific rubrics], a Final Project for CSC/ECE 517, Fall 2021.
== Purpose==
In CSC/ECE 517, there are Expertiza-based course projects, Mozilla-based course projects, etc. However, currently, we can only specify one kind of rubric for all kinds of course projects. This means that refactoring projects, testing projects, and Mozilla projects need to use the same rubric.  We hope we could specify different rubrics to be used with different kinds of course projects.

The project objective is to merge [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2147._Role-based_reviewing#E2147._Role-based_reviewing E2147]with the existing [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Spring_2020_-_E2026._Specialized_rubrics_for_different_topic_types E2026]. While merging, we found out that the code for E2026 has already been merged on Expertiza beta branch and so now the actual implementation is to write test cases for both E2147 and E2026.

==Brief Overview of E2147 and E2026 Projects==
*[https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2147._Role-based_reviewing#E2147._Role-based_reviewing E2147(Role based reviewing)]: The project objective was to develop teammate review varying by the roles that each team member has in order to evaluate the particular role that each team member has taken rather than asking a generic questionnaire which was same for all the team members previously. For eg, the instructor would create a separate teammate questionnaire for developer role and for tester role respectively.
*[https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Spring_2020_-_E2026._Specialized_rubrics_for_different_topic_types E2026(Specialized rubrics for different topics)]: The project objective was to develop specialized rubrics based on project topics, previously we had same type of questionnaire for all the different types of projects. This implementation made sure that we evaluate different projects based on personalised rubrics rather than having same rubrics.

==Testing Plan==

===Test Scenarios for E2147  (Role based reviewing)===
*Only Instructor/TA/Admin should be able to create or update duties: Only the instructor/admin should be able to create or update duties. Also, the corresponding TA of the assignment should be able to create or update duties.
*Create new roles: If the assignment is role based reviewing then the instructor should be able to create new roles for the same and save it in the database using new role functionality.
*Only allow student to update duties in Student View: Only student should be able to update his duties in Student View. 
*Edit existing roles: Once the new roles have been created, then the instructor should be able to edit its details such as the maximum number of each role allowed.
*Delete existing roles: The instructor should also be able to delete any roles if he wants.
*Review rubric by role: On Rubrics tab, if the instructor clicks on Review rubric by role, then the corresponding rubric roles based teammate questionnaire should appear and get saved successfully.
*Student View: If the assignment is role based assignment, then the respective students should be able to see the respective teammate questionnaire based on roles for his other team members.
*Selection of Roles: On student login, a student should be able to see the existing roles that he can take up in the project. Only the roles which are available should be shown to the student.

===Test Scenarios for E2026  (Specialized rubrics for different topics)===

As part of our implementation, we modified existing code as well as added new code. To ensure that existing functionality was not broken, and new functionality worked as expected, we used the following Test Strategy (which was also used by previous team):

===Run and pass existing RSpec Tests===
*The following existing RSpec test files have been modified and they pass as part of testing:
**spec/controllers/assignments_controller_spec.rb
**spec/controllers/questionnaires_controller_spec.rb
**spec/controllers/response_controller_spec.rb
**spec/factories/factories.rb
**spec/features/assignment_creation_spec.rb
**spec/features/quiz_spec.rb
**spec/features/staggered_deadline_spec.rb
**spec/models/assignment_form_spec.rb
**spec/models/assignment_spec.rb
**spec/models/on_the_fly_calc_spec.rb
**spec/models/response_spec.rb
**spec/models/review_response_map_spec.rb

===Develop New RSpec Tests===
*spec/helpers/duties_controller_spec.rb
*All these rspec tests passed.


== duties_controller_spec.rb ==
We have created this new test file to unit test the functionality of duties_controller.

The first step was to create mock data so that the test could run on objects created for the tests. The code below shows how the mock data was created for the specific tests that we created to ensure the functionality of the projects.

''
 describe DutiesController do
  let(:assignment) { build(:assignment, id: 1,course_id: 1,instructor_id: 6, due_dates: [due_date], microtask: true, staggered_deadline: true)}
  let(:admin) { build(:admin) }
  let(:instructor) { build(:instructor, id: 6) }
  let(:instructor2) { build(:instructor, id: 66) }
  let(:ta) { build(:teaching_assistant, id: 8) }
  let(:duty) { build(:duty, id: 1, duty_name: "Role", max_members_for_duty: 2, assignment_id: 1) }
  let(:due_date) { build(:assignment_due_date, deadline_type_id: 1) } ''

We then run the arbitrary code that is needed before each test. We include method stubs here that we can use for each test. We stub the current user to be instructor for now.

''
 before(:each) do
    allow(Assignment).to receive(:find).with('1').and_return(assignment)
    allow(Assignment).to receive(:find).with(1).and_return(assignment)
    stub_current_user(instructor, instructor.role.name, instructor.role)
    allow(Duty).to receive(:find).with('1').and_return(duty)
  end
''

Test scenarios for duties controller:

1) Only admin, superadmin, TA, Instructor can perform edit or update action

'' describe '#action_allowed?' do ''

The whole context that defines if the action allowed is edit or update. We do this by mocking the action to edit. We then run the arbitrary code that is needed before each test in this context. We include method stubs here that we can use for each test. We stub the current user to be instructor for now.

    context 'when params action is edit or update' do
      before(:each) do
        controller.params = {id: '1', action: 'edit'}
      end

When the current logged in user role is super admin or admin. We check if the action is allowed by stubbing the current user to admin. We then verify if the controller is able to perform the action for the current admin user
 
      context 'when the role name of current user is super admin or admin' do
        it 'allows certain action' do
          stub_current_user(admin, admin.role.name, admin.role)
          expect(controller.send(:action_allowed?)).to be true
        end
      end

When the current logged in user role is instructor of the current assignment. We check if the action is allowed by stubbing the current user to instructor. We then verify if the controller is able to perform the action for the current instructor user

      context 'when current user is the instructor of current assignment' do
        it 'allows certain action' do
          expect(controller.send(:action_allowed?)).to be true
        end
      end

When the current logged in user role is the TA of the course. We check if the action is allowed by stubbing the current user to TA. We then verify if the controller is able to perform the action for the current TA user

      context 'when current user is the ta of the course which current assignment belongs to' do
        it 'allows certain action' do
          stub_current_user(ta, ta.role.name, ta.role)
          allow(TaMapping).to receive(:exists?).with(ta_id: 8, course_id: 1).and_return(true)
          expect(controller.send(:action_allowed?)).to be true
        end
      end

When the current logged in user is the instructor of the course. We check if the action is allowed by stubbing the current user to instructor. We then verify if the controller is able to perform the action for the current instructor user

      context 'when current user is the instructor of the course which current assignment belongs to' do
        it 'allows certain action' do
          stub_current_user(instructor2, instructor2.role.name, instructor2.role)
          allow(Course).to receive(:find).with(1).and_return(double('Course', instructor_id: 66))
          expect(controller.send(:action_allowed?)).to be true
        end
      end
    end

When the parameter action is neither edit nor update, we do this by mocking the action to new(create) in the arbitrary code that runs before each test.

    context 'when params action is not edit and update' do
      before(:each) do
        controller.params = {id: '1', action: 'new'}
      end

We now check if super admin/admin/instructor/ta can perform this action which is neither edit nor update. We check if the controller can perform the action 'new' which we mocked in the before each step.

      context 'when the role current user is super admin/admin/instructor/ta' do
        it 'allows certain action except edit and update' do
          expect(controller.send(:action_allowed?)).to be true
        end
      end
    end
  end

We also added few tests to test creation/edit/deletion of a duty. 

Creation of a duty, we create a duty and check if it is saved successfully

describe '#create' do

We write a test to check if the right page is rendered after successful creation and saving of a new duty in the database. We do this by stubbing the duty object to get saved in the database, and then perform the create action. We also check if the flash message rendered matches to the expected message.

      context 'when new duty can be saved successfully' do
        it 'sets up a new duty and redirects to assignment#edit page' do
          allow(duty).to receive(:save).and_return('OK')
          params = {
              id: 1,
              duty: {
                  duty_name: 'Scrum Master',
                  max_members_for_duty: 2,
                  assignment_id: 1
              }
          }
          post :create, params
          expect(response).to redirect_to('/assignments/1/edit')
          expect(flash[:notice]).to match(/Role was successfully created.*/)
        end
      end

Test to check if the right error message is shown if the creation of duty is failed. We stub duty to receive errors, perform the create action, and then check if the right page is rendered and also the check if the flash message matches with the expected error message.

      context 'when new duty cannot be saved successfully' do
        it 'shows error message and redirects to duty#new page' do
          allow(duty).to receive(:errors)
          params = {
              id: 1,
              duty: {
                  duty_name: 'Scrum Master',
                  max_members_for_duty: -1,
                  assignment_id: 1
              }
          }
          post :create, params
          expect(flash[:error]).to eq('Max members for duty must be greater than or equal to 1. ')
          expect(response).to redirect_to('/duties/new?id=1')
        end
      end
    end

Test to check the edit functionality of a duty.

describe '#update' do

When the duty can be edited successfully, we check if the right page is rendered. We first stub the duty active record find function to return a mocked duty object. We then try to edit the duty object by performing the update action, and then check if the right page is rendered and the flash message matches to the expected message.

    context 'when duty can be found' do
      it 'updates current duty and redirects to assignment#edit page' do
        allow(Duty).to receive(:find).with('1').and_return(build(:duty, id: 1))
          params = {
            id: 1,
            assignment_id: 1,
            duty: {
                duty_name: 'Scrum Master',
                max_members_for_duty: 5,
                assignment_id: 1
            }
        }
        post :update, params
        expect(response).to redirect_to('/assignments/1/edit')
        expect(flash[:notice]).to match(/Role was successfully updated.*/)
      end
    end

When the duty cannot be updated, we check if the right error message is displayed. We stub the duty object to receive errors. And then we try to save the duty by performing the create action, which results in an unsuccessful attempt, where we check if the right page is rendered and the flash message matches with the expected one. 

    context 'when new duty cannot be updated successfully' do
      it 'shows error message and redirects to duty#new page' do
        allow(duty).to receive(:errors)
        params = {
            id: 1,
            duty: {
                duty_name: 'SM',
                max_members_for_duty: 1,
                assignment_id: 1
            }
        }
        post :create, params
        expect(flash[:error]).to eq('Duty name is too short (minimum is 3 characters). ')
        expect(response).to redirect_to('/duties/new?id=1')
      end
    end
  end


Now finally, we test the delete functionality of the duty. We perform the delete action, and then check if the right page is rendered after the delete action, and also check if the flash message matches with the expected one 

 describe '#destroy' do
    context 'when duty can be found' do
      it 'redirects to assignment#edit page' do
        params = {id: 1, assignment_id: 1}
        post :destroy, params
        expect(response).to redirect_to('/assignments/1/edit')
        expect(flash[:notice]).to match(/Role was successfully destroyed.*/)
      end
      end
  end

== team_users_controller_spec.rb ==

We have added a test case in this file which tests if the action is allowed only by specific user roles. The first step was to create mock data so that the test could run on objects created for the tests. The code below shows how the mock data was created for the specific tests that we created to ensure the functionality of the projects.

''
 describe TeamsUsersController do
  let(:assignment) do
    build(:assignment, id: 1, name: 'test assignment', instructor_id: 6, staggered_deadline: true, directory_path: 'same path',
          participants: [build(:participant)], teams: [build(:assignment_team)], course_id: 1)
  end
  let(:assignment_form) { double('AssignmentForm', assignment: assignment) }
  let(:student) { build(:student) }

  before(:each) do
    allow(Assignment).to receive(:find).with('1').and_return(assignment)
    stub_current_user(student, student.role.name, student.role)
 end
''

Then we check if the student can perform the update action for duties. We run an arbitrary code, in before each block, where we mock the action to update duties. In the inner context, we then stub the current user to be a student user, and check if the controller allows the update duties action.

''
 describe '#action_allowed?' do
  context 'when params action is update duties' do
    before(:each) do
      controller.params = {id: '1', action: 'update_duties'}
    end
    context 'when the role current user is student' do
      it 'allows certain action' do
        stub_current_user(student, student.role.name, student.role)
        expect(controller.send(:action_allowed?)).to be true
      end
    end
  end
 end 
''

This file also contains a test case to ensure the duties can be properly updated. The test case checks this by ensuring that the user is redirected to the student view page. just as with the previous section the first step is to mock the data by creating a team user object.

''
  let(:teams_user1) {TeamsUser.new id:1, duty_id:1}
''

Then the data is actually updated and we ensure that the user is redirected to the proper place.

''
  describe '#update_duties' do
      it 'updates the duties for the participant' do
        allow(TeamsUser).to receive(:find).with('1').and_return(teams_user1)
        allow(teams_user1).to receive(:update_attribute).with(any_args).and_return('OK')
        params = {
            teams_user_id: '1',
            teams_user: {duty_id: '1'},
            participant_id: '1'
        }
        session = {user: stub_current_user(student, student.role.name, student.role)}
        get :update_duties, params, session
        expect(response).to redirect_to('/student_teams/view?student_id=1')
      end
  end
''

== assignment_form_spec.rb ==
The assignment_form_spec.rb file contains all the test files for the assignment form, which includes adding object to a queue. The first step was to create mock data so that the test could run on objects created for the tests. The code below shows how the mock data was created for the specific tests that we created to ensure the functionality of the projects.
''
  let(:new_assignment_questionnaire) { build(:assignment_questionnaire) }
''
The new_assignment_questionnaire object is used for both test cases in this file. The first test case checks for when the active record cannot be found for a specific assignment id or duty id. This test case ensures no errors will happen when a user is attempting to search for this. This scenario could very easily happen and this test ensures the functionality when it cannot be found.
''
  context 'when active record for assignment_questionnaire is not found for a given assignment_id and duty_id' do
      it 'returns new instance of assignment_questionnaire with default values' do
        allow(assignment).to receive(:questionnaire_varies_by_duty).and_return(true)
        allow(AssignmentQuestionnaire).to receive(:where).with(assignment_id: 1, duty_id: anything).and_return([])
        allow(AssignmentQuestionnaire).to receive(:where).with(user_id: anything, assignment_id: nil, questionnaire_id: nil).and_return([])
        allow(AssignmentQuestionnaire).to receive(:new).and_return(new_assignment_questionnaire)
        expect(assignment_form.assignment_questionnaire('TeammateReviewQuestionnaire', nil, nil, 1)).to eq(new_assignment_questionnaire)
      end
    end
''
The next next test checks when the assignment questionnaire is found given the assignment id and duty id parameters. This test is very similar to the one where the test is not found because the it searches through the active record to find the assignment questionnaire. Ensuring that this test passes will be useful because if the questionnaire cannot be returned then it will not be possible to do role based reviewing.
''
  context 'when active record for assignment_questionnaire is found for a given assignment_id and duty_id' do
      it 'returns new instance of assignment_questionnaire with default values' do
        allow(assignment).to receive(:questionnaire_varies_by_duty).and_return(true)
        allow(Questionnaire).to receive(:find).with(1).and_return(questionnaire3)
        allow(AssignmentQuestionnaire).to receive(:where).with(assignment_id: 1, duty_id: 1).and_return([assignment_questionnaire2])
        allow(AssignmentQuestionnaire).to receive(:where).with(user_id: anything, assignment_id: nil, questionnaire_id: nil).and_return([])
        allow(AssignmentQuestionnaire).to receive(:new).and_return(new_assignment_questionnaire)
        expect(assignment_form.assignment_questionnaire('TeammateReviewQuestionnaire', nil, nil, 1)).to eq(assignment_questionnaire2)
      end
    end
''

== duty_spec.rb ==
The main purpose of the duty_spec.rb file is to test any duty to make sure a duty can be assigned to a team member. it also tests if a duty is unavailable to a team member because the max number of people have been assigned that duty. to test this functionality the mock data is created to allow for testing of the object. The mock data is shown below which is used for all tests in this file.
''
  let(:assignment) { build(:assignment, id: 1, name: 'no assgt') }
  let(:participant) { build(:participant, id:1, user_id: 1) }
  let(:participant2) { build(:participant, id:2, user_id: 2) }
  let(:participant3) { build(:participant, id:3, user_id: 3) }
  let(:user) { build(:student, id: 1, name: 'no name', fullname: 'no one', participants: [participant]) }
  let(:user2) { build(:student, id: 2, name: 'no name2', fullname: 'no one2', participants: [participant2]) }
  let(:user3) { build(:student, id: 3, name: 'no name3', fullname: 'no one3', participants: [participant3]) }

  let(:team1) { build(:assignment_team, id: 1, name: 'no team', users: [user, user2, user3]) }
  let(:sample_duty_taken) { build(:duty, id: 1, max_members_for_duty:1, assignment_id:1) }
  let(:sample_duty_not_taken) { build(:duty, id: 1, max_members_for_duty:2, assignment_id:1) }

  let(:team_user1) { build(:team_user, id: 1, user: user) }
  let(:team_user2) { build(:team_user, id: 2, user: user2) }
  let(:team_user3) { build(:team_user, id: 3, user: user3, duty_id:1) }
''
An assignment is created on which the duties can be tested. for the testing multiple users and participants are created. Finally we created team users that could be assigned duties through the duty_id field. the next step was to set up the reoccurring steps for each of the tests.
''
  before(:each) do
    allow(team1).to receive(:participants).and_return([participant, participant2, participant3])
    allow(participant).to receive(:get_team_user).and_return(team_user1)
    allow(participant2).to receive(:get_team_user).and_return(team_user2)
    allow(participant3).to receive(:get_team_user).and_return(team_user3)
  end
''
Finally the actual tests were implemented. The tests are fairly simple because they just need to check if the duty can be assigned or not. The first test checks to make sure a duty can be assigned to a student when the role is available. This makes sure the students will not run into a problem when the roles are assigned within the group. If the roles could not be assigned, then the specific rubrics could not be used during the reviewing phase of the project.
''
    describe '#can_be_assigned?' do
    context 'when users in current team want to assign roles that are available'
      it 'returns true' do
        expect(sample_duty_not_taken.can_be_assigned?(team1)).to be true
      end
The next test case makes sure a duty cannot be assigned if there is already the max number of people assigned that role. This test case is important because if too many people are assigned that role the team dynamic will not be correct.
    context 'when users in current team want to assign roles that are unavailable'
    it 'returns false' do
      expect(sample_duty_taken.can_be_assigned?(team1)).to be false
    end
''

== teammate_review_response_map_spec.rb ==
This Rspec testing file tests that a specific questionnaire can be returned when the student has a role assigned to them. This is needed for the role based reviewing because when one student goes to review another they must get the correct questionnaire for their role. We tested this functionality as well as the inverse, when the role could not be found. initially we created the mock data for testing which is shown below.
''
  let(:questionnaire) { Questionnaire.new name: "abc", private: 0, min_question_score: 0, max_question_score: 10, instructor_id: 1234 }
  let(:assignment) { build(:assignment, id: 1, name: 'no assgt', is_duty_based_assignment: true, questionnaires: [questionnaire]) }
  let(:assignment_questionnaire1) { build(:assignment_questionnaire, id: 1, assignment_id: 1, questionnaire_id: 2, duty_id: 1) }
  let(:participant) { build(:participant, id: 1, user_id: 6, assignment: assignment) }
  let(:teammate_review_response_map) { TeammateReviewResponseMap.new reviewer: participant, reviewer_is_team: true, assignment:assignment }
''
This step created the review response map that we would be testing as well as and assignment which contained a questionnaire. The next step was to actually test the data. First we decided to ensure that the correct questionnaire was returned with the following test. It compares the returned questionnaire with the one we know is correct.
''
  it 'returns questionnaire specific to a duty' do
      allow(AssignmentQuestionnaire).to receive(:where).with(assignment_id: 1, duty_id: 1).and_return([assignment_questionnaire1])
      allow(Questionnaire).to receive(:find).with(assignment_questionnaire1.questionnaire_id).and_return(questionnaire)
      expect(teammate_review_response_map.questionnaire_by_duty(1)).to eq questionnaire
    end
''
Finally we added the inverse for when there was an error returning the questionnaire. in this case the program should return the default questionnaire.
''
  it 'returns default questionnaire when no questionnaire is found for duty' do
      allow(AssignmentQuestionnaire).to receive(:where).with(assignment_id: 1, duty_id: 1).and_return([])
      allow(assignment.questionnaires).to receive(:find_by).with(type: 'TeammateReviewQuestionnaire').and_return(questionnaire)
      expect(teammate_review_response_map.questionnaire_by_duty(1)).to eq questionnaire
    end
''


== Manual Testing in Expertiza UI ==
E2161
Here we describe manual UI Testing steps to edit an existing assignment to allow it have to specialized rubrics for different topic types. These steps are also shown in recorded demo video.
*Login to Expertiza using instructor account (For testing, username: '''instructor6''', password: '''password''')
*Click on Manage > Assignments
*Click on Edit option for any assignment, you should get following view. Make sure '''Has topics?''' box is checked.
[[File:Assignment edit 3.png]]
*Click on Rubrics tab. You will see 2 checkboxes ('''Review rubric varies by round?''', '''Review rubric varies by topic?''')
*Check the box for '''Review rubric varies by topic?'''
*Go to Topics tab and verify that there is dropdown menu beside each Topic.
*Select a rubric from dropdown menu, and click '''Save'''
[[File:Topics rubric 2.png]]
*Go back to Home, and select the same assignment to edit. When you click on Topics tab, you should see the rubric you had selected.


==Demonstration Videos==
* Functionality Walk-through: https://youtu.be/FRt3ZyofGf4
* RSpec Testing Video: https://youtu.be/PdlbX5Dlgnw

==Deployments and Pull Requests==
* Our version of expertiza is deployed at : http://152.7.176.120:8080
* Our github repository is : https://github.com/namanshrimali/expertiza/tree/beta
* Our Pull Request : https://github.com/expertiza/expertiza/pull/2137
==Team Information==
* Naman Shrimali
* Kanchan Rawat
* Subodh Thota
* Andrew Shipman
Mentor : Prof. Edward F. Gehringer