Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-27T00:54:04Z

Rkaur: /* Important Links */

This wiki page is for the description of changes made under ECE517 SPRING 2019 PROJECT E1931 CONFLICT NOTIFICATIONS

=Problem Statement=

Current conflict notification sends an email to the instructor whenever two reviews for the same submission differ “significantly” on their scoring (the threshold is specified in the “Notification limit” on the Rubrics tab of assignment creation). Right now an email is sent at any such time one of these conflicts happen, and it links to the review or submission which initiated the notification. It gives the link in raw HTML, but it should give it as a link associated with text, such as the text “this new review” linking to the page that shows the review.The email send to the instructor should also link to page that shows data on each of the reviews done on this team.

=Existing Feature Description=

Currently, this feature works as follows:

[[File:Flowwork.PNG|center]]

Whenever a new review is submitted, it is compared with the average of the previously submitted reviews. If the difference between the new submission and existing average is more than the threshold limit set, then a mail is sent to the instructor.With every review submitted for an assignment of a particular student, the average is updated.The mail sent to the instructor contains the links to the conflicted review and a summary of the reviews.

==Flaws with the existing Implementation==

===No view exists to analyze the conflict report===
Currently whenever the conflict happens,a summary link is sent to the instructor which contains the score of all the reviews but there is no view where the instructor can see all the conflicts and analyze them.

===Incorrect email message links===
The existing email body uses hardcoded URLs mentioned in models/response.rb file in the method notify_instructor_on_difference. Being hardcoded, these links wouldn't work on other servers where Expertiza is running. For Example, if the setup is done on localhost, the links will not be functional.

[[File:EMAIL.png|center]]

*Conflicting Review which triggered the mail
[[File:Score conflicting.png|center]]

=What needs to be done=
The scope of this project is to send an email notification to instructor which will contain links to the conflicting review, summary link, and a link to report which can be used for analyzing. The new report will have the information like the Team(having conflicts), the standard deviation for the team review score and pictorial representation of all the review scores. <br>

The email sent to the instructor should link to a page that shows the data on each of the reviews done on this team:
* The name of the team.
* The members (user-IDs and/or names)
* One line for each review received.In this line would be
# The name of the person (or team) that submitted the review, which would be hyperlinked to the review itself
# The score given for the review
# A bar chart showing pictorial view
* The average score received from reviewers.
* The standard deviation of the score received from reviewers.
* Review scores that are outside some “tolerance” limit should be highlighted, e.g., boldfaced or shown in a different color.Probably the tolerance should be, perhaps, one or two standard deviations from the mean.

=Implementation=

*'''Feature 1:''' New page to show the conflict report
A new view page of the report which will have the following information:-<br>
# The name of the team.
# The members of the team.
# The name of the reviewer which would be hyperlinked to the review itself.
# The score given for the review.
# A bar chart showing pictorial view of all the reviews for a particular team.
# The average score received from reviewers.
# The standard deviation of the score received from reviewers.
# Conflicting reviewers are highlighted.These are those which are two standard deviations away from the mean.

This view have a report of all the teams on one page.This report link is added to the mail that is already being sent whenever some conflict happens.This report page will be also accessible from the reports drop-down in the instructor view.

*A new partial is created for this view -

<pre>
<div style="color:green;text-align:adjust">
Note: <br>
X- axis represents precentage score of the team<br>
Y- axis represents the reviewers <br>
Highlighted reviewers are outside the tolerance limit<br>
</div><br><br>
<table id="myTable" border="1px solid black" margin-top="-9px" width="100%" cellspacing='0' cellpadding='2' border='0'>
<thead>
<tr bgcolor='#CCCCCC'>

<th width="30%" class="sorter-false" border="1px black solid" style="text-align:center">Team <span></span></th>
<th width="70%" class="sorter-false" border="1px black solid" style="text-align:center" >Reviewers/Metrics <span></span></th>
</tr>
</thead>
<tbody>
<% max_scores= max_assignment_review_score_per_round(@id) %>
<%@reviewers.keys.each do |team_name| %>
<tr>
<td style="padding-left: 50px;" >
<% members = team_members(team_name, @id)%>
<strong>Team Name: </strong><%= team_name %><br>
<strong>Team Members: </strong><br>
<% members.each do |member|%>
<%=User.find(member.user_id).fullname %><br>
<% end %>
</td>
<td>
<%= render 'review_conflict_metric' , team_name: team_name , max_scores: max_scores %>
</td>
</tr>
<% end %>
</tbody>
</table>
</pre>

* Partial for the display for reviewers,scores and metrics
<pre>
<table width="100%" border="0" >
<%question_answers = review_score_for_team(@id, team_name) %>
<% i = 1 %>
<% question_answers.each do |question_answer| %>
<% average = average_of_round(question_answer) %>
<% std = std_of_round(average,question_answer) %>
<% upper_tolerance_limit = (average+(2*std)).round(2) %>
<% lower_tolerance_limit = (average-(2*std)).round(2) %>
<tr>
<td width="30%">
<div>
<strong>Reviewers with scores</strong><br>
<ol>
<% question_answer.each do |reviewer, answer| %>
<% if answer > upper_tolerance_limit or answer < lower_tolerance_limit %>
<li><% response_id = response_id_review_conflict(reviewer, @teams[team_name], @id) %>
<mark style="background-color:#FFFF00">
<%= link_to "("+@reviewers[team_name.to_s][reviewer.to_s].to_s+")", :controller => 'popup', :action => 'team_users_popup', :id => @teams[team_name], :id2 => response_id, :assignment_id=>@id %>
<%= list_review_submissions(reviewer, @teams[team_name], response_id) %>
<%= ": " + answer.to_s %></mark>
</li>
<% else %>
<li><% response_id = response_id_review_conflict(reviewer, @teams[team_name], @id) %>
<%= link_to "("+@reviewers[team_name.to_s][reviewer.to_s].to_s+")", :controller => 'popup', :action => 'team_users_popup', :id => @teams[team_name], :id2 => response_id, :assignment_id=>@id %>
<%= list_review_submissions(reviewer, @teams[team_name], response_id) %>
<%= ": " + answer.to_s %>
</li>
<% end %>
<% end %>
</ol>
</div>
<strong>Max Review Score: </strong><%=max_scores[i-1]%><br>
<strong>Average Score: </strong><%= average.to_s %><br>
<strong>Standard Deviation: </strong><%= std.to_s %><br><br>
</td>
<td>
<div>
<strong><u><%= "Round: " + i.to_s %></u></strong>
<%= generate_score_chart(max_scores[i-1], question_answer) %>
</div>
</td>
</tr>
<% i+=1 %>
<% end %>
</table>
</pre>

*Sample view of the new report -
[[File: Oodd report.PNG|center]]
<br>

* Method to get all the answers(scores) for a reviewee on an assignment for a particular round
<pre>
def self.answers_by_round_for_reviewee(assignment_id, reviewee_id,round)
#get all answers for a reviewee on an assignment for a particular round
question_answers = Answer.select("answers.answer, response_maps.reviewer_id")
.joins("join responses on responses.id = answers.response_id")
.joins("join response_maps on responses.map_id = response_maps.id")
.where("response_maps.reviewed_object_id = ? and
response_maps.reviewee_id = ? and
responses.round = ?", assignment_id, reviewee_id,round)
question_answers
end
</pre>

* Method to get the maximum score for an assignment and for a particular round
<pre>
def max_score_of_assignment_per_round(assignment,round)
rubric = get_questions_by_assignment(assignment)
rubric_questions_used = rubric[assignment.varying_rubrics_by_round? ? round : 0]
assignment_max_score_per_round = 0
rubric_questions_used.each do |q|
next if q.type.eql?("SectionHeader")
q_max_score = get_max_score_for_question(q)
assignment_max_score_per_round += q_max_score
end
assignment_max_score_per_round
end

def max_assignment_review_score_per_round(reviewed_object_id)
assignment = Assignment.find(reviewed_object_id)
total_rounds = assignment.rounds_of_reviews
review_max_scores = Array.new(total_rounds)
(0..total_rounds-1).each do |round|
review_max_scores[round] = SummaryHelper::Summary.new.max_score_of_assignment_per_round(assignment, round)
end
review_max_scores
end
</pre>

* Method to create the bar chart showing the review scores per round
<pre>
def generate_score_chart(review_max_score, question_answer)
scores = Array.new
question_answer.each do |reviewer,answer|
scores << ((answer.to_f/review_max_score.to_f)*100).round(2)
end
labels = (1..scores.length).to_a
data = {
labels: labels,
datasets: [
{
label: "score%",
backgroundColor: "rgba(255,99,132,0.8)",
borderWidth: 1,
data: scores,
hoverBackgroundColor: "orange",
yAxisID: "bar-y-axis1"
}
]
}
options = {
legend: {
position: 'top',
labels: {
usePointStyle: true
}
},
width: "125",
height: "75",
scales: {
yAxes: [{
stacked: true,
id: "bar-y-axis1",
barThickness: 10
}, {
display: false,
stacked: true,
id: "bar-y-axis2",
barThickness: 15,
type: 'category',
categoryPercentage: 0.8,
barPercentage: 0.9,
gridLines: {
offsetGridLines: true
}
}],
xAxes: [{
stacked: false,
ticks: {
beginAtZero: true,
stepSize: 10,
max: 100
}
}]
}
}
horizontal_bar_chart data, options
end
</pre>

* Method to get the team members
<pre>
def team_members(team_name, assignment_id)
team_id = Team.select(:id).where(name: team_name)
members = TeamsUser.where(team_id: team_id)
members
end
</pre>

* Method to find the standard deviation
<pre>
def std_of_round(average,question_answer)
accum=0.0
i=0
question_answer.each do |reviewer,answer|
accum+=(answer-average)**2
i+=1
end
if i != 0
accum=Math.sqrt(accum/i)
end
accum.round(2)
end
</pre>

* Method to find the average score per round per team
<pre>
def average_of_round(question_answer)
average=0.0
i=0
question_answer.each do |reviewer,answer|
average+=answer
i+=1
end
if i != 0
average=average/i
end
average.round(2)
end
</pre>

* Method to get the review score for each round of particular team
<pre>
def review_score_for_team(reviewed_object_id, team_name)
question_answers=[]
reviewee_id = Team.select(:id).where(name: team_name, parent_id: reviewed_object_id)
reviewee_id.each do |reviewee|
total_rounds = Assignment.find(reviewed_object_id).rounds_of_reviews
question_answers = Array.new(total_rounds)
(0..total_rounds-1).each do |round|
temp_values = Answer.answers_by_round_for_reviewee(reviewed_object_id, reviewee,round+1)
question_answers[round] = review_score_helper_for_team(temp_values)
end
end
question_answers
end

def review_score_helper_for_team(temp_values)
question_answers={}
temp_values.each do |temp_value|
if question_answers.key?(temp_value[:reviewer_id])
if temp_value[:answer].nil?
question_answers[temp_value[:reviewer_id]] += 0
else
question_answers[temp_value[:reviewer_id]] += temp_value[:answer]
end
else
if temp_value[:answer].nil?
question_answers[temp_value[:reviewer_id]] = 0
else
question_answers[temp_value[:reviewer_id]] = temp_value[:answer]
end
end
end
question_answers
end
</pre>

* Method to create a response map for the reviewers and reviewee as well as the teams and the reviewee_id
<pre>
def review_conflict_response_map(params, _session = nil)
assign_basics(params)
teams = Team.select(:id, :name).where(parent_id: @id).order(:name)
@teams = ({})
@reviewers = ({})
teams.each do |reviewee|
@reviewers[reviewee.name] = reviewers_name_id_by_reviewee_and_assignment(reviewee, @id)
@teams[reviewee.name] = reviewee.id
end
end
</pre>

* Method to get the reviewers of a particular assignment and particular reviewee
<pre>
def reviewers_name_id_by_reviewee_and_assignment(reviewee, id)
temp_reviewers = User.select(" DISTINCT participants.id, users.name")
.joins("JOIN participants ON participants.user_id = users.id")
.joins("JOIN response_maps ON response_maps.reviewer_id = participants.id")
.where("response_maps.reviewee_id = ? and response_maps.reviewed_object_id = ?", reviewee.id, id)
reviewers = ({})
temp_reviewers.each do |reviewer|
reviewers[reviewer[:id].to_s] = reviewer[:name]
end
reviewers
end
</pre>

*'''Feature 2:''' No hardcoded URLs
Hardcoded URLs are be managed by the config file in order to make these links functional on every server running Expertiza and raw URLs will be given some meaningful names.

* Code changed for removing the hardcoded URLs
[[File:Harcord link.PNG|center]]

=Test Plan=

To test code implementation and correctness of the modified feature, we had done the following tests:-

*Run and pass existing RSpec Tests
*UI testing for the new functionality
*Develop New RSpec Tests for the new code

==Rspec Testing==

All the existing related rspec tests have tested and they all passed wit 0 failures.
Majority of the new functionality concerns with the new view,so we have mainly done the UI testing shown below.Some of the Rspec test added are shown below:

*Rspec Tests for Answer Model
<pre>
let(:response_map) { create(:review_response_map, id: 1, reviewed_object_id: 1, reviewer_id: 1) }
it "returns answers by reviewer for reviewee in round from db which is not empty" do
expect(Answer.answers_by_round_for_reviewee(@assignment_id, @reviewee_id,@round)).not_to be_empty
end
</pre>

*Rspec Tests for Report Formatter Helper methods
<pre>
describe 'review_conflict_response_map' do
context 'when type is ReviewConflictResponseMap' do
it 'renders response_report page with corresponding data' do
allow(Team).to receive(:where).with(parent_id: '1').and_return([integer('id')]).ordered
params = {
id: 1,
report: {type: 'ReviewResponseMap'},
user: 'no one'
}
get :response_report, params
expect(response).to render_template(:response_report)
end
end
end

describe "reviewers_name_id_by_reviewee_and_assignment" do
before(:each) do
@assignment_id = 1
@reviewee_id = 1
end
it "returns reviewers name from Answer by reviewee ans assignment id from db which is not empty" do
expect(allow(User).to receive(:where).with(@reviewee_id, @assignment_id).and_return([integer('id')])).not_to be_empty
end
end
</pre>

==UI Testing==

We have created a [https://cl.ly/ab6b9c8bac36 Demo Video] showing the new functionality.

=Files changed in this Project=
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/controllers/response_controller.rb Response Controller]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/report_formatter_helper.rb Report Formatter Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/review_mapping_helper.rb Review Mapping Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/summary_helper.rb Summary Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/mailers/mailer.rb Mailer]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/models/answer.rb Answer model]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/models/response.rb Response Model]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/mailer/notify_grade_conflict_message.html.erb Notify Grade Conflict Message View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_conflict_metric.html.erb Review Conflict Metric View] (New Created)
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_conflict_report.html.erb Review Conflict Report View] (New Created)
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_report.html.erb Review Report View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_searchbox.html.erb Searchbox View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/response_report.html.haml Response Report View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/spec/controllers/reports_controller_spec.rb Reports Controller Spec]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/spec/models/answer_spec.rb Answer model spec]

=Important Links=

* [https://github.com/expertiza/expertiza/pull/1445 Pull Request]
* [https://github.com/AryaAbhishek/expertiza Expertiza Github Repo]

=References=
* [https://github.com/expertiza/expertiza expertiza]
* [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018-_Project_E1865._Conflict_Notification Previous Work]
* [https://github.com/expertiza/expertiza/pull/1329 Previous Pull Request]
* [https://rubygems.org/gems/standard-deviation/versions/0.1.1 Standard-Deviation gem]
* [https://github.com/jtescher/descriptive-statistics descriptive-statistics gem]

=Team=

[mailto:aarya@ncsu.edu Abhishek Arya]<br>
[mailto:bbdeshpa@ncsu.edu Bhargav Deshpande]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:uschatto@ncsu.edu Udita Chattopadhyay]<br>

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-27T00:53:42Z

Rkaur:

This wiki page is for the description of changes made under ECE517 SPRING 2019 PROJECT E1931 CONFLICT NOTIFICATIONS

=Problem Statement=

Current conflict notification sends an email to the instructor whenever two reviews for the same submission differ “significantly” on their scoring (the threshold is specified in the “Notification limit” on the Rubrics tab of assignment creation). Right now an email is sent at any such time one of these conflicts happen, and it links to the review or submission which initiated the notification. It gives the link in raw HTML, but it should give it as a link associated with text, such as the text “this new review” linking to the page that shows the review.The email send to the instructor should also link to page that shows data on each of the reviews done on this team.

=Existing Feature Description=

Currently, this feature works as follows:

[[File:Flowwork.PNG|center]]

Whenever a new review is submitted, it is compared with the average of the previously submitted reviews. If the difference between the new submission and existing average is more than the threshold limit set, then a mail is sent to the instructor.With every review submitted for an assignment of a particular student, the average is updated.The mail sent to the instructor contains the links to the conflicted review and a summary of the reviews.

==Flaws with the existing Implementation==

===No view exists to analyze the conflict report===
Currently whenever the conflict happens,a summary link is sent to the instructor which contains the score of all the reviews but there is no view where the instructor can see all the conflicts and analyze them.

===Incorrect email message links===
The existing email body uses hardcoded URLs mentioned in models/response.rb file in the method notify_instructor_on_difference. Being hardcoded, these links wouldn't work on other servers where Expertiza is running. For Example, if the setup is done on localhost, the links will not be functional.

[[File:EMAIL.png|center]]

*Conflicting Review which triggered the mail
[[File:Score conflicting.png|center]]

=What needs to be done=
The scope of this project is to send an email notification to instructor which will contain links to the conflicting review, summary link, and a link to report which can be used for analyzing. The new report will have the information like the Team(having conflicts), the standard deviation for the team review score and pictorial representation of all the review scores. <br>

The email sent to the instructor should link to a page that shows the data on each of the reviews done on this team:
* The name of the team.
* The members (user-IDs and/or names)
* One line for each review received.In this line would be
# The name of the person (or team) that submitted the review, which would be hyperlinked to the review itself
# The score given for the review
# A bar chart showing pictorial view
* The average score received from reviewers.
* The standard deviation of the score received from reviewers.
* Review scores that are outside some “tolerance” limit should be highlighted, e.g., boldfaced or shown in a different color.Probably the tolerance should be, perhaps, one or two standard deviations from the mean.

=Implementation=

*'''Feature 1:''' New page to show the conflict report
A new view page of the report which will have the following information:-<br>
# The name of the team.
# The members of the team.
# The name of the reviewer which would be hyperlinked to the review itself.
# The score given for the review.
# A bar chart showing pictorial view of all the reviews for a particular team.
# The average score received from reviewers.
# The standard deviation of the score received from reviewers.
# Conflicting reviewers are highlighted.These are those which are two standard deviations away from the mean.

This view have a report of all the teams on one page.This report link is added to the mail that is already being sent whenever some conflict happens.This report page will be also accessible from the reports drop-down in the instructor view.

*A new partial is created for this view -

<pre>
<div style="color:green;text-align:adjust">
Note: <br>
X- axis represents precentage score of the team<br>
Y- axis represents the reviewers <br>
Highlighted reviewers are outside the tolerance limit<br>
</div><br><br>
<table id="myTable" border="1px solid black" margin-top="-9px" width="100%" cellspacing='0' cellpadding='2' border='0'>
<thead>
<tr bgcolor='#CCCCCC'>

<th width="30%" class="sorter-false" border="1px black solid" style="text-align:center">Team <span></span></th>
<th width="70%" class="sorter-false" border="1px black solid" style="text-align:center" >Reviewers/Metrics <span></span></th>
</tr>
</thead>
<tbody>
<% max_scores= max_assignment_review_score_per_round(@id) %>
<%@reviewers.keys.each do |team_name| %>
<tr>
<td style="padding-left: 50px;" >
<% members = team_members(team_name, @id)%>
<strong>Team Name: </strong><%= team_name %><br>
<strong>Team Members: </strong><br>
<% members.each do |member|%>
<%=User.find(member.user_id).fullname %><br>
<% end %>
</td>
<td>
<%= render 'review_conflict_metric' , team_name: team_name , max_scores: max_scores %>
</td>
</tr>
<% end %>
</tbody>
</table>
</pre>

* Partial for the display for reviewers,scores and metrics
<pre>
<table width="100%" border="0" >
<%question_answers = review_score_for_team(@id, team_name) %>
<% i = 1 %>
<% question_answers.each do |question_answer| %>
<% average = average_of_round(question_answer) %>
<% std = std_of_round(average,question_answer) %>
<% upper_tolerance_limit = (average+(2*std)).round(2) %>
<% lower_tolerance_limit = (average-(2*std)).round(2) %>
<tr>
<td width="30%">
<div>
<strong>Reviewers with scores</strong><br>
<ol>
<% question_answer.each do |reviewer, answer| %>
<% if answer > upper_tolerance_limit or answer < lower_tolerance_limit %>
<li><% response_id = response_id_review_conflict(reviewer, @teams[team_name], @id) %>
<mark style="background-color:#FFFF00">
<%= link_to "("+@reviewers[team_name.to_s][reviewer.to_s].to_s+")", :controller => 'popup', :action => 'team_users_popup', :id => @teams[team_name], :id2 => response_id, :assignment_id=>@id %>
<%= list_review_submissions(reviewer, @teams[team_name], response_id) %>
<%= ": " + answer.to_s %></mark>
</li>
<% else %>
<li><% response_id = response_id_review_conflict(reviewer, @teams[team_name], @id) %>
<%= link_to "("+@reviewers[team_name.to_s][reviewer.to_s].to_s+")", :controller => 'popup', :action => 'team_users_popup', :id => @teams[team_name], :id2 => response_id, :assignment_id=>@id %>
<%= list_review_submissions(reviewer, @teams[team_name], response_id) %>
<%= ": " + answer.to_s %>
</li>
<% end %>
<% end %>
</ol>
</div>
<strong>Max Review Score: </strong><%=max_scores[i-1]%><br>
<strong>Average Score: </strong><%= average.to_s %><br>
<strong>Standard Deviation: </strong><%= std.to_s %><br><br>
</td>
<td>
<div>
<strong><u><%= "Round: " + i.to_s %></u></strong>
<%= generate_score_chart(max_scores[i-1], question_answer) %>
</div>
</td>
</tr>
<% i+=1 %>
<% end %>
</table>
</pre>

*Sample view of the new report -
[[File: Oodd report.PNG|center]]
<br>

* Method to get all the answers(scores) for a reviewee on an assignment for a particular round
<pre>
def self.answers_by_round_for_reviewee(assignment_id, reviewee_id,round)
#get all answers for a reviewee on an assignment for a particular round
question_answers = Answer.select("answers.answer, response_maps.reviewer_id")
.joins("join responses on responses.id = answers.response_id")
.joins("join response_maps on responses.map_id = response_maps.id")
.where("response_maps.reviewed_object_id = ? and
response_maps.reviewee_id = ? and
responses.round = ?", assignment_id, reviewee_id,round)
question_answers
end
</pre>

* Method to get the maximum score for an assignment and for a particular round
<pre>
def max_score_of_assignment_per_round(assignment,round)
rubric = get_questions_by_assignment(assignment)
rubric_questions_used = rubric[assignment.varying_rubrics_by_round? ? round : 0]
assignment_max_score_per_round = 0
rubric_questions_used.each do |q|
next if q.type.eql?("SectionHeader")
q_max_score = get_max_score_for_question(q)
assignment_max_score_per_round += q_max_score
end
assignment_max_score_per_round
end

def max_assignment_review_score_per_round(reviewed_object_id)
assignment = Assignment.find(reviewed_object_id)
total_rounds = assignment.rounds_of_reviews
review_max_scores = Array.new(total_rounds)
(0..total_rounds-1).each do |round|
review_max_scores[round] = SummaryHelper::Summary.new.max_score_of_assignment_per_round(assignment, round)
end
review_max_scores
end
</pre>

* Method to create the bar chart showing the review scores per round
<pre>
def generate_score_chart(review_max_score, question_answer)
scores = Array.new
question_answer.each do |reviewer,answer|
scores << ((answer.to_f/review_max_score.to_f)*100).round(2)
end
labels = (1..scores.length).to_a
data = {
labels: labels,
datasets: [
{
label: "score%",
backgroundColor: "rgba(255,99,132,0.8)",
borderWidth: 1,
data: scores,
hoverBackgroundColor: "orange",
yAxisID: "bar-y-axis1"
}
]
}
options = {
legend: {
position: 'top',
labels: {
usePointStyle: true
}
},
width: "125",
height: "75",
scales: {
yAxes: [{
stacked: true,
id: "bar-y-axis1",
barThickness: 10
}, {
display: false,
stacked: true,
id: "bar-y-axis2",
barThickness: 15,
type: 'category',
categoryPercentage: 0.8,
barPercentage: 0.9,
gridLines: {
offsetGridLines: true
}
}],
xAxes: [{
stacked: false,
ticks: {
beginAtZero: true,
stepSize: 10,
max: 100
}
}]
}
}
horizontal_bar_chart data, options
end
</pre>

* Method to get the team members
<pre>
def team_members(team_name, assignment_id)
team_id = Team.select(:id).where(name: team_name)
members = TeamsUser.where(team_id: team_id)
members
end
</pre>

* Method to find the standard deviation
<pre>
def std_of_round(average,question_answer)
accum=0.0
i=0
question_answer.each do |reviewer,answer|
accum+=(answer-average)**2
i+=1
end
if i != 0
accum=Math.sqrt(accum/i)
end
accum.round(2)
end
</pre>

* Method to find the average score per round per team
<pre>
def average_of_round(question_answer)
average=0.0
i=0
question_answer.each do |reviewer,answer|
average+=answer
i+=1
end
if i != 0
average=average/i
end
average.round(2)
end
</pre>

* Method to get the review score for each round of particular team
<pre>
def review_score_for_team(reviewed_object_id, team_name)
question_answers=[]
reviewee_id = Team.select(:id).where(name: team_name, parent_id: reviewed_object_id)
reviewee_id.each do |reviewee|
total_rounds = Assignment.find(reviewed_object_id).rounds_of_reviews
question_answers = Array.new(total_rounds)
(0..total_rounds-1).each do |round|
temp_values = Answer.answers_by_round_for_reviewee(reviewed_object_id, reviewee,round+1)
question_answers[round] = review_score_helper_for_team(temp_values)
end
end
question_answers
end

def review_score_helper_for_team(temp_values)
question_answers={}
temp_values.each do |temp_value|
if question_answers.key?(temp_value[:reviewer_id])
if temp_value[:answer].nil?
question_answers[temp_value[:reviewer_id]] += 0
else
question_answers[temp_value[:reviewer_id]] += temp_value[:answer]
end
else
if temp_value[:answer].nil?
question_answers[temp_value[:reviewer_id]] = 0
else
question_answers[temp_value[:reviewer_id]] = temp_value[:answer]
end
end
end
question_answers
end
</pre>

* Method to create a response map for the reviewers and reviewee as well as the teams and the reviewee_id
<pre>
def review_conflict_response_map(params, _session = nil)
assign_basics(params)
teams = Team.select(:id, :name).where(parent_id: @id).order(:name)
@teams = ({})
@reviewers = ({})
teams.each do |reviewee|
@reviewers[reviewee.name] = reviewers_name_id_by_reviewee_and_assignment(reviewee, @id)
@teams[reviewee.name] = reviewee.id
end
end
</pre>

* Method to get the reviewers of a particular assignment and particular reviewee
<pre>
def reviewers_name_id_by_reviewee_and_assignment(reviewee, id)
temp_reviewers = User.select(" DISTINCT participants.id, users.name")
.joins("JOIN participants ON participants.user_id = users.id")
.joins("JOIN response_maps ON response_maps.reviewer_id = participants.id")
.where("response_maps.reviewee_id = ? and response_maps.reviewed_object_id = ?", reviewee.id, id)
reviewers = ({})
temp_reviewers.each do |reviewer|
reviewers[reviewer[:id].to_s] = reviewer[:name]
end
reviewers
end
</pre>

*'''Feature 2:''' No hardcoded URLs
Hardcoded URLs are be managed by the config file in order to make these links functional on every server running Expertiza and raw URLs will be given some meaningful names.

* Code changed for removing the hardcoded URLs
[[File:Harcord link.PNG|center]]

=Test Plan=

To test code implementation and correctness of the modified feature, we had done the following tests:-

*Run and pass existing RSpec Tests
*UI testing for the new functionality
*Develop New RSpec Tests for the new code

==Rspec Testing==

All the existing related rspec tests have tested and they all passed wit 0 failures.
Majority of the new functionality concerns with the new view,so we have mainly done the UI testing shown below.Some of the Rspec test added are shown below:

*Rspec Tests for Answer Model
<pre>
let(:response_map) { create(:review_response_map, id: 1, reviewed_object_id: 1, reviewer_id: 1) }
it "returns answers by reviewer for reviewee in round from db which is not empty" do
expect(Answer.answers_by_round_for_reviewee(@assignment_id, @reviewee_id,@round)).not_to be_empty
end
</pre>

*Rspec Tests for Report Formatter Helper methods
<pre>
describe 'review_conflict_response_map' do
context 'when type is ReviewConflictResponseMap' do
it 'renders response_report page with corresponding data' do
allow(Team).to receive(:where).with(parent_id: '1').and_return([integer('id')]).ordered
params = {
id: 1,
report: {type: 'ReviewResponseMap'},
user: 'no one'
}
get :response_report, params
expect(response).to render_template(:response_report)
end
end
end

describe "reviewers_name_id_by_reviewee_and_assignment" do
before(:each) do
@assignment_id = 1
@reviewee_id = 1
end
it "returns reviewers name from Answer by reviewee ans assignment id from db which is not empty" do
expect(allow(User).to receive(:where).with(@reviewee_id, @assignment_id).and_return([integer('id')])).not_to be_empty
end
end
</pre>

==UI Testing==

We have created a [https://cl.ly/ab6b9c8bac36 Demo Video] showing the new functionality.

=Files changed in this Project=
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/controllers/response_controller.rb Response Controller]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/report_formatter_helper.rb Report Formatter Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/review_mapping_helper.rb Review Mapping Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/summary_helper.rb Summary Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/mailers/mailer.rb Mailer]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/models/answer.rb Answer model]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/models/response.rb Response Model]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/mailer/notify_grade_conflict_message.html.erb Notify Grade Conflict Message View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_conflict_metric.html.erb Review Conflict Metric View] (New Created)
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_conflict_report.html.erb Review Conflict Report View] (New Created)
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_report.html.erb Review Report View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_searchbox.html.erb Searchbox View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/response_report.html.haml Response Report View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/spec/controllers/reports_controller_spec.rb Reports Controller Spec]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/spec/models/answer_spec.rb Answer model spec]

