E1863 Issues related to assignment creation
Introduction
Assignment creation can be done in Expertiza by an instructor or a TA for a particular course. It is available from the Manage -> Assignments drop down on the expertiza site. Two types of assignments can be created - private and public. The various properties of assignments are grouped into tabs - General, Topics, Rubrics, Review strategy, Due dates and Other stuff. There were various issues related to saving data in these. The main motive behind this project is to fix these issues and maintain data integrity.
There are 5 issues that are being dealt with in this project and each of them are explained in detail below.
Problem Statement
If there are multiple topics added in an staggered-deadlined assignment and if the deadline of those topics are in future, students are not able to sign up for any of them. Overall assignment due dates are taking precedence, on the Due Dates tab. Due dates for the assignment needs to be changed manually to the due dates for the new topics just added. Students are able to sign up for topics only when all of the assignment due dates are in the future AND the due date for the topic they are choosing is in the future. They should be able to sign up if only the due date for the topic they are choosing is in the future. The overall assignment due dates should be irrelevant. In other words, topic should obey just the topic deadlines and not the assignment deadlines.
Proposed Solution
For staggered deadline assignments, different topics might have different deadlines. So, the deadline for each topic needs to be checked on an individual basis. Deadlines for individual topics are stored in a table called due_dates with the parent_id as the identifier of the particular topic in place of the assignment identifier. A logic for getting the due dates of individual topics for staggered deadline assignments is found in the method check_topic_due_date_value in the module SignUpSheetHelper.
topic_due_date = TopicDueDate.where(parent_id: topic_id, deadline_type_id: deadline_type_id, round:review_round).first rescue nil if !topic_due_date.nil? due_date = topic_due_date.due_at else due_date = assignment_due_dates[review_round - 1].due_at.to_s
Currently, if the signup deadline of the the assignment is earlier that the submission deadline for a topic, that topic is not available for taking. This is restricted by the following piece of code in _all_actions.html.erb.
<% elsif (@signup_topic_deadline.nil? || (Time.now < @signup_topic_deadline.due_at)) &&
(!@assignment.staggered_deadline? || (Time.now < get_topic_deadline([@assignment.due_dates.find_by(deadline_type_id: 1)], topic.id))) %>
<td align="center"><%= link_to image_tag('Check-icon.png', :border => 0, :title => 'Signup', :align => 'middle'), :controller=>'sign_up_sheet', :action=> 'sign_up',
:id=>params[:id],:topic_id => topic.id, :assignment_id => params[:assignment_id] %></td>
<% end %>
Fix to be implemented(tentative): Change the above piece of code to allow signing up staggered assignment topics bases on the topic deadline and not on the assignment sign up deadline. The prospective change is shown below.
<% elsif ((@signup_topic_deadline.nil? || (Time.now < @signup_topic_deadline.due_at)) &&
(!@assignment.staggered_deadline? || (Time.now < get_topic_deadline([@assignment.due_dates.find_by(deadline_type_id: 1)], topic.id)))) ||
(@assignment.staggered_deadline? && (Time.now < get_topic_deadline([@assignment.due_dates.find_by(deadline_type_id: 1)], topic.id))) %>
<td align="center"><%= link_to image_tag('Check-icon.png', :border => 0, :title => 'Signup', :align => 'middle'), :controller=>'sign_up_sheet', :action=> 'sign_up',
:id=>params[:id],:topic_id => topic.id, :assignment_id => params[:assignment_id] %></td>
<% end %>
Test Plan
Manual Testing
- Log in to Expertiza with the credentials: instructor6/password (on google chrome)
- Go to the Manage -> Assignments.
- Click on New Public Assignment
- On the new assignment creation page, under the General tab, give details for Assignment name, Course (choose CSC 517, Spring 2016) and Submission directory.
- Check the Staggered deadline assignment? checkbox.
- Click on the Rubrics tab and give some values for Review and Author Feedback, if there are any other fields apart from these give values to that too.
- Click on Create at the bottom.
- Now, click on the the Topics tab and further click on New Topic.
- Give suitable values to the fields and click on Create.
- Click on the Due dates tab.
- Check the Use signup deadline checkbox and give suitable dates for Signup, Round1: Submission and Round1: Review.
- Go back to the Topics tab and click on Show start/due date at the bottom.
- Change the Submission deadline date to date later than the date given for Signup on the Due dates tab.
- Click on save at the bottom.
- Now, click on the Other stuff tab and and further click on Add participant. It will open in a new tab.
- Click on Copy participants from course. After it adds the participants, close the tab.
- Go to Manage -> Impersonate User.
- Give student6360 for the Enter user account field and click on Impersonate.
- Click on Assignments and further click on the assignment that was created in the earlier steps.
- Click on Signup Sheet.
- You should be able to see a green tick mark under the Actions header and should be able to click on it and signup, which means the issue is fixed.
Automated Testing
No automated test cases, only manual testing since the modifications are made to the view files.
References
2. Pull Request
Problem Statement
At present, when the TA creates a new assignment, only he has the ability to delete it. The task at hand is to allow the instructor to delete the assignment that is created by a TA. When an assignment is created by the instructor or by the TA , the delete option is assigned to self, that is, to the one who created the assignment. This needs to be overridden in that the instructor must be able to view the assignment list by having an additional parameter to classify the creator of that assignment as Teaching Assistant or Instructor, and giving the Instructor the option to delete an assignment created by TA.
Proposed Solution
The flow diagram given below specifies the sequence of events that will aim at resolving the issue related to deleting an assignment :
The fix to resolve the issue involves the delete function in the assignment controller. The code at present checks for the current user and allows it the permission to delete the file.
if id != @assignment_form.assignment.instructor_id
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "You are not authorized to delete this assignment.", request)
raise "You are not authorized to delete this assignment."
else
@assignment_form.delete(params[:force])
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{@assignment_form.assignment.id} was deleted.", request)
flash[:success] = "The assignment was successfully deleted."
end
To allow instructor to delete an assignment created by TA, the if-else block is fixed as given:
if @user.role.name == "Instructor" or (@user.role.name == "Teaching Assistant" and id == @assignment_form.assignment.instructor_id)
# allow delete
@assignment_form.delete(params[:force])
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{@assignment_form.assignment.id} was deleted.", request)
flash[:success] = "The assignment was successfully deleted."
else
# throw error
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "You are not authorized to delete this assignment.", request)
raise "You are not authorized to delete this assignment."
end
Modifications made to the code check for the form id and allow permission to delete when the id belongs to assignment creator or when the role of the current user is 'Instructor'.
Test Plan
Manual Testing
Task Description: Teaching Assistant creating an assignment
Precondition: The instructor has set up the page for assignment creation.
Primary Flow:
- Log in to Expertiza
- Select New Assignment
- Enter the Assignment Name and select Course.
- Check the parameters for the teams, quiz, badges categories etc.
- Click Create
Task Description: Instructor or Teaching Assistant deleting an assignment
Precondition: There exists at least one assignment created by TA.
Primary Flow:
- Log in to Expertiza
- Select the Delete option in the action section for an assignment created by the TA.
- If Logged in as Instructor or Teaching Assistant, the assignment gets deleted for that action.
- Else, the current participant is shown the error message that they do not have authorization to execute the delete option.
Automated Testing
Test cases mainly involve controller testing to check the assignment created by TA and it can be deleted by instructor through invocation of the delete method.
References
2. Pull Request
Problem Statement
There are assignments that have rubrics that vary by round, i.e. each round of the assignment review will have a different set of rubrics. When such an assignment is copied, there exists a problem where the rubrics from the original assignment are not copied over properly to the new assignment. This new assignment does not have rubrics that vary by round, as in the original assignment, but have the rubric from Round 1 in the original assignment copied over for all rounds. This makes the copying of assignments incomplete and thus, needs to be fixed.
Proposed Solution
When an assignment is copied, all the rubric data is to be copied over. As the copying over was incomplete, there were issues. This is fixed in the copy_assignment_questionnaire method of assignment_form.rb file.
The current code is shown below. We can see that the data (columns) related to the various rounds are not copied over. This needs to be fixed.
def self.copy_assignment_questionnaire(old_assign, new_assign, user)
old_assign.assignment_questionnaires.each do |aq|
AssignmentQuestionnaire.create(
assignment_id: new_assign.id,
questionnaire_id: aq.questionnaire_id,
user_id: user.id,
notification_limit: aq.notification_limit,
questionnaire_weight: aq.questionnaire_weight
)
end
end
After the fix, the code looks like below. The columns related to the various rounds will be copied over from now on.
def self.copy_assignment_questionnaire(old_assign, new_assign, user)
old_assign.assignment_questionnaires.each do |aq|
AssignmentQuestionnaire.create(
assignment_id: new_assign.id,
questionnaire_id: aq.questionnaire_id,
user_id: user.id,
notification_limit: aq.notification_limit,
questionnaire_weight: aq.questionnaire_weight,
used_in_round: aq.used_in_round,
dropdown: aq.dropdown
)
end
end
Test Plan
Manual Testing
- Log into Expertiza as instructor
- Access the Assignments listing
- Access an assignment and go to the 'Rubrics' tab to see if there are multiple rubrics defined
- Copy the assignment, if there are multiple rubrics, from the assignments listing screen
- Access the newly copied assignment
- Check if the rubrics tab lists down rubrics for all rounds in the original assignment
Automated Testing
This would reuse the existing automation tests.
References
2. Pull Request
Problem Statement
The Issue is that the Instructor is not able to participate in a given assignment. This issues require a fix so that the instructor can also add himself as a participant in the assignment created .This fix will allow the instructor to perform the same functionality as the other student participants as reviewing peer assignments , submitting assignment, etc.
Proposed Solution
When the instructor is logged in he can create a new assignment or use an existing assignment . All the assignments that are created would require participants which happens to be the responsibility of the instructor . Thus the Instructor adds the participants to the assignment and with this fix the instructor will be added as a participant by default every time a new assignment is created.The new functionality in the assignment controller will be modified to resolve the issue.
The issue was not with the instructor been not able to get added as the participant but it was that the list was not getting updated after adding the participant and that was due to inconsistent referencing in the view and javascript file. the inconsistent version of both the file is shown as below
add.js.erb file
<% unless flash[:error] %>
$("#plist").append(
"<%= j render :partial => 'participant', :locals => {participant: @participant, :userid => @participant.user_id, :controller => 'participants'} %>");
<% end %>
+$("#flash_message").html("<%= j render partial: "shared/flash_messages" %>");
_user_list.html.erb file
<table id="" class="table table-striped" style="font-size: 15px">
The changed versions are as follows:
add.js.erb file
<% unless flash[:error] %>
$("#user_list").append(
"<%= j render :partial => 'participant', :locals => {participant: @participant, :userid => @participant.user_id, :controller => 'participants'} %>");
<% end %>
+$("#flash_message").html("<%= j render partial: "shared/flash_messages" %>");
_user_list.html.erb file
<table id="user_list" class="table table-striped" style="font-size: 15px"> # here the table id is changed to user_list
Test Plan
Manual Testing
- Log into Expertiza as instructor
- Create a new Assignment
- Click on the add participant button for the assignment created
- Modify the participant list by adding instructor as a participant
- Check the list to see the instructor added to the assignment as participant
Automated Testing
This would involve the controller and view testing to see if the instructor is added to the assignment as a participant through the add and list methods that are being invoked with the participant list showing instructor as the participant.
References
2. Pull request
Problem Statement
While creating/editing an assignment, one of the configurations is setting up of the rubrics. This is done in the 'Rubrics' tab of the assignment page. While setting up the rubrics, we can assign a weight to each of it, so that the final score will be the weighted sum of the different rubric scores. The default weight of a rubric is always 0%, meaning that it's not counted towards the final score. But if any of the rubric is assigned with a non-zero weight, we need to make sure that the sum of all the weights of the various rubrics add up to 100%. This is not enforced currently, allowing the user to have a total weight which is between 0% and 100%. This needs to be fixed to bring in a hard stop, if the sum of the weights don't add up to 0% or 100%.
Proposed Solution
When we save a new/existing assignment, we need to bring in a check to see if the sum of the weights of the various rubrics add up to 0 or 100%. If this check fails, we need to throw a notice that says, "Sum of weights of rubrics need to be 0 or 100%.". The user has to make the corresponding modification to the weights and try again. If the check succeeds, we continue with the assignment saving process.
A new validation method validate_assignment_questionnaires_weights called in the update_assignment_questionnaires method of assignment_form.rb would do the checking:
# code to save assignment questionnaires
def update_assignment_questionnaires(attributes)
return false unless attributes
validate_assignment_questionnaires_weights(attributes)
@errors = @assignment.errors
unless @has_errors
existing_aqs = AssignmentQuestionnaire.where(assignment_id: @assignment.id)
existing_aqs.each(&:delete)
attributes.each do |assignment_questionnaire|
if assignment_questionnaire[:id].nil? or assignment_questionnaire[:id].blank?
aq = AssignmentQuestionnaire.new(assignment_questionnaire)
unless aq.save
@errors = @assignment.errors.to_s
@has_errors = true
end
else
aq = AssignmentQuestionnaire.find(assignment_questionnaire[:id])
unless aq.update_attributes(assignment_questionnaire)
@errors = @assignment.errors.to_s
@has_errors = true
end
end
end
end
end
# checks to see if the sum of weights of all rubrics add up to either 0 or 100%
def validate_assignment_questionnaires_weights(attributes)
total_weight=0
attributes.each do |assignment_questionnaire|
total_weight+=assignment_questionnaire[:questionnaire_weight].to_i
end
if total_weight != 0 and total_weight != 100
@assignment.errors.add(:message,'Total weight of rubrics should add up to either 0 or 100%')
@has_errors = true
end
end
Test Plan
Manual Testing
- Log into Expertiza as instructor
- Access the Assignments listing
- Access an assignment and go to the 'Rubrics' tab
- Modify the data in the rubric weights column in such a way that it adds up to a value other than 0 or 100
- Click save
- Check to see if there is an error thrown and the rubrics tab is not saved
Automated Testing
The following test was added to assignment_form_spec.rb to test if there is an error if the sum of weights of the rubrics do not add up to 0 or 100.
context 'when both save and update_attributes method do not work with wrong rubrics weights' do
it 'changes @has_errors value to true and returns nil' do
allow(assignment_questionnaire).to receive(:save).and_return(true)
allow(assignment_questionnaire2).to receive(:update_attributes).with(assignment_questionnaire2).and_return(true)
allow(assignment_questionnaire2).to receive(:[]).with(:questionnaire_weight).and_return(12)
expect(assignment_form.update_assignment_questionnaires(attributes)).to eq(nil)
expect(assignment_form.instance_variable_get(:@has_errors)).to be true
end
end
References
2. Pull Request