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
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
Existing methods returned an instructor and his/her TA(s). We used these methods to change action_allowed? method in quesionnaire_controller to fix this. To achieve this, we checked if instructor or TA’s under the same instructor has access of current questionnaire using include? and current_user_id? in questionnaires_controller.
Modified Files
- app/controllers/export_file_controller.rb
- app/controllers/questionnaires_controller.rb
- app/models/question.rb
expertiza/app/controllers/export_file_controller.rb
This file is shared between all views and is responsible for rendering all the elements on the navigation bar including the menu items. In order to switch between default view of the user and student view, the following code was added. When in default view, the instructor is presented with a link, Open Student View, to switch to student view and when in student view, the Close Student View link can be used to switch back to the default view.
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
A new role_switch_controller.rb file has been added. This controller contains the actions to open/close student view.
def self.export_fields(_options) fields = ["Seq", "Question", "Type", "Weight", "text area size", "max_label", "min_label"] fields end
expertiza/apps/controllers/questionnaires_controller.rb
New post methods are added in config/routes.rb. The routes are directed to the role switch controller's open_student_view and close_student_view actions.
def action_allowed? if action_name == "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
Issue 577
Dumping and loading rubric criteria from/to csv. 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
A similar export functionality was added for the question_advice model. 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..
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
A new menu node with the below format is added under 'Manage...' node in the yml configuration files for admin, superadmin , teaching assistants and instructor.
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
RSpec
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
describe '#action_allowed?' do context 'when params action is edit or update' do before(:each) do controller.params = {id: '1', action: 'edit'} end 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 context 'when current user is the instructor of current questionnaires' do it 'allows certain action' do expect(controller.send(:action_allowed?)).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 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 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 stub_current_user(ta, ta.role.name, ta.role) allow(TaMapping).to receive(:exists?).with(ta_id: 8, course_id: 1).and_return(false) expect(controller.send(:action_allowed?)).to be true end end context 'when current user is the instructor of the course which current questionnaires 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 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 stub_current_user(instructor2, instructor2.role.name, instructor2.role) allow(Course).to receive(:find).with(1).and_return(double('Course', instructor_id: 666)) expect(controller.send(:action_allowed?)).to be true 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 expect(controller.send(:action_allowed?)).to be true end end end end
External Links
- Link to forked repository [[1]]
References
Expertiza
Expertiza Github
Expertiza Documentation