=Important Links=

* [https://github.com/expertiza/expertiza/pull/1445 Pull Request]
* [https://github.com/AryaAbhishek/expertiza Expertiza Github]

=References=
* [https://github.com/expertiza/expertiza expertiza]
* [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018-_Project_E1865._Conflict_Notification Previous Work]
* [https://github.com/expertiza/expertiza/pull/1329 Previous Pull Request]
* [https://rubygems.org/gems/standard-deviation/versions/0.1.1 Standard-Deviation gem]
* [https://github.com/jtescher/descriptive-statistics descriptive-statistics gem]

=Team=

[mailto:aarya@ncsu.edu Abhishek Arya]<br>
[mailto:bbdeshpa@ncsu.edu Bhargav Deshpande]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:uschatto@ncsu.edu Udita Chattopadhyay]<br>

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-27T00:39:18Z

Rkaur: /* UI Testing */

This wiki page is for the description of changes made under ECE517 SPRING 2019 PROJECT E1931 CONFLICT NOTIFICATIONS

=Problem Statement=

Current conflict notification sends an email to the instructor whenever two reviews for the same submission differ “significantly” on their scoring (the threshold is specified in the “Notification limit” on the Rubrics tab of assignment creation). Right now an email is sent at any such time one of these conflicts happen, and it links to the review or submission which initiated the notification. It gives the link in raw HTML, but it should give it as a link associated with text, such as the text “this new review” linking to the page that shows the review.The email send to the instructor should also link to page that shows data on each of the reviews done on this team.

=Existing Feature Description=

Currently, this feature works as follows:

[[File:Flowwork.PNG|center]]

Whenever a new review is submitted, it is compared with the average of the previously submitted reviews. If the difference between the new submission and existing average is more than the threshold limit set, then a mail is sent to the instructor.With every review submitted for an assignment of a particular student, the average is updated.The mail sent to the instructor contains the links to the conflicted review and a summary of the reviews.

==Flaws with the existing Implementation==

===No view exists to analyze the conflict report===
Currently whenever the conflict happens,a summary link is sent to the instructor which contains the score of all the reviews but there is no view where the instructor can see all the conflicts and analyze them.

===Incorrect email message links===
The existing email body uses hardcoded URLs mentioned in models/response.rb file in the method notify_instructor_on_difference. Being hardcoded, these links wouldn't work on other servers where Expertiza is running. For Example, if the setup is done on localhost, the links will not be functional.

[[File:EMAIL.png|center]]

*Conflicting Review which triggered the mail
[[File:Score conflicting.png|center]]

=What needs to be done=
The scope of this project is to send an email notification to instructor which will contain links to the conflicting review, summary link, and a link to report which can be used for analyzing. The new report will have the information like the Team(having conflicts), the standard deviation for the team review score and pictorial representation of all the review scores. <br>

The email sent to the instructor should link to a page that shows the data on each of the reviews done on this team:
* The name of the team.
* The members (user-IDs and/or names)
* One line for each review received.In this line would be
# The name of the person (or team) that submitted the review, which would be hyperlinked to the review itself
# The score given for the review
# A bar chart showing pictorial view
* The average score received from reviewers.
* The standard deviation of the score received from reviewers.
* Review scores that are outside some “tolerance” limit should be highlighted, e.g., boldfaced or shown in a different color.Probably the tolerance should be, perhaps, one or two standard deviations from the mean.

=Implementation=

*'''Feature 1:''' New page to show the conflict report
A new view page of the report which will have the following information:-<br>
# The name of the team.
# The members of the team.
# The name of the reviewer which would be hyperlinked to the review itself.
# The score given for the review.
# A bar chart showing pictorial view of all the reviews for a particular team.
# The average score received from reviewers.
# The standard deviation of the score received from reviewers.
# Conflicting reviewers are highlighted.These are those which are two standard deviations away from the mean.

This view have a report of all the teams on one page.This report link is added to the mail that is already being sent whenever some conflict happens.This report page will be also accessible from the reports drop-down in the instructor view.

*A new partial is created for this view -

<pre>
<div style="color:green;text-align:adjust">
Note: <br>
X- axis represents precentage score of the team<br>
Y- axis represents the reviewers <br>
Highlighted reviewers are outside the tolerance limit<br>
</div><br><br>
<table id="myTable" border="1px solid black" margin-top="-9px" width="100%" cellspacing='0' cellpadding='2' border='0'>
<thead>
<tr bgcolor='#CCCCCC'>

<th width="30%" class="sorter-false" border="1px black solid" style="text-align:center">Team <span></span></th>
<th width="70%" class="sorter-false" border="1px black solid" style="text-align:center" >Reviewers/Metrics <span></span></th>
</tr>
</thead>
<tbody>
<% max_scores= max_assignment_review_score_per_round(@id) %>
<%@reviewers.keys.each do |team_name| %>
<tr>
<td style="padding-left: 50px;" >
<% members = team_members(team_name, @id)%>
<strong>Team Name: </strong><%= team_name %><br>
<strong>Team Members: </strong><br>
<% members.each do |member|%>
<%=User.find(member.user_id).fullname %><br>
<% end %>
</td>
<td>
<%= render 'review_conflict_metric' , team_name: team_name , max_scores: max_scores %>
</td>
</tr>
<% end %>
</tbody>
</table>
</pre>

* Partial for the display for reviewers,scores and metrics
<pre>
<table width="100%" border="0" >
<%question_answers = review_score_for_team(@id, team_name) %>
<% i = 1 %>
<% question_answers.each do |question_answer| %>
<% average = average_of_round(question_answer) %>
<% std = std_of_round(average,question_answer) %>
<% upper_tolerance_limit = (average+(2*std)).round(2) %>
<% lower_tolerance_limit = (average-(2*std)).round(2) %>
<tr>
<td width="30%">
<div>
<strong>Reviewers with scores</strong><br>
<ol>
<% question_answer.each do |reviewer, answer| %>
<% if answer > upper_tolerance_limit or answer < lower_tolerance_limit %>
<li><% response_id = response_id_review_conflict(reviewer, @teams[team_name], @id) %>
<mark style="background-color:#FFFF00">
<%= link_to "("+@reviewers[team_name.to_s][reviewer.to_s].to_s+")", :controller => 'popup', :action => 'team_users_popup', :id => @teams[team_name], :id2 => response_id, :assignment_id=>@id %>
<%= list_review_submissions(reviewer, @teams[team_name], response_id) %>
<%= ": " + answer.to_s %></mark>
</li>
<% else %>
<li><% response_id = response_id_review_conflict(reviewer, @teams[team_name], @id) %>
<%= link_to "("+@reviewers[team_name.to_s][reviewer.to_s].to_s+")", :controller => 'popup', :action => 'team_users_popup', :id => @teams[team_name], :id2 => response_id, :assignment_id=>@id %>
<%= list_review_submissions(reviewer, @teams[team_name], response_id) %>
<%= ": " + answer.to_s %>
</li>
<% end %>
<% end %>
</ol>
</div>
<strong>Max Review Score: </strong><%=max_scores[i-1]%><br>
<strong>Average Score: </strong><%= average.to_s %><br>
<strong>Standard Deviation: </strong><%= std.to_s %><br><br>
</td>
<td>
<div>
<strong><u><%= "Round: " + i.to_s %></u></strong>
<%= generate_score_chart(max_scores[i-1], question_answer) %>
</div>
</td>
</tr>
<% i+=1 %>
<% end %>
</table>
</pre>

*Sample view of the new report -
[[File: Oodd report.PNG|center]]
<br>

* Method to get all the answers(scores) for a reviewee on an assignment for a particular round
<pre>
def self.answers_by_round_for_reviewee(assignment_id, reviewee_id,round)
#get all answers for a reviewee on an assignment for a particular round
question_answers = Answer.select("answers.answer, response_maps.reviewer_id")
.joins("join responses on responses.id = answers.response_id")
.joins("join response_maps on responses.map_id = response_maps.id")
.where("response_maps.reviewed_object_id = ? and
response_maps.reviewee_id = ? and
responses.round = ?", assignment_id, reviewee_id,round)
question_answers
end
</pre>

* Method to get the maximum score for an assignment and for a particular round
<pre>
def max_score_of_assignment_per_round(assignment,round)
rubric = get_questions_by_assignment(assignment)
rubric_questions_used = rubric[assignment.varying_rubrics_by_round? ? round : 0]
assignment_max_score_per_round = 0
rubric_questions_used.each do |q|
next if q.type.eql?("SectionHeader")
q_max_score = get_max_score_for_question(q)
assignment_max_score_per_round += q_max_score
end
assignment_max_score_per_round
end

def max_assignment_review_score_per_round(reviewed_object_id)
assignment = Assignment.find(reviewed_object_id)
total_rounds = assignment.rounds_of_reviews
review_max_scores = Array.new(total_rounds)
(0..total_rounds-1).each do |round|
review_max_scores[round] = SummaryHelper::Summary.new.max_score_of_assignment_per_round(assignment, round)
end
review_max_scores
end
</pre>

* Method to create the bar chart showing the review scores per round
<pre>
def generate_score_chart(review_max_score, question_answer)
scores = Array.new
question_answer.each do |reviewer,answer|
scores << ((answer.to_f/review_max_score.to_f)*100).round(2)
end
labels = (1..scores.length).to_a
data = {
labels: labels,
datasets: [
{
label: "score%",
backgroundColor: "rgba(255,99,132,0.8)",
borderWidth: 1,
data: scores,
hoverBackgroundColor: "orange",
yAxisID: "bar-y-axis1"
}
]
}
options = {
legend: {
position: 'top',
labels: {
usePointStyle: true
}
},
width: "125",
height: "75",
scales: {
yAxes: [{
stacked: true,
id: "bar-y-axis1",
barThickness: 10
}, {
display: false,
stacked: true,
id: "bar-y-axis2",
barThickness: 15,
type: 'category',
categoryPercentage: 0.8,
barPercentage: 0.9,
gridLines: {
offsetGridLines: true
}
}],
xAxes: [{
stacked: false,
ticks: {
beginAtZero: true,
stepSize: 10,
max: 100
}
}]
}
}
horizontal_bar_chart data, options
end
</pre>

* Method to get the team members
<pre>
def team_members(team_name, assignment_id)
team_id = Team.select(:id).where(name: team_name)
members = TeamsUser.where(team_id: team_id)
members
end
</pre>

* Method to find the standard deviation
<pre>
def std_of_round(average,question_answer)
accum=0.0
i=0
question_answer.each do |reviewer,answer|
accum+=(answer-average)**2
i+=1
end
if i != 0
accum=Math.sqrt(accum/i)
end
accum.round(2)
end
</pre>

* Method to find the average score per round per team
<pre>
def average_of_round(question_answer)
average=0.0
i=0
question_answer.each do |reviewer,answer|
average+=answer
i+=1
end
if i != 0
average=average/i
end
average.round(2)
end
</pre>

* Method to get the review score for each round of particular team
<pre>
def review_score_for_team(reviewed_object_id, team_name)
question_answers=[]
reviewee_id = Team.select(:id).where(name: team_name, parent_id: reviewed_object_id)
reviewee_id.each do |reviewee|
total_rounds = Assignment.find(reviewed_object_id).rounds_of_reviews
question_answers = Array.new(total_rounds)
(0..total_rounds-1).each do |round|
temp_values = Answer.answers_by_round_for_reviewee(reviewed_object_id, reviewee,round+1)
question_answers[round] = review_score_helper_for_team(temp_values)
end
end
question_answers
end

def review_score_helper_for_team(temp_values)
question_answers={}
temp_values.each do |temp_value|
if question_answers.key?(temp_value[:reviewer_id])
if temp_value[:answer].nil?
question_answers[temp_value[:reviewer_id]] += 0
else
question_answers[temp_value[:reviewer_id]] += temp_value[:answer]
end
else
if temp_value[:answer].nil?
question_answers[temp_value[:reviewer_id]] = 0
else
question_answers[temp_value[:reviewer_id]] = temp_value[:answer]
end
end
end
question_answers
end
</pre>

* Method to create a response map for the reviewers and reviewee as well as the teams and the reviewee_id
<pre>
def review_conflict_response_map(params, _session = nil)
assign_basics(params)
teams = Team.select(:id, :name).where(parent_id: @id).order(:name)
@teams = ({})
@reviewers = ({})
teams.each do |reviewee|
@reviewers[reviewee.name] = reviewers_name_id_by_reviewee_and_assignment(reviewee, @id)
@teams[reviewee.name] = reviewee.id
end
end
</pre>

* Method to get the reviewers of a particular assignment and particular reviewee
<pre>
def reviewers_name_id_by_reviewee_and_assignment(reviewee, id)
temp_reviewers = User.select(" DISTINCT participants.id, users.name")
.joins("JOIN participants ON participants.user_id = users.id")
.joins("JOIN response_maps ON response_maps.reviewer_id = participants.id")
.where("response_maps.reviewee_id = ? and response_maps.reviewed_object_id = ?", reviewee.id, id)
reviewers = ({})
temp_reviewers.each do |reviewer|
reviewers[reviewer[:id].to_s] = reviewer[:name]
end
reviewers
end
</pre>

*'''Feature 2:''' No hardcoded URLs
Hardcoded URLs are be managed by the config file in order to make these links functional on every server running Expertiza and raw URLs will be given some meaningful names.

* Code changed for removing the hardcoded URLs
[[File:Harcord link.PNG|center]]

=Test Plan=

To test code implementation and correctness of the modified feature, we had done the following tests:-

*Run and pass existing RSpec Tests
*UI testing for the new functionality
*Develop New RSpec Tests for the new code

==Rspec Testing==

All the existing related rspec tests have tested and they all passed wit 0 failures.
Majority of the new functionality concerns with the new view,so we have mainly done the UI testing shown below.Some of the Rspec test added are shown below:

*Rspec Tests for Answer Model
<pre>
let(:response_map) { create(:review_response_map, id: 1, reviewed_object_id: 1, reviewer_id: 1) }
it "returns answers by reviewer for reviewee in round from db which is not empty" do
expect(Answer.answers_by_round_for_reviewee(@assignment_id, @reviewee_id,@round)).not_to be_empty
end
</pre>

*Rspec Tests for Report Formatter Helper methods
<pre>
describe 'review_conflict_response_map' do
context 'when type is ReviewConflictResponseMap' do
it 'renders response_report page with corresponding data' do
allow(Team).to receive(:where).with(parent_id: '1').and_return([integer('id')]).ordered
params = {
id: 1,
report: {type: 'ReviewResponseMap'},
user: 'no one'
}
get :response_report, params
expect(response).to render_template(:response_report)
end
end
end

describe "reviewers_name_id_by_reviewee_and_assignment" do
before(:each) do
@assignment_id = 1
@reviewee_id = 1
end
it "returns reviewers name from Answer by reviewee ans assignment id from db which is not empty" do
expect(allow(User).to receive(:where).with(@reviewee_id, @assignment_id).and_return([integer('id')])).not_to be_empty
end
end
</pre>

==UI Testing==

We have created a [https://cl.ly/ab6b9c8bac36 Demo Video] showing the new functionality.

=Files changed in this Project=
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/controllers/response_controller.rb Response Controller]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/report_formatter_helper.rb Report Formatter Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/review_mapping_helper.rb Review Mapping Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/summary_helper.rb Summary Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/mailers/mailer.rb Mailer]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/models/answer.rb Answer model]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/models/response.rb Response Model]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/mailer/notify_grade_conflict_message.html.erb Notify Grade Conflict Message View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_conflict_metric.html.erb Review Conflict Metric View] (New Created)
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_conflict_report.html.erb Review Conflict Report View] (New Created)
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_report.html.erb Review Report View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_searchbox.html.erb Searchbox View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/response_report.html.haml Response Report View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/spec/controllers/reports_controller_spec.rb Reports Controller Spec]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/spec/models/answer_spec.rb Answer model spec]

=References=
* [https://github.com/expertiza/expertiza expertiza]
* [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018-_Project_E1865._Conflict_Notification Previous Work]
* [https://github.com/expertiza/expertiza/pull/1329 Previous Pull Request]
* [https://rubygems.org/gems/standard-deviation/versions/0.1.1 Standard-Deviation gem]
* [https://github.com/jtescher/descriptive-statistics descriptive-statistics gem]

=Team=

[mailto:aarya@ncsu.edu Abhishek Arya]<br>
[mailto:bbdeshpa@ncsu.edu Bhargav Deshpande]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:uschatto@ncsu.edu Udita Chattopadhyay]<br>

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-27T00:38:53Z

Rkaur: /* UI Testing */

This wiki page is for the description of changes made under ECE517 SPRING 2019 PROJECT E1931 CONFLICT NOTIFICATIONS

=Problem Statement=

Current conflict notification sends an email to the instructor whenever two reviews for the same submission differ “significantly” on their scoring (the threshold is specified in the “Notification limit” on the Rubrics tab of assignment creation). Right now an email is sent at any such time one of these conflicts happen, and it links to the review or submission which initiated the notification. It gives the link in raw HTML, but it should give it as a link associated with text, such as the text “this new review” linking to the page that shows the review.The email send to the instructor should also link to page that shows data on each of the reviews done on this team.

=Existing Feature Description=

Currently, this feature works as follows:

[[File:Flowwork.PNG|center]]

Whenever a new review is submitted, it is compared with the average of the previously submitted reviews. If the difference between the new submission and existing average is more than the threshold limit set, then a mail is sent to the instructor.With every review submitted for an assignment of a particular student, the average is updated.The mail sent to the instructor contains the links to the conflicted review and a summary of the reviews.

==Flaws with the existing Implementation==

===No view exists to analyze the conflict report===
Currently whenever the conflict happens,a summary link is sent to the instructor which contains the score of all the reviews but there is no view where the instructor can see all the conflicts and analyze them.

===Incorrect email message links===
The existing email body uses hardcoded URLs mentioned in models/response.rb file in the method notify_instructor_on_difference. Being hardcoded, these links wouldn't work on other servers where Expertiza is running. For Example, if the setup is done on localhost, the links will not be functional.

[[File:EMAIL.png|center]]

*Conflicting Review which triggered the mail
[[File:Score conflicting.png|center]]

=What needs to be done=
The scope of this project is to send an email notification to instructor which will contain links to the conflicting review, summary link, and a link to report which can be used for analyzing. The new report will have the information like the Team(having conflicts), the standard deviation for the team review score and pictorial representation of all the review scores. <br>

The email sent to the instructor should link to a page that shows the data on each of the reviews done on this team:
* The name of the team.
* The members (user-IDs and/or names)
* One line for each review received.In this line would be
# The name of the person (or team) that submitted the review, which would be hyperlinked to the review itself
# The score given for the review
# A bar chart showing pictorial view
* The average score received from reviewers.
* The standard deviation of the score received from reviewers.
* Review scores that are outside some “tolerance” limit should be highlighted, e.g., boldfaced or shown in a different color.Probably the tolerance should be, perhaps, one or two standard deviations from the mean.

=Implementation=

*'''Feature 1:''' New page to show the conflict report
A new view page of the report which will have the following information:-<br>
# The name of the team.
# The members of the team.
# The name of the reviewer which would be hyperlinked to the review itself.
# The score given for the review.
# A bar chart showing pictorial view of all the reviews for a particular team.
# The average score received from reviewers.
# The standard deviation of the score received from reviewers.
# Conflicting reviewers are highlighted.These are those which are two standard deviations away from the mean.

This view have a report of all the teams on one page.This report link is added to the mail that is already being sent whenever some conflict happens.This report page will be also accessible from the reports drop-down in the instructor view.

*A new partial is created for this view -

<pre>
<div style="color:green;text-align:adjust">
Note: <br>
X- axis represents precentage score of the team<br>
Y- axis represents the reviewers <br>
Highlighted reviewers are outside the tolerance limit<br>
</div><br><br>
<table id="myTable" border="1px solid black" margin-top="-9px" width="100%" cellspacing='0' cellpadding='2' border='0'>
<thead>
<tr bgcolor='#CCCCCC'>

<th width="30%" class="sorter-false" border="1px black solid" style="text-align:center">Team <span></span></th>
<th width="70%" class="sorter-false" border="1px black solid" style="text-align:center" >Reviewers/Metrics <span></span></th>
</tr>
</thead>
<tbody>
<% max_scores= max_assignment_review_score_per_round(@id) %>
<%@reviewers.keys.each do |team_name| %>
<tr>
<td style="padding-left: 50px;" >
<% members = team_members(team_name, @id)%>
<strong>Team Name: </strong><%= team_name %><br>
<strong>Team Members: </strong><br>
<% members.each do |member|%>
<%=User.find(member.user_id).fullname %><br>
<% end %>
</td>
<td>
<%= render 'review_conflict_metric' , team_name: team_name , max_scores: max_scores %>
</td>
</tr>
<% end %>
</tbody>
</table>
</pre>

* Partial for the display for reviewers,scores and metrics
<pre>
<table width="100%" border="0" >
<%question_answers = review_score_for_team(@id, team_name) %>
<% i = 1 %>
<% question_answers.each do |question_answer| %>
<% average = average_of_round(question_answer) %>
<% std = std_of_round(average,question_answer) %>
<% upper_tolerance_limit = (average+(2*std)).round(2) %>
<% lower_tolerance_limit = (average-(2*std)).round(2) %>
<tr>
<td width="30%">
<div>
<strong>Reviewers with scores</strong><br>
<ol>
<% question_answer.each do |reviewer, answer| %>
<% if answer > upper_tolerance_limit or answer < lower_tolerance_limit %>
<li><% response_id = response_id_review_conflict(reviewer, @teams[team_name], @id) %>
<mark style="background-color:#FFFF00">
<%= link_to "("+@reviewers[team_name.to_s][reviewer.to_s].to_s+")", :controller => 'popup', :action => 'team_users_popup', :id => @teams[team_name], :id2 => response_id, :assignment_id=>@id %>
<%= list_review_submissions(reviewer, @teams[team_name], response_id) %>
<%= ": " + answer.to_s %></mark>
</li>
<% else %>
<li><% response_id = response_id_review_conflict(reviewer, @teams[team_name], @id) %>
<%= link_to "("+@reviewers[team_name.to_s][reviewer.to_s].to_s+")", :controller => 'popup', :action => 'team_users_popup', :id => @teams[team_name], :id2 => response_id, :assignment_id=>@id %>
<%= list_review_submissions(reviewer, @teams[team_name], response_id) %>
<%= ": " + answer.to_s %>
</li>
<% end %>
<% end %>
</ol>
</div>
<strong>Max Review Score: </strong><%=max_scores[i-1]%><br>
<strong>Average Score: </strong><%= average.to_s %><br>
<strong>Standard Deviation: </strong><%= std.to_s %><br><br>
</td>
<td>
<div>
<strong><u><%= "Round: " + i.to_s %></u></strong>
<%= generate_score_chart(max_scores[i-1], question_answer) %>
</div>
</td>
</tr>
<% i+=1 %>
<% end %>
</table>
</pre>

*Sample view of the new report -
[[File: Oodd report.PNG|center]]
<br>

* Method to get all the answers(scores) for a reviewee on an assignment for a particular round
<pre>
def self.answers_by_round_for_reviewee(assignment_id, reviewee_id,round)
#get all answers for a reviewee on an assignment for a particular round
question_answers = Answer.select("answers.answer, response_maps.reviewer_id")
.joins("join responses on responses.id = answers.response_id")
.joins("join response_maps on responses.map_id = response_maps.id")
.where("response_maps.reviewed_object_id = ? and
response_maps.reviewee_id = ? and
responses.round = ?", assignment_id, reviewee_id,round)
question_answers
end
</pre>

* Method to get the maximum score for an assignment and for a particular round
<pre>
def max_score_of_assignment_per_round(assignment,round)
rubric = get_questions_by_assignment(assignment)
rubric_questions_used = rubric[assignment.varying_rubrics_by_round? ? round : 0]
assignment_max_score_per_round = 0
rubric_questions_used.each do |q|
next if q.type.eql?("SectionHeader")
q_max_score = get_max_score_for_question(q)
assignment_max_score_per_round += q_max_score
end
assignment_max_score_per_round
end

def max_assignment_review_score_per_round(reviewed_object_id)
assignment = Assignment.find(reviewed_object_id)
total_rounds = assignment.rounds_of_reviews
review_max_scores = Array.new(total_rounds)
(0..total_rounds-1).each do |round|
review_max_scores[round] = SummaryHelper::Summary.new.max_score_of_assignment_per_round(assignment, round)
end
review_max_scores
end
</pre>

* Method to create the bar chart showing the review scores per round
<pre>
def generate_score_chart(review_max_score, question_answer)
scores = Array.new
question_answer.each do |reviewer,answer|
scores << ((answer.to_f/review_max_score.to_f)*100).round(2)
end
labels = (1..scores.length).to_a
data = {
labels: labels,
datasets: [
{
label: "score%",
backgroundColor: "rgba(255,99,132,0.8)",
borderWidth: 1,
data: scores,
hoverBackgroundColor: "orange",
yAxisID: "bar-y-axis1"
}
]
}
options = {
legend: {
position: 'top',
labels: {
usePointStyle: true
}
},
width: "125",
height: "75",
scales: {
yAxes: [{
stacked: true,
id: "bar-y-axis1",
barThickness: 10
}, {
display: false,
stacked: true,
id: "bar-y-axis2",
barThickness: 15,
type: 'category',
categoryPercentage: 0.8,
barPercentage: 0.9,
gridLines: {
offsetGridLines: true
}
}],
xAxes: [{
stacked: false,
ticks: {
beginAtZero: true,
stepSize: 10,
max: 100
}
}]
}
}
horizontal_bar_chart data, options
end
</pre>

* Method to get the team members
<pre>
def team_members(team_name, assignment_id)
team_id = Team.select(:id).where(name: team_name)
members = TeamsUser.where(team_id: team_id)
members
end
</pre>

* Method to find the standard deviation
<pre>
def std_of_round(average,question_answer)
accum=0.0
i=0
question_answer.each do |reviewer,answer|
accum+=(answer-average)**2
i+=1
end
if i != 0
accum=Math.sqrt(accum/i)
end
accum.round(2)
end
</pre>

* Method to find the average score per round per team
<pre>
def average_of_round(question_answer)
average=0.0
i=0
question_answer.each do |reviewer,answer|
average+=answer
i+=1
end
if i != 0
average=average/i
end
average.round(2)
end
</pre>

* Method to get the review score for each round of particular team
<pre>
def review_score_for_team(reviewed_object_id, team_name)
question_answers=[]
reviewee_id = Team.select(:id).where(name: team_name, parent_id: reviewed_object_id)
reviewee_id.each do |reviewee|
total_rounds = Assignment.find(reviewed_object_id).rounds_of_reviews
question_answers = Array.new(total_rounds)
(0..total_rounds-1).each do |round|
temp_values = Answer.answers_by_round_for_reviewee(reviewed_object_id, reviewee,round+1)
question_answers[round] = review_score_helper_for_team(temp_values)
end
end
question_answers
end

def review_score_helper_for_team(temp_values)
question_answers={}
temp_values.each do |temp_value|
if question_answers.key?(temp_value[:reviewer_id])
if temp_value[:answer].nil?
question_answers[temp_value[:reviewer_id]] += 0
else
question_answers[temp_value[:reviewer_id]] += temp_value[:answer]
end
else
if temp_value[:answer].nil?
question_answers[temp_value[:reviewer_id]] = 0
else
question_answers[temp_value[:reviewer_id]] = temp_value[:answer]
end
end
end
question_answers
end
</pre>

* Method to create a response map for the reviewers and reviewee as well as the teams and the reviewee_id
<pre>
def review_conflict_response_map(params, _session = nil)
assign_basics(params)
teams = Team.select(:id, :name).where(parent_id: @id).order(:name)
@teams = ({})
@reviewers = ({})
teams.each do |reviewee|
@reviewers[reviewee.name] = reviewers_name_id_by_reviewee_and_assignment(reviewee, @id)
@teams[reviewee.name] = reviewee.id
end
end
</pre>

* Method to get the reviewers of a particular assignment and particular reviewee
<pre>
def reviewers_name_id_by_reviewee_and_assignment(reviewee, id)
temp_reviewers = User.select(" DISTINCT participants.id, users.name")
.joins("JOIN participants ON participants.user_id = users.id")
.joins("JOIN response_maps ON response_maps.reviewer_id = participants.id")
.where("response_maps.reviewee_id = ? and response_maps.reviewed_object_id = ?", reviewee.id, id)
reviewers = ({})
temp_reviewers.each do |reviewer|
reviewers[reviewer[:id].to_s] = reviewer[:name]
end
reviewers
end
</pre>

*'''Feature 2:''' No hardcoded URLs
Hardcoded URLs are be managed by the config file in order to make these links functional on every server running Expertiza and raw URLs will be given some meaningful names.

* Code changed for removing the hardcoded URLs
[[File:Harcord link.PNG|center]]

=Test Plan=

To test code implementation and correctness of the modified feature, we had done the following tests:-

*Run and pass existing RSpec Tests
*UI testing for the new functionality
*Develop New RSpec Tests for the new code

==Rspec Testing==

All the existing related rspec tests have tested and they all passed wit 0 failures.
Majority of the new functionality concerns with the new view,so we have mainly done the UI testing shown below.Some of the Rspec test added are shown below:

*Rspec Tests for Answer Model
<pre>
let(:response_map) { create(:review_response_map, id: 1, reviewed_object_id: 1, reviewer_id: 1) }
it "returns answers by reviewer for reviewee in round from db which is not empty" do
expect(Answer.answers_by_round_for_reviewee(@assignment_id, @reviewee_id,@round)).not_to be_empty
end
</pre>

*Rspec Tests for Report Formatter Helper methods
<pre>
describe 'review_conflict_response_map' do
context 'when type is ReviewConflictResponseMap' do
it 'renders response_report page with corresponding data' do
allow(Team).to receive(:where).with(parent_id: '1').and_return([integer('id')]).ordered
params = {
id: 1,
report: {type: 'ReviewResponseMap'},
user: 'no one'
}
get :response_report, params
expect(response).to render_template(:response_report)
end
end
end

describe "reviewers_name_id_by_reviewee_and_assignment" do
before(:each) do
@assignment_id = 1
@reviewee_id = 1
end
it "returns reviewers name from Answer by reviewee ans assignment id from db which is not empty" do
expect(allow(User).to receive(:where).with(@reviewee_id, @assignment_id).and_return([integer('id')])).not_to be_empty
end
end
</pre>

==UI Testing==

We have crated a [https://cl.ly/ab6b9c8bac36 Demo Video] showing the new functionality.

=Files changed in this Project=
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/controllers/response_controller.rb Response Controller]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/report_formatter_helper.rb Report Formatter Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/review_mapping_helper.rb Review Mapping Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/summary_helper.rb Summary Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/mailers/mailer.rb Mailer]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/models/answer.rb Answer model]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/models/response.rb Response Model]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/mailer/notify_grade_conflict_message.html.erb Notify Grade Conflict Message View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_conflict_metric.html.erb Review Conflict Metric View] (New Created)
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_conflict_report.html.erb Review Conflict Report View] (New Created)
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_report.html.erb Review Report View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_searchbox.html.erb Searchbox View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/response_report.html.haml Response Report View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/spec/controllers/reports_controller_spec.rb Reports Controller Spec]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/spec/models/answer_spec.rb Answer model spec]

=References=
* [https://github.com/expertiza/expertiza expertiza]
* [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018-_Project_E1865._Conflict_Notification Previous Work]
* [https://github.com/expertiza/expertiza/pull/1329 Previous Pull Request]
* [https://rubygems.org/gems/standard-deviation/versions/0.1.1 Standard-Deviation gem]
* [https://github.com/jtescher/descriptive-statistics descriptive-statistics gem]

=Team=

[mailto:aarya@ncsu.edu Abhishek Arya]<br>
[mailto:bbdeshpa@ncsu.edu Bhargav Deshpande]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:uschatto@ncsu.edu Udita Chattopadhyay]<br>

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-27T00:35:41Z

Rkaur:

This wiki page is for the description of changes made under ECE517 SPRING 2019 PROJECT E1931 CONFLICT NOTIFICATIONS

=Problem Statement=

Current conflict notification sends an email to the instructor whenever two reviews for the same submission differ “significantly” on their scoring (the threshold is specified in the “Notification limit” on the Rubrics tab of assignment creation). Right now an email is sent at any such time one of these conflicts happen, and it links to the review or submission which initiated the notification. It gives the link in raw HTML, but it should give it as a link associated with text, such as the text “this new review” linking to the page that shows the review.The email send to the instructor should also link to page that shows data on each of the reviews done on this team.

=Existing Feature Description=

Currently, this feature works as follows:

[[File:Flowwork.PNG|center]]

Whenever a new review is submitted, it is compared with the average of the previously submitted reviews. If the difference between the new submission and existing average is more than the threshold limit set, then a mail is sent to the instructor.With every review submitted for an assignment of a particular student, the average is updated.The mail sent to the instructor contains the links to the conflicted review and a summary of the reviews.

==Flaws with the existing Implementation==

===No view exists to analyze the conflict report===
Currently whenever the conflict happens,a summary link is sent to the instructor which contains the score of all the reviews but there is no view where the instructor can see all the conflicts and analyze them.

===Incorrect email message links===
The existing email body uses hardcoded URLs mentioned in models/response.rb file in the method notify_instructor_on_difference. Being hardcoded, these links wouldn't work on other servers where Expertiza is running. For Example, if the setup is done on localhost, the links will not be functional.

[[File:EMAIL.png|center]]

*Conflicting Review which triggered the mail
[[File:Score conflicting.png|center]]

=What needs to be done=
The scope of this project is to send an email notification to instructor which will contain links to the conflicting review, summary link, and a link to report which can be used for analyzing. The new report will have the information like the Team(having conflicts), the standard deviation for the team review score and pictorial representation of all the review scores. <br>

The email sent to the instructor should link to a page that shows the data on each of the reviews done on this team:
* The name of the team.
* The members (user-IDs and/or names)
* One line for each review received.In this line would be
# The name of the person (or team) that submitted the review, which would be hyperlinked to the review itself
# The score given for the review
# A bar chart showing pictorial view
* The average score received from reviewers.
* The standard deviation of the score received from reviewers.
* Review scores that are outside some “tolerance” limit should be highlighted, e.g., boldfaced or shown in a different color.Probably the tolerance should be, perhaps, one or two standard deviations from the mean.

=Implementation=

*'''Feature 1:''' New page to show the conflict report
A new view page of the report which will have the following information:-<br>
# The name of the team.
# The members of the team.
# The name of the reviewer which would be hyperlinked to the review itself.
# The score given for the review.
# A bar chart showing pictorial view of all the reviews for a particular team.
# The average score received from reviewers.
# The standard deviation of the score received from reviewers.
# Conflicting reviewers are highlighted.These are those which are two standard deviations away from the mean.

This view have a report of all the teams on one page.This report link is added to the mail that is already being sent whenever some conflict happens.This report page will be also accessible from the reports drop-down in the instructor view.

*A new partial is created for this view -

<pre>
<div style="color:green;text-align:adjust">
Note: <br>
X- axis represents precentage score of the team<br>
Y- axis represents the reviewers <br>
Highlighted reviewers are outside the tolerance limit<br>
</div><br><br>
<table id="myTable" border="1px solid black" margin-top="-9px" width="100%" cellspacing='0' cellpadding='2' border='0'>
<thead>
<tr bgcolor='#CCCCCC'>

<th width="30%" class="sorter-false" border="1px black solid" style="text-align:center">Team <span></span></th>
<th width="70%" class="sorter-false" border="1px black solid" style="text-align:center" >Reviewers/Metrics <span></span></th>
</tr>
</thead>
<tbody>
<% max_scores= max_assignment_review_score_per_round(@id) %>
<%@reviewers.keys.each do |team_name| %>
<tr>
<td style="padding-left: 50px;" >
<% members = team_members(team_name, @id)%>
<strong>Team Name: </strong><%= team_name %><br>
<strong>Team Members: </strong><br>
<% members.each do |member|%>
<%=User.find(member.user_id).fullname %><br>
<% end %>
</td>
<td>
<%= render 'review_conflict_metric' , team_name: team_name , max_scores: max_scores %>
</td>
</tr>
<% end %>
</tbody>
</table>
</pre>

* Partial for the display for reviewers,scores and metrics
<pre>
<table width="100%" border="0" >
<%question_answers = review_score_for_team(@id, team_name) %>
<% i = 1 %>
<% question_answers.each do |question_answer| %>
<% average = average_of_round(question_answer) %>
<% std = std_of_round(average,question_answer) %>
<% upper_tolerance_limit = (average+(2*std)).round(2) %>
<% lower_tolerance_limit = (average-(2*std)).round(2) %>
<tr>
<td width="30%">
<div>
<strong>Reviewers with scores</strong><br>
<ol>
<% question_answer.each do |reviewer, answer| %>
<% if answer > upper_tolerance_limit or answer < lower_tolerance_limit %>
<li><% response_id = response_id_review_conflict(reviewer, @teams[team_name], @id) %>
<mark style="background-color:#FFFF00">
<%= link_to "("+@reviewers[team_name.to_s][reviewer.to_s].to_s+")", :controller => 'popup', :action => 'team_users_popup', :id => @teams[team_name], :id2 => response_id, :assignment_id=>@id %>
<%= list_review_submissions(reviewer, @teams[team_name], response_id) %>
<%= ": " + answer.to_s %></mark>
</li>
<% else %>
<li><% response_id = response_id_review_conflict(reviewer, @teams[team_name], @id) %>
<%= link_to "("+@reviewers[team_name.to_s][reviewer.to_s].to_s+")", :controller => 'popup', :action => 'team_users_popup', :id => @teams[team_name], :id2 => response_id, :assignment_id=>@id %>
<%= list_review_submissions(reviewer, @teams[team_name], response_id) %>
<%= ": " + answer.to_s %>
</li>
<% end %>
<% end %>
</ol>
</div>
<strong>Max Review Score: </strong><%=max_scores[i-1]%><br>
<strong>Average Score: </strong><%= average.to_s %><br>
<strong>Standard Deviation: </strong><%= std.to_s %><br><br>
</td>
<td>
<div>
<strong><u><%= "Round: " + i.to_s %></u></strong>
<%= generate_score_chart(max_scores[i-1], question_answer) %>
</div>
</td>
</tr>
<% i+=1 %>
<% end %>
</table>
</pre>

*Sample view of the new report -
[[File: Oodd report.PNG|center]]
<br>

* Method to get all the answers(scores) for a reviewee on an assignment for a particular round
<pre>
def self.answers_by_round_for_reviewee(assignment_id, reviewee_id,round)
#get all answers for a reviewee on an assignment for a particular round
question_answers = Answer.select("answers.answer, response_maps.reviewer_id")
.joins("join responses on responses.id = answers.response_id")
.joins("join response_maps on responses.map_id = response_maps.id")
.where("response_maps.reviewed_object_id = ? and
response_maps.reviewee_id = ? and
responses.round = ?", assignment_id, reviewee_id,round)
question_answers
end
</pre>

* Method to get the maximum score for an assignment and for a particular round
<pre>
def max_score_of_assignment_per_round(assignment,round)
rubric = get_questions_by_assignment(assignment)
rubric_questions_used = rubric[assignment.varying_rubrics_by_round? ? round : 0]
assignment_max_score_per_round = 0
rubric_questions_used.each do |q|
next if q.type.eql?("SectionHeader")
q_max_score = get_max_score_for_question(q)
assignment_max_score_per_round += q_max_score
end
assignment_max_score_per_round
end

def max_assignment_review_score_per_round(reviewed_object_id)
assignment = Assignment.find(reviewed_object_id)
total_rounds = assignment.rounds_of_reviews
review_max_scores = Array.new(total_rounds)
(0..total_rounds-1).each do |round|
review_max_scores[round] = SummaryHelper::Summary.new.max_score_of_assignment_per_round(assignment, round)
end
review_max_scores
end
</pre>

* Method to create the bar chart showing the review scores per round
<pre>
def generate_score_chart(review_max_score, question_answer)
scores = Array.new
question_answer.each do |reviewer,answer|
scores << ((answer.to_f/review_max_score.to_f)*100).round(2)
end
labels = (1..scores.length).to_a
data = {
labels: labels,
datasets: [
{
label: "score%",
backgroundColor: "rgba(255,99,132,0.8)",
borderWidth: 1,
data: scores,
hoverBackgroundColor: "orange",
yAxisID: "bar-y-axis1"
}
]
}
options = {
legend: {
position: 'top',
labels: {
usePointStyle: true
}
},
width: "125",
height: "75",
scales: {
yAxes: [{
stacked: true,
id: "bar-y-axis1",
barThickness: 10
}, {
display: false,
stacked: true,
id: "bar-y-axis2",
barThickness: 15,
type: 'category',
categoryPercentage: 0.8,
barPercentage: 0.9,
gridLines: {
offsetGridLines: true
}
}],
xAxes: [{
stacked: false,
ticks: {
beginAtZero: true,
stepSize: 10,
max: 100
}
}]
}
}
horizontal_bar_chart data, options
end
</pre>

* Method to get the team members
<pre>
def team_members(team_name, assignment_id)
team_id = Team.select(:id).where(name: team_name)
members = TeamsUser.where(team_id: team_id)
members
end
</pre>

* Method to find the standard deviation
<pre>
def std_of_round(average,question_answer)
accum=0.0
i=0
question_answer.each do |reviewer,answer|
accum+=(answer-average)**2
i+=1
end
if i != 0
accum=Math.sqrt(accum/i)
end
accum.round(2)
end
</pre>

* Method to find the average score per round per team
<pre>
def average_of_round(question_answer)
average=0.0
i=0
question_answer.each do |reviewer,answer|
average+=answer
i+=1
end
if i != 0
average=average/i
end
average.round(2)
end
</pre>

* Method to get the review score for each round of particular team
<pre>
def review_score_for_team(reviewed_object_id, team_name)
question_answers=[]
reviewee_id = Team.select(:id).where(name: team_name, parent_id: reviewed_object_id)
reviewee_id.each do |reviewee|
total_rounds = Assignment.find(reviewed_object_id).rounds_of_reviews
question_answers = Array.new(total_rounds)
(0..total_rounds-1).each do |round|
temp_values = Answer.answers_by_round_for_reviewee(reviewed_object_id, reviewee,round+1)
question_answers[round] = review_score_helper_for_team(temp_values)
end
end
question_answers
end

def review_score_helper_for_team(temp_values)
question_answers={}
temp_values.each do |temp_value|
if question_answers.key?(temp_value[:reviewer_id])
if temp_value[:answer].nil?
question_answers[temp_value[:reviewer_id]] += 0
else
question_answers[temp_value[:reviewer_id]] += temp_value[:answer]
end
else
if temp_value[:answer].nil?
question_answers[temp_value[:reviewer_id]] = 0
else
question_answers[temp_value[:reviewer_id]] = temp_value[:answer]
end
end
end
question_answers
end
</pre>

* Method to create a response map for the reviewers and reviewee as well as the teams and the reviewee_id
<pre>
def review_conflict_response_map(params, _session = nil)
assign_basics(params)
teams = Team.select(:id, :name).where(parent_id: @id).order(:name)
@teams = ({})
@reviewers = ({})
teams.each do |reviewee|
@reviewers[reviewee.name] = reviewers_name_id_by_reviewee_and_assignment(reviewee, @id)
@teams[reviewee.name] = reviewee.id
end
end
</pre>

* Method to get the reviewers of a particular assignment and particular reviewee
<pre>
def reviewers_name_id_by_reviewee_and_assignment(reviewee, id)
temp_reviewers = User.select(" DISTINCT participants.id, users.name")
.joins("JOIN participants ON participants.user_id = users.id")
.joins("JOIN response_maps ON response_maps.reviewer_id = participants.id")
.where("response_maps.reviewee_id = ? and response_maps.reviewed_object_id = ?", reviewee.id, id)
reviewers = ({})
temp_reviewers.each do |reviewer|
reviewers[reviewer[:id].to_s] = reviewer[:name]
end
reviewers
end
</pre>

*'''Feature 2:''' No hardcoded URLs
Hardcoded URLs are be managed by the config file in order to make these links functional on every server running Expertiza and raw URLs will be given some meaningful names.

* Code changed for removing the hardcoded URLs
[[File:Harcord link.PNG|center]]

=Test Plan=

To test code implementation and correctness of the modified feature, we had done the following tests:-

*Run and pass existing RSpec Tests
*UI testing for the new functionality
*Develop New RSpec Tests for the new code

==Rspec Testing==

All the existing related rspec tests have tested and they all passed wit 0 failures.
Majority of the new functionality concerns with the new view,so we have mainly done the UI testing shown below.Some of the Rspec test added are shown below:

*Rspec Tests for Answer Model
<pre>
let(:response_map) { create(:review_response_map, id: 1, reviewed_object_id: 1, reviewer_id: 1) }
it "returns answers by reviewer for reviewee in round from db which is not empty" do
expect(Answer.answers_by_round_for_reviewee(@assignment_id, @reviewee_id,@round)).not_to be_empty
end
</pre>

*Rspec Tests for Report Formatter Helper methods
<pre>
describe 'review_conflict_response_map' do
context 'when type is ReviewConflictResponseMap' do
it 'renders response_report page with corresponding data' do
allow(Team).to receive(:where).with(parent_id: '1').and_return([integer('id')]).ordered
params = {
id: 1,
report: {type: 'ReviewResponseMap'},
user: 'no one'
}
get :response_report, params
expect(response).to render_template(:response_report)
end
end
end

describe "reviewers_name_id_by_reviewee_and_assignment" do
before(:each) do
@assignment_id = 1
@reviewee_id = 1
end
it "returns reviewers name from Answer by reviewee ans assignment id from db which is not empty" do
expect(allow(User).to receive(:where).with(@reviewee_id, @assignment_id).and_return([integer('id')])).not_to be_empty
end
end
</pre>

==UI Testing==

We have crated a video showing the new functionality.

=Files changed in this Project=
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/controllers/response_controller.rb Response Controller]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/report_formatter_helper.rb Report Formatter Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/review_mapping_helper.rb Review Mapping Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/summary_helper.rb Summary Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/mailers/mailer.rb Mailer]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/models/answer.rb Answer model]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/models/response.rb Response Model]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/mailer/notify_grade_conflict_message.html.erb Notify Grade Conflict Message View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_conflict_metric.html.erb Review Conflict Metric View] (New Created)
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_conflict_report.html.erb Review Conflict Report View] (New Created)
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_report.html.erb Review Report View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_searchbox.html.erb Searchbox View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/response_report.html.haml Response Report View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/spec/controllers/reports_controller_spec.rb Reports Controller Spec]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/spec/models/answer_spec.rb Answer model spec]

=References=
* [https://github.com/expertiza/expertiza expertiza]
* [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018-_Project_E1865._Conflict_Notification Previous Work]
* [https://github.com/expertiza/expertiza/pull/1329 Previous Pull Request]
* [https://rubygems.org/gems/standard-deviation/versions/0.1.1 Standard-Deviation gem]
* [https://github.com/jtescher/descriptive-statistics descriptive-statistics gem]

=Team=

[mailto:aarya@ncsu.edu Abhishek Arya]<br>
[mailto:bbdeshpa@ncsu.edu Bhargav Deshpande]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:uschatto@ncsu.edu Udita Chattopadhyay]<br>

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-27T00:35:06Z

Rkaur:

This wiki page is for the description of changes made under ECE517 SPRING 2019 PROJECT E1931 CONFLICT NOTIFICATIONS

=Problem Statement=

Current conflict notification sends an email to the instructor whenever two reviews for the same submission differ “significantly” on their scoring (the threshold is specified in the “Notification limit” on the Rubrics tab of assignment creation). Right now an email is sent at any such time one of these conflicts happen, and it links to the review or submission which initiated the notification. It gives the link in raw HTML, but it should give it as a link associated with text, such as the text “this new review” linking to the page that shows the review.The email send to the instructor should also link to page that shows data on each of the reviews done on this team.

=Existing Feature Description=

Currently, this feature works as follows:

[[File:Flowwork.PNG|center]]

Whenever a new review is submitted, it is compared with the average of the previously submitted reviews. If the difference between the new submission and existing average is more than the threshold limit set, then a mail is sent to the instructor.With every review submitted for an assignment of a particular student, the average is updated.The mail sent to the instructor contains the links to the conflicted review and a summary of the reviews.

==Flaws with the existing Implementation==

===No view exists to analyze the conflict report===
Currently whenever the conflict happens,a summary link is sent to the instructor which contains the score of all the reviews but there is no view where the instructor can see all the conflicts and analyze them.

===Incorrect email message links===
The existing email body uses hardcoded URLs mentioned in models/response.rb file in the method notify_instructor_on_difference. Being hardcoded, these links wouldn't work on other servers where Expertiza is running. For Example, if the setup is done on localhost, the links will not be functional.

[[File:EMAIL.png|center]]

*Conflicting Review which triggered the mail
[[File:Score conflicting.png|center]]

=What needs to be done=
The scope of this project is to send an email notification to instructor which will contain links to the conflicting review, summary link, and a link to report which can be used for analyzing. The new report will have the information like the Team(having conflicts), the standard deviation for the team review score and pictorial representation of all the review scores. <br>

The email sent to the instructor should link to a page that shows the data on each of the reviews done on this team:
* The name of the team.
* The members (user-IDs and/or names)
* One line for each review received.In this line would be
# The name of the person (or team) that submitted the review, which would be hyperlinked to the review itself
# The score given for the review
# A bar chart showing pictorial view
* The average score received from reviewers.
* The standard deviation of the score received from reviewers.
* Review scores that are outside some “tolerance” limit should be highlighted, e.g., boldfaced or shown in a different color.Probably the tolerance should be, perhaps, one or two standard deviations from the mean.

=Implementation=

*'''Feature 1:''' New page to show the conflict report
A new view page of the report which will have the following information:-<br>
# The name of the team.
# The members of the team.
# The name of the reviewer which would be hyperlinked to the review itself.
# The score given for the review.
# A bar chart showing pictorial view of all the reviews for a particular team.
# The average score received from reviewers.
# The standard deviation of the score received from reviewers.
# Conflicting reviewers are highlighted.These are those which are two standard deviations away from the mean.

This view have a report of all the teams on one page.This report link is added to the mail that is already being sent whenever some conflict happens.This report page will be also accessible from the reports drop-down in the instructor view.

*A new partial is created for this view -

<pre>
<div style="color:green;text-align:adjust">
Note: <br>
X- axis represents precentage score of the team<br>
Y- axis represents the reviewers <br>
Highlighted reviewers are outside the tolerance limit<br>
</div><br><br>
<table id="myTable" border="1px solid black" margin-top="-9px" width="100%" cellspacing='0' cellpadding='2' border='0'>
<thead>
<tr bgcolor='#CCCCCC'>

<th width="30%" class="sorter-false" border="1px black solid" style="text-align:center">Team <span></span></th>
<th width="70%" class="sorter-false" border="1px black solid" style="text-align:center" >Reviewers/Metrics <span></span></th>
</tr>
</thead>
<tbody>
<% max_scores= max_assignment_review_score_per_round(@id) %>
<%@reviewers.keys.each do |team_name| %>
<tr>
<td style="padding-left: 50px;" >
<% members = team_members(team_name, @id)%>
<strong>Team Name: </strong><%= team_name %><br>
<strong>Team Members: </strong><br>
<% members.each do |member|%>
<%=User.find(member.user_id).fullname %><br>
<% end %>
</td>
<td>
<%= render 'review_conflict_metric' , team_name: team_name , max_scores: max_scores %>
</td>
</tr>
<% end %>
</tbody>
</table>
</pre>

* Partial for the display for reviewers,scores and metrics
<pre>
<table width="100%" border="0" >
<%question_answers = review_score_for_team(@id, team_name) %>
<% i = 1 %>
<% question_answers.each do |question_answer| %>
<% average = average_of_round(question_answer) %>
<% std = std_of_round(average,question_answer) %>
<% upper_tolerance_limit = (average+(2*std)).round(2) %>
<% lower_tolerance_limit = (average-(2*std)).round(2) %>
<tr>
<td width="30%">
<div>
<strong>Reviewers with scores</strong><br>
<ol>
<% question_answer.each do |reviewer, answer| %>
<% if answer > upper_tolerance_limit or answer < lower_tolerance_limit %>
<li><% response_id = response_id_review_conflict(reviewer, @teams[team_name], @id) %>
<mark style="background-color:#FFFF00">
<%= link_to "("+@reviewers[team_name.to_s][reviewer.to_s].to_s+")", :controller => 'popup', :action => 'team_users_popup', :id => @teams[team_name], :id2 => response_id, :assignment_id=>@id %>
<%= list_review_submissions(reviewer, @teams[team_name], response_id) %>
<%= ": " + answer.to_s %></mark>
</li>
<% else %>
<li><% response_id = response_id_review_conflict(reviewer, @teams[team_name], @id) %>
<%= link_to "("+@reviewers[team_name.to_s][reviewer.to_s].to_s+")", :controller => 'popup', :action => 'team_users_popup', :id => @teams[team_name], :id2 => response_id, :assignment_id=>@id %>
<%= list_review_submissions(reviewer, @teams[team_name], response_id) %>
<%= ": " + answer.to_s %>
</li>
<% end %>
<% end %>
</ol>
</div>
<strong>Max Review Score: </strong><%=max_scores[i-1]%><br>
<strong>Average Score: </strong><%= average.to_s %><br>
<strong>Standard Deviation: </strong><%= std.to_s %><br><br>
</td>
<td>
<div>
<strong><u><%= "Round: " + i.to_s %></u></strong>
<%= generate_score_chart(max_scores[i-1], question_answer) %>
</div>
</td>
</tr>
<% i+=1 %>
<% end %>
</table>
</pre>

*Sample view of the new report -
[[File: Oodd report.PNG|center]]
<br>

* Method to get all the answers(scores) for a reviewee on an assignment for a particular round
<pre>
def self.answers_by_round_for_reviewee(assignment_id, reviewee_id,round)
#get all answers for a reviewee on an assignment for a particular round
question_answers = Answer.select("answers.answer, response_maps.reviewer_id")
.joins("join responses on responses.id = answers.response_id")
.joins("join response_maps on responses.map_id = response_maps.id")
.where("response_maps.reviewed_object_id = ? and
response_maps.reviewee_id = ? and
responses.round = ?", assignment_id, reviewee_id,round)
question_answers
end
</pre>

* Method to get the maximum score for an assignment and for a particular round
<pre>
def max_score_of_assignment_per_round(assignment,round)
rubric = get_questions_by_assignment(assignment)
rubric_questions_used = rubric[assignment.varying_rubrics_by_round? ? round : 0]
assignment_max_score_per_round = 0
rubric_questions_used.each do |q|
next if q.type.eql?("SectionHeader")
q_max_score = get_max_score_for_question(q)
assignment_max_score_per_round += q_max_score
end
assignment_max_score_per_round
end

def max_assignment_review_score_per_round(reviewed_object_id)
assignment = Assignment.find(reviewed_object_id)
total_rounds = assignment.rounds_of_reviews
review_max_scores = Array.new(total_rounds)
(0..total_rounds-1).each do |round|
review_max_scores[round] = SummaryHelper::Summary.new.max_score_of_assignment_per_round(assignment, round)
end
review_max_scores
end
</pre>

* Method to create the bar chart showing the review scores per round
<pre>
def generate_score_chart(review_max_score, question_answer)
scores = Array.new
question_answer.each do |reviewer,answer|
scores << ((answer.to_f/review_max_score.to_f)*100).round(2)
end
labels = (1..scores.length).to_a
data = {
labels: labels,
datasets: [
{
label: "score%",
backgroundColor: "rgba(255,99,132,0.8)",
borderWidth: 1,
data: scores,
hoverBackgroundColor: "orange",
yAxisID: "bar-y-axis1"
}
]
}
options = {
legend: {
position: 'top',
labels: {
usePointStyle: true
}
},
width: "125",
height: "75",
scales: {
yAxes: [{
stacked: true,
id: "bar-y-axis1",
barThickness: 10
}, {
display: false,
stacked: true,
id: "bar-y-axis2",
barThickness: 15,
type: 'category',
categoryPercentage: 0.8,
barPercentage: 0.9,
gridLines: {
offsetGridLines: true
}
}],
xAxes: [{
stacked: false,
ticks: {
beginAtZero: true,
stepSize: 10,
max: 100
}
}]
}
}
horizontal_bar_chart data, options
end
</pre>

* Method to get the team members
<pre>
def team_members(team_name, assignment_id)
team_id = Team.select(:id).where(name: team_name)
members = TeamsUser.where(team_id: team_id)
members
end
</pre>

* Method to find the standard deviation
<pre>
def std_of_round(average,question_answer)
accum=0.0
i=0
question_answer.each do |reviewer,answer|
accum+=(answer-average)**2
i+=1
end
if i != 0
accum=Math.sqrt(accum/i)
end
accum.round(2)
end
</pre>

* Method to find the average score per round per team
<pre>
def average_of_round(question_answer)
average=0.0
i=0
question_answer.each do |reviewer,answer|
average+=answer
i+=1
end
if i != 0
average=average/i
end
average.round(2)
end
</pre>

* Method to get the review score for each round of particular team
<pre>
def review_score_for_team(reviewed_object_id, team_name)
question_answers=[]
reviewee_id = Team.select(:id).where(name: team_name, parent_id: reviewed_object_id)
reviewee_id.each do |reviewee|
total_rounds = Assignment.find(reviewed_object_id).rounds_of_reviews
question_answers = Array.new(total_rounds)
(0..total_rounds-1).each do |round|
temp_values = Answer.answers_by_round_for_reviewee(reviewed_object_id, reviewee,round+1)
question_answers[round] = review_score_helper_for_team(temp_values)
end
end
question_answers
end

def review_score_helper_for_team(temp_values)
question_answers={}
temp_values.each do |temp_value|
if question_answers.key?(temp_value[:reviewer_id])
if temp_value[:answer].nil?
question_answers[temp_value[:reviewer_id]] += 0
else
question_answers[temp_value[:reviewer_id]] += temp_value[:answer]
end
else
if temp_value[:answer].nil?
question_answers[temp_value[:reviewer_id]] = 0
else
question_answers[temp_value[:reviewer_id]] = temp_value[:answer]
end
end
end
question_answers
end
</pre>

* Method to create a response map for the reviewers and reviewee as well as the teams and the reviewee_id
<pre>
def review_conflict_response_map(params, _session = nil)
assign_basics(params)
teams = Team.select(:id, :name).where(parent_id: @id).order(:name)
@teams = ({})
@reviewers = ({})
teams.each do |reviewee|
@reviewers[reviewee.name] = reviewers_name_id_by_reviewee_and_assignment(reviewee, @id)
@teams[reviewee.name] = reviewee.id
end
end
</pre>

* Method to get the reviewers of a particular assignment and particular reviewee
<pre>
def reviewers_name_id_by_reviewee_and_assignment(reviewee, id)
temp_reviewers = User.select(" DISTINCT participants.id, users.name")
.joins("JOIN participants ON participants.user_id = users.id")
.joins("JOIN response_maps ON response_maps.reviewer_id = participants.id")
.where("response_maps.reviewee_id = ? and response_maps.reviewed_object_id = ?", reviewee.id, id)
reviewers = ({})
temp_reviewers.each do |reviewer|
reviewers[reviewer[:id].to_s] = reviewer[:name]
end
reviewers
end
</pre>

*'''Feature 2:''' No hardcoded URLs
Hardcoded URLs are be managed by the config file in order to make these links functional on every server running Expertiza and raw URLs will be given some meaningful names.

* Code changed for removing the hardcoded URLs
[[File:Harcord link.PNG|center]]

=Test Plan=

To test code implementation and correctness of the modified feature, we had done the following tests:-

*Run and pass existing RSpec Tests
*UI testing for the new functionality
*Develop New RSpec Tests for the new code

==Rspec Testing==

All the existing related rspec tests have tested and they all passed wit 0 failures.
Majority of the new functionality concerns with the new view,so we have mainly done the UI testing shown below.Some of the Rspec test added are shown below:

*Rspec Tests for Answer Model
<pre>
let(:response_map) { create(:review_response_map, id: 1, reviewed_object_id: 1, reviewer_id: 1) }
it "returns answers by reviewer for reviewee in round from db which is not empty" do
expect(Answer.answers_by_round_for_reviewee(@assignment_id, @reviewee_id,@round)).not_to be_empty
end
</pre>

*Rspec Tests for Report Formatter Helper methods
<pre>
describe 'review_conflict_response_map' do
context 'when type is ReviewConflictResponseMap' do
it 'renders response_report page with corresponding data' do
allow(Team).to receive(:where).with(parent_id: '1').and_return([integer('id')]).ordered
params = {
id: 1,
report: {type: 'ReviewResponseMap'},
user: 'no one'
}
get :response_report, params
expect(response).to render_template(:response_report)
end
end
end

describe "reviewers_name_id_by_reviewee_and_assignment" do
before(:each) do
@assignment_id = 1
@reviewee_id = 1
end
it "returns reviewers name from Answer by reviewee ans assignment id from db which is not empty" do
expect(allow(User).to receive(:where).with(@reviewee_id, @assignment_id).and_return([integer('id')])).not_to be_empty
end
end
</pre>

* Method to

==UI Testing==

We have crated a video showing the new functionality.

=Files changed in this Project=
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/controllers/response_controller.rb Response Controller]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/report_formatter_helper.rb Report Formatter Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/review_mapping_helper.rb Review Mapping Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/summary_helper.rb Summary Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/mailers/mailer.rb Mailer]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/models/answer.rb Answer model]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/models/response.rb Response Model]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/mailer/notify_grade_conflict_message.html.erb Notify Grade Conflict Message View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_conflict_metric.html.erb Review Conflict Metric View] (New Created)
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_conflict_report.html.erb Review Conflict Report View] (New Created)
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_report.html.erb Review Report View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_searchbox.html.erb Searchbox View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/response_report.html.haml Response Report View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/spec/controllers/reports_controller_spec.rb Reports Controller Spec]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/spec/models/answer_spec.rb Answer model spec]

=References=
* [https://github.com/expertiza/expertiza expertiza]
* [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018-_Project_E1865._Conflict_Notification Previous Work]
* [https://github.com/expertiza/expertiza/pull/1329 Previous Pull Request]
* [https://rubygems.org/gems/standard-deviation/versions/0.1.1 Standard-Deviation gem]
* [https://github.com/jtescher/descriptive-statistics descriptive-statistics gem]

=Team=

[mailto:aarya@ncsu.edu Abhishek Arya]<br>
[mailto:bbdeshpa@ncsu.edu Bhargav Deshpande]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:uschatto@ncsu.edu Udita Chattopadhyay]<br>

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-27T00:25:17Z

Rkaur:

This wiki page is for the description of changes made under ECE517 SPRING 2019 PROJECT E1931 CONFLICT NOTIFICATIONS

=Problem Statement=

Current conflict notification sends an email to the instructor whenever two reviews for the same submission differ “significantly” on their scoring (the threshold is specified in the “Notification limit” on the Rubrics tab of assignment creation). Right now an email is sent at any such time one of these conflicts happen, and it links to the review or submission which initiated the notification. It gives the link in raw HTML, but it should give it as a link associated with text, such as the text “this new review” linking to the page that shows the review.The email send to the instructor should also link to page that shows data on each of the reviews done on this team.

=Existing Feature Description=

Currently, this feature works as follows:

[[File:Flowwork.PNG|center]]

Whenever a new review is submitted, it is compared with the average of the previously submitted reviews. If the difference between the new submission and existing average is more than the threshold limit set, then a mail is sent to the instructor.With every review submitted for an assignment of a particular student, the average is updated.The mail sent to the instructor contains the links to the conflicted review and a summary of the reviews.

==Flaws with the existing Implementation==

===No view exists to analyze the conflict report===
Currently whenever the conflict happens,a summary link is sent to the instructor which contains the score of all the reviews but there is no view where the instructor can see all the conflicts and analyze them.

===Incorrect email message links===
The existing email body uses hardcoded URLs mentioned in models/response.rb file in the method notify_instructor_on_difference. Being hardcoded, these links wouldn't work on other servers where Expertiza is running. For Example, if the setup is done on localhost, the links will not be functional.

[[File:EMAIL.png|center]]

*Conflicting Review which triggered the mail
[[File:Score conflicting.png|center]]

=What needs to be done=
The scope of this project is to send an email notification to instructor which will contain links to the conflicting review, summary link, and a link to report which can be used for analyzing. The new report will have the information like the Team(having conflicts), the standard deviation for the team review score and pictorial representation of all the review scores. <br>

The email sent to the instructor should link to a page that shows the data on each of the reviews done on this team:
* The name of the team.
* The members (user-IDs and/or names)
* One line for each review received.In this line would be
# The name of the person (or team) that submitted the review, which would be hyperlinked to the review itself
# The score given for the review
# A bar chart showing pictorial view
* The average score received from reviewers.
* The standard deviation of the score received from reviewers.
* Review scores that are outside some “tolerance” limit should be highlighted, e.g., boldfaced or shown in a different color.Probably the tolerance should be, perhaps, one or two standard deviations from the mean.

=Implementation=

*'''Feature 1:''' New page to show the conflict report
A new view page of the report which will have the following information:-<br>
# The name of the team.
# The members of the team.
# The name of the reviewer which would be hyperlinked to the review itself.
# The score given for the review.
# A bar chart showing pictorial view of all the reviews for a particular team.
# The average score received from reviewers.
# The standard deviation of the score received from reviewers.
# Conflicting reviewers are highlighted.These are those which are two standard deviations away from the mean.

This view have a report of all the teams on one page.This report link is added to the mail that is already being sent whenever some conflict happens.This report page will be also accessible from the reports drop-down in the instructor view.

*A new partial is created for this view -

<pre>
<div style="color:green;text-align:adjust">
Note: <br>
X- axis represents precentage score of the team<br>
Y- axis represents the reviewers <br>
Highlighted reviewers are outside the tolerance limit<br>
</div><br><br>
<table id="myTable" border="1px solid black" margin-top="-9px" width="100%" cellspacing='0' cellpadding='2' border='0'>
<thead>
<tr bgcolor='#CCCCCC'>

<th width="30%" class="sorter-false" border="1px black solid" style="text-align:center">Team <span></span></th>
<th width="70%" class="sorter-false" border="1px black solid" style="text-align:center" >Reviewers/Metrics <span></span></th>
</tr>
</thead>
<tbody>
<% max_scores= max_assignment_review_score_per_round(@id) %>
<%@reviewers.keys.each do |team_name| %>
<tr>
<td style="padding-left: 50px;" >
<% members = team_members(team_name, @id)%>
<strong>Team Name: </strong><%= team_name %><br>
<strong>Team Members: </strong><br>
<% members.each do |member|%>
<%=User.find(member.user_id).fullname %><br>
<% end %>
</td>
<td>
<%= render 'review_conflict_metric' , team_name: team_name , max_scores: max_scores %>
</td>
</tr>
<% end %>
</tbody>
</table>
</pre>

* Partial for the display for reviewers,scores and metrics
<pre>
<table width="100%" border="0" >
<%question_answers = review_score_for_team(@id, team_name) %>
<% i = 1 %>
<% question_answers.each do |question_answer| %>
<% average = average_of_round(question_answer) %>
<% std = std_of_round(average,question_answer) %>
<% upper_tolerance_limit = (average+(2*std)).round(2) %>
<% lower_tolerance_limit = (average-(2*std)).round(2) %>
<tr>
<td width="30%">
<div>
<strong>Reviewers with scores</strong><br>
<ol>
<% question_answer.each do |reviewer, answer| %>
<% if answer > upper_tolerance_limit or answer < lower_tolerance_limit %>
<li><% response_id = response_id_review_conflict(reviewer, @teams[team_name], @id) %>
<mark style="background-color:#FFFF00">
<%= link_to "("+@reviewers[team_name.to_s][reviewer.to_s].to_s+")", :controller => 'popup', :action => 'team_users_popup', :id => @teams[team_name], :id2 => response_id, :assignment_id=>@id %>
<%= list_review_submissions(reviewer, @teams[team_name], response_id) %>
<%= ": " + answer.to_s %></mark>
</li>
<% else %>
<li><% response_id = response_id_review_conflict(reviewer, @teams[team_name], @id) %>
<%= link_to "("+@reviewers[team_name.to_s][reviewer.to_s].to_s+")", :controller => 'popup', :action => 'team_users_popup', :id => @teams[team_name], :id2 => response_id, :assignment_id=>@id %>
<%= list_review_submissions(reviewer, @teams[team_name], response_id) %>
<%= ": " + answer.to_s %>
</li>
<% end %>
<% end %>
</ol>
</div>
<strong>Max Review Score: </strong><%=max_scores[i-1]%><br>
<strong>Average Score: </strong><%= average.to_s %><br>
<strong>Standard Deviation: </strong><%= std.to_s %><br><br>
</td>
<td>
<div>
<strong><u><%= "Round: " + i.to_s %></u></strong>
<%= generate_score_chart(max_scores[i-1], question_answer) %>
</div>
</td>
</tr>
<% i+=1 %>
<% end %>
</table>
</pre>

*Sample view of the new report -
[[File: Oodd report.PNG|center]]
<br>

* Method to get all the answers(scores) for a reviewee on an assignment for a particular round
<pre>
def self.answers_by_round_for_reviewee(assignment_id, reviewee_id,round)
#get all answers for a reviewee on an assignment for a particular round
question_answers = Answer.select("answers.answer, response_maps.reviewer_id")
.joins("join responses on responses.id = answers.response_id")
.joins("join response_maps on responses.map_id = response_maps.id")
.where("response_maps.reviewed_object_id = ? and
response_maps.reviewee_id = ? and
responses.round = ?", assignment_id, reviewee_id,round)
question_answers
end
</pre>

* Method to get the maximum score for an assignment and for a particular round
<pre>
def max_score_of_assignment_per_round(assignment,round)
rubric = get_questions_by_assignment(assignment)
rubric_questions_used = rubric[assignment.varying_rubrics_by_round? ? round : 0]
assignment_max_score_per_round = 0
rubric_questions_used.each do |q|
next if q.type.eql?("SectionHeader")
q_max_score = get_max_score_for_question(q)
assignment_max_score_per_round += q_max_score
end
assignment_max_score_per_round
end
</pre>

* Method to create the bar chart showing the review scores per round
<pre>
def generate_score_chart(review_max_score, question_answer)
scores = Array.new
question_answer.each do |reviewer,answer|
scores << ((answer.to_f/review_max_score.to_f)*100).round(2)
end
labels = (1..scores.length).to_a
data = {
labels: labels,
datasets: [
{
label: "score%",
backgroundColor: "rgba(255,99,132,0.8)",
borderWidth: 1,
data: scores,
hoverBackgroundColor: "orange",
yAxisID: "bar-y-axis1"
}
]
}
options = {
legend: {
position: 'top',
labels: {
usePointStyle: true
}
},
width: "125",
height: "75",
scales: {
yAxes: [{
stacked: true,
id: "bar-y-axis1",
barThickness: 10
}, {
display: false,
stacked: true,
id: "bar-y-axis2",
barThickness: 15,
type: 'category',
categoryPercentage: 0.8,
barPercentage: 0.9,
gridLines: {
offsetGridLines: true
}
}],
xAxes: [{
stacked: false,
ticks: {
beginAtZero: true,
stepSize: 10,
max: 100
}
}]
}
}
horizontal_bar_chart data, options
end
</pre>

*'''Feature 2:''' No hardcoded URLs
Hardcoded URLs are be managed by the config file in order to make these links functional on every server running Expertiza and raw URLs will be given some meaningful names.

* Code changed for removing the hardcoded URLs
[[File:Harcord link.PNG|center]]

=Test Plan=

To test code implementation and correctness of the modified feature, we had done the following tests:-

*Run and pass existing RSpec Tests
*UI testing for the new functionality
*Develop New RSpec Tests for the new code

==Rspec Testing==

All the existing related rspec tests have tested and they all passed wit 0 failures.
Majority of the new functionality concerns with the new view,so we have mainly done the UI testing shown below.Some of the Rspec test added are shown below:

*Rspec Tests for Answer Model
<pre>
let(:response_map) { create(:review_response_map, id: 1, reviewed_object_id: 1, reviewer_id: 1) }
it "returns answers by reviewer for reviewee in round from db which is not empty" do
expect(Answer.answers_by_round_for_reviewee(@assignment_id, @reviewee_id,@round)).not_to be_empty
end
</pre>

*Rspec Tests for Report Formatter Helper methods
<pre>
describe 'review_conflict_response_map' do
context 'when type is ReviewConflictResponseMap' do
it 'renders response_report page with corresponding data' do
allow(Team).to receive(:where).with(parent_id: '1').and_return([integer('id')]).ordered
params = {
id: 1,
report: {type: 'ReviewResponseMap'},
user: 'no one'
}
get :response_report, params
expect(response).to render_template(:response_report)
end
end
end

describe "reviewers_name_id_by_reviewee_and_assignment" do
before(:each) do
@assignment_id = 1
@reviewee_id = 1
end
it "returns reviewers name from Answer by reviewee ans assignment id from db which is not empty" do
expect(allow(User).to receive(:where).with(@reviewee_id, @assignment_id).and_return([integer('id')])).not_to be_empty
end
end
</pre>

==UI Testing==

We have crated a video showing the new functionality.

=Files changed in this Project=
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/controllers/response_controller.rb Response Controller]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/report_formatter_helper.rb Report Formatter Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/review_mapping_helper.rb Review Mapping Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/helpers/summary_helper.rb Summary Helper]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/mailers/mailer.rb Mailer]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/models/answer.rb Answer model]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/models/response.rb Response Model]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/mailer/notify_grade_conflict_message.html.erb Notify Grade Conflict Message View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_conflict_metric.html.erb Review Conflict Metric View] (New Created)
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_conflict_report.html.erb Review Conflict Report View] (New Created)
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_review_report.html.erb Review Report View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/_searchbox.html.erb Searchbox View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/app/views/reports/response_report.html.haml Response Report View]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/spec/controllers/reports_controller_spec.rb Reports Controller Spec]
* [https://github.com/AryaAbhishek/expertiza/blob/beta/spec/models/answer_spec.rb Answer model spec]

=References=
* [https://github.com/expertiza/expertiza expertiza]
* [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018-_Project_E1865._Conflict_Notification Previous Work]
* [https://github.com/expertiza/expertiza/pull/1329 Previous Pull Request]
* [https://rubygems.org/gems/standard-deviation/versions/0.1.1 Standard-Deviation gem]
* [https://github.com/jtescher/descriptive-statistics descriptive-statistics gem]

=Team=

[mailto:aarya@ncsu.edu Abhishek Arya]<br>
[mailto:bbdeshpa@ncsu.edu Bhargav Deshpande]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:uschatto@ncsu.edu Udita Chattopadhyay]<br>

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-27T00:13:43Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-27T00:00:50Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-26T23:44:06Z

Rkaur:

File:Harcord link.PNG

2019-04-26T23:43:28Z

Rkaur:

File:Oodd report.PNG

2019-04-26T23:33:56Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-26T23:31:16Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-26T23:08:47Z

Rkaur: /* Implementation */

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-26T23:00:22Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-26T22:56:06Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-26T21:53:46Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-26T21:51:42Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-26T21:14:12Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-12T23:23:11Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-12T22:43:40Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-12T22:42:37Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-06T22:00:18Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-06T21:48:52Z

Rkaur:

File:EMAIL.png

2019-04-06T21:48:19Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-06T21:46:05Z

Rkaur:

File:Score conflicting.png

2019-04-06T21:44:36Z

Rkaur:

File:SUMMARY.png

2019-04-06T21:43:52Z

Rkaur:

File:Email.png

2019-04-06T21:39:36Z

Rkaur: uploaded a new version of "File:Email.png"

File:Flowwork.PNG

2019-04-06T21:38:11Z

Rkaur: uploaded a new version of "File:Flowwork.PNG"

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-06T21:07:50Z

Rkaur:

File:Flowwork.PNG

2019-04-06T21:05:21Z

Rkaur: uploaded a new version of "File:Flowwork.PNG"

File:Flowwork.PNG

2019-04-06T21:04:02Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-06T20:44:51Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-06T20:25:13Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-06T20:19:56Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-06T20:03:26Z

Rkaur:

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-06T19:56:14Z

Rkaur:

This wiki page is for the description of changes made under ECE517 SPRING 2019 PROJECT E1931 CONFLICT NOTIFICATIONS

=''Problem Statement''=

Current conflict notification sends an email to the instructor whenever two reviews for the same submission differ “significantly” on their scoring (the threshold is specified in the “Notification limit” on the Rubrics tab of assignment creation). Right now an email is sent at any such time one of these conflicts happen, however, it does not link to the particular review or submission which initiated the notification. The scope of this project is to send an email notification to instructor which contains links to two reviews (one of which is the newly submitted review) that differ by more than the notification limit where such a conflict has occurred. The email should state that a new conflicting review has been made.

=''Existing Feature Description''=
=''What needs to be done''=
=''UML Diagram''=
=''Files to be changed in this Project''=
=''Test Plan''=
=''Implementation''=
==''Rspec Testing''==
==''UI Testing''==
=''References''=

CSC/ECE 517 Spring 2019 - Project E1931. Conflict notification

2019-04-06T19:43:14Z

Rkaur:

This wiki page is for the description of changes made under ECE517 SPRING 2019 PROJECT E1931 CONFLICT NOTIFICATIONS

='''Problem Statement'''=

Current conflict notification sends an email to the instructor whenever two reviews for the same submission differ “significantly” on their scoring (the threshold is specified in the “Notification limit” on the Rubrics tab of assignment creation). Right now an email is sent at any such time one of these conflicts happen, however, it does not link to the particular review or submission which initiated the notification. The scope of this project is to send an email notification to instructor which contains links to two reviews (one of which is the newly submitted review) that differ by more than the notification limit where such a conflict has occurred. The email should state that a new conflicting review has been made.

='''Existing Feature Description'''=
='''UML Diagram'''=
='''Files to be changed in this Project'''=
='''Test Plan'''=
='''Implementation'''=
=='''Rspec Testing'''==
=='''UI Testing'''==
='''References'''=

CSC/ECE 517 Spring 2019 - Project E1906. Refactor stage deadlines in Assignment.rb

2019-03-30T21:42:41Z

Rkaur:

This wiki page is for the description of changes made under E1906 OSS assignment for Spring 2019, CSC/ECE 517.

=='''Expertiza Background'''==
[http://expertiza.ncsu.edu/ Expertiza] is a web application developed using Ruby on Rails Framework whose creation and maintenance is taken care of by students as well as the faculty of NCSU.It's code is available on Github [https://github.com/expertiza/expertiza Expertiza on GitHub].Expertiza allows the instructor to create and edit new as well as existing assignments.This also includes creating a wide range of topics under each assignment which students can sign up for.They can also publish surveys and reviews,view statistical results, set deadlines for assignments and make announcements.It provides a platform for students to signup for topics,form teams,view and submit assignments and give peer reviews and feedback.

=='''Stage Deadline Background'''==
An assignment in Expertiza has many deadlines possible: deadlines for submission and review in each round,deadlines for signing up and dropping topics,submitting meta-reviews,etc.Depending on what the next deadline is,an Expertiza assignment is said to be in a particular kind of what is called "stage".For example,if the next deadline(after the current time) is a submission deadline which is a DueDate object,then the assignment is said to be in a "submission" phase.

[[File: StageBackground.png|center]]

Depending on which stage an assignment is in, certain activities may be permissible or impermissible.For example, the default for a submission stage is to allow submission but not review,and in a review stage,the default is to allow review but not submission.These defaults may be changed by checking the relevant boxes on the Due Dates tab of assignment creation or editing.In the figure below it can be seen the "Other's work" link is grayed out as review is not permissible during submission phase.

[[File: Statusinadeadline.jpg|center]]

=='''Important Links'''==

[http://oss.flazzle.com/ Deployed_Project_Link]

[https://github.com/jwboykin/expertiza Expertiza_Link_for_this_Project]

[https://github.com/expertiza/expertiza/pull/1376 Pull_request_link]

=='''Problem Statement'''==
E1906 is an Expertiza based OSS project which deals basically with refactoring stage deadlines in assignment.rb file.Class assignment.rb has several methods for checking what kind of stage an assignment is in at the current time.These are:

* current_stage_name(topic_id = nil)
* find_current_stage(topic_id = nil)
* get_current_stage(topic_id = nil)
* link_for_current_stage(topic_id = nil)
* stage_deadline(topic_id = nil)

The goal of the project focuses on refactoring some of the above more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. This project is basically an attempt to make this part of the application easier to read and maintain.

=='''Files modified in this project'''==

*assignment.rb
*assignment_spec.rb

=='''Solutions Implemented'''==

==='''Problem 1'''===

Multiple functions in the code were calling DueDate.get_next_due_date(self.id, topic_id).

===='''Solution'''====

A new private function next_due_date was added. Other calls to the DueDate.get_next_due_date were replaced.

<pre>
#returns the next due date
def next_due_date(topic_id = nil)
DueDate.get_next_due_date(self.id, topic_id)
end
</pre>

*One example of refactoring
[[File:Duedate E1906.png|center]]

==='''Problem 2'''===

The above five methods were being used at various places to check if the assignment is 'Finished' which made the code very redundant. Assignment is said to be finished if the next_due_date is nil.

===='''Solution'''====

We have created a new method "finished?" that checks if the next_due_date is nil (which means that the assignment is "finished").

<pre>
# New function to check if the assignment is finished
def finished?( topic_id = nil )
next_due_date(topic_id).nil?
end
</pre>

We have called this method at all the places where previously different methods were being used to check the same thing.

*Signup sheet table uses assignment finished? method instead of current_stage_name
[[File:Problem1_1.png‎|center]]

*Use assignment finished? method where only a comparison to "Finished" was being done using get_current_stage and find_current_stage
[[File:Problem1_2.png|center]] [[File:Problem1_3.png|center]]

==='''Problem 3'''===

The check for a topic_id to be nil when it is a staggered assignment and return 'Unknown' when it is true was being done at many places.This was not following Ruby and Rails rules.

===='''Solution'''====

To make the code DRY and more easy to understand,we have created a new private method "topic_missing?" to check if the topic_id is nil in case of staggered assignment.

<pre>
# Function to check if topic id is null when its a staggered assignment
def topic_missing?( topic_id = nil)
topic_id.nil? and self.staggered_deadline?
end
</pre>

*Refactoring with new method topic_missing?

[[File:Topic missing.png|center]]

*stage_deadline( topic_id = nil) changed according to the modifications done

<pre>
#Gives the next stage deadline date and nil if its finished assignment
def stage_deadline(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
return nil if finished?(topic_id)
next_due_date(topic_id).due_at.to_s
end
</pre>

==='''Problem 4'''===

find_current_stage(topic_id=nil) method is being used only locally in the assignment.rb and once in the student_teams controller to check to check whether to display or not the reviews option for team members.This method was calling another function to get the next_due_date for an assignment that was like nesting of calls and not very Ruby standard.

===='''Solution'''====

We have removed this function and replaced it with finished? method in the student_teams controller(shown in screenshot above in Problem 2) and with DueDate.get_next_due_date where the next due date was required.

<pre>
Before :
due_date = find_current_stage(topic_id)

After :
due_date = DueDate.get_next_due_date(self.id, topic_id)
</pre>

==='''Problem 5'''===

get_current_stage method is returning the stage name which the assignment is in at current time.This is not very clear from the name of the method.

===='''Solution'''====

We have changed the method name to current_stage_name(topic_id=nil) to make it more clear to understand.

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

*Refactored get_current_stage with current_stage_name

[[File:Current stage name new.png|center]]

==='''Problem 6'''===

current_stage_name(topic_id=nil)[This is the initial one] method was implementing things which was already being done by other methods and thus this method was redundant.
The initial code given was:

<pre>
def current_stage_name(topic_id = nil)
if self.staggered_deadline?
return (topic_id.nil? ? 'Unknown' : get_current_stage(topic_id))
end
due_date = find_current_stage(topic_id)

unless self.staggered_deadline?
if due_date != 'Finished' && !due_date.nil? && !due_date.deadline_name.nil?
return due_date.deadline_name
else
return get_current_stage(topic_id)
end
end
end
</pre>

First thing we noticed was that due_date.deadline_name was always nil because there was no field to enter the deadline name on the form, and therefore, the statement <pre> return due_date.deadline_name </pre> was never getting executed.

===='''Solution'''====

This method was checking for the due_date.deadline_name which was always nil. After removing the dead code, we get:

<pre>
def current_stage_name(topic_id = nil)
if topic_missing?(topic_id)
return 'Unknown'
else
get_current_stage(topic_id)
end
end
</pre>

Looking at the current_stage_name(topic_id=nil)[New one] method shown below and comparing it to what we have above, this code was surely redundant.That is why we removed this code and made appropriate changes wherever required within assignment.rb.
*Below is the current_stage_name method for reference

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

The associated rspec tests that tested the old current_stage_name method were also no longer needed.

==='''Problem 7'''===

link_for_current_stage function served no purpose. The function checked to see if any given due date had a link (presumably web URL) and would return it. The database had no values and there is no where in the application this URL could be provided. There doesn't appear to be any use for having a separate URL for each stage.

===='''Solution'''====

Removed function.

[[File:Big1.png|center]]

Removed test related to the function. Removed the one call to this function.

[[File:big2.png|center]]

=='''Automated Testing using RSPEC'''==

We have created and modified the tests as per the changes done in our project.We have added the tests to the file [https://github.com/jwboykin/expertiza/blob/master/spec/models/assignment_spec.rb expertiza/spec/models/assignment_spec.rb] and they are passing 100%.

===='''Tests for get_current_stage_name'''====

The current version of expertiza did not have any tests for get_current_stage(topic_id=nil) method.Now as we have changed it to current_stage_name(topic_id=nil),we have added its tests as well.

<pre>
describe '#current_stage_name' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.current_stage_name).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns Finished' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.current_stage_name(123)).to eq('Finished')
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns current stage name' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:topic_missing?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
deadline = create(:deadline_type, id: 1, name:"Review")
allow(DeadlineType).to receive(:find).with(1).and_return(deadline)
expect(assignment.current_stage_name(123)).to eq('Review')
end
end
end
end
</pre>

===='''Tests for finished?'''====

We have added tests for the new method that we have added finished?.

<pre>
describe '#finished?' do
context 'when assignment next due date is nil' do
it 'returns True' do
allow(DueDate).to receive(:get_next_due_date).with(1, 123).and_return(nil)
expect(assignment.finished?(123)).to eq(true)
end
end
end

describe '#finished?' do
context 'when there is a next due date' do
it 'returns False' do
allow(DueDate).to receive(:get_next_due_date).with(1,123).and_return('2021-11-11 11:11:11')
expect(assignment.finished?(123)).to eq(false)
end
end
end
</pre>

As topic_missing? method is made private,there was no need to test this.

===='''Tests for stage_deadline'''====

We also have corrected the tests for stage_deadline according to the modifications done(It is using finished? and topic_missing? methods).

<pre>
describe '#stage_deadline' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.stage_deadline).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns nil' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.stage_deadline(123)).to be nil
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns due date' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
expect(assignment.stage_deadline(123)).to match('2011-11-11 11:11:11')
end
end
end
end
</pre>

=='''Team'''==

[mailto:jwboykin@ncsu.edu Jimmy Boykin]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:sbasnet2@ncsu.edu Sushan Basnet]

CSC/ECE 517 Spring 2019 - Project E1906. Refactor stage deadlines in Assignment.rb

2019-03-27T05:30:08Z

Rkaur:

This wiki page is for the description of changes made under E1906 OSS assignment for Spring 2019, CSC/ECE 517.

=='''Expertiza Background'''==
[http://expertiza.ncsu.edu/ Expertiza] is a web application developed using Ruby on Rails Framework whose creation and maintenance is taken care of by students as well as the faculty of NCSU.It's code is available on Github [https://github.com/expertiza/expertiza Expertiza on GitHub].Expertiza allows the instructor to create and edit new as well as existing assignments.This also includes creating a wide range of topics under each assignment which students can sign up for.They can also publish surveys and reviews,view statistical results, set deadlines for assignments and make announcements.It provides a platform for students to signup for topics,form teams,view and submit assignments and give peer reviews and feedback.

=='''Stage Deadline Background'''==
An assignment in Expertiza has many deadlines possible: deadlines for submission and review in each round,deadlines for signing up and dropping topics,submitting meta-reviews,etc.Depending on what the next deadline is,an Expertiza assignment is said to be in a particular kind of what is called "stage".For example,if the next deadline(after the current time) is a submission deadline which is a DueDate object,then the assignment is said to be in a "submission" phase.

[[File: StageBackground.png|center]]

Depending on which stage an assignment is in, certain activities may be permissible or impermissible.For example, the default for a submission stage is to allow submission but not review,and in a review stage,the default is to allow review but not submission.These defaults may be changed by checking the relevant boxes on the Due Dates tab of assignment creation or editing.In the figure below it can be seen the "Other's work" link is grayed out as review is not permissible during submission phase.

[[File: Statusinadeadline.jpg|center]]

=='''Important Links'''==

[http://oss.flazzle.com/ Deployed_Project_Link]

[https://github.com/jwboykin/expertiza Expertiza_Link_for_this_Project]

[https://github.com/expertiza/expertiza/pull/1376 Pull_request_link]

=='''Problem Statement'''==
E1906 is an Expertiza based OSS project which deals basically with refactoring stage deadlines in assignment.rb file.Class assignment.rb has several methods for checking what kind of stage an assignment is in at the current time.These are:

* current_stage_name(topic_id = nil)
* find_current_stage(topic_id = nil)
* get_current_stage(topic_id = nil)
* link_for_current_stage(topic_id = nil)
* stage_deadline(topic_id = nil)

The goal of the project focuses on refactoring some of the above more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. This project is basically an attempt to make this part of the application easier to read and maintain.

=='''Solutions Implemented'''==

==='''Problem 1'''===

Multiple functions in the code were calling DueDate.get_next_due_date(self.id, topic_id).

===='''Solution'''====

A new private function next_due_date was added. Other calls to the DueDate.get_next_due_date were replaced.

<pre>
#returns the next due date
def next_due_date(topic_id = nil)
DueDate.get_next_due_date(self.id, topic_id)
end
</pre>

*One example of refactoring
[[File:Duedate E1906.png|center]]

==='''Problem 2'''===

The above five methods were being used at various places to check if the assignment is 'Finished' which made the code very redundant. Assignment is said to be finished if the next_due_date is nil.

===='''Solution'''====

We have created a new method "finished?" that checks if the next_due_date is nil (which means that the assignment is "finished").

<pre>
# New function to check if the assignment is finished
def finished?( topic_id = nil )
next_due_date(topic_id).nil?
end
</pre>

We have called this method at all the places where previously different methods were being used to check the same thing.

*Signup sheet table uses assignment finished? method instead of current_stage_name
[[File:Problem1_1.png‎|center]]

*Use assignment finished? method where only a comparison to "Finished" was being done using get_current_stage and find_current_stage
[[File:Problem1_2.png|center]] [[File:Problem1_3.png|center]]

==='''Problem 3'''===

The check for a topic_id to be nil when it is a staggered assignment and return 'Unknown' when it is true was being done at many places.This was not following Ruby and Rails rules.

===='''Solution'''====

To make the code DRY and more easy to understand,we have created a new private method "topic_missing?" to check if the topic_id is nil in case of staggered assignment.

<pre>
# Function to check if topic id is null when its a staggered assignment
def topic_missing?( topic_id = nil)
topic_id.nil? and self.staggered_deadline?
end
</pre>

*Refactoring with new method topic_missing?

[[File:Topic missing.png|center]]

*stage_deadline( topic_id = nil) changed according to the modifications done

<pre>
#Gives the next stage deadline date and nil if its finished assignment
def stage_deadline(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
return nil if finished?(topic_id)
next_due_date(topic_id).due_at.to_s
end
</pre>

==='''Problem 4'''===

find_current_stage(topic_id=nil) method is being used only locally in the assignment.rb and once in the student_teams controller to check to check whether to display or not the reviews option for team members.This method was calling another function to get the next_due_date for an assignment that was like nesting of calls and not very Ruby standard.

===='''Solution'''====

We have removed this function and replaced it with finished? method in the student_teams controller(shown in screenshot above in Problem 2) and with DueDate.get_next_due_date where the next due date was required.

<pre>
Before :
due_date = find_current_stage(topic_id)

After :
due_date = DueDate.get_next_due_date(self.id, topic_id)
</pre>

==='''Problem 5'''===

get_current_stage method is returning the stage name which the assignment is in at current time.This is not very clear from the name of the method.

===='''Solution'''====

We have changed the method name to current_stage_name(topic_id=nil) to make it more clear to understand.

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

*Refactored get_current_stage with current_stage_name

[[File:Current stage name new.png|center]]

==='''Problem 6'''===

current_stage_name(topic_id=nil)[This is the initial one] method was implementing things which was already being done by other methods and thus this method was redundant.
The initial code given was:

<pre>
def current_stage_name(topic_id = nil)
if self.staggered_deadline?
return (topic_id.nil? ? 'Unknown' : get_current_stage(topic_id))
end
due_date = find_current_stage(topic_id)

unless self.staggered_deadline?
if due_date != 'Finished' && !due_date.nil? && !due_date.deadline_name.nil?
return due_date.deadline_name
else
return get_current_stage(topic_id)
end
end
end
</pre>

First thing we noticed was that due_date.deadline_name was always nil because there was no field to enter the deadline name on the form, and therefore, the statement <pre> return due_date.deadline_name </pre> was never getting executed.

===='''Solution'''====

This method was checking for the due_date.deadline_name which was always nil. After removing the dead code, we get:

<pre>
def current_stage_name(topic_id = nil)
if topic_missing?(topic_id)
return 'Unknown'
else
get_current_stage(topic_id)
end
end
</pre>

Looking at the current_stage_name(topic_id=nil)[New one] method shown below and comparing it to what we have above, this code was surely redundant.That is why we removed this code and made appropriate changes wherever required within assignment.rb.
*Below is the current_stage_name method for reference

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

The associated rspec tests that tested the old current_stage_name method were also no longer needed.

==='''Problem 7'''===

link_for_current_stage function served no purpose. The function checked to see if any given due date had a link (presumably web URL) and would return it. The database had no values and there is no where in the application this URL could be provided. There doesn't appear to be any use for having a separate URL for each stage.

===='''Solution'''====

Removed function.

[[File:Big1.png|center]]

Removed test related to the function. Removed the one call to this function.

[[File:big2.png|center]]

=='''Automated Testing using RSPEC'''==

We have created and modified the tests as per the changes done in our project.We have added the tests to the file [https://github.com/jwboykin/expertiza/blob/master/spec/models/assignment_spec.rb expertiza/spec/models/assignment_spec.rb] and they are passing 100%.

===='''Tests for get_current_stage_name'''====

The current version of expertiza did not have any tests for get_current_stage(topic_id=nil) method.Now as we have changed it to current_stage_name(topic_id=nil),we have added its tests as well.

<pre>
describe '#current_stage_name' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.current_stage_name).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns Finished' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.current_stage_name(123)).to eq('Finished')
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns current stage name' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:topic_missing?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
deadline = create(:deadline_type, id: 1, name:"Review")
allow(DeadlineType).to receive(:find).with(1).and_return(deadline)
expect(assignment.current_stage_name(123)).to eq('Review')
end
end
end
end
</pre>

===='''Tests for finished?'''====

We have added tests for the new method that we have added finished?.

<pre>
describe '#finished?' do
context 'when assignment next due date is nil' do
it 'returns True' do
allow(DueDate).to receive(:get_next_due_date).with(1, 123).and_return(nil)
expect(assignment.finished?(123)).to eq(true)
end
end
end

describe '#finished?' do
context 'when there is a next due date' do
it 'returns False' do
allow(DueDate).to receive(:get_next_due_date).with(1,123).and_return('2021-11-11 11:11:11')
expect(assignment.finished?(123)).to eq(false)
end
end
end
</pre>

As topic_missing? method is made private,there was no need to test this.

===='''Tests for stage_deadline'''====

We also have corrected the tests for stage_deadline according to the modifications done(It is using finished? and topic_missing? methods).

<pre>
describe '#stage_deadline' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.stage_deadline).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns nil' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.stage_deadline(123)).to be nil
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns due date' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
expect(assignment.stage_deadline(123)).to match('2011-11-11 11:11:11')
end
end
end
end
</pre>

=='''Team'''==

[mailto:jwboykin@ncsu.edu Jimmy Boykin]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:sbasnet2@ncsu.edu Sushan Basnet]

CSC/ECE 517 Spring 2019 - Project E1906. Refactor stage deadlines in Assignment.rb

2019-03-26T00:52:21Z

Rkaur:

This wiki page is for the description of changes made under E1906 OSS assignment for Spring 2019, CSC/ECE 517.

=='''Expertiza Background'''==
[http://expertiza.ncsu.edu/ Expertiza] is a web application developed using Ruby on Rails Framework whose creation and maintenance is taken care of by students as well as the faculty of NCSU.It's code is available on Github [https://github.com/expertiza/expertiza Expertiza on GitHub].Expertiza allows the instructor to create and edit new as well as existing assignments.This also includes creating a wide range of topics under each assignment which students can sign up for.They can also publish surveys and reviews,view statistical results, set deadlines for assignments and make announcements.It provides a platform for students to signup for topics,form teams,view and submit assignments and give peer reviews and feedback.

=='''Stage Deadline Background'''==
An assignment in Expertiza has many deadlines possible: deadlines for submission and review in each round,deadlines for signing up and dropping topics,submitting meta-reviews,etc.Depending on what the next deadline is,an Expertiza assignment is said to be in a particular kind of what is called "stage".For example,if the next deadline(after the current time) is a submission deadline which is a DueDate object,then the assignment is said to be in a "submission" phase.

[[File: StageBackground.png|center]]

Depending on which stage an assignment is in, certain activities may be permissible or impermissible.For example, the default for a submission stage is to allow submission but not review,and in a review stage,the default is to allow review but not submission.These defaults may be changed by checking the relevant boxes on the Due Dates tab of assignment creation or editing.In the figure below it can be seen the "Other's work" link is grayed out as review is not permissible during submission phase.

[[File: Statusinadeadline.jpg|center]]

=='''Important Links'''==

[http://oss.flazzle.com/ Deployed_Project_Link]

[https://github.com/jwboykin/expertiza Expertiza_Link_for_this_Project]

[https://github.com/expertiza/expertiza/pull/1376 Pull_request_link]

=='''Problem Statement'''==
E1906 is an Expertiza based OSS project which deals basically with refactoring stage deadlines in assignment.rb file.Class assignment.rb has several methods for checking what kind of stage an assignment is in at the current time.These are:

* current_stage_name(topic_id = nil)
* find_current_stage(topic_id = nil)
* get_current_stage(topic_id = nil)
* link_for_current_stage(topic_id = nil)
* stage_deadline(topic_id = nil)

The goal of the project focuses on refactoring some of the above more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. This project is basically an attempt to make this part of the application easier to read and maintain.

=='''Solutions Implemented'''==

==='''Problem 1'''===

Multiple functions in the code were calling DueDate.get_next_due_date(self.id, topic_id).

===='''Solution'''====

A new private function next_due_date was added. Other calls to the DueDate.get_next_due_date were replaced.

<pre>
#returns the next due date
def next_due_date(topic_id = nil)
DueDate.get_next_due_date(self.id, topic_id)
end
</pre>

*One example of refactoring
[[File:Duedate E1906.png|center]]

==='''Problem 2'''===

The above five methods were being used at various places to check if the assignment is 'Finished' which made the code very redundant. Assignment is said to be finished if the next_due_date is nil.

===='''Solution'''====

We have created a new method "finished?" that checks if the next_due_date is nil (which means that the assignment is "finished").

<pre>
# New function to check if the assignment is finished
def finished?( topic_id = nil )
next_due_date(topic_id).nil?
end
</pre>

We have called this method at all the places where previously different methods were being used to check the same thing.

*Signup sheet table uses assignment finished? method instead of current_stage_name
[[File:Problem1_1.png‎|center]]

*Use assignment finished? method where only a comparison to "Finished" was being done using get_current_stage and find_current_stage
[[File:Problem1_2.png|center]] [[File:Problem1_3.png|center]]

==='''Problem 3'''===

The check for a topic_id to be nil when it is a staggered assignment and return 'Unknown' when it is true was being done at many places.This was not following Ruby and Rails rules.

===='''Solution'''====

To make the code DRY and more easy to understand,we have created a new private method "topic_missing?" to check if the topic_id is nil in case of staggered assignment.

<pre>
# Function to check if topic id is null when its a staggered assignment
def topic_missing?( topic_id = nil)
topic_id.nil? and self.staggered_deadline?
end
</pre>

*Refactoring with new method topic_missing?

[[File:Topic missing.png|center]]

*stage_deadline( topic_id = nil) changed according to the modifications done

<pre>
#Gives the next stage deadline date and nil if its finished assignment
def stage_deadline(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
return nil if finished?(topic_id)
next_due_date(topic_id).due_at.to_s
end
</pre>

==='''Problem 4'''===

find_current_stage(topic_id=nil) method is being used only locally in the assignment.rb and once in the student_teams controller to check to check whether to display or not the reviews option for team members.This method was calling another function to get the next_due_date for an assignment that was like nesting of calls and not very Ruby standard.

===='''Solution'''====

We have removed this function and replaced it with finished? method in the student_teams controller(shown in screenshot above in Problem 2) and with DueDate.get_next_due_date where the next due date was required.

<pre>
Before :
due_date = find_current_stage(topic_id)

After :
due_date = DueDate.get_next_due_date(self.id, topic_id)
</pre>

==='''Problem 5'''===

get_current_stage method is returning the stage name which the assignment is in at current time.This is not very clear from the name of the method.

===='''Solution'''====

We have changed the method name to current_stage_name(topic_id=nil) to make it more clear to understand.

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

*Refactored get_current_stage with current_stage_name

[[File:Current stage name new.png|center]]

==='''Problem 6'''===

current_stage_name(topic_id=nil)[This is the initial one] method was implementing things which was already being done by other methods and thus this method was redundant.
The initial code given was:

<pre>
def current_stage_name(topic_id = nil)
if self.staggered_deadline?
return (topic_id.nil? ? 'Unknown' : get_current_stage(topic_id))
end
due_date = find_current_stage(topic_id)

unless self.staggered_deadline?
if due_date != 'Finished' && !due_date.nil? && !due_date.deadline_name.nil?
return due_date.deadline_name
else
return get_current_stage(topic_id)
end
end
end
</pre>

First thing we noticed was that due_date.deadline_name was always nil because there was no field to enter the deadline name on the form, and therefore, the statement <pre> return due_date.deadline_name </pre> was never getting executed.

===='''Solution'''====

This method was checking for the due_date.deadline_name which was always nil. After removing the dead code, we get:

<pre>
def current_stage_name(topic_id = nil)
if topic_missing?(topic_id)
return 'Unknown'
else
get_current_stage(topic_id)
end
end
</pre>

Looking at the current_stage_name(topic_id=nil)[New one] method shown below and comparing it to what we have above, this code was surely redundant.That is why we removed this code and made appropriate changes wherever required within assignment.rb.
*Below is the current_stage_name method for reference

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

The associated rspec tests that tested the old current_stage_name method were also no longer needed.

==='''Problem 7'''===

link_for_current_stage function served no purpose. The function checked to see if any given due date had a link (presumably web URL) and would return it. The database had no values and there is no where in the application this URL could be provided. There doesn't appear to be any use for having a separate URL for each stage.

===='''Solution'''====

Removed function.

[[File:Big1.png|center]]

Removed test related to the function. Removed the one call to this function.

[[File:big2.png|center]]

=='''Automated Testing using RSPEC'''==

===='''Tests for get_current_stage_name'''====

The current version of expertiza did not have any tests for get_current_stage(topic_id=nil) method.Now as we have changed it to current_stage_name(topic_id=nil),we have added its tests as well.

<pre>
describe '#current_stage_name' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.current_stage_name).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns Finished' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.current_stage_name(123)).to eq('Finished')
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns current stage name' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:topic_missing?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
deadline = create(:deadline_type, id: 1, name:"Review")
allow(DeadlineType).to receive(:find).with(1).and_return(deadline)
expect(assignment.current_stage_name(123)).to eq('Review')
end
end
end
end
</pre>

===='''Tests for finished?'''====

We have added tests for the new method that we have added finished?.

<pre>
describe '#finished?' do
context 'when assignment next due date is nil' do
it 'returns True' do
allow(DueDate).to receive(:get_next_due_date).with(1, 123).and_return(nil)
expect(assignment.finished?(123)).to eq(true)
end
end
end

describe '#finished?' do
context 'when there is a next due date' do
it 'returns False' do
allow(DueDate).to receive(:get_next_due_date).with(1,123).and_return('2021-11-11 11:11:11')
expect(assignment.finished?(123)).to eq(false)
end
end
end
</pre>

As topic_missing? method is made private,there was no need to test this.

===='''Tests for stage_deadline'''====

We also have corrected the tests for stage_deadline according to the modifications done(It is using finished? and topic_missing? methods).

<pre>
describe '#stage_deadline' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.stage_deadline).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns nil' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.stage_deadline(123)).to be nil
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns due date' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
expect(assignment.stage_deadline(123)).to match('2011-11-11 11:11:11')
end
end
end
end
</pre>

=='''Team'''==

[mailto:jwboykin@ncsu.edu Jimmy Boykin]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:sbasnet2@ncsu.edu Sushan Basnet]

CSC/ECE 517 Spring 2019 - Project E1906. Refactor stage deadlines in Assignment.rb

2019-03-25T23:26:48Z

Rkaur:

This wiki page is for the description of changes made under E1906 OSS assignment for Spring 2019, CSC/ECE 517.

=='''Expertiza Background'''==
[http://expertiza.ncsu.edu/ Expertiza] is a web application developed using Ruby on Rails Framework whose creation and maintenance is taken care of by students as well as the faculty of NCSU.It's code is available on Github [https://github.com/expertiza/expertiza Expertiza on GitHub].Expertiza allows the instructor to create and edit new as well as existing assignments.This also includes creating a wide range of topics under each assignment which students can sign up for.They can also publish surveys and reviews,view statistical results, set deadlines for assignments and make announcements.It provides a platform for students to signup for topics,form teams,view and submit assignments and give peer reviews and feedback.

=='''Stage Deadline Background'''==
An assignment in Expertiza has many deadlines possible: deadlines for submission and review in each round,deadlines for signing up and dropping topics,submitting meta-reviews,etc.Depending on what the next deadline is,an Expertiza assignment is said to be in a particular kind of what is called "stage".For example,if the next deadline(after the current time) is a submission deadline which is a DueDate object,then the assignment is said to be in a "submission" phase.

[[File: StageBackground.png|center]]

Depending on which stage an assignment is in, certain activities may be permissible or impermissible.For example, the default for a submission stage is to allow submission but not review,and in a review stage,the default is to allow review but not submission.These defaults may be changed by checking the relevant boxes on the Due Dates tab of assignment creation or editing.In the figure below it can be seen the "Other's work" link is grayed out as review is not permissible during submission phase.

[[File: Statusinadeadline.jpg|center]]

=='''Important Links'''==

[http://oss.flazzle.com/ Deployed_Project_Link]

[https://github.com/jwboykin/expertiza Expertiza_Link_for_this_Project]

=='''Problem Statement'''==
E1906 is an Expertiza based OSS project which deals basically with refactoring stage deadlines in assignment.rb file.Class assignment.rb has several methods for checking what kind of stage an assignment is in at the current time.These are:

* current_stage_name(topic_id = nil)
* find_current_stage(topic_id = nil)
* get_current_stage(topic_id = nil)
* link_for_current_stage(topic_id = nil)
* stage_deadline(topic_id = nil)

The goal of the project focuses on refactoring some of the above more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. This project is basically an attempt to make this part of the application easier to read and maintain.

=='''Solutions Implemented'''==

==='''Problem 1'''===

Multiple functions in the code were calling DueDate.get_next_due_date(self.id, topic_id).

===='''Solution'''====

A new private function next_due_date was added. Other calls to the DueDate.get_next_due_date were replaced.

<pre>
#returns the next due date
def next_due_date(topic_id = nil)
DueDate.get_next_due_date(self.id, topic_id)
end
</pre>

*One example of refactoring
[[File:Duedate E1906.png|center]]

==='''Problem 2'''===

The above five methods were being used at various places to check if the assignment is 'Finished' which made the code very redundant. Assignment is said to be finished if the next_due_date is nil.

===='''Solution'''====

We have created a new method "finished?" that checks if the next_due_date is nil (which means that the assignment is "finished").

<pre>
# New function to check if the assignment is finished
def finished?( topic_id = nil )
next_due_date(topic_id).nil?
end
</pre>

We have called this method at all the places where previously different methods were being used to check the same thing.

*Signup sheet table uses assignment finished? method instead of current_stage_name
[[File:Problem1_1.png‎|center]]

*Use assignment finished? method where only a comparison to "Finished" was being done using get_current_stage and find_current_stage
[[File:Problem1_2.png|center]] [[File:Problem1_3.png|center]]

==='''Problem 3'''===

The check for a topic_id to be nil when it is a staggered assignment and return 'Unknown' when it is true was being done at many places.This was not following Ruby and Rails rules.

===='''Solution'''====

To make the code DRY and more easy to understand,we have created a new private method "topic_missing?" to check if the topic_id is nil in case of staggered assignment.

<pre>
# Function to check if topic id is null when its a staggered assignment
def topic_missing?( topic_id = nil)
topic_id.nil? and self.staggered_deadline?
end
</pre>

*Refactoring with new method topic_missing?

[[File:Topic missing.png|center]]

*stage_deadline( topic_id = nil) changed according to the modifications done

<pre>
#Gives the next stage deadline date and nil if its finished assignment
def stage_deadline(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
return nil if finished?(topic_id)
next_due_date(topic_id).due_at.to_s
end
</pre>

==='''Problem 4'''===

find_current_stage(topic_id=nil) method is being used only locally in the assignment.rb and once in the student_teams controller to check to check whether to display or not the reviews option for team members.This method was calling another function to get the next_due_date for an assignment that was like nesting of calls and not very Ruby standard.

===='''Solution'''====

We have removed this function and replaced it with finished? method in the student_teams controller(shown in screenshot above in Problem 2) and with DueDate.get_next_due_date where the next due date was required.

<pre>
Before :
due_date = find_current_stage(topic_id)

After :
due_date = DueDate.get_next_due_date(self.id, topic_id)
</pre>

==='''Problem 5'''===

get_current_stage method is returning the stage name which the assignment is in at current time.This is not very clear from the name of the method.

===='''Solution'''====

We have changed the method name to current_stage_name(topic_id=nil) to make it more clear to understand.

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

*Refactored get_current_stage with current_stage_name

[[File:Current stage name new.png|center]]

==='''Problem 6'''===

current_stage_name(topic_id=nil)[This is the initial one] method was implementing things which was already being done by other methods and thus this method was redundant.
The initial code given was:

<pre>
def current_stage_name(topic_id = nil)
if self.staggered_deadline?
return (topic_id.nil? ? 'Unknown' : get_current_stage(topic_id))
end
due_date = find_current_stage(topic_id)

unless self.staggered_deadline?
if due_date != 'Finished' && !due_date.nil? && !due_date.deadline_name.nil?
return due_date.deadline_name
else
return get_current_stage(topic_id)
end
end
end
</pre>

First thing we noticed was that due_date.deadline_name was always nil because there was no field to enter the deadline name on the form, and therefore, the statement <pre> return due_date.deadline_name </pre> was never getting executed.

===='''Solution'''====

This method was checking for the due_date.deadline_name which was always nil. After removing the dead code, we get:

<pre>
def current_stage_name(topic_id = nil)
if topic_missing?(topic_id)
return 'Unknown'
else
get_current_stage(topic_id)
end
end
</pre>

Looking at the current_stage_name(topic_id=nil)[New one] method shown below and comparing it to what we have above, this code was surely redundant.That is why we removed this code and made appropriate changes wherever required within assignment.rb.
*Below is the current_stage_name method for reference

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

The associated rspec tests that tested the old current_stage_name method were also no longer needed.

==='''Problem 7'''===

link_for_current_stage function served no purpose. The function checked to see if any given due date had a link (presumably web URL) and would return it. The database had no values and there is no where in the application this URL could be provided. There doesn't appear to be any use for having a separate URL for each stage.

===='''Solution'''====

Removed function.

[[File:Big1.png|center]]

Removed test related to the function. Removed the one call to this function.

[[File:big2.png|center]]

=='''Automated Testing using RSPEC'''==

===='''Tests for get_current_stage_name'''====

The current version of expertiza did not have any tests for get_current_stage(topic_id=nil) method.Now as we have changed it to current_stage_name(topic_id=nil),we have added its tests as well.

<pre>
describe '#current_stage_name' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.current_stage_name).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns Finished' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.current_stage_name(123)).to eq('Finished')
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns current stage name' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:topic_missing?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
deadline = create(:deadline_type, id: 1, name:"Review")
allow(DeadlineType).to receive(:find).with(1).and_return(deadline)
expect(assignment.current_stage_name(123)).to eq('Review')
end
end
end
end
</pre>

===='''Tests for finished?'''====

We have added tests for the new method that we have added finished?.

<pre>
describe '#finished?' do
context 'when assignment next due date is nil' do
it 'returns True' do
allow(DueDate).to receive(:get_next_due_date).with(1, 123).and_return(nil)
expect(assignment.finished?(123)).to eq(true)
end
end
end

describe '#finished?' do
context 'when there is a next due date' do
it 'returns False' do
allow(DueDate).to receive(:get_next_due_date).with(1,123).and_return('2021-11-11 11:11:11')
expect(assignment.finished?(123)).to eq(false)
end
end
end
</pre>

As topic_missing? method is made private,there was no need to test this.

===='''Tests for stage_deadline'''====

We also have corrected the tests for stage_deadline according to the modifications done(It is using finished? and topic_missing? methods).

<pre>
describe '#stage_deadline' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.stage_deadline).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns nil' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.stage_deadline(123)).to be nil
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns due date' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
expect(assignment.stage_deadline(123)).to match('2011-11-11 11:11:11')
end
end
end
end
</pre>

=='''Team'''==

[mailto:jwboykin@ncsu.edu Jimmy Boykin]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:sbasnet2@ncsu.edu Sushan Basnet]

CSC/ECE 517 Spring 2019 - Project E1906. Refactor stage deadlines in Assignment.rb

2019-03-25T23:26:11Z

Rkaur:

This wiki page is for the description of changes made under E1906 OSS assignment for Spring 2019, CSC/ECE 517.

=='''Expertiza Background'''==
[http://expertiza.ncsu.edu/ Expertiza] is a web application developed using Ruby on Rails Framework whose creation and maintenance is taken care of by students as well as the faculty of NCSU.It's code is available on Github [https://github.com/expertiza/expertiza Expertiza on GitHub].Expertiza allows the instructor to create and edit new as well as existing assignments.This also includes creating a wide range of topics under each assignment which students can sign up for.They can also publish surveys and reviews,view statistical results, set deadlines for assignments and make announcements.It provides a platform for students to signup for topics,form teams,view and submit assignments and give peer reviews and feedback.

=='''Stage Deadline Background'''==
An assignment in Expertiza has many deadlines possible: deadlines for submission and review in each round,deadlines for signing up and dropping topics,submitting meta-reviews,etc.Depending on what the next deadline is,an Expertiza assignment is said to be in a particular kind of what is called "stage".For example,if the next deadline(after the current time) is a submission deadline which is a DueDate object,then the assignment is said to be in a "submission" phase.

[[File: StageBackground.png|center]]

Depending on which stage an assignment is in, certain activities may be permissible or impermissible.For example, the default for a submission stage is to allow submission but not review,and in a review stage,the default is to allow review but not submission.These defaults may be changed by checking the relevant boxes on the Due Dates tab of assignment creation or editing.In the figure below it can be seen the "Other's work" link is grayed out as review is not permissible during submission phase.

[[File: Statusinadeadline.jpg|center]]

=='''Important Links'''==

[http://oss.flazzle.com/ Deployed_Project_Link]

[https://github.com/jwboykin/expertiza Expertiza_Link_for_this_Project]

=='''Problem Statement'''==
E1906 is an Expertiza based OSS project which deals basically with refactoring stage deadlines in assignment.rb file.Class assignment.rb has several methods for checking what kind of stage an assignment is in at the current time.These are:

* current_stage_name(topic_id = nil)
* find_current_stage(topic_id = nil)
* get_current_stage(topic_id = nil)
* link_for_current_stage(topic_id = nil)
* stage_deadline(topic_id = nil)

The goal of the project focuses on refactoring some of the above more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. This project is basically an attempt to make this part of the application easier to read and maintain.

=='''Solutions Implemented'''==

==='''Problem 1'''===

Multiple functions in the code were calling DueDate.get_next_due_date(self.id, topic_id).

===='''Solution'''====

A new private function next_due_date was added. Other calls to the DueDate.get_next_due_date were replaced.

<pre>
#returns the next due date
def next_due_date(topic_id = nil)
DueDate.get_next_due_date(self.id, topic_id)
end
</pre>

*One example of refactoring
[[File:Duedate E1906.png|center]]

==='''Problem 2'''===

The above five methods were being used at various places to check if the assignment is 'Finished' which made the code very redundant. Assignment is said to be finished if the next_due_date is nil.

===='''Solution'''====

We have created a new method "finished?" that checks if the next_due_date is nil (which means that the assignment is "finished").

<pre>
# New function to check if the assignment is finished
def finished?( topic_id = nil )
next_due_date(topic_id).nil?
end
</pre>

We have called this method at all the places where previously different methods were being used to check the same thing.

*Signup sheet table uses assignment finished? method instead of current_stage_name
[[File:Problem1_1.png‎|center]]

*Use assignment finished? method where only a comparison to "Finished" was being done using get_current_stage and find_current_stage
[[File:Problem1_2.png|center]] [[File:Problem1_3.png|center]]

==='''Problem 3'''===

The check for a topic_id to be nil when it is a staggered assignment and return 'Unknown' when it is true was being done at many places.This was not following Ruby and Rails rules.

===='''Solution'''====

To make the code DRY and more easy to understand,we have created a new private method "topic_missing?" to check if the topic_id is nil in case of staggered assignment.

<pre>
# Function to check if topic id is null when its a staggered assignment
def topic_missing?( topic_id = nil)
topic_id.nil? and self.staggered_deadline?
end
</pre>

*Refactoring with new method topic_missing?

[[File:Topic missing.png|center]]

*stage_deadline( topic_id = nil) changed according to the modifications done

<pre>
#Gives the next stage deadline date and nil if its finished assignment
def stage_deadline(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
return nil if finished?(topic_id)
next_due_date(topic_id).due_at.to_s
end
</pre>

==='''Problem 4'''===

find_current_stage(topic_id=nil) method is being used only locally in the assignment.rb and once in the student_teams controller to check to check whether to display or not the reviews option for team members.This method was calling another function to get the next_due_date for an assignment that was like nesting of calls and not very Ruby standard.

===='''Solution'''====

We have removed this function and replaced it with finished? method in the student_teams controller(shown in screenshot above in Problem 2) and with DueDate.get_next_due_date where the next due date was required.

<pre>
Before :
due_date = find_current_stage(topic_id)

After :
due_date = DueDate.get_next_due_date(self.id, topic_id)
</pre>

==='''Problem 5'''===

get_current_stage method is returning the stage name which the assignment is in at current time.This is not very clear from the name of the method.

===='''Solution'''====

We have changed the method name to current_stage_name(topic_id=nil) to make it more clear to understand.

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

*Refactored get_current_stage with current_stage_name

[[File:Current stage name new.png|center]]

==='''Problem 6'''===

current_stage_name(topic_id=nil)[This is the initial one] method was implementing things which was already being done by other methods and thus this method was redundant.
The initial code given was:

<pre>
def current_stage_name(topic_id = nil)
if self.staggered_deadline?
return (topic_id.nil? ? 'Unknown' : get_current_stage(topic_id))
end
due_date = find_current_stage(topic_id)

unless self.staggered_deadline?
if due_date != 'Finished' && !due_date.nil? && !due_date.deadline_name.nil?
return due_date.deadline_name
else
return get_current_stage(topic_id)
end
end
end
</pre>

First thing we noticed was that due_date.deadline_name was always nil because there was no field to enter the deadline name on the form, and therefore, the statement <pre> return due_date.deadline_name </pre> was never getting executed.

===='''Solution'''====

This method was checking for the due_date.deadline_name which was always nil. After removing the dead code, we get:

<pre>
def current_stage_name(topic_id = nil)
if topic_missing?(topic_id)
return 'Unknown'
else
get_current_stage(topic_id)
end
end
</pre>

Looking at the current_stage_name(topic_id=nil)[New one] method shown below and comparing it to what we have above, this code was surely redundant.That is why we removed this code and made appropriate changes wherever required within assignment.rb.
*Below is the current_stage_name method for reference

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

The associated rspec tests that tested the old current_stage_name method were also no longer needed.

==='''Problem 7'''===

link_for_current_stage function served no purpose. The function checked to see if any given due date had a link (presumably web URL) and would return it. The database had no values and there is no where in the application this URL could be provided. There doesn't appear to be any use for having a separate URL for each stage.

===='''Solution'''====

Removed function.

[[File:Big1.png|center]]

Removed test related to the function. Removed the one call to this function.

[[File:big2.png|center]]

=='''Automated Testing using RSPEC'''==

===='''Tests for get_current_stage_name'''====

The current version of expertiza did not have any tests for get_current_stage(topic_id=nil) method.Now as we have changed it to current_stage_name(topic_id=nil),we have added its tests as well.

<pre>
describe '#current_stage_name' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.current_stage_name).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns Finished' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.current_stage_name(123)).to eq('Finished')
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns current stage name' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:topic_missing?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
deadline = create(:deadline_type, id: 1, name:"Review")
allow(DeadlineType).to receive(:find).with(1).and_return(deadline)
expect(assignment.current_stage_name(123)).to eq('Review')
end
end
end
end
</pre>

===='''Tests for finished?'''====

We have added tests for the new method that we have added finished?.

<pre>
describe '#finished?' do
context 'when assignment next due date is nil' do
it 'returns True' do
allow(DueDate).to receive(:get_next_due_date).with(1, 123).and_return(nil)
expect(assignment.finished?(123)).to eq(true)
end
end
end

describe '#finished?' do
context 'when there is a next due date' do
it 'returns False' do
allow(DueDate).to receive(:get_next_due_date).with(1,123).and_return('2021-11-11 11:11:11')
expect(assignment.finished?(123)).to eq(false)
end
end
end
</pre>

As topic_missing? method is made private,there was no need to test this.

===='''Tests for stage_deadline'''====

We also have corrected the tests for stage_deadline according to the modifications done(It is using finished? and topic_missing? methods).

<pre>
describe '#stage_deadline' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.stage_deadline).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns nil' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.stage_deadline(123)).to be nil
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns due date' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
expect(assignment.stage_deadline(123)).to match('2011-11-11 11:11:11')
end
end
end
end
</pre>

=='''Team'''==

[mailto:jwboykin@ncsu.edu Jimmy Boykin]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:sbasnet2@ncsu.edu Sushan Basnet]

CSC/ECE 517 Spring 2019 - Project E1906. Refactor stage deadlines in Assignment.rb

2019-03-25T23:20:15Z

Rkaur:

This wiki page is for the description of changes made under E1906 OSS assignment for Spring 2019, CSC/ECE 517.

=='''Expertiza Background'''==
[http://expertiza.ncsu.edu/ Expertiza] is a web application developed using Ruby on Rails Framework whose creation and maintenance is taken care of by students as well as the faculty of NCSU.It's code is available on Github [https://github.com/expertiza/expertiza Expertiza on GitHub].Expertiza allows the instructor to create and edit new as well as existing assignments.This also includes creating a wide range of topics under each assignment which students can sign up for.They can also publish surveys and reviews,view statistical results, set deadlines for assignments and make announcements.It provides a platform for students to signup for topics,form teams,view and submit assignments and give peer reviews and feedback.

=='''Stage Deadline Background'''==
An assignment in Expertiza has many deadlines possible: deadlines for submission and review in each round,deadlines for signing up and dropping topics,submitting meta-reviews,etc.Depending on what the next deadline is,an Expertiza assignment is said to be in a particular kind of what is called "stage".For example,if the next deadline(after the current time) is a submission deadline which is a DueDate object,then the assignment is said to be in a "submission" phase.

[[File: StageBackground.png|center]]

Depending on which stage an assignment is in, certain activities may be permissible or impermissible.For example, the default for a submission stage is to allow submission but not review,and in a review stage,the default is to allow review but not submission.These defaults may be changed by checking the relevant boxes on the Due Dates tab of assignment creation or editing.In the figure below it can be seen the "Other's work" link is grayed out as review is not permissible during submission phase.

[[File: Statusinadeadline.jpg|center]]

=='''Important Links'''==

[http://oss.flazzle.com/ Deployed_Project_Link]

[https://github.com/jwboykin/expertiza Expertiza_Link_for_this_Project]

=='''Problem Statement'''==
E1906 is an Expertiza based OSS project which deals basically with refactoring stage deadlines in assignment.rb file.Class assignment.rb has several methods for checking what kind of stage an assignment is in at the current time.These are:

* current_stage_name(topic_id = nil)
* find_current_stage(topic_id = nil)
* get_current_stage(topic_id = nil)
* link_for_current_stage(topic_id = nil)
* stage_deadline(topic_id = nil)

The goal of the project focuses on refactoring some of the above more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. This project is basically an attempt to make this part of the application easier to read and maintain.

=='''Solutions Implemented'''==

==='''Problem 1'''===

Multiple functions in the code were calling DueDate.get_next_due_date(self.id, topic_id).

===='''Solution'''====

A new private function next_due_date was added. Other calls to the DueDate.get_next_due_date were replaced.

<pre>
#returns the next due date
def next_due_date(topic_id = nil)
DueDate.get_next_due_date(self.id, topic_id)
end
</pre>

*One example of refactoring
[[File:Duedate E1906.png|center]]

==='''Problem 2'''===

The above five methods were being used at various places to check if the assignment is 'Finished' which made the code very redundant. Assignment is said to be finished if the next_due_date is nil.

===='''Solution'''====

We have created a new method "finished?" that checks if the next_due_date is nil (which means that the assignment is "finished").

<pre>
# New function to check if the assignment is finished
def finished?( topic_id = nil )
next_due_date(topic_id).nil?
end
</pre>

We have called this method at all the places where previously different methods were being used to check the same thing.

*Signup sheet table uses assignment finished? method instead of current_stage_name
[[File:Problem1_1.png‎|center]]

*Use assignment finished? method where only a comparison to "Finished" was being done using get_current_stage and find_current_stage
[[File:Problem1_2.png|center]] [[File:Problem1_3.png|center]]

==='''Problem 3'''===

The check for a topic_id to be nil when it is a staggered assignment and return 'Unknown' when it is true was being done at many places.This was not following Ruby and Rails rules.

===='''Solution'''====

To make the code DRY and more easy to understand,we have created a new private method "topic_missing?" to check if the topic_id is nil in case of staggered assignment.

<pre>
# Function to check if topic id is null when its a staggered assignment
def topic_missing?( topic_id = nil)
topic_id.nil? and self.staggered_deadline?
end
</pre>

*Refactoring with new method topic_missing?

[[File:Topic missing.png|center]]

==='''Problem 4'''===

find_current_stage(topic_id=nil) method is being used only locally in the assignment.rb and once in the student_teams controller to check to check whether to display or not the reviews option for team members.This method was calling another function to get the next_due_date for an assignment that was like nesting of calls and not very Ruby standard.

===='''Solution'''====

We have removed this function and replaced it with finished? method in the student_teams controller(shown in screenshot above in Problem 2) and with DueDate.get_next_due_date where the next due date was required.

<pre>
Before :
due_date = find_current_stage(topic_id)

After :
due_date = DueDate.get_next_due_date(self.id, topic_id)
</pre>

==='''Problem 5'''===

get_current_stage method is returning the stage name which the assignment is in at current time.This is not very clear from the name of the method.

===='''Solution'''====

We have changed the method name to current_stage_name(topic_id=nil) to make it more clear to understand.

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

*Refactored get_current_stage with current_stage_name

[[File:Current stage name new.png|center]]

==='''Problem 6'''===

current_stage_name(topic_id=nil)[This is the initial one] method was implementing things which was already being done by other methods and thus this method was redundant.
The initial code given was:

<pre>
def current_stage_name(topic_id = nil)
if self.staggered_deadline?
return (topic_id.nil? ? 'Unknown' : get_current_stage(topic_id))
end
due_date = find_current_stage(topic_id)

unless self.staggered_deadline?
if due_date != 'Finished' && !due_date.nil? && !due_date.deadline_name.nil?
return due_date.deadline_name
else
return get_current_stage(topic_id)
end
end
end
</pre>

First thing we noticed was that due_date.deadline_name was always nil because there was no field to enter the deadline name on the form, and therefore, the statement <pre> return due_date.deadline_name </pre> was never getting executed.

===='''Solution'''====

This method was checking for the due_date.deadline_name which was always nil. After removing the dead code, we get:

<pre>
def current_stage_name(topic_id = nil)
if topic_missing?(topic_id)
return 'Unknown'
else
get_current_stage(topic_id)
end
end
</pre>

Looking at the current_stage_name(topic_id=nil)[New one] method shown below and comparing it to what we have above, this code was surely redundant.That is why we removed this code and made appropriate changes wherever required within assignment.rb.
*Below is the current_stage_name method for reference

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

The associated rspec tests that tested the old current_stage_name method were also no longer needed.

==='''Problem 7'''===

link_for_current_stage function served no purpose. The function checked to see if any given due date had a link (presumably web URL) and would return it. The database had no values and there is no where in the application this URL could be provided. There doesn't appear to be any use for having a separate URL for each stage.

===='''Solution'''====

Removed function.

[[File:Big1.png|center]]

Removed test related to the function. Removed the one call to this function.

[[File:big2.png|center]]

=='''Automated Testing using RSPEC'''==

===='''Tests for get_current_stage_name'''====

The current version of expertiza did not have any tests for get_current_stage(topic_id=nil) method.Now as we have changed it to current_stage_name(topic_id=nil),we have added its tests as well.

<pre>
describe '#current_stage_name' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.current_stage_name).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns Finished' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.current_stage_name(123)).to eq('Finished')
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns current stage name' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:topic_missing?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
deadline = create(:deadline_type, id: 1, name:"Review")
allow(DeadlineType).to receive(:find).with(1).and_return(deadline)
expect(assignment.current_stage_name(123)).to eq('Review')
end
end
end
end
</pre>

===='''Tests for finished?'''====

We have added tests for the new method that we have added finished?.

<pre>
describe '#finished?' do
context 'when assignment next due date is nil' do
it 'returns True' do
allow(DueDate).to receive(:get_next_due_date).with(1, 123).and_return(nil)
expect(assignment.finished?(123)).to eq(true)
end
end
end

describe '#finished?' do
context 'when there is a next due date' do
it 'returns False' do
allow(DueDate).to receive(:get_next_due_date).with(1,123).and_return('2021-11-11 11:11:11')
expect(assignment.finished?(123)).to eq(false)
end
end
end
</pre>

As topic_missing? method is made private,there was no need to test this.

===='''Tests for stage_deadline'''====

We also have corrected the tests for stage_deadline according to the modifications done(It is using finished? and topic_missing? methods).

<pre>
describe '#stage_deadline' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.stage_deadline).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns nil' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.stage_deadline(123)).to be nil
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns due date' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
expect(assignment.stage_deadline(123)).to match('2011-11-11 11:11:11')
end
end
end
end
</pre>

=='''Team'''==

[mailto:jwboykin@ncsu.edu Jimmy Boykin]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:sbasnet2@ncsu.edu Sushan Basnet]

CSC/ECE 517 Spring 2019 - Project E1906. Refactor stage deadlines in Assignment.rb

2019-03-25T23:18:15Z

Rkaur:

This wiki page is for the description of changes made under E1906 OSS assignment for Spring 2019, CSC/ECE 517.

=='''Expertiza Background'''==
[http://expertiza.ncsu.edu/ Expertiza] is a web application developed using Ruby on Rails Framework whose creation and maintenance is taken care of by students as well as the faculty of NCSU.It's code is available on Github [https://github.com/expertiza/expertiza Expertiza on GitHub].Expertiza allows the instructor to create and edit new as well as existing assignments.This also includes creating a wide range of topics under each assignment which students can sign up for.They can also publish surveys and reviews,view statistical results, set deadlines for assignments and make announcements.It provides a platform for students to signup for topics,form teams,view and submit assignments and give peer reviews and feedback.

=='''Stage Deadline Background'''==
An assignment in Expertiza has many deadlines possible: deadlines for submission and review in each round,deadlines for signing up and dropping topics,submitting meta-reviews,etc.Depending on what the next deadline is,an Expertiza assignment is said to be in a particular kind of what is called "stage".For example,if the next deadline(after the current time) is a submission deadline which is a DueDate object,then the assignment is said to be in a "submission" phase.

[[File: StageBackground.png|center]]

Depending on which stage an assignment is in, certain activities may be permissible or impermissible.For example, the default for a submission stage is to allow submission but not review,and in a review stage,the default is to allow review but not submission.These defaults may be changed by checking the relevant boxes on the Due Dates tab of assignment creation or editing.In the figure below it can be seen the "Other's work" link is grayed out as review is not permissible during submission phase.

[[File: Statusinadeadline.jpg|center]]

=='''Important Links'''==

[http://oss.flazzle.com/ Deployed_Project_Link]

[https://github.com/jwboykin/expertiza Expertiza_Link_for_this_Project]

=='''Problem Statement'''==
E1906 is an Expertiza based OSS project which deals basically with refactoring stage deadlines in assignment.rb file.Class assignment.rb has several methods for checking what kind of stage an assignment is in at the current time.These are:

* current_stage_name(topic_id = nil)
* find_current_stage(topic_id = nil)
* get_current_stage(topic_id = nil)
* link_for_current_stage(topic_id = nil)
* stage_deadline(topic_id = nil)

The goal of the project focuses on refactoring some of the above more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. This project is basically an attempt to make this part of the application easier to read and maintain.

=='''Solutions Implemented'''==

==='''Problem 1'''===

Multiple functions in the code were calling DueDate.get_next_due_date(self.id, topic_id).

===='''Solution'''====

A new private function next_due_date was added. Other calls to the DueDate.get_next_due_date were replaced.

<pre>
#returns the next due date
def next_due_date(topic_id = nil)
DueDate.get_next_due_date(self.id, topic_id)
end
</pre>

*One example of refactoring
[[File:Duedate E1906.png|center]]

==='''Problem 2'''===

The above five methods were being used at various places to check if the assignment is 'Finished' which made the code very redundant. Assignment is said to be finished if the next_due_date is nil.

===='''Solution'''====

We have created a new method "finished?" that checks if the next_due_date is nil (which means that the assignment is "finished").

<pre>
# New function to check if the assignment is finished
def finished?( topic_id = nil )
next_due_date(topic_id).nil?
end
</pre>

We have called this method at all the places where previously different methods were being used to check the same thing.

*Signup sheet table uses assignment finished? method instead of current_stage_name
[[File:Problem1_1.png‎|center]]

*Use assignment finished? method where only a comparison to "Finished" was being done using get_current_stage and find_current_stage
[[File:Problem1_2.png|center]] [[File:Problem1_3.png|center]]

==='''Problem 3'''===

The check for a topic_id to be nil when it is a staggered assignment and return 'Unknown' when it is true was being done at many places.This was not following Ruby and Rails rules.

===='''Solution'''====

To make the code DRY and more easy to understand,we have created a new private method "topic_missing?" to check if the topic_id is nil in case of staggered assignment.

<pre>
# Function to check if topic id is null when its a staggered assignment
def topic_missing?( topic_id = nil)
topic_id.nil? and self.staggered_deadline?
end
</pre>

*Refactoring with new method topic_missing?

[[File:Topic missing.png|center]]

==='''Problem 4'''===

find_current_stage(topic_id=nil) method is being used only locally in the assignment.rb and once in the student_teams controller to check to check whether to display or not the reviews option for team members.This method was calling another function to get the next_due_date for an assignment that was like nesting of calls and not very Ruby standard.

===='''Solution'''====

We have removed this function and replaced it with finished? method in the student_teams controller(shown in screenshot above in Problem 2) and with DueDate.get_next_due_date where the next due date was required.

<pre>
Before :
due_date = find_current_stage(topic_id)

After :
due_date = DueDate.get_next_due_date(self.id, topic_id)
</pre>

==='''Problem 5'''===

get_current_stage method is returning the stage name which the assignment is in at current time.This is not very clear from the name of the method.

===='''Solution'''====

We have changed the method name to current_stage_name(topic_id=nil) to make it more clear to understand.

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

*Refactored get_current_stage with current_stage_name

[[File:Current stage name new.png|center]]

==='''Problem 6'''===

current_stage_name(topic_id=nil)[This is the initial one] method was implementing things which was already being done by other methods and thus this method was redundant.
The initial code given was:

<pre>
def current_stage_name(topic_id = nil)
if self.staggered_deadline?
return (topic_id.nil? ? 'Unknown' : get_current_stage(topic_id))
end
due_date = find_current_stage(topic_id)

unless self.staggered_deadline?
if due_date != 'Finished' && !due_date.nil? && !due_date.deadline_name.nil?
return due_date.deadline_name
else
return get_current_stage(topic_id)
end
end
end
</pre>

First thing we noticed was that due_date.deadline_name was always nil because there was no field to enter the deadline name on the form, and therefore, the statement <pre> return due_date.deadline_name </pre> was never getting executed.

===='''Solution'''====

This method was checking for the due_date.deadline_name which was always nil. After removing the dead code, we get:

<pre>
def current_stage_name(topic_id = nil)
if topic_missing?(topic_id)
return 'Unknown'
else
get_current_stage(topic_id)
end
end
</pre>

Looking at the current_stage_name(topic_id=nil)[New one] method shown below and comparing it to what we have above, this code was surely redundant.That is why we removed this code and made appropriate changes wherever required within assignment.rb.
*Below is the current_stage_name method for reference

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

The associated rspec tests that tested the old current_stage_name method were also no longer needed.

==='''Problem 7'''===

link_for_current_stage function served no purpose. The function checked to see if any given due date had a link (presumably web URL) and would return it. The database had no values and there is no where in the application this URL could be provided. There doesn't appear to be any use for having a separate URL for each stage.

===='''Solution'''====

Removed function.

[[File:Big1.png|center]]

Removed test related to the function. Removed the one call to this function.

[[File:big2.png|center]]

=='''Automated Testing using RSPEC'''==

===='''Tests for get_current_stage_name'''====

The current version of expertiza did not have any tests for get_current_stage(topic_id=nil) method.Now as we have changed it to current_stage_name(topic_id=nil),we have added its tests as well.

<pre>
describe '#current_stage_name' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.current_stage_name).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns Finished' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.current_stage_name(123)).to eq('Finished')
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns current stage name' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:topic_missing?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
deadline = create(:deadline_type, id: 1, name:"Review")
allow(DeadlineType).to receive(:find).with(1).and_return(deadline)
expect(assignment.current_stage_name(123)).to eq('Review')
end
end
end
end
</pre>

===='''Tests for finished?'''====

We have added tests for the new method that we have added finished?.

<pre>
describe '#finished?' do
context 'when assignment next due date is nil' do
it 'returns True' do
allow(DueDate).to receive(:get_next_due_date).with(1, 123).and_return(nil)
expect(assignment.finished?(123)).to eq(true)
end
end
end

describe '#finished?' do
context 'when there is a next due date' do
it 'returns False' do
allow(DueDate).to receive(:get_next_due_date).with(1,123).and_return('2021-11-11 11:11:11')
expect(assignment.finished?(123)).to eq(false)
end
end
end
</pre>

As topic_missing? method is made private,there was no need to test this.

===='''Tests for stage_deadline'''====

We also have corrected the tests for stage_deadline according to the modifications done(It is using finished? and topic_missing? methods).

<pre>
describe '#stage_deadline' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.stage_deadline).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns nil' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.stage_deadline(123)).to be nil
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns due date' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
expect(assignment.stage_deadline(123)).to match('2011-11-11 11:11:11')
end
end
end
end
</pre>

=='''Team'''==
[mailto:jwboykin@ncsu.edu Jimmy Boykin]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:sbasnet2@ncsu.edu Sushan Basnet]

CSC/ECE 517 Spring 2019 - Project E1906. Refactor stage deadlines in Assignment.rb

2019-03-25T23:17:34Z

Rkaur:

This wiki page is for the description of changes made under E1906 OSS assignment for Spring 2019, CSC/ECE 517.

=='''Expertiza Background'''==
[http://expertiza.ncsu.edu/ Expertiza] is a web application developed using Ruby on Rails Framework whose creation and maintenance is taken care of by students as well as the faculty of NCSU.It's code is available on Github [https://github.com/expertiza/expertiza Expertiza on GitHub].Expertiza allows the instructor to create and edit new as well as existing assignments.This also includes creating a wide range of topics under each assignment which students can sign up for.They can also publish surveys and reviews,view statistical results, set deadlines for assignments and make announcements.It provides a platform for students to signup for topics,form teams,view and submit assignments and give peer reviews and feedback.

=='''Stage Deadline Background'''==
An assignment in Expertiza has many deadlines possible: deadlines for submission and review in each round,deadlines for signing up and dropping topics,submitting meta-reviews,etc.Depending on what the next deadline is,an Expertiza assignment is said to be in a particular kind of what is called "stage".For example,if the next deadline(after the current time) is a submission deadline which is a DueDate object,then the assignment is said to be in a "submission" phase.

[[File: StageBackground.png|center]]

Depending on which stage an assignment is in, certain activities may be permissible or impermissible.For example, the default for a submission stage is to allow submission but not review,and in a review stage,the default is to allow review but not submission.These defaults may be changed by checking the relevant boxes on the Due Dates tab of assignment creation or editing.In the figure below it can be seen the "Other's work" link is grayed out as review is not permissible during submission phase.

[[File: Statusinadeadline.jpg|center]]

=='''Important Links'''==

[http://oss.flazzle.com/ Deployed_Project_Link]

[https://github.com/jwboykin/expertiza Expertiza_Link_for_this_Project]

=='''Problem Statement'''==
E1906 is an Expertiza based OSS project which deals basically with refactoring stage deadlines in assignment.rb file.Class assignment.rb has several methods for checking what kind of stage an assignment is in at the current time.These are:

* current_stage_name(topic_id = nil)
* find_current_stage(topic_id = nil)
* get_current_stage(topic_id = nil)
* link_for_current_stage(topic_id = nil)
* stage_deadline(topic_id = nil)

The goal of the project focuses on refactoring some of the above more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. This project is basically an attempt to make this part of the application easier to read and maintain.

=='''Solutions Implemented'''==

==='''Problem 1'''===

Multiple functions in the code were calling DueDate.get_next_due_date(self.id, topic_id).

===='''Solution'''====

A new private function next_due_date was added. Other calls to the DueDate.get_next_due_date were replaced.

<pre>
#returns the next due date
def next_due_date(topic_id = nil)
DueDate.get_next_due_date(self.id, topic_id)
end
</pre>

*One example of refactoring
[[File:Duedate E1906.png|center]]

==='''Problem 2'''===

The above five methods were being used at various places to check if the assignment is 'Finished' which made the code very redundant. Assignment is said to be finished if the next_due_date is nil.

===='''Solution'''====

We have created a new method "finished?" that checks if the next_due_date is nil (which means that the assignment is "finished").

<pre>
# New function to check if the assignment is finished
def finished?( topic_id = nil )
next_due_date(topic_id).nil?
end
</pre>

We have called this method at all the places where previously different methods were being used to check the same thing.

*Signup sheet table uses assignment finished? method instead of current_stage_name
[[File:Problem1_1.png‎|center]]

*Use assignment finished? method where only a comparison to "Finished" was being done using get_current_stage and find_current_stage
[[File:Problem1_2.png|center]] [[File:Problem1_3.png|center]]

==='''Problem 3'''===

The check for a topic_id to be nil when it is a staggered assignment and return 'Unknown' when it is true was being done at many places.This was not following Ruby and Rails rules.

===='''Solution'''====

To make the code DRY and more easy to understand,we have created a new private method "topic_missing?" to check if the topic_id is nil in case of staggered assignment.

<pre>
# Function to check if topic id is null when its a staggered assignment
def topic_missing?( topic_id = nil)
topic_id.nil? and self.staggered_deadline?
end
</pre>

*Refactoring with new method topic_missing?

[[File:Topic missing.png|center]]

==='''Problem 4'''===

find_current_stage(topic_id=nil) method is being used only locally in the assignment.rb and once in the student_teams controller to check to check whether to display or not the reviews option for team members.This method was calling another function to get the next_due_date for an assignment that was like nesting of calls and not very Ruby standard.

===='''Solution'''====

We have removed this function and replaced it with finished? method in the student_teams controller(shown in screenshot above in Problem 2) and with DueDate.get_next_due_date where the next due date was required.

<pre>
Before :
due_date = find_current_stage(topic_id)

After :
due_date = DueDate.get_next_due_date(self.id, topic_id)
</pre>

==='''Problem 5'''===

get_current_stage method is returning the stage name which the assignment is in at current time.This is not very clear from the name of the method.

===='''Solution'''====

We have changed the method name to current_stage_name(topic_id=nil) to make it more clear to understand.

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

*Refactored get_current_stage with current_stage_name

[[File:Current stage name new.png|center]]

==='''Problem 6'''===

current_stage_name(topic_id=nil)[This is the initial one] method was implementing things which was already being done by other methods and thus this method was redundant.
The initial code given was:

<pre>
def current_stage_name(topic_id = nil)
if self.staggered_deadline?
return (topic_id.nil? ? 'Unknown' : get_current_stage(topic_id))
end
due_date = find_current_stage(topic_id)

unless self.staggered_deadline?
if due_date != 'Finished' && !due_date.nil? && !due_date.deadline_name.nil?
return due_date.deadline_name
else
return get_current_stage(topic_id)
end
end
end
</pre>

First thing we noticed was that due_date.deadline_name was always nil because there was no field to enter the deadline name on the form, and therefore, the statement <pre> return due_date.deadline_name </pre> was never getting executed.

===='''Solution'''====

This method was checking for the due_date.deadline_name which was always nil. After removing the dead code, we get:

<pre>
def current_stage_name(topic_id = nil)
if topic_missing?(topic_id)
return 'Unknown'
else
get_current_stage(topic_id)
end
end
</pre>

Looking at the current_stage_name(topic_id=nil)[New one] method shown below and comparing it to what we have above, this code was surely redundant.That is why we removed this code and made appropriate changes wherever required within assignment.rb.
*Below is the current_stage_name method for reference

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

The associated rspec tests that tested the old current_stage_name method were also no longer needed.

==='''Problem 7'''===

link_for_current_stage function served no purpose. The function checked to see if any given due date had a link (presumably web URL) and would return it. The database had no values and there is no where in the application this URL could be provided. There doesn't appear to be any use for having a separate URL for each stage.

===='''Solution'''====

Removed function.

[[File:Big1.png|center]]

Removed test related to the function. Removed the one call to this function.

[[File:big2.png|center]]

=='''Automated Testing using RSPEC'''==

===='''Tests for get_current_stage_name'''====

The current version of expertiza did not have any tests for get_current_stage(topic_id=nil) method.Now as we have changed it to current_stage_name(topic_id=nil),we have added its tests as well.

<pre>
describe '#current_stage_name' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.current_stage_name).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns Finished' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.current_stage_name(123)).to eq('Finished')
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns current stage name' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:topic_missing?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
deadline = create(:deadline_type, id: 1, name:"Review")
allow(DeadlineType).to receive(:find).with(1).and_return(deadline)
expect(assignment.current_stage_name(123)).to eq('Review')
end
end
end
end
</pre>

===='''Tests for finished?'''====

We have added tests for the new method that we have added finished?.

<pre>
describe '#finished?' do
context 'when assignment next due date is nil' do
it 'returns True' do
allow(DueDate).to receive(:get_next_due_date).with(1, 123).and_return(nil)
expect(assignment.finished?(123)).to eq(true)
end
end
end

describe '#finished?' do
context 'when there is a next due date' do
it 'returns False' do
allow(DueDate).to receive(:get_next_due_date).with(1,123).and_return('2021-11-11 11:11:11')
expect(assignment.finished?(123)).to eq(false)
end
end
end
</pre>

As topic_missing? method is made private,there was no need to test this.

===='''Tests for stage_deadline'''====

We also have corrected the tests for stage_deadline according to the modifications done(It is using finished? and topic_missing? methods).

<pre>
describe '#stage_deadline' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.stage_deadline).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns nil' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.stage_deadline(123)).to be nil
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns due date' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
expect(assignment.stage_deadline(123)).to match('2011-11-11 11:11:11')
end
end
end
end
</pre>

=='''Team'''==
[mailto:jwboykin@ncsu.edu Jimmy Boykin]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:sbasnet2@ncsu.edu Sushan Basnet]

CSC/ECE 517 Spring 2019 - Project E1906. Refactor stage deadlines in Assignment.rb

2019-03-25T23:13:28Z

Rkaur:

This wiki page is for the description of changes made under E1906 OSS assignment for Spring 2019, CSC/ECE 517.

=='''Expertiza Background'''==
[http://expertiza.ncsu.edu/ Expertiza] is a web application developed using Ruby on Rails Framework whose creation and maintenance is taken care of by students as well as the faculty of NCSU.It's code is available on Github [https://github.com/expertiza/expertiza Expertiza on GitHub].Expertiza allows the instructor to create and edit new as well as existing assignments.This also includes creating a wide range of topics under each assignment which students can sign up for.They can also publish surveys and reviews,view statistical results, set deadlines for assignments and make announcements.It provides a platform for students to signup for topics,form teams,view and submit assignments and give peer reviews and feedback.

=='''Stage Deadline Background'''==
An assignment in Expertiza has many deadlines possible: deadlines for submission and review in each round,deadlines for signing up and dropping topics,submitting meta-reviews,etc.Depending on what the next deadline is,an Expertiza assignment is said to be in a particular kind of what is called "stage".For example,if the next deadline(after the current time) is a submission deadline which is a DueDate object,then the assignment is said to be in a "submission" phase.

[[File: StageBackground.png|center]]

Depending on which stage an assignment is in, certain activities may be permissible or impermissible.For example, the default for a submission stage is to allow submission but not review,and in a review stage,the default is to allow review but not submission.These defaults may be changed by checking the relevant boxes on the Due Dates tab of assignment creation or editing.In the figure below it can be seen the "Other's work" link is grayed out as review is not permissible during submission phase.

[[File: Statusinadeadline.jpg|center]]

=='''Important Links'''==

[http://oss.flazzle.com/ Deployed_Project_Link]

[https://github.com/jwboykin/expertiza Expertiza_Link_for_this_Project]

=='''Problem Statement'''==
E1906 is an Expertiza based OSS project which deals basically with refactoring stage deadlines in assignment.rb file.Class assignment.rb has several methods for checking what kind of stage an assignment is in at the current time.These are:

* current_stage_name(topic_id = nil)
* find_current_stage(topic_id = nil)
* get_current_stage(topic_id = nil)
* link_for_current_stage(topic_id = nil)
* stage_deadline(topic_id = nil)

The goal of the project focuses on refactoring some of the above more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. This project is basically an attempt to make this part of the application easier to read and maintain.

=='''Solutions Implemented'''==

==='''Problem 1'''===

Multiple functions in the code were calling DueDate.get_next_due_date(self.id, topic_id).

===='''Solution'''====

A new private function next_due_date was added. Other calls to the DueDate.get_next_due_date were replaced.

<pre>
#returns the next due date
def next_due_date(topic_id = nil)
DueDate.get_next_due_date(self.id, topic_id)
end
</pre>

*One example of refactoring
[[File:Duedate E1906.png|center]]

==='''Problem 2'''===

The above five methods were being used at various places to check if the assignment is 'Finished' which made the code very redundant. Assignment is said to be finished if the next_due_date is nil.

===='''Solution'''====

We have created a new method "finished?" that checks if the next_due_date is nil (which means that the assignment is "finished").

<pre>
# New function to check if the assignment is finished
def finished?( topic_id = nil )
next_due_date(topic_id).nil?
end
</pre>

We have called this method at all the places where previously different methods were being used to check the same thing.

*Signup sheet table uses assignment finished? method instead of current_stage_name
[[File:Problem1_1.png‎|center]]

*Use assignment finished? method where only a comparison to "Finished" was being done using get_current_stage and find_current_stage
[[File:Problem1_2.png|center]] [[File:Problem1_3.png|center]]

==='''Problem 3'''===

The check for a topic_id to be nil when it is a staggered assignment and return 'Unknown' when it is true was being done at many places.This was not following Ruby and Rails rules.

===='''Solution'''====

To make the code DRY and more easy to understand,we have created a new private method "topic_missing?" to check if the topic_id is nil in case of staggered assignment.

<pre>
# Function to check if topic id is null when its a staggered assignment
def topic_missing?( topic_id = nil)
topic_id.nil? and self.staggered_deadline?
end
</pre>

*Refactoring with new method topic_missing?

[[File:Topic missing.png|center]]

==='''Problem 4'''===

find_current_stage(topic_id=nil) method is being used only locally in the assignment.rb and once in the student_teams controller to check to check whether to display or not the reviews option for team members.This method was calling another function to get the next_due_date for an assignment that was like nesting of calls and not very Ruby standard.

===='''Solution'''====

We have removed this function and replaced it with finished? method in the student_teams controller(shown in screenshot above in Problem 2) and with DueDate.get_next_due_date where the next due date was required.

<pre>
Before :
due_date = find_current_stage(topic_id)

After :
due_date = DueDate.get_next_due_date(self.id, topic_id)
</pre>

==='''Problem 5'''===

get_current_stage method is returning the stage name which the assignment is in at current time.This is not very clear from the name of the method.

===='''Solution'''====

We have changed the method name to current_stage_name(topic_id=nil) to make it more clear to understand.

<pre>
#Gives the current stage name of the assignment
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

*Refactored get_current_stage with current_stage_name

[[File:Current stage name new.png|center]]

==='''Problem 6'''===

current_stage_name(topic_id=nil)[This is the initial one] method was implementing things which was already being done by other methods and thus this method was redundant.
The initial code given was:

<pre>
def current_stage_name(topic_id = nil)
if self.staggered_deadline?
return (topic_id.nil? ? 'Unknown' : get_current_stage(topic_id))
end
due_date = find_current_stage(topic_id)

unless self.staggered_deadline?
if due_date != 'Finished' && !due_date.nil? && !due_date.deadline_name.nil?
return due_date.deadline_name
else
return get_current_stage(topic_id)
end
end
end
</pre>

First thing we noticed was that due_date.deadline_name was always nil because there was no field to enter the deadline name on the form, and therefore, the statement <pre> return due_date.deadline_name </pre> was never getting executed.

===='''Solution'''====

This method was checking for the due_date.deadline_name which was always nil. After removing the dead code, we get:

<pre>
def current_stage_name(topic_id = nil)
if topic_missing?(topic_id)
return 'Unknown'
else
get_current_stage(topic_id)
end
end
</pre>

Looking at the get_current_stage_name(topic_id=nil) method shown below and comparing it to what we have above, this code was surely redundant.That is why we removed this code and made appropriate changes wherever required within assignment.rb.
*Below is the get_current_stage_name method for reference

<pre>
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = DueDate.get_next_due_date(self.id, topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

The associated rspec tests that tested the current_stage method were also no longer needed.

==='''Problem 7'''===

link_for_current_stage function served no purpose. The function checked to see if any given due date had a link (presumably web URL) and would return it. The database had no values and there is no where in the application this URL could be provided. There doesn't appear to be any use for having a separate URL for each stage.

===='''Solution'''====

Removed function.

[[File:Big1.png|center]]

Removed test related to the function. Removed the one call to this function.

[[File:big2.png|center]]

=='''Automated Testing using RSPEC'''==

===='''Tests for get_current_stage_name'''====

The current version of expertiza did not have any tests for get_current_stage(topic_id=nil) method.Now as we have changed it to current_stage_name(topic_id=nil),we have added its tests as well.

<pre>
describe '#current_stage_name' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.current_stage_name).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns Finished' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.current_stage_name(123)).to eq('Finished')
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns current stage name' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:topic_missing?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
deadline = create(:deadline_type, id: 1, name:"Review")
allow(DeadlineType).to receive(:find).with(1).and_return(deadline)
expect(assignment.current_stage_name(123)).to eq('Review')
end
end
end
end
</pre>

===='''Tests for finished?'''====

We have added tests for the new method that we have added finished?.

<pre>
describe '#finished?' do
context 'when assignment next due date is nil' do
it 'returns True' do
allow(DueDate).to receive(:get_next_due_date).with(1, 123).and_return(nil)
expect(assignment.finished?(123)).to eq(true)
end
end
end

describe '#finished?' do
context 'when there is a next due date' do
it 'returns False' do
allow(DueDate).to receive(:get_next_due_date).with(1,123).and_return('2021-11-11 11:11:11')
expect(assignment.finished?(123)).to eq(false)
end
end
end
</pre>

As topic_missing? method is made private,there was no need to test this.

===='''Tests for stage_deadline'''====

We also have corrected the tests for stage_deadline according to the modifications done(It is using finished? and topic_missing? methods).

<pre>
describe '#stage_deadline' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.stage_deadline).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns nil' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.stage_deadline(123)).to be nil
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns due date' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
expect(assignment.stage_deadline(123)).to match('2011-11-11 11:11:11')
end
end
end
end
</pre>

=='''Team'''==
[mailto:jwboykin@ncsu.edu Jimmy Boykin]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:sbasnet2@ncsu.edu Sushan Basnet]

CSC/ECE 517 Spring 2019 - Project E1906. Refactor stage deadlines in Assignment.rb

2019-03-25T23:09:30Z

Rkaur:

This wiki page is for the description of changes made under E1906 OSS assignment for Spring 2019, CSC/ECE 517.

=='''Expertiza Background'''==
[http://expertiza.ncsu.edu/ Expertiza] is a web application developed using Ruby on Rails Framework whose creation and maintenance is taken care of by students as well as the faculty of NCSU.It's code is available on Github [https://github.com/expertiza/expertiza Expertiza on GitHub].Expertiza allows the instructor to create and edit new as well as existing assignments.This also includes creating a wide range of topics under each assignment which students can sign up for.They can also publish surveys and reviews,view statistical results, set deadlines for assignments and make announcements.It provides a platform for students to signup for topics,form teams,view and submit assignments and give peer reviews and feedback.

=='''Stage Deadline Background'''==
An assignment in Expertiza has many deadlines possible: deadlines for submission and review in each round,deadlines for signing up and dropping topics,submitting meta-reviews,etc.Depending on what the next deadline is,an Expertiza assignment is said to be in a particular kind of what is called "stage".For example,if the next deadline(after the current time) is a submission deadline which is a DueDate object,then the assignment is said to be in a "submission" phase.

[[File: StageBackground.png|center]]

Depending on which stage an assignment is in, certain activities may be permissible or impermissible.For example, the default for a submission stage is to allow submission but not review,and in a review stage,the default is to allow review but not submission.These defaults may be changed by checking the relevant boxes on the Due Dates tab of assignment creation or editing.In the figure below it can be seen the "Other's work" link is grayed out as review is not permissible during submission phase.

[[File: Statusinadeadline.jpg|center]]

=='''Important Links'''==

[http://oss.flazzle.com/ Deployed_Project_Link]

[https://github.com/jwboykin/expertiza Expertiza_Link_for_this_Project]

=='''Problem Statement'''==
E1906 is an Expertiza based OSS project which deals basically with refactoring stage deadlines in assignment.rb file.Class assignment.rb has several methods for checking what kind of stage an assignment is in at the current time.These are:

* current_stage_name(topic_id = nil)
* find_current_stage(topic_id = nil)
* get_current_stage(topic_id = nil)
* link_for_current_stage(topic_id = nil)
* stage_deadline(topic_id = nil)

The goal of the project focuses on refactoring some of the above more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. This project is basically an attempt to make this part of the application easier to read and maintain.

=='''Solutions Implemented'''==

==='''Problem 1'''===

Multiple functions in the code were calling DueDate.get_next_due_date(self.id, topic_id).

===='''Solution'''====

A new private function next_due_date was added. Other calls to the DueDate.get_next_due_date were replaced.

<pre>
#returns the next due date
def next_due_date(topic_id = nil)
DueDate.get_next_due_date(self.id, topic_id)
end
</pre>

*One example of refactoring
[[File:Duedate E1906.png|center]]

==='''Problem 2'''===

The above five methods were being used at various places to check if the assignment is 'Finished' which made the code very redundant. Assignment is said to be finished if the next_due_date is nil.

===='''Solution'''====

We have created a new method "finished?" that checks if the next_due_date is nil (which means that the assignment is "finished").

<pre>
# New function to check if the assignment is finished
def finished?( topic_id = nil )
next_due_date(topic_id).nil?
end
</pre>

We have called this method at all the places where previously different methods were being used to check the same thing.

*Signup sheet table uses assignment finished? method instead of current_stage_name
[[File:Problem1_1.png‎|center]]

*Use assignment finished? method where only a comparison to "Finished" was being done using get_current_stage and find_current_stage
[[File:Problem1_2.png|center]] [[File:Problem1_3.png|center]]

==='''Problem 3'''===

The check for a topic_id to be nil when it is a staggered assignment and return 'Unknown' when it is true was being done at many places.This was not following Ruby and Rails rules.

===='''Solution'''====

To make the code DRY and more easy to understand,we have created a new private method "topic_missing?" to check if the topic_id is nil in case of staggered assignment.

<pre>
# Function to check if topic id is null when its a staggered assignment
def topic_missing?( topic_id = nil)
topic_id.nil? and self.staggered_deadline?
end
</pre>

*Refactoring with new method topic_missing?

[[File:Topic missing.png|center]]

==='''Problem 4'''===

find_current_stage(topic_id=nil) method is being used only locally in the assignment.rb and once in the student_teams controller to check to check whether to display or not the reviews option for team members.This method was calling another function to get the next_due_date for an assignment that was like nesting of calls and not very Ruby standard.

===='''Solution'''====

We have removed this function and replaced it with finished? method in the student_teams controller(shown in screenshot above in Problem 2) and with DueDate.get_next_due_date where the next due date was required.

<pre>
Before :
due_date = find_current_stage(topic_id)

After :
due_date = DueDate.get_next_due_date(self.id, topic_id)
</pre>

==='''Problem 5'''===

get_current_stage method is returning the stage name which the assignment is in at current time.This is not very clear from the name of the method.

===='''Solution'''====

We have changed the method name to current_stage_name(topic_id=nil) to make it more clear to understand.

<pre>
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = next_due_date(topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

*Refactored get_current_stage with current_stage_name

[[File:Current stage name new.png|center]]

==='''Problem 6'''===

current_stage_name(topic_id=nil)[This is the initial one] method was implementing things which was already being done by other methods and thus this method was redundant.
The initial code given was:

<pre>
def current_stage_name(topic_id = nil)
if self.staggered_deadline?
return (topic_id.nil? ? 'Unknown' : get_current_stage(topic_id))
end
due_date = find_current_stage(topic_id)

unless self.staggered_deadline?
if due_date != 'Finished' && !due_date.nil? && !due_date.deadline_name.nil?
return due_date.deadline_name
else
return get_current_stage(topic_id)
end
end
end
</pre>

First thing we noticed was that due_date.deadline_name was always nil because there was no field to enter the deadline name on the form, and therefore, the statement <pre> return due_date.deadline_name </pre> was never getting executed.

===='''Solution'''====

This method was checking for the due_date.deadline_name which was always nil. After removing the dead code, we get:

<pre>
def current_stage_name(topic_id = nil)
if topic_missing?(topic_id)
return 'Unknown'
else
get_current_stage(topic_id)
end
end
</pre>

Looking at the get_current_stage_name(topic_id=nil) method shown below and comparing it to what we have above, this code was surely redundant.That is why we removed this code and made appropriate changes wherever required within assignment.rb.
*Below is the get_current_stage_name method for reference

<pre>
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = DueDate.get_next_due_date(self.id, topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

The associated rspec tests that tested the current_stage method were also no longer needed.

==='''Problem 7'''===

link_for_current_stage function served no purpose. The function checked to see if any given due date had a link (presumably web URL) and would return it. The database had no values and there is no where in the application this URL could be provided. There doesn't appear to be any use for having a separate URL for each stage.

===='''Solution'''====

Removed function.

[[File:Big1.png|center]]

Removed test related to the function. Removed the one call to this function.

[[File:big2.png|center]]

=='''Automated Testing using RSPEC'''==

===='''Tests for get_current_stage_name'''====

The current version of expertiza did not have any tests for get_current_stage(topic_id=nil) method.Now as we have changed it to current_stage_name(topic_id=nil),we have added its tests as well.

<pre>
describe '#current_stage_name' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.current_stage_name).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns Finished' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.current_stage_name(123)).to eq('Finished')
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns current stage name' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:topic_missing?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
deadline = create(:deadline_type, id: 1, name:"Review")
allow(DeadlineType).to receive(:find).with(1).and_return(deadline)
expect(assignment.current_stage_name(123)).to eq('Review')
end
end
end
end
</pre>

===='''Tests for finished?'''====

We have added tests for the new method that we have added finished?.

<pre>
describe '#finished?' do
context 'when assignment next due date is nil' do
it 'returns True' do
allow(DueDate).to receive(:get_next_due_date).with(1, 123).and_return(nil)
expect(assignment.finished?(123)).to eq(true)
end
end
end

describe '#finished?' do
context 'when there is a next due date' do
it 'returns False' do
allow(DueDate).to receive(:get_next_due_date).with(1,123).and_return('2021-11-11 11:11:11')
expect(assignment.finished?(123)).to eq(false)
end
end
end
</pre>

As topic_missing? method is made private,there was no need to test this.

===='''Tests for stage_deadline'''====

We also have corrected the tests for stage_deadline according to the modifications done(It is using finished? and topic_missing? methods).

<pre>
describe '#stage_deadline' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.stage_deadline).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns nil' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.stage_deadline(123)).to be nil
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns due date' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
expect(assignment.stage_deadline(123)).to match('2011-11-11 11:11:11')
end
end
end
end
</pre>

=='''Team'''==
[mailto:jwboykin@ncsu.edu Jimmy Boykin]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:sbasnet2@ncsu.edu Sushan Basnet]

CSC/ECE 517 Spring 2019 - Project E1906. Refactor stage deadlines in Assignment.rb

2019-03-25T22:57:34Z

Rkaur:

This wiki page is for the description of changes made under E1906 OSS assignment for Spring 2019, CSC/ECE 517.

=='''Expertiza Background'''==
[http://expertiza.ncsu.edu/ Expertiza] is a web application developed using Ruby on Rails Framework whose creation and maintenance is taken care of by students as well as the faculty of NCSU.It's code is available on Github [https://github.com/expertiza/expertiza Expertiza on GitHub].Expertiza allows the instructor to create and edit new as well as existing assignments.This also includes creating a wide range of topics under each assignment which students can sign up for.They can also publish surveys and reviews,view statistical results, set deadlines for assignments and make announcements.It provides a platform for students to signup for topics,form teams,view and submit assignments and give peer reviews and feedback.

=='''Stage Deadline Background'''==
An assignment in Expertiza has many deadlines possible: deadlines for submission and review in each round,deadlines for signing up and dropping topics,submitting meta-reviews,etc.Depending on what the next deadline is,an Expertiza assignment is said to be in a particular kind of what is called "stage".For example,if the next deadline(after the current time) is a submission deadline which is a DueDate object,then the assignment is said to be in a "submission" phase.

[[File: StageBackground.png|center]]

Depending on which stage an assignment is in, certain activities may be permissible or impermissible.For example, the default for a submission stage is to allow submission but not review,and in a review stage,the default is to allow review but not submission.These defaults may be changed by checking the relevant boxes on the Due Dates tab of assignment creation or editing.In the figure below it can be seen the "Other's work" link is grayed out as review is not permissible during submission phase.

[[File: Statusinadeadline.jpg|center]]

=='''Important Links'''==

[http://oss.flazzle.com/ Deployed_Project_Link]

[https://github.com/jwboykin/expertiza Expertiza_Link_for_this_Project]

=='''Problem Statement'''==
E1906 is an Expertiza based OSS project which deals basically with refactoring stage deadlines in assignment.rb file.Class assignment.rb has several methods for checking what kind of stage an assignment is in at the current time.These are:

* current_stage_name(topic_id = nil)
* find_current_stage(topic_id = nil)
* get_current_stage(topic_id = nil)
* link_for_current_stage(topic_id = nil)
* stage_deadline(topic_id = nil)

The goal of the project focuses on refactoring some of the above more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. This project is basically an attempt to make this part of the application easier to read and maintain.

=='''Solutions Implemented'''==

==='''Problem 1'''===

Multiple functions in the code were calling DueDate.get_next_due_date(self.id, topic_id).

===='''Solution'''====

A new private function next_due_date was added. Other calls to the DueDate.get_next_due_date were replaced.

<pre>
#returns the next due date
def next_due_date(topic_id = nil)
DueDate.get_next_due_date(self.id, topic_id)
end
</pre>

*One example of refactoring
[[File:Duedate E1906.png|center]]

==='''Problem 2'''===

The above five methods were being used at various places to check if the assignment is 'Finished' which made the code very redundant. Assignment is said to be finished if the next_due_date is nil.

===='''Solution'''====

We have created a new method "finished?" that checks if the next_due_date is nil (which means that the assignment is "finished").

<pre>
# New function to check if the assignment is finished
def finished?( topic_id = nil )
next_due_date(topic_id).nil?
end
</pre>

We have called this method at all the places where previously different methods were being used to check the same thing.

*Signup sheet table uses assignment finished? method instead of current_stage_name
[[File:Problem1_1.png‎|center]]

*Use assignment finished? method where only a comparison to "Finished" was being done using get_current_stage and find_current_stage
[[File:Problem1_2.png|center]] [[File:Problem1_3.png|center]]

==='''Problem 3'''===

The check for a topic_id to be nil when it is a staggered assignment and return 'Unknown' when it is true was being done at many places.This was not following Ruby and Rails rules.

===='''Solution'''====

To make the code DRY and more easy to understand,we have created a new private method "topic_missing?" to check if the topic_id is nil in case of staggered assignment.

<pre>
# Function to check if topic id is null when its a staggered assignment
def topic_missing?( topic_id = nil)
topic_id.nil? and self.staggered_deadline?
end
</pre>

*Refactoring with new method topic_missing?

[[File:Topic missing.png|center]]

==='''Problem 4'''===

find_current_stage(topic_id=nil) method is being used only locally in the assignment.rb and once in the student_teams controller to check to check whether to display or not the reviews option for team members.This method was calling another function to get the next_due_date for an assignment that was like nesting of calls and not very Ruby standard.

===='''Solution'''====

We have removed this function.We have replaced it with finished? method in the student_teams controller(shown in screenshot above in Problem 1) and with DueDate.get_next_due_date where the next due date was required.

<pre>
Before :
due_date = find_current_stage(topic_id)

After :
due_date = DueDate.get_next_due_date(self.id, topic_id)
</pre>

==='''Problem 5'''===

get_current_stage method is returning the stage name which the assignment is in at current time.This is not very clear from the name of the method.

===='''Solution'''====

We have changed the method name to current_stage_name(topic_id=nil) to make it more clear to understand.

<pre>
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = DueDate.get_next_due_date(self.id, topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

*Refactored get_current_stage with current_stage_name

[[File:Current stage name new.png|center]]

==='''Problem 6'''===

current_stage_name(topic_id=nil)[This is the initial one] method was implementing things which was already being done by other methods and thus this method was redundant.
The initial code given was:

<pre>
def current_stage_name(topic_id = nil)
if self.staggered_deadline?
return (topic_id.nil? ? 'Unknown' : get_current_stage(topic_id))
end
due_date = find_current_stage(topic_id)

unless self.staggered_deadline?
if due_date != 'Finished' && !due_date.nil? && !due_date.deadline_name.nil?
return due_date.deadline_name
else
return get_current_stage(topic_id)
end
end
end
</pre>

First thing we noticed was that due_date.deadline_name was always nil because there was no field to enter the deadline name on the form, and therefore, the statement <pre> return due_date.deadline_name </pre> was never getting executed.

===='''Solution'''====

This method was checking for the due_date.deadline_name which was always nil. After removing the dead code, we get:

<pre>
def current_stage_name(topic_id = nil)
if topic_missing?(topic_id)
return 'Unknown'
else
get_current_stage(topic_id)
end
end
</pre>

Looking at the get_current_stage_name(topic_id=nil) method shown below and comparing it to what we have above, this code was surely redundant.That is why we removed this code and made appropriate changes wherever required within assignment.rb.
*Below is the get_current_stage_name method for reference

<pre>
def current_stage_name(topic_id = nil)
return 'Unknown' if topic_missing?(topic_id)
due_date = DueDate.get_next_due_date(self.id, topic_id)
finished?(topic_id)? "Finished" : DeadlineType.find(due_date.deadline_type_id).name
end
</pre>

The associated rspec tests that tested the current_stage method were also no longer needed.

==='''Problem 7'''===

link_for_current_stage function served no purpose. The function checked to see if any given due date had a link (presumably web URL) and would return it. The database had no values and there is no where in the application this URL could be provided. There doesn't appear to be any use for having a separate URL for each stage.

===='''Solution'''====

Removed function.

[[File:Big1.png|center]]

Removed test related to the function. Removed the one call to this function.

[[File:big2.png|center]]

=='''Automated Testing using RSPEC'''==

===='''Tests for get_current_stage_name'''====

The current version of expertiza did not have any tests for get_current_stage(topic_id=nil) method.Now as we have changed it to current_stage_name(topic_id=nil),we have added its tests as well.

<pre>
describe '#current_stage_name' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.current_stage_name).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns Finished' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.current_stage_name(123)).to eq('Finished')
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns current stage name' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:topic_missing?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
deadline = create(:deadline_type, id: 1, name:"Review")
allow(DeadlineType).to receive(:find).with(1).and_return(deadline)
expect(assignment.current_stage_name(123)).to eq('Review')
end
end
end
end
</pre>

===='''Tests for finished?'''====

We have added tests for the new method that we have added finished?.

<pre>
describe '#finished?' do
context 'when assignment next due date is nil' do
it 'returns True' do
allow(DueDate).to receive(:get_next_due_date).with(1, 123).and_return(nil)
expect(assignment.finished?(123)).to eq(true)
end
end
end

describe '#finished?' do
context 'when there is a next due date' do
it 'returns False' do
allow(DueDate).to receive(:get_next_due_date).with(1,123).and_return('2021-11-11 11:11:11')
expect(assignment.finished?(123)).to eq(false)
end
end
end
</pre>

As topic_missing? method is made private,there was no need to test this.

===='''Tests for stage_deadline'''====

We also have corrected the tests for stage_deadline according to the modifications done(It is using finished? and topic_missing? methods).

<pre>
describe '#stage_deadline' do
context 'when topic id is nil and current assignment has staggered deadline' do
it 'returns Unknown' do
allow(assignment).to receive(:topic_missing?).and_return(true)
expect(assignment.stage_deadline).to eq('Unknown')
end
end

context 'when current assignment does not have staggered deadline' do
context 'when due date is nil' do
it 'returns nil' do
allow(assignment).to receive(:finished?).with(123).and_return(true)
expect(assignment.stage_deadline(123)).to be nil
end
end

context 'when due date is not nil and due date is not equal to Finished' do
it 'returns due date' do
allow(assignment).to receive(:finished?).with(123).and_return(false)
allow(assignment).to receive(:next_due_date).with(123).and_return(assignment_due_date)
expect(assignment.stage_deadline(123)).to match('2011-11-11 11:11:11')
end
end
end
end
</pre>

=='''Team'''==
[mailto:jwboykin@ncsu.edu Jimmy Boykin]<br>
[mailto:rkaur@ncsu.edu Ramandeep Kaur]<br>
[mailto:sbasnet2@ncsu.edu Sushan Basnet]