E1728. Remove useless partials from grades view and move view logic to grades helper.rb: Difference between revisions
Line 609: | Line 609: | ||
! Description | ! Description | ||
| | | | ||
# Login to the site as a | # Login to the site as a student2064 | ||
# Click on "Assignments" on the top menu | # Click on "Assignments" on the top menu | ||
# Select an assignment from the list | # Select an assignment from the list | ||
Line 631: | Line 631: | ||
|- | |- | ||
! Scenario | ! Scenario | ||
| | | Test to see if logic elimination from _participant.html.erb is working | ||
|- | |- | ||
! Pre-Conditions | ! Pre-Conditions | ||
| | | | ||
# Has one course assigned | |||
# Has one assigned as part of that course | |||
# Has more than one review for that assignment | |||
|- | |- | ||
! Description | ! Description |
Revision as of 22:55, 22 March 2017
Introduction
This wiki provides details on the refactoring tasks that were undertaken as part of the continuous improvement to the Expertiza project
Background
Expertiza is a web application where students can submit and peer-review learning objects (articles, code, web sites, etc). The Expertiza project is supported by the National Science Foundation.
The application provides a complete system through which students and instructors collaborate on the learning objects as well as submit, review and grade assignments for the courses.
Motivation
By participating in the overall refactoring effort as part of the continuous improvement of Expertiza, students get an opportunity to work on a open source software project. This helps them gain exposure on the technologies used in the project as well as much needed experience in collaborating with peers as part of the software development process.
Tasks
The tasks involved as part of this refactoring effort were geared towards cleaning up the grade view logic.
Initial Task List
The initial set of tasks were:
- For files: _scores_author_feedback.html.erb, _scores_metareview.html.erb, _scores_submitted_work.html.erb
- Move javascript code to assets
- For files: _scores_header.html.erb
- Move logical code (such as L43-96) to helper file and assign self-explanatory method name
- For files: _participant.html.erb, view_team.html.erb
- Move javascript code to assets
- Move logical code to helper file and assign self-explanatory method name
- Such as L8-22 in _participant.html.erb
- Create test file named grades_helper_spec.rb in spec/helpers
- Write test cases for all methods in grades_helper.rb
Revised Task List
On working with files _scores_author_feedback.html.erb, _scores_metareview.html.erb, _scores_submitted_work.html.erb, _scores_header.html.erb, the team came to find out that these files were not being used anymore in the application and the overall tasks were updated to the following:
- Remove useless partials from grades view, such as _scores_author_feedback.html.erb, _scores_metareview.html.erb, _scores_submitted_work.html.erb, _scores_header.html.erb, etc.
- For files _participant.html.erb, view_team.html.erb to grades_helper.rb
- Move javascript code to assets
- Move logical code to helper file and assign self-explanatory method name such as L8-22 in _participant.html.erb
- Create test file named grades_helper_spec.rb in spec/helpers
- Write test cases for all methods in grades_helper.rb by using factories
Refactoring Tasks
Remove useless partials from grades view, such as _scores_author_feedback.html.erb, _scores_metareview.html.erb, _scores_submitted_work.html.erb, _scores_header.html.erb, etc.
The named files were removed from the project and the deletions were committed to the repository
For files _participant.html.erb, view_team.html.erb to grades_helper.rb
Move javascript code to assets
A new file was created and added to the project: app/assets/javascripts/grades/view_team.js
For the view_team.html.erb file, the javascript code was moved to view_team.js
$=jQuery; $(function () { $("[data-toggle='tooltip']").tooltip(); $("#scoresTable").tablesorter(); }); var lesser = false; // Function to sort the columns based on the total review score function col_sort(m) { lesser = !lesser // Swaps two columns of the table jQuery.moveColumn = function (table, from, to) { var rows = jQuery('tr', table); var hidden_child_row = table.find('tr.tablesorter-childRow'); hidden_child_row.each(function () { inner_table = jQuery(this).find('table.tbl_questlist') hidden_table = inner_table.eq(0).find('tr') hidden_table.eq(from - 1).detach().insertBefore(hidden_table.eq(to - 1)); if (from - to > 1) { hidden_table.eq(to - 1).detach().insertAfter((hidden_table.eq(from - 2))); } }); var cols; rows.each(function () { cols = jQuery(this).children('th, td'); cols.eq(from).detach().insertBefore(cols.eq(to)); if (from - to > 1) { cols.eq(to).detach().insertAfter((cols.eq(from - 1))); } }); } // Gets all the table with the class "tbl_heat" var tables = $("table.tbl_heat"); // Get all the rows with the class accordion-toggle var tbr = tables.eq(m).find('tr.accordion-toggle'); // Get the cells from the last row of the table var columns = tbr.eq(tbr.length - 1).find('td'); // Init an array to hold the review total var sum_array = []; // iterate through the rows and calculate the total of each review for (var l = 2; l < columns.length - 2; l++) { var total = 0; for (var n = 0; n < tbr.length; n++) { var row_slice = tbr.eq(n).find('td'); if (parseInt(row_slice[l].innerHTML) > 0) { total = total + parseInt(row_slice[l].innerHTML) } } sum_array.push(total) } // The sorting algorithm for (var i = 3; i < columns.length - 2; i++) { var j = i; while (j > 2 && compare(sum_array[j - 2], sum_array[j - 3], lesser)) { var tmp tmp = sum_array[j - 3] sum_array[j - 3] = sum_array[j - 2] sum_array[j - 2] = tmp jQuery.moveColumn($("table.tbl_heat").eq(m), j, j - 1); // This part is repeated since the table is updated tables = $("table.tbl_heat") tbr = tables.eq(m).find('tr.accordion-toggle'); columns = tbr.eq(tbr.length - 1).find('td') j = j - 1; } } } // Function to return boolean based on lesser or greater operator function compare(a, b, less) { if (less) { return a < b } else { return a > b } }
Move logical code to helper file and assign self-explanatory method name such as L8-22 in _participant.html.erb
The following lines of code were removed
<% participant = pscore[:participant] if pscore[:review] @rscore_review=Rscore.new(pscore,:review) end if pscore[:metareview] @rscore_metareview=Rscore.new(pscore,:metareview) end if pscore[:feedback] @rscore_feedback=Rscore.new(pscore,:feedback) end if pscore[:teammate] @rscore_teammate=Rscore.new(pscore,:teammate) end %>
and converted to properties in the GradesHelper module defined in the app/helpers/grades_helper.rb file
def participant @participant = Participant.find(params[:id]) end def rscore_review @participant = Participant.find(params[:id]) @pscore = @participant.scores(@questions) if @pscore[:review] @rscore_review=Rscore.new(@pscore,:review) end @rscore_review end def rscore_metareview @participant = Participant.find(params[:id]) @pscore = @participant.scores(@questions) if @pscore[:metareview] @rscore_metareview=Rscore.new(@pscore,:metareview) end @rscore_metareview end def rscore_feedback @participant = Participant.find(params[:id]) @pscore = @participant.scores(@questions) if @pscore[:feedback] @rscore_feedback=Rscore.new(@pscore,:feedback) end @rscore_feedback end def rscore_teammate @participant = Participant.find(params[:id]) @pscore = @participant.scores(@questions) if @pscore[:teammate] @rscore_teammate=Rscore.new(@pscore,:teammate) end @rscore_teammate end def p_total_score @participant = Participant.find(params[:id]) @pscore = @participant.scores(@questions) if @participant.grade @total_score = participant.grade else @total_score = @pscore[:total_score] end @total_score end def p_title @participant = Participant.find(params[:id]) if @participant.grade @title = "A score in blue indicates that the value was overwritten by the instructor or teaching assistant." else @title = nil end @title end
The following lines of code in the _participant.html.erb file were also replaced to use the above properties created
Original | Replacement |
---|---|
<% if !@rscore_review.nil? && @rscore_review.my_avg %> |
<% if !rscore_review.nil? && rscore_review.my_avg %> |
<%= @rscore_review.my_avg.is_a?(Float)? sprintf("%.2f",@rscore_review.my_avg): @rscore_review.my_avg %>%<BR/> |
<%= rscore_review.my_avg.is_a?(Float)? sprintf("%.2f",rscore_review.my_avg): rscore_review.my_avg %>%<BR/> |
<%= @rscore_review.my_min.is_a?(Float)? sprintf("%.0f",@rscore_review.my_min): @rscore_review.my_min %>% - <%= @rscore_review.my_max.is_a?(Float)? sprintf("%.0f",@rscore_review.my_max): @rscore_review.my_max %>% |
<%= rscore_review.my_min.is_a?(Float)? sprintf("%.0f",rscore_review.my_min): rscore_review.my_min %>% - <%= rscore_review.my_max.is_a?(Float)? sprintf("%.0f",rscore_review.my_max): rscore_review.my_max %>% |
<% if @rscore_metareview && @rscore_metareview.my_avg %> <TD ALIGN="CENTER" VALIGN="TOP"><%= @rscore_metareview.my_avg.is_a?(Float)? sprintf("%.2f",@rscore_metareview.my_avg):@rscore_metareview.my_avg %>% |
<% if rscore_metareview && rscore_metareview.my_avg %> <TD ALIGN="CENTER" VALIGN="TOP"><%= rscore_metareview.my_avg.is_a?(Float)? sprintf("%.2f",rscore_metareview.my_avg):rscore_metareview.my_avg %>% |
<TD ALIGN="CENTER" VALIGN="TOP"><%= @rscore_metareview.my_min.is_a?(Float)? sprintf("%.0f",@rscore_metareview.my_min):@rscore_metareview.my_min %>% - <%= @rscore_metareview.my_max.is_a?(Float)?sprintf("%.0f",@rscore_metareview.my_max):@rscore_metareview.my_max %>% |
<TD ALIGN="CENTER" VALIGN="TOP"><%= rscore_metareview.my_min.is_a?(Float)? sprintf("%.0f",rscore_metareview.my_min):rscore_metareview.my_min %>% - <%= rscore_metareview.my_max.is_a?(Float)?sprintf("%.0f",rscore_metareview.my_max):rscore_metareview.my_max %>% |
<% if !@rscore_feedback.nil? && @rscore_feedback.my_avg %> <TD ALIGN="CENTER" VALIGN="TOP"><%= @rscore_feedback.my_avg.is_a?(Float)? sprintf("%.2f",@rscore_feedback.my_avg): @rscore_feedback.my_avg %>% |
<% if !rscore_feedback.nil? && rscore_feedback.my_avg %> <TD ALIGN="CENTER" VALIGN="TOP"><%= rscore_feedback.my_avg.is_a?(Float)? sprintf("%.2f",rscore_feedback.my_avg): rscore_feedback.my_avg %>% |
<TD ALIGN="CENTER" VALIGN="TOP"><%= @rscore_feedback.my_min.is_a?(Float)? sprintf("%.0f",@rscore_feedback.my_min): @rscore_feedback.my_min %>% - <%= @rscore_feedback.my_max.is_a?(Float)? sprintf("%.0f",@rscore_feedback.my_max):@rscore_feedback.my_max %>% |
<TD ALIGN="CENTER" VALIGN="TOP"><%= rscore_feedback.my_min.is_a?(Float)? sprintf("%.0f",rscore_feedback.my_min): rscore_feedback.my_min %>% - <%= rscore_feedback.my_max.is_a?(Float)? sprintf("%.0f",rscore_feedback.my_max):rscore_feedback.my_max %>% |
<% if @rscore_teammate and @rscore_teammate.my_avg %> |
<% if rscore_teammate and rscore_teammate.my_avg %> |
<% range = @rscore_teammate.my_min.is_a?(Float) ? sprintf("%.0f",@rscore_teammate.my_min) : @rscore_teammate.my_min.to_s + '%' + ' - ' %> <% range += @rscore_teammate.my_max.is_a?(Float) ? sprintf("%.0f",@rscore_teammate.my_max) : @rscore_teammate.my_max.to_s + '%' %> |
<% range = rscore_teammate.my_min.is_a?(Float) ? sprintf("%.0f",rscore_teammate.my_min) : rscore_teammate.my_min.to_s + '%' + ' - ' %> <% range += rscore_teammate.my_max.is_a?(Float) ? sprintf("%.0f",rscore_teammate.my_max) : rscore_teammate.my_max.to_s + '%' %> |
<% if participant.grade %> <% total_score = participant.grade %> <% title = "A score in blue indicates that the value was overwritten by the instructor or teaching assistant." %> <% else %> <% total_score = pscore[:total_score] %> <% title = nil %> <% end %> <div <% if title %>title="<%=title%>" style="color:#0033FF"<% end %>><%= total_score==(-1)? "N/A": sprintf("%.2f",total_score) %><%= score_postfix %></div> |
<div <% if p_title %>title="<%=p_title%>" style="color:#0033FF"<% end %>><%= p_total_score==(-1)? "N/A": sprintf("%.2f",p_total_score) %><%= score_postfix %></div> |
<% if !@rscore_review.nil? && @rscore_review.my_avg %> <%= @rscore_review.my_avg.is_a?(Float)?sprintf("%.2f",@rscore_review.my_avg):@rscore_review.my_avg %><%= score_postfix %> |
<% if !rscore_review.nil? && rscore_review.my_avg %> <%= rscore_review.my_avg.is_a?(Float)?sprintf("%.2f",rscore_review.my_avg):rscore_review.my_avg %><%= score_postfix %> |
Create test file named grades_helper_spec.rb in spec/helpers
A new test file was created and added to the project: spec/helpers/grades_helper_spec.rb
Write test cases for all methods in grades_helper.rb by using factories
The following test cases were added to test all methods in grades_helper.rb
require 'rails_helper' describe GradesHelper, :type => :helper do before(:each) do @assignment = create(:assignment, max_team_size: 1) @deadline_type = create(:deadline_type, id: 5, name: 'metareview') @deadline_right = create(:deadline_right) end describe 'get_accordion_title' do it 'should render is_first:true if last_topic is nil' do get_accordion_title(nil, 'last question') expect(response).to render_template(partial: 'response/_accordion', locals: {title: 'last question', is_first: true}) end it 'should render is_first:false if last_topic is not equal to next_topic' do get_accordion_title('last question', 'next question') expect(response).to render_template(partial: 'response/_accordion', locals: {title: 'next question', is_first: false}) end it 'should render nothing if last_topic is equal to next_topic' do get_accordion_title('question', 'question') expect(response).to render_template(nil) end end describe 'has_team_and_metareview?' do it 'should correctly identify the assignment from an assignment id' do params[:id] = @assignment.id result = Assignment.find(params[:id]) expect(result).to eq(@assignment) end it 'should correctly identify the assignment from a participant id' do participant = create(:participant, assignment: @assignment) params[:id] = participant.id result = Participant.find(params[:id]).parent_id expect(result).to eq(@assignment.id) end it 'should return 0 for an assignment without a team or a metareview deadline after a view action' do params[:action] = 'view' params[:id] = @assignment.id result = has_team_and_metareview? expect(result).to be == {has_team: false, has_metareview: false, true_num: 0} end it 'should return 1 for an assignment with a team but no metareview deadline after a view action' do @assignment.max_team_size = 2 @assignment.save params[:action] = 'view' params[:id] = @assignment.id result = has_team_and_metareview? expect(result).to be == {has_team: true, has_metareview: false, true_num: 1} end it 'should return 1 for an assignment without a team but with a metareview deadline after a view action' do @assignment.max_team_size = 1 @assignment.save @assignment_due_date = create(:assignment_due_date, assignment: @assignment, deadline_type: @deadline_type, submission_allowed_id: @deadline_right.id, review_allowed_id: @deadline_right.id, review_of_review_allowed_id: @deadline_right.id, due_at: '2015-12-30 23:30:12') params[:action] = 'view' params[:id] = @assignment.id result = has_team_and_metareview? expect(result).to be == {has_team: false, has_metareview: true, true_num: 1} end it 'should return 2 for an assignment without a team but with a metareview after a view action' do @assignment.max_team_size = 3 @assignment.save @assignment_due_date = create(:assignment_due_date, assignment: @assignment, deadline_type: @deadline_type, submission_allowed_id: @deadline_right.id, review_allowed_id: @deadline_right.id, review_of_review_allowed_id: @deadline_right.id, due_at: '2015-12-30 23:30:12') params[:action] = 'view' params[:id] = @assignment.id result = has_team_and_metareview? expect(result).to be == {has_team: true, has_metareview: true, true_num: 2} end end describe 'participant' do it 'should return the correct particpant' do new_participant = create(:participant) params[:id] = new_participant.id result = participant() expect(result).to eq(new_participant) end end describe 'rscore_review' do it 'should return a record of type :review if available' do new_participant = create(:participant) questionnaire = create(:questionnaire) assignment_questionnaire = create(:assignment_questionnaire, user_id: new_participant.id, questionnaire: questionnaire) @questions = {} @questions[questionnaire.symbol] = questionnaire.questions params[:id] = new_participant.id result = rscore_review() expect(result).to_not be_nil end it 'should return nil if no record of type :review is available' do new_participant = create(:participant) questionnaire = create(:metareview_questionnaire) assignment_questionnaire = create(:assignment_questionnaire, user_id: new_participant.id, questionnaire: questionnaire) @questions = {} @questions[questionnaire.symbol] = questionnaire.questions params[:id] = new_participant.id result = rscore_review() expect(result).to be_nil end end describe 'rscore_metareview' do it 'should return a record of type :metareview if available' do new_participant = create(:participant) questionnaire = create(:metareview_questionnaire) assignment_questionnaire = create(:assignment_questionnaire, user_id: new_participant.id, questionnaire: questionnaire) @questions = {} @questions[questionnaire.symbol] = questionnaire.questions params[:id] = new_participant.id result = rscore_metareview() expect(result).to_not be_nil end it 'should return nil if no record of type :metareview is available' do new_participant = create(:participant) questionnaire = create(:questionnaire) assignment_questionnaire = create(:assignment_questionnaire, user_id: new_participant.id, questionnaire: questionnaire) @questions = {} @questions[questionnaire.symbol] = questionnaire.questions params[:id] = new_participant.id result = rscore_metareview() expect(result).to be_nil end end describe 'rscore_feedback' do it 'should return a record of type :feedback if available' do new_participant = create(:participant) questionnaire = create(:author_feedback_questionnaire) assignment_questionnaire = create(:assignment_questionnaire, user_id: new_participant.id, questionnaire: questionnaire) @questions = {} @questions[questionnaire.symbol] = questionnaire.questions params[:id] = new_participant.id result = rscore_feedback() expect(result).to_not be_nil end it 'should return nil if no record of type :feedback is available' do new_participant = create(:participant) questionnaire = create(:questionnaire) assignment_questionnaire = create(:assignment_questionnaire, user_id: new_participant.id, questionnaire: questionnaire) @questions = {} @questions[questionnaire.symbol] = questionnaire.questions params[:id] = new_participant.id result = rscore_feedback() expect(result).to be_nil end end describe 'rscore_teammate' do it 'should return a record of type :teammate if available' do new_participant = create(:participant) questionnaire = create(:teammate_review_questionnaire) assignment_questionnaire = create(:assignment_questionnaire, user_id: new_participant.id, questionnaire: questionnaire) @questions = {} @questions[questionnaire.symbol] = questionnaire.questions params[:id] = new_participant.id result = rscore_teammate() expect(result).to_not be_nil end it 'should return nil if no record of type :teammate is available' do new_participant = create(:participant) questionnaire = create(:questionnaire) assignment_questionnaire = create(:assignment_questionnaire, user_id: new_participant.id, questionnaire: questionnaire) @questions = {} @questions[questionnaire.symbol] = questionnaire.questions params[:id] = new_participant.id result = rscore_teammate() expect(result).to be_nil end end describe 'p_total_score' do it 'should return the grade if available' do new_participant = create(:participant, grade: 90) questionnaire = create(:questionnaire) assignment_questionnaire = create(:assignment_questionnaire, user_id: new_participant.id, questionnaire: questionnaire) @questions = {} @questions[questionnaire.symbol] = questionnaire.questions params[:id] = new_participant.id result = p_total_score() expect(result).to eq(90) end it 'should return :total_score if no grade is available' do new_participant = create(:participant) questionnaire = create(:questionnaire) assignment_questionnaire = create(:assignment_questionnaire, user_id: new_participant.id, questionnaire: questionnaire) @questions = {} @questions[questionnaire.symbol] = questionnaire.questions params[:id] = new_participant.id result = p_total_score() expect(result).to eq(0) end end describe 'p_title' do it 'should return a title when the participant has a grade' do new_participant = create(:participant) params[:id] = new_participant.id new_participant.grade = 90 new_participant.save result = p_title() expect(result).to eq('A score in blue indicates that the value was overwritten by the instructor or teaching assistant.') end it 'should return nil when the participant has no grade' do new_participant = create(:participant) params[:id] = new_participant.id result = p_title() expect(result).to eq(nil) end end describe 'get_css_style_for_hamer_reputation' do it 'should return correct css for a reputation of -0.1' do result = get_css_style_for_hamer_reputation(-0.1) expect(result).to be == 'c1' end it 'should return correct css for a reputation of 0' do result = get_css_style_for_hamer_reputation(0) expect(result).to be == 'c1' end it 'should return correct css for a reputation of 0.5' do result = get_css_style_for_hamer_reputation(0.5) expect(result).to be == 'c2' end it 'should return correct css for a reputation of 1' do result = get_css_style_for_hamer_reputation(1) expect(result).to be == 'c2' end it 'should return correct css for a reputation of 1.5' do result = get_css_style_for_hamer_reputation(1.5) expect(result).to be == 'c3' end it 'should return correct css for a reputation of 2' do result = get_css_style_for_hamer_reputation(2) expect(result).to be == 'c4' end it 'should return correct css for a reputation of 2.1' do result = get_css_style_for_hamer_reputation(2.1) expect(result).to be == 'c5' end end describe 'get_css_style_for_lauw_reputation' do it 'should return correct css for a reputation of -0.1' do result = get_css_style_for_lauw_reputation(-0.1) expect(result).to be == 'c1' end it 'should return correct css for a reputation of 0' do result = get_css_style_for_lauw_reputation(0) expect(result).to be == 'c1' end it 'should return correct css for a reputation of 0.2' do result = get_css_style_for_lauw_reputation(0.2) expect(result).to be == 'c2' end it 'should return correct css for a reputation of 0.4' do result = get_css_style_for_lauw_reputation(0.4) expect(result).to be == 'c2' end it 'should return correct css for a reputation of 0.6' do result = get_css_style_for_lauw_reputation(0.6) expect(result).to be == 'c3' end it 'should return correct css for a reputation of 0.8' do result = get_css_style_for_lauw_reputation(0.8) expect(result).to be == 'c4' end it 'should return correct css for a reputation of 0.9' do result = get_css_style_for_lauw_reputation(0.9) expect(result).to be == 'c5' end end end
Test Plans
Test Case 1 | |
---|---|
Test URL | http://csc517.eastus.cloudapp.azure.com:8080 |
Test Type | Functional |
Scenario | Test to see if javascript changes for view_team.html.erb are working |
Pre-Conditions |
Make sure student to test with has the following:
|
Description |
|
Test Execution using existing login |
|
Test Case 2 | |
Test Type | Functional |
Scenario | Test to see if logic elimination from _participant.html.erb is working |
Pre-Conditions |
|
Description | [Insert description here] |
Test Execution using existing login | [Insert test execution steps using existing login] |