E1833 Fix and improve rubric criteria
Introduction
Background
Expertiza is an educational web application created and maintained by the joint efforts of the students and the faculty at NCSU. It’s an open source project developed on Ruby on Rails platform and it’s code is available on Github. It provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.
Problem statement
In Expertiza, instructors (also admin, super admin and TAs) can create rubrics (they are called questionnaires in DB, there are different types like review rubric, teammate review rubric, etc. Each rubric may have one or many criteria (called questions in DB). For each criterion, it may have 0 to many suggestions/advices.
Issues to be resolved
Issue 696
Issue Description: Instructors and TA’s under the same instructor can make changes to each other’s rubric.
Expectations
Only the owner (an instructor) and his/her TA(s) should be able to edit the corresponding rubric.
Solution Implemented
The first step towards solving this issue was to retrieve id of current user and check its authorization with regards to questionnaire.To achieve this, we are checking if current user is a 'Teaching Assistant' and if 'instructor' of teaching assistant has access to current questionnaire. If instructor has access of questionnaire, his/her Teaching Assistant should also have access for the respective questionnaire. Technically, as view edit_questionnaire.html.erb returns current user_id to method action_allowed? in controller questionnaire_controller.rb, we have checked the same user_id with assign_instructor_id. Assign_instructor_id returns instructor_id of Teaching_assistant using function get_my_instructor. And finally we have validated if current_user_role is 'Teaching Assistant'. If above conditions are met, Teaching assistant will have access to edit the questionnaire.
Modified Files
- app/controllers/export_file_controller.rb
- app/controllers/questionnaires_controller.rb
- app/models/question.rb
expertiza/apps/controllers/questionnaires_controller.rb
def action_allowed?
if params[:action] == "edit"
@questionnaire = Questionnaire.find(params[:id])
(['Super-Administrator',
'Administrator'
].include? current_role_name) ||
((['Instructor'].include? current_role_name) && current_user_id?(@questionnaire.try(:instructor_id))) ||
((['Teaching Assistant'].include? current_role_name) && assign_instructor_id == @questionnaire.try(:instructor_id))
else
['Super-Administrator',
'Administrator',
'Instructor',
'Teaching Assistant', 'Student'].include? current_role_name
end
end
expertiza/app/controllers/export_file_controller.rb
def start
@model = params[:model]%>
titles = {"Assignment" => "Grades", "CourseParticipant" => "Course Participants", "AssignmentTeam" => "Teams",
"CourseTeam" => "Teams", "User" => "Users", "Question" => "Questions"}
@title = titles[@model]
@id = params[:id]
end
expertiza/app/models/question.rb
def self.export_fields(_options)
fields = ["Seq", "Question", "Type", "Weight", "text area size", "max_label", "min_label"]
fields
end
Instructor6 has access for review 517 F09 wiki
TA teaching_assistant20 of instructor6 does not have access for review bilim3
Issue 577
When logged in as super admin. At the bottom of the rubric, we will find links to import it and export it. The Export Details does not work and is not relevant to the context. Export page does not display the names of the rubric. When a rubric is exported, the headers on each column were incorrect and not related to the rubric. It seems to be headers from an export of information on users.
Expectations
Export page needs to give the name of the rubric When a rubric is exported, the column names should be proper. Exporting "details" for rubrics should mean exporting the advice associated with the rubrics. And then could rename "export details" to "export advice". Also change name of columns as per advice information extracted.
Solution Implemented
In order to implement export functionality for questionnaire, two functions were added to question_advice model namely export_field and export. Export_fields returns an array of model fields and Export returns the advice data for current questionnaire. A new route has also been defined to handle the question advice export request. Views were also changed to display “Export Advices” instead of “Export Details” in the case of the Question model. You can find similar export functionalities in other modules as well including question model.
Modified Files
- app/controllers/export_file_controller.rb
- app/views/export_file/start.html.erb
- app/config/routes.rb
- app/models/question_advice.rb
expertiza/app/controllers/export_file_controller.rb
def export_advices
@delim_type = params[:delim_type]
filename, delimiter = find_delim_filename(@delim_type, params[:other_char])
allowed_models = ['Question']
advice_model = 'QuestionAdvice'
csv_data = CSV.generate(col_sep: delimiter) do |csv|
if allowed_models.include? params[:model]
csv << Object.const_get(advice_model).export_fields(params[:options])
Object.const_get(advice_model).export(csv, params[:id], params[:options])
end
end
send_data csv_data,
type: 'text/csv; charset=iso-8859-1; header=present',
disposition: "attachment; filename=#{filename}"
end
expertiza/app/models/question_advice.rb
class QuestionAdvice < ActiveRecord::Base
belongs_to :question
def self.export_fields(_options)
fields = []
QuestionAdvice.columns.each do |column|
fields.push(column.name)
end
fields
end
def self.export(csv, parent_id, _options)
questionnaire = Questionnaire.find(parent_id)
questions = questionnaire.questions
questions.each do |question|
question_advices = QuestionAdvice.where("question_id = ?", question.id)
question_advices.each do |advice|
tcsv = []
advice.attributes.each_pair do |_name, value|
tcsv.push(value)
end
csv << tcsv
end
end
end
end
expertiza/app/views/export_file/start.html.erb
<% if @model == 'Question' %>
<h2>Export Advices</h2>
<%= form_tag({:controller => 'export_file', :action => 'export_advices'}, {:method => 'post', :multipart => true, :name => 'export_advices'}) do %>
<table>
<tr>
<td valign='Top'>Delimiter:</td>
<td>
<%= radio_button_tag 'delim_type', 'comma', true %> Comma<br/>
<%= radio_button_tag 'delim_type', 'space' %> Space<br/>
<%= radio_button_tag 'delim_type', 'tab' %> Tab<br/>
<%= radio_button_tag 'delim_type', 'other' %> Other <%= text_field_tag 'other_char' %>
<br/>
</td>
</tr>
</table>
<br/>
<%= hidden_field_tag('model', @model) %>
<%= hidden_field_tag('id', @id) %>
<input type='submit' value='Export Advices' onclick='{export_advices.submit();}'/>
<br/><br/>
<% end %>
expertiza/app/config/routes.rb
resources :export_file, only: [] do
collection do
get :start
get :export
post :export
post :exportdetails
post :export_advices
end
end
Export Advices Output File
Test Plan
Manual Test
Below are the steps to perform manual testing for the issues resolved. Issue :
- Step 1: Traverse to manage->Questionnaires-> Review rubric
- Step 2: Select any review from the list, which Instructor/Teaching Assistant do not have access to.(Eg.)
- Step 3: Choose any review and click of edit review button(pencil symbol).
- Step 4: Issue is replicated and Instructor/Teaching Assistant should not be able to edit the review and system will be show an error message. (If Instructor/Teaching assistant has access to current questionnaire, he/she should be able to access it.)
Issue :
- Step 1: Traverse to manage->Questionnaires-> Review rubric
- Step 2: Select any review from the list, which Instructor/Teaching Assistant do not have access to.(Eg.)
- Step 3: Choose any review and click of edit review button(pencil symbol).
- Step 4: Click on the link 'Export questionnaire' at the bottom of the page.
- Step 5: Bottom of the navigated page will show 'Export Advices' and when clicked on Export, you will be able to export advices for current rubric.
There were no existing test cases for the Questionnaires_controller for Issue 696. We have made changes in questionnaires_controller_spec.rb which covers testing scenarios to check access over questionnaire module as per the role defined in the system i.e. for roles Instructor, TA and admin. The specs were run on the previous and current files and they return the same results implying that the changed code does not break anything. Below are the test Scenarios covered:
- when the role name of current user is super admin or admin.
- when current user is the instructor of current questionnaires.
- when current user is the ta of the course which current questionnaires belongs to.
- when current user is a ta but not the ta of the course which current questionnaires belongs to.
- when current user is the instructor of the course which current questionnaires belongs to.
- when current user is an instructor but not the instructor of current course or current questionnaires.
expertiza/spec/controllers/questionnaires_controller_spec.rb
def check_access username
stub_current_user(username,username.role.name,username.role)
expect(controller.send(:action_allowed?))
end
describe '#action_allowed?' do
let(:questionnaire) { build(:questionnaire, id: 1) }
let(:instructor) { build(:instructor, id: 1) }
let(:ta) { build(:teaching_assistant, id: 10, parent_id: 66) }
context 'when params action is edit or update' do
before(:each) do
controller.params = {id: '1', action: 'edit'}
controller.request.session[:user] = instructor
end
context 'when the role name of current user is super admin or admin' do
it 'allows certain action' do
check_access(admin).to be true
end
end
context 'when current user is the instructor of current questionnaires' do
it 'allows certain action' do
check_access(instructor).to be true
end
end
context 'when current user is the ta of the course which current questionnaires belongs to' do
it 'allows certain action' do
allow(TaMapping).to receive(:exists?).with(ta_id: 8, course_id: 1).and_return(true)
check_access(ta).to be true
end
end
context 'when current user is a ta but not the ta of the course which current questionnaires belongs to' do
it 'does not allow certain action' do
allow(TaMapping).to receive(:exists?).with(ta_id: 10, course_id: 1).and_return(false)
controller.request.session[:user] = instructor2
check_access(ta).to be false
end
end
context 'when current user is the instructor of the course which current questionnaires belongs to' do
it 'allows certain action' do
allow(Course).to receive(:find).with(1).and_return(double('Course', instructor_id: 6))
check_access(instructor).to be true
end
end
context 'when current user is an instructor but not the instructor of current course or current questionnaires' do
it 'does not allow certain action' do
allow(Course).to receive(:find).with(1).and_return(double('Course', instructor_id: 66))
check_access(instructor2).to be false
end
end
end
context 'when params action is not edit and update' do
before(:each) do
controller.params = {id: '1', action: 'new'}
end
context 'when the role current user is super admin/admin/instructor/ta' do
it 'allows certain action except edit and update' do
check_access(admin).to be true
end
end
end
end
External Links
- Link to forked repository [[1]]
References
Expertiza
Expertiza Github
Expertiza Documentation