Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Spring 2016 E1631 Team-based reviewing

2016-04-12T02:57:11Z

Kvysyar: /* RSpec */

== Purpose ==

When a participant of a team reviewed an assignment, his/her review is independent of his teammate’s reviews. To allow teammates to discuss and review together, allowing teams to submit reviews for the team as a whole should be allowed ideally. We intend to do this in our project.

*Currently all reviews in Expertiza are done by individuals. This is true regardless of whether the assignment is done by individuals or teams .
*There are occasions when it's advantageous to review projects as a team .
*It helps foster discussion and thereby improves the process of learning.

== Task Description ==
This project comprises of the following steps :

# Objects of ResponseMap record the information of who reviews whom. The field reviewee_id refers to the team who is being reviewed. The field reviewer_id refers to the individual/team performing the review. We added a boolean field "reviewer_is_team" to identify if the review is performed by a team or an individual.
# If reviewer_is_team is true, the reviewer_id will be a reference to the Teams table.
# For an Instructor to specify whether the review is a Team/Individual based review, we will be providing a dropdown on the Review Strategy tab of assignment creation.
# Ensure that features such as "view my scores", or the "alternate view/heat map" should continue working.
# Concurrency control: Shouldn't allow multiple teammates to edit the team's review at the same time. The scope of the project doesn't include concurrent editing to allow multiple teammates to edit a review simultaneously.

== Design ==

==== Design Pattern ====
'''Model View Controller'''

In an MVC model, a software application is divided into three major components: Model, View and Controller.

'''Model''' - Keeps data, logic and relations between objects and the database. It also handles validations, associations and transactions.

'''View''' - Displays the data received from the controller in the user interface.

'''Controller''' - Accepts the client's input and converts it into action points for the model or view. Other responsibilities include querying models and organizing data in a structure that is displayed by the view.

==== Use Case Diagram ====

'''Name:''' Specify review type on Review Strategy tab.

'''Actor:''' Instructor.

'''Description:''' The instructor specifies whether this review is an individual review or a team based review on the Review Strategy tab.

'''Name:''' Perform Review.

'''Actor:''' Individual Student/Team.

'''Description:''' The individual/team will perform a review for the Assignment.

[[File:OODDUseCase.jpg]]

=== Database Design ===

ResponseMap in essence, records who reviews whom. Schema of ResponseMap:

{| class="wikitable"
|-
! Field
! Data type
|-
| id
| int(11)
|-
| reviewed_object_id
| int(11)
|-
| reviewer_id
| int(11)
|-
| reviewee_id
| int(11)
|-
| type
| varchar(255)
|-
| created_at
| datetime
|-
| updated_at
| datetime
|-
| caliberate_to
| boolean
|-
| reviewer_is_team
| boolean
|}

The description of the fields of the database for ResponseMap:

1. '''id:''' The unique record identifier.

2. '''reviewed_object_id:''' The id of the object that is reviewed. Assignments or ReviewMaps could be reviewed.

3. '''reviewer_id:''' The reviewer can either be an “AssignmentTeam” or “AssignmentParticipant”, which is indicated by the field reviwer_is_team.

4. '''reviewee_id:''' The id “AssignmentTeam” who is getting the “Response” i.e., the one whose work is being reviewed.

5. '''type :''' Indicates the type of the ResponseMap.

6. '''created_at:''' The timestamp when the Response was created.

7. '''updated_at:''' The timestamp when the Response was updated.

8. '''calibrate_to:''' A field of boolean data type.

9. '''reviewer_is_team:''' If this field is ‘true’, the reviewer_id refers to the id of the “AssignmentTeam” to which the participant belongs. Else, it refers to the id of the “Participant” itself.

===Implementation===

Below are the key files to be modified:
====Models====
* '''review_response_map.rb:''' Add a field reviewer_is_team , which determines if the reviewer is doing an individual review or on behalf of the team.
* '''assignment.rb:''' Add a field review_type , that indicates if the assignment is a team based or an individual assignment decided by the instructor.

====Views====
* '''assignments/edit/_review_report.html.erb:''' Add a dropdown for the instructors to select if the reviews are individual or team-based.

====Controllers====
* review_mapping_controller.rb
* participants_controller.rb

== Testing Details ==

=== RSpec ===
RSpec is a testing framework for Rails, and is a Behavioral-Driven Development tool. It is a domain specific language(DSL). All the tests can be executed by rspec spec command, or can also be executed individually using the command "rspec spec/models/assignment_team_spec.rb".

We intend to write unit tests using RSpec for all the methods which we modified/created.

===Factory Girl===
Factory Girl is used for creating Assignment and Team objects to be used for testing. Factory Girl is a replacement for fixtures. Fixtures have to be updated whenever we change a data model whereas adding and removing fields is much easier in Factory Girl. Fixture definitions are global whereas Factories can be local, so isolated cases can be tested. Factories are defined to create objects for testing.

===Running RSpec===
<code>
$ rspec spec/models/assignment_team_spec.rb
</code>

CSC/ECE 517 Spring 2016 E1631 Team-based reviewing

2016-04-11T23:09:12Z

Kvysyar: /* Task Description */

CSC/ECE 517 Spring 2016 E1631 Team-based reviewing

2016-04-11T23:08:42Z

Kvysyar: /* Task Description update */

CSC/ECE 517 Spring 2016 E1631 Team-based reviewing

2016-04-03T00:18:48Z

Kvysyar: /* Purpose */

CSC/ECE 517 Spring 2016/OSS E1601

2016-04-02T04:06:11Z

Kvysyar: /* Example RSpec */

=== E1601: Refactor methods related to submitted_hyperlinks. ===

This wiki is for the open source refactoring project E1601: Refactor methods related to submitted_hyperlinks for the course ECE/CSC 517.

== Peer Review Information ==

The reviewers may use the below student and instructor logins for the purpose of reviewing and testing the functionality. These students belong to the same team and work on an assignment created by the below instructor:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student5884, password -> password
* Student login: username -> student6420, password -> password
'''Please note''': Please do not delete the above users or their team. If you wish to do so, please add them back so as to aid other reviewers. Also note that the option to submit hyperlinks in the UI will be displayed to the enrolled students only if assignment deadline is in future and the assignment is not in "Finish" state.

== Expertiza Introduction==

Expertiza is an open source web application which enhances the student's learning through peer reviews. The students can form teams and work on assignments given by course instructors. The peers can review the submissions and give feedback. The students also get to review the reviewers!

== Problem statement ==

'''Before''': When a student submitted files and hyperlinks for an assignment, that content was submitted on behalf of his team, and not individually (even if there is only one person in the team i.e a student working on an assignment cannot be without a team even if he is working alone). Some of the previous contributors had worked on a project which made this change and associated all the submitted content to teams instead of the individual participants: the submitted_hyperlinks and dirctory_num fields were moved to teams table. All the hyperlinks were now recorded in one submitted_hyperlinks field in the team model instead of being present in all the participants of the team. All the submitted files similarly were uploaded to the common file space and in the “course_folder/assignment_folder/team_folder” path.

'''What was wrong with that''': There are multiple methods in assignment_participant.rb which still give the impression that the author is submitting the hyperlinks on behalf of himself and not the team even though the relevant fields have been moved to teams model. These methods are to be moved to assignment_teams.rb for improving the clarity and the efficiency of code.

'''What refactoring we have done to improve the code''':

''Task1::'' Removed “hyperlinks_array” method and “hyperlinks” method from assignment_participant.rb. Created “hyperlinks” method in assignment_team.rb.

''Task2::'' Removed “submit_hyperlink” and “remove_hyperlink” from assignment_pariticpant.rb and created equivalent methods in assignment_team.rb.

''Task3::'' Removed “has_submissions?” from assginment_participant.rb and created the equivalent method in assignment_team.rb.

''Task4::'' All the calls to the above methods were refactored to call the appropriate method's functions.

''Task5::'' Refactored some of the logic of controller methods.

''Task6::'' Tests for submitting and removing submitted hyperlinks and files.

== List of files changed ==

Model Files:

#assignment_participant.rb
#assignment_team.rb
#student_task.rb

View Files:

#submitted_content/_hyperlink.html.erb
#assignments/edit/_calibration.html.erb
#assignments/list_submissions.html.erb
#submitted_content/_main.html.erb

Controller Files:

#sign_up_sheet_controller.rb
#submitted_content_controller.rb

Files with major modifications:

#The model: assignment_participant.rb
#The model: assignment_team.rb
#The controller: submitted_content_controller.rb

== Key changes ==

Four methods <code>hyperlinks</code>, <code>has_submissions?</code>, <code>submit_hyperlink</code> and <code>remove_hyperlink</code> were moved from the assignment_participant.rb model to assignment_team.rb, and the <code>hyperlinks_array</code> method was removed. 

'''Task1::''' The below methods <code>hyperlinks_array</code> and <code>hyperlinks</code> were removed from the assignment_participant.rb. The <code>hyperlinks</code> method which existed here seemed suspect as there was no <code>hyperlinks</code> method in the AssignmentTeam/Team models so the <code>try</code> would always fail and return an empty array. Also the <code>hyperlinks_array</code> method retrieved the hyperlinks from the <code>submitted_hyperlinks</code> field of the team model so in moving to AssignmentTeam model we just had to remove the .team quantifier.

<code>
def hyperlinks
team.try(:hyperlinks) || []
end

def hyperlinks_array
self.team.submitted_hyperlinks.blank? ? [] : YAML::load(self.team.submitted_hyperlinks)
end

</code>

They are both merged into a new single <code>hyperlinks</code> method in assignment_team.rb. All callers of both methods were refactored to use <code>hyperlinks</code> method of AssignmentTeam.

<code>
def hyperlinks
self.submitted_hyperlinks.blank? ? [] : YAML::load(self.submitted_hyperlinks)
end
</code>

'''Task2::''' Earlier, the following method <code>submit_hyperlink</code> was in assignment_participant.rb:

<code>
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks_array
hyperlinks << hyperlink
team_object = self.team
team_object.submitted_hyperlinks = YAML::dump(hyperlinks)
team_object.save
end
</code>

It has been now moved to assignment_team.rb as below. Again, as the method was moved to AssignmentTeam model, it obviated the need to invoke the <code>team</code> method.
<code>
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks
hyperlinks << hyperlink
self.submitted_hyperlinks = YAML::dump(hyperlinks)
self.save
end
</code>
A similar refactoring was done for <code>remove_hyperlinks</code> method in assignment_participant.rb.

'''Task3::''' The methods <code>has_submissions?</code> earlier existed both in AssignmentTeam model and AssignmentParticipant model.

'' In assignment_team.rb''

<code>
def has_submissions?
list_of_users = participants;
list_of_users.each { |participant| return true if participant.has_submissions? }
false
end
</code>

''In assignment_participant.rb''

<code>
def has_submissions?
return ((self.team.submitted_files.length > 0) or (hyperlinks_array.length > 0))
end
</code>

What was happening earlier was that <code>has_submissions?</code> (AssignmentTeam method) used to call <code>has_submissions?</code> for each of the AssignmentParticipants in the team. This was redundant as the <code>submitted_files</code> and <code>submitted_hyperlinks</code> fields had been moved from the participant to the team model in earlier projects. So the participant objects again ended up again going to the team for retrieving these fields.

So we removed <code>has_submissions?</code> from the AssignmentParticipant and refactored the one already present in AssignmentTeam to the simple code below:

''In assignment_team.rb''

<code>
def has_submissions?
return ((self.submitted_files.length > 0) or (hyperlinks.length > 0))
end
</code>

'''Task4::''' Most importantly, at all the places in the Expertiza code, where there were calls to the above mentioned methods in assignment_participant.rb, they were refactored appropriately to call the newly created methods in assignment_team.rb.

''Sample example :''

<code>
if (participant.submitted_at.nil? && participant.hyperlinks.empty?)
</code>

''This was refactored as''

<code>
if (participant.submitted_at.nil? && participant.team.hyperlinks.empty?)
</code>

In refactoring the call points, we used the existing <code>team</code> method of AssignmentParticipant to get the AssignmentTeam object associated with the particular participant and then invoked the equivalent methods on the <code>team</code> object.

'''Task5 ::''' Refactoring some of controller code.

In submitted_content_controller, <code>remove_hyperlink</code> method used to cycle through all the participants belonging to the team of current participant, and for each participant call the <code>remove_hyperlink</code> method of the AssignmentParticipant Model to delete the instance of input hyperlink.

Since all the required hyperlink methods and fields are now in AssignmentTeam this would be redundant and waste cycles. So we refactored it to directly call the AssignmentTeam <code>remove_hyperlink</code> method.

''Earlier in submitted_content_controller.rb''

<code>
def remove_hyperlink
@participant = AssignmentParticipant.find(params[:hyperlinks][:participant_id])

return unless current_user_id?(@participant.user_id)
hyperlink_to_delete = @participant.hyperlinks_array[params['chk_links'].to_i]

team_id = TeamsUser.team_id(@participant.parent_id, @participant.user_id)
team_participants = Array.new
if Team.exists?(team_id)
team_users = TeamsUser.where(team_id: team_id)
team_users.each do |team_user|
team_participants << AssignmentParticipant.where(parent_id: @participant.parent_id, user_id: team_user.user_id).first
end
else
team_participants << @participant
end

team_participants.each do |team_participant|
team_participant.remove_hyperlink(hyperlink_to_delete)
end
</code>

''Refactored:''
<code>
def remove_hyperlink
@participant = AssignmentParticipant.find(params[:hyperlinks][:participant_id])

return unless current_user_id?(@participant.user_id)

team = @participant.team
hyperlink_to_delete = team.hyperlinks[params['chk_links'].to_i]
team.remove_hyperlink(hyperlink_to_delete)
</code>

== Testing Details ==

=== RSpec ===
RSpec is a testing framework for Rails, and is a Behavioral-Driven Development tool. It is a domain specific language(DSL).

There were no existing tests for the hyperlinks related methods. We used RSpec to write test cases using TTD approach. The assignment_team_spec.rb in the spec folder will have these tests. All the tests can be executed by rspec spec command, or can also be executed individually using the command "rspec spec/models/assignment_team_spec.rb".

===Factory Girl===
We have used Factory Girl for creating Assignment and Team objects to be used for testing. Factory Girl is a replacement for fixtures. Fixtures have to be updated whenever we change a data model whereas adding and removing fields is much easier in Factory Girl. Fixture definitions are global whereas Factories can be local, so isolated cases can be tested. Factories are defined to create objects for testing.

===Setup===
Below code has to be added to the gemfile

<code>
gem 'rspec-rails'
gem "factory_girl_rails", "~> 4.0"
</code>

===Building Factory===
Below is the code to create object of team class.

<code>
FactoryGirl.define do
factory :team do
name "Wikipedia contribution_Team2"
parent_id 999
#parent_id 741
type "AssignmentTeam"
submitted_hyperlinks "---\n- http://www.goo.gl.com/2122\n- http://www.goo.gl.com/8767"
end
end
</code>

===Example RSpec===
We have written unit tests for each of the methods we have re-factored in this project. Here's a unit test example contained in spec/model/assignment_team_spec.rb:

<code>
describe "#has_submissions?" do
it "checks if a team has submitted hyperlinks" do
# assignment = build(:assignment)
assign_team = build(:assignment_team)
assign_team.submitted_hyperlinks << "\n- https://www.csc.ncsu.edu/2341"
expect(assign_team.has_submissions?).to be true
end
end
</code>

===Running RSpec===
<code>
$ rspec spec/models/assignment_team_spec.rb
</code>

=== UI Testing ===

Following are some of the steps which we performed to test this code from the User interface.

''Before the test::'' Have two students in an assignment team (say student1 and student2, you can also use the student logins listed in this wiki) for an active assignment. Active assignment simply means that the due date is in future and the assignment is not in "Finish" state. You can use an instructor login (example username: instructor6 password: password) to create a new assignment or update the due dates of some previous assignment and assign the team to it.

''Test1:'' Login as student1. Submit a hyperlink (say : linkA) as student1.

''Expected:'' Submit should go through. The hyperlink should be visible when you login as student2.

''Test2:'' Now as student2, submit another hyperlink (say linkB).

''Expected:'' Submit should go through. Now both hyperlinks should be visible from both student1 and student2.

''Test3:'' As student1, try submitting linkB.

''Expected:'' As linkB already exists in team's hyperlinks the submit should not go through.

''Test4:'' Remove a hyperlink (say linkA).

''Expected:'' Remove goes through. Now only linkB should be visible from both student accounts.

''Test5:'' Remove all hyperlinks.

''Expected:'' Hyperlinks list is empty for team and this can be seen from both student accounts.

''Test6:'' Submit some hyperlink. Try dropping topic after submitting.

''Expected:'' You are not allowed to drop if you have submitted some work. Only if you remove the submitted work it is possible for you to drop.

''Test7:'' Submit some hyperlink. Do a review of the team after logging in as a student in another team. The assignment should be in review stage for this to be possible, you can move the due dates via instructor login to achieve this.

''Expected:'' Peer review should show the links submitted by the other team. Review should be submitted successfully.

CSC/ECE 517 Spring 2016/OSS E1601

2016-04-02T04:05:26Z

Kvysyar: /* Building Factory */

=== E1601: Refactor methods related to submitted_hyperlinks. ===

This wiki is for the open source refactoring project E1601: Refactor methods related to submitted_hyperlinks for the course ECE/CSC 517.

== Peer Review Information ==

The reviewers may use the below student and instructor logins for the purpose of reviewing and testing the functionality. These students belong to the same team and work on an assignment created by the below instructor:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student5884, password -> password
* Student login: username -> student6420, password -> password
'''Please note''': Please do not delete the above users or their team. If you wish to do so, please add them back so as to aid other reviewers. Also note that the option to submit hyperlinks in the UI will be displayed to the enrolled students only if assignment deadline is in future and the assignment is not in "Finish" state.

== Expertiza Introduction==

Expertiza is an open source web application which enhances the student's learning through peer reviews. The students can form teams and work on assignments given by course instructors. The peers can review the submissions and give feedback. The students also get to review the reviewers!

== Problem statement ==

'''Before''': When a student submitted files and hyperlinks for an assignment, that content was submitted on behalf of his team, and not individually (even if there is only one person in the team i.e a student working on an assignment cannot be without a team even if he is working alone). Some of the previous contributors had worked on a project which made this change and associated all the submitted content to teams instead of the individual participants: the submitted_hyperlinks and dirctory_num fields were moved to teams table. All the hyperlinks were now recorded in one submitted_hyperlinks field in the team model instead of being present in all the participants of the team. All the submitted files similarly were uploaded to the common file space and in the “course_folder/assignment_folder/team_folder” path.

'''What was wrong with that''': There are multiple methods in assignment_participant.rb which still give the impression that the author is submitting the hyperlinks on behalf of himself and not the team even though the relevant fields have been moved to teams model. These methods are to be moved to assignment_teams.rb for improving the clarity and the efficiency of code.

'''What refactoring we have done to improve the code''':

''Task1::'' Removed “hyperlinks_array” method and “hyperlinks” method from assignment_participant.rb. Created “hyperlinks” method in assignment_team.rb.

''Task2::'' Removed “submit_hyperlink” and “remove_hyperlink” from assignment_pariticpant.rb and created equivalent methods in assignment_team.rb.

''Task3::'' Removed “has_submissions?” from assginment_participant.rb and created the equivalent method in assignment_team.rb.

''Task4::'' All the calls to the above methods were refactored to call the appropriate method's functions.

''Task5::'' Refactored some of the logic of controller methods.

''Task6::'' Tests for submitting and removing submitted hyperlinks and files.

== List of files changed ==

Model Files:

#assignment_participant.rb
#assignment_team.rb
#student_task.rb

View Files:

#submitted_content/_hyperlink.html.erb
#assignments/edit/_calibration.html.erb
#assignments/list_submissions.html.erb
#submitted_content/_main.html.erb

Controller Files:

#sign_up_sheet_controller.rb
#submitted_content_controller.rb

Files with major modifications:

#The model: assignment_participant.rb
#The model: assignment_team.rb
#The controller: submitted_content_controller.rb

== Key changes ==

Four methods <code>hyperlinks</code>, <code>has_submissions?</code>, <code>submit_hyperlink</code> and <code>remove_hyperlink</code> were moved from the assignment_participant.rb model to assignment_team.rb, and the <code>hyperlinks_array</code> method was removed. 

'''Task1::''' The below methods <code>hyperlinks_array</code> and <code>hyperlinks</code> were removed from the assignment_participant.rb. The <code>hyperlinks</code> method which existed here seemed suspect as there was no <code>hyperlinks</code> method in the AssignmentTeam/Team models so the <code>try</code> would always fail and return an empty array. Also the <code>hyperlinks_array</code> method retrieved the hyperlinks from the <code>submitted_hyperlinks</code> field of the team model so in moving to AssignmentTeam model we just had to remove the .team quantifier.

<code>
def hyperlinks
team.try(:hyperlinks) || []
end

def hyperlinks_array
self.team.submitted_hyperlinks.blank? ? [] : YAML::load(self.team.submitted_hyperlinks)
end

</code>

They are both merged into a new single <code>hyperlinks</code> method in assignment_team.rb. All callers of both methods were refactored to use <code>hyperlinks</code> method of AssignmentTeam.

<code>
def hyperlinks
self.submitted_hyperlinks.blank? ? [] : YAML::load(self.submitted_hyperlinks)
end
</code>

'''Task2::''' Earlier, the following method <code>submit_hyperlink</code> was in assignment_participant.rb:

<code>
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks_array
hyperlinks << hyperlink
team_object = self.team
team_object.submitted_hyperlinks = YAML::dump(hyperlinks)
team_object.save
end
</code>

It has been now moved to assignment_team.rb as below. Again, as the method was moved to AssignmentTeam model, it obviated the need to invoke the <code>team</code> method.
<code>
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks
hyperlinks << hyperlink
self.submitted_hyperlinks = YAML::dump(hyperlinks)
self.save
end
</code>
A similar refactoring was done for <code>remove_hyperlinks</code> method in assignment_participant.rb.

'''Task3::''' The methods <code>has_submissions?</code> earlier existed both in AssignmentTeam model and AssignmentParticipant model.

'' In assignment_team.rb''

<code>
def has_submissions?
list_of_users = participants;
list_of_users.each { |participant| return true if participant.has_submissions? }
false
end
</code>

''In assignment_participant.rb''

<code>
def has_submissions?
return ((self.team.submitted_files.length > 0) or (hyperlinks_array.length > 0))
end
</code>

What was happening earlier was that <code>has_submissions?</code> (AssignmentTeam method) used to call <code>has_submissions?</code> for each of the AssignmentParticipants in the team. This was redundant as the <code>submitted_files</code> and <code>submitted_hyperlinks</code> fields had been moved from the participant to the team model in earlier projects. So the participant objects again ended up again going to the team for retrieving these fields.

So we removed <code>has_submissions?</code> from the AssignmentParticipant and refactored the one already present in AssignmentTeam to the simple code below:

''In assignment_team.rb''

<code>
def has_submissions?
return ((self.submitted_files.length > 0) or (hyperlinks.length > 0))
end
</code>

'''Task4::''' Most importantly, at all the places in the Expertiza code, where there were calls to the above mentioned methods in assignment_participant.rb, they were refactored appropriately to call the newly created methods in assignment_team.rb.

''Sample example :''

<code>
if (participant.submitted_at.nil? && participant.hyperlinks.empty?)
</code>

''This was refactored as''

<code>
if (participant.submitted_at.nil? && participant.team.hyperlinks.empty?)
</code>

In refactoring the call points, we used the existing <code>team</code> method of AssignmentParticipant to get the AssignmentTeam object associated with the particular participant and then invoked the equivalent methods on the <code>team</code> object.

'''Task5 ::''' Refactoring some of controller code.

In submitted_content_controller, <code>remove_hyperlink</code> method used to cycle through all the participants belonging to the team of current participant, and for each participant call the <code>remove_hyperlink</code> method of the AssignmentParticipant Model to delete the instance of input hyperlink.

Since all the required hyperlink methods and fields are now in AssignmentTeam this would be redundant and waste cycles. So we refactored it to directly call the AssignmentTeam <code>remove_hyperlink</code> method.

''Earlier in submitted_content_controller.rb''

<code>
def remove_hyperlink
@participant = AssignmentParticipant.find(params[:hyperlinks][:participant_id])

return unless current_user_id?(@participant.user_id)
hyperlink_to_delete = @participant.hyperlinks_array[params['chk_links'].to_i]

team_id = TeamsUser.team_id(@participant.parent_id, @participant.user_id)
team_participants = Array.new
if Team.exists?(team_id)
team_users = TeamsUser.where(team_id: team_id)
team_users.each do |team_user|
team_participants << AssignmentParticipant.where(parent_id: @participant.parent_id, user_id: team_user.user_id).first
end
else
team_participants << @participant
end

team_participants.each do |team_participant|
team_participant.remove_hyperlink(hyperlink_to_delete)
end
</code>

''Refactored:''
<code>
def remove_hyperlink
@participant = AssignmentParticipant.find(params[:hyperlinks][:participant_id])

return unless current_user_id?(@participant.user_id)

team = @participant.team
hyperlink_to_delete = team.hyperlinks[params['chk_links'].to_i]
team.remove_hyperlink(hyperlink_to_delete)
</code>

== Testing Details ==

=== RSpec ===
RSpec is a testing framework for Rails, and is a Behavioral-Driven Development tool. It is a domain specific language(DSL).

There were no existing tests for the hyperlinks related methods. We used RSpec to write test cases using TTD approach. The assignment_team_spec.rb in the spec folder will have these tests. All the tests can be executed by rspec spec command, or can also be executed individually using the command "rspec spec/models/assignment_team_spec.rb".

===Factory Girl===
We have used Factory Girl for creating Assignment and Team objects to be used for testing. Factory Girl is a replacement for fixtures. Fixtures have to be updated whenever we change a data model whereas adding and removing fields is much easier in Factory Girl. Fixture definitions are global whereas Factories can be local, so isolated cases can be tested. Factories are defined to create objects for testing.

===Setup===
Below code has to be added to the gemfile

<code>
gem 'rspec-rails'
gem "factory_girl_rails", "~> 4.0"
</code>

===Building Factory===
Below is the code to create object of team class.

<code>
FactoryGirl.define do
factory :team do
name "Wikipedia contribution_Team2"
parent_id 999
#parent_id 741
type "AssignmentTeam"
submitted_hyperlinks "---\n- http://www.goo.gl.com/2122\n- http://www.goo.gl.com/8767"
end
end
</code>

===Example RSpec===
We have written unit tests for each of the methods we have re-factored in this project. Here's a unit test example contained in spec/model/assignment_team_spec.rb:

<code>
describe "#has_submissions?" do
it "checks if a team has submitted hyperlinks" do
# assignment = build(:assignment)
assign_team = build(:assignment_team)
assign_team.submitted_hyperlinks << "\n- https://www.harrypotter.ncsu.edu"
expect(assign_team.has_submissions?).to be true
end
end
</code>

===Running RSpec===
<code>
$ rspec spec/models/assignment_team_spec.rb
</code>

=== UI Testing ===

Following are some of the steps which we performed to test this code from the User interface.

''Before the test::'' Have two students in an assignment team (say student1 and student2, you can also use the student logins listed in this wiki) for an active assignment. Active assignment simply means that the due date is in future and the assignment is not in "Finish" state. You can use an instructor login (example username: instructor6 password: password) to create a new assignment or update the due dates of some previous assignment and assign the team to it.

''Test1:'' Login as student1. Submit a hyperlink (say : linkA) as student1.

''Expected:'' Submit should go through. The hyperlink should be visible when you login as student2.

''Test2:'' Now as student2, submit another hyperlink (say linkB).

''Expected:'' Submit should go through. Now both hyperlinks should be visible from both student1 and student2.

''Test3:'' As student1, try submitting linkB.

''Expected:'' As linkB already exists in team's hyperlinks the submit should not go through.

''Test4:'' Remove a hyperlink (say linkA).

''Expected:'' Remove goes through. Now only linkB should be visible from both student accounts.

''Test5:'' Remove all hyperlinks.

''Expected:'' Hyperlinks list is empty for team and this can be seen from both student accounts.

''Test6:'' Submit some hyperlink. Try dropping topic after submitting.

''Expected:'' You are not allowed to drop if you have submitted some work. Only if you remove the submitted work it is possible for you to drop.

''Test7:'' Submit some hyperlink. Do a review of the team after logging in as a student in another team. The assignment should be in review stage for this to be possible, you can move the due dates via instructor login to achieve this.

''Expected:'' Peer review should show the links submitted by the other team. Review should be submitted successfully.

CSC/ECE 517 Spring 2016/OSS E1601

2016-04-02T04:02:37Z

Kvysyar: /* Factory Girl */

=== E1601: Refactor methods related to submitted_hyperlinks. ===

This wiki is for the open source refactoring project E1601: Refactor methods related to submitted_hyperlinks for the course ECE/CSC 517.

== Peer Review Information ==

The reviewers may use the below student and instructor logins for the purpose of reviewing and testing the functionality. These students belong to the same team and work on an assignment created by the below instructor:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student5884, password -> password
* Student login: username -> student6420, password -> password
'''Please note''': Please do not delete the above users or their team. If you wish to do so, please add them back so as to aid other reviewers. Also note that the option to submit hyperlinks in the UI will be displayed to the enrolled students only if assignment deadline is in future and the assignment is not in "Finish" state.

== Expertiza Introduction==

Expertiza is an open source web application which enhances the student's learning through peer reviews. The students can form teams and work on assignments given by course instructors. The peers can review the submissions and give feedback. The students also get to review the reviewers!

== Problem statement ==

'''Before''': When a student submitted files and hyperlinks for an assignment, that content was submitted on behalf of his team, and not individually (even if there is only one person in the team i.e a student working on an assignment cannot be without a team even if he is working alone). Some of the previous contributors had worked on a project which made this change and associated all the submitted content to teams instead of the individual participants: the submitted_hyperlinks and dirctory_num fields were moved to teams table. All the hyperlinks were now recorded in one submitted_hyperlinks field in the team model instead of being present in all the participants of the team. All the submitted files similarly were uploaded to the common file space and in the “course_folder/assignment_folder/team_folder” path.

'''What was wrong with that''': There are multiple methods in assignment_participant.rb which still give the impression that the author is submitting the hyperlinks on behalf of himself and not the team even though the relevant fields have been moved to teams model. These methods are to be moved to assignment_teams.rb for improving the clarity and the efficiency of code.

'''What refactoring we have done to improve the code''':

''Task1::'' Removed “hyperlinks_array” method and “hyperlinks” method from assignment_participant.rb. Created “hyperlinks” method in assignment_team.rb.

''Task2::'' Removed “submit_hyperlink” and “remove_hyperlink” from assignment_pariticpant.rb and created equivalent methods in assignment_team.rb.

''Task3::'' Removed “has_submissions?” from assginment_participant.rb and created the equivalent method in assignment_team.rb.

''Task4::'' All the calls to the above methods were refactored to call the appropriate method's functions.

''Task5::'' Refactored some of the logic of controller methods.

''Task6::'' Tests for submitting and removing submitted hyperlinks and files.

== List of files changed ==

Model Files:

#assignment_participant.rb
#assignment_team.rb
#student_task.rb

View Files:

#submitted_content/_hyperlink.html.erb
#assignments/edit/_calibration.html.erb
#assignments/list_submissions.html.erb
#submitted_content/_main.html.erb

Controller Files:

#sign_up_sheet_controller.rb
#submitted_content_controller.rb

Files with major modifications:

#The model: assignment_participant.rb
#The model: assignment_team.rb
#The controller: submitted_content_controller.rb

== Key changes ==

Four methods <code>hyperlinks</code>, <code>has_submissions?</code>, <code>submit_hyperlink</code> and <code>remove_hyperlink</code> were moved from the assignment_participant.rb model to assignment_team.rb, and the <code>hyperlinks_array</code> method was removed. 

'''Task1::''' The below methods <code>hyperlinks_array</code> and <code>hyperlinks</code> were removed from the assignment_participant.rb. The <code>hyperlinks</code> method which existed here seemed suspect as there was no <code>hyperlinks</code> method in the AssignmentTeam/Team models so the <code>try</code> would always fail and return an empty array. Also the <code>hyperlinks_array</code> method retrieved the hyperlinks from the <code>submitted_hyperlinks</code> field of the team model so in moving to AssignmentTeam model we just had to remove the .team quantifier.

<code>
def hyperlinks
team.try(:hyperlinks) || []
end

def hyperlinks_array
self.team.submitted_hyperlinks.blank? ? [] : YAML::load(self.team.submitted_hyperlinks)
end

</code>

They are both merged into a new single <code>hyperlinks</code> method in assignment_team.rb. All callers of both methods were refactored to use <code>hyperlinks</code> method of AssignmentTeam.

<code>
def hyperlinks
self.submitted_hyperlinks.blank? ? [] : YAML::load(self.submitted_hyperlinks)
end
</code>

'''Task2::''' Earlier, the following method <code>submit_hyperlink</code> was in assignment_participant.rb:

<code>
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks_array
hyperlinks << hyperlink
team_object = self.team
team_object.submitted_hyperlinks = YAML::dump(hyperlinks)
team_object.save
end
</code>

It has been now moved to assignment_team.rb as below. Again, as the method was moved to AssignmentTeam model, it obviated the need to invoke the <code>team</code> method.
<code>
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks
hyperlinks << hyperlink
self.submitted_hyperlinks = YAML::dump(hyperlinks)
self.save
end
</code>
A similar refactoring was done for <code>remove_hyperlinks</code> method in assignment_participant.rb.

'''Task3::''' The methods <code>has_submissions?</code> earlier existed both in AssignmentTeam model and AssignmentParticipant model.

'' In assignment_team.rb''

<code>
def has_submissions?
list_of_users = participants;
list_of_users.each { |participant| return true if participant.has_submissions? }
false
end
</code>

''In assignment_participant.rb''

<code>
def has_submissions?
return ((self.team.submitted_files.length > 0) or (hyperlinks_array.length > 0))
end
</code>

What was happening earlier was that <code>has_submissions?</code> (AssignmentTeam method) used to call <code>has_submissions?</code> for each of the AssignmentParticipants in the team. This was redundant as the <code>submitted_files</code> and <code>submitted_hyperlinks</code> fields had been moved from the participant to the team model in earlier projects. So the participant objects again ended up again going to the team for retrieving these fields.

So we removed <code>has_submissions?</code> from the AssignmentParticipant and refactored the one already present in AssignmentTeam to the simple code below:

''In assignment_team.rb''

<code>
def has_submissions?
return ((self.submitted_files.length > 0) or (hyperlinks.length > 0))
end
</code>

'''Task4::''' Most importantly, at all the places in the Expertiza code, where there were calls to the above mentioned methods in assignment_participant.rb, they were refactored appropriately to call the newly created methods in assignment_team.rb.

''Sample example :''

<code>
if (participant.submitted_at.nil? && participant.hyperlinks.empty?)
</code>

''This was refactored as''

<code>
if (participant.submitted_at.nil? && participant.team.hyperlinks.empty?)
</code>

In refactoring the call points, we used the existing <code>team</code> method of AssignmentParticipant to get the AssignmentTeam object associated with the particular participant and then invoked the equivalent methods on the <code>team</code> object.

'''Task5 ::''' Refactoring some of controller code.

In submitted_content_controller, <code>remove_hyperlink</code> method used to cycle through all the participants belonging to the team of current participant, and for each participant call the <code>remove_hyperlink</code> method of the AssignmentParticipant Model to delete the instance of input hyperlink.

Since all the required hyperlink methods and fields are now in AssignmentTeam this would be redundant and waste cycles. So we refactored it to directly call the AssignmentTeam <code>remove_hyperlink</code> method.

''Earlier in submitted_content_controller.rb''

<code>
def remove_hyperlink
@participant = AssignmentParticipant.find(params[:hyperlinks][:participant_id])

return unless current_user_id?(@participant.user_id)
hyperlink_to_delete = @participant.hyperlinks_array[params['chk_links'].to_i]

team_id = TeamsUser.team_id(@participant.parent_id, @participant.user_id)
team_participants = Array.new
if Team.exists?(team_id)
team_users = TeamsUser.where(team_id: team_id)
team_users.each do |team_user|
team_participants << AssignmentParticipant.where(parent_id: @participant.parent_id, user_id: team_user.user_id).first
end
else
team_participants << @participant
end

team_participants.each do |team_participant|
team_participant.remove_hyperlink(hyperlink_to_delete)
end
</code>

''Refactored:''
<code>
def remove_hyperlink
@participant = AssignmentParticipant.find(params[:hyperlinks][:participant_id])

return unless current_user_id?(@participant.user_id)

team = @participant.team
hyperlink_to_delete = team.hyperlinks[params['chk_links'].to_i]
team.remove_hyperlink(hyperlink_to_delete)
</code>

== Testing Details ==

=== RSpec ===
RSpec is a testing framework for Rails, and is a Behavioral-Driven Development tool. It is a domain specific language(DSL).

There were no existing tests for the hyperlinks related methods. We used RSpec to write test cases using TTD approach. The assignment_team_spec.rb in the spec folder will have these tests. All the tests can be executed by rspec spec command, or can also be executed individually using the command "rspec spec/models/assignment_team_spec.rb".

===Factory Girl===
We have used Factory Girl for creating Assignment and Team objects to be used for testing. Factory Girl is a replacement for fixtures. Fixtures have to be updated whenever we change a data model whereas adding and removing fields is much easier in Factory Girl. Fixture definitions are global whereas Factories can be local, so isolated cases can be tested. Factories are defined to create objects for testing.

===Setup===
Below code has to be added to the gemfile

<code>
gem 'rspec-rails'
gem "factory_girl_rails", "~> 4.0"
</code>

===Building Factory===
Below is the code to create object of team class.

<code>
FactoryGirl.define do
factory :team do
name "Wikipedia contribution_Team2"
parent_id 999
#parent_id 741
type "AssignmentTeam"
submitted_hyperlinks "---\n- http://water.com\n- http://shed.com"
end
end
</code>

===Example RSpec===
We have written unit tests for each of the methods we have re-factored in this project. Here's a unit test example contained in spec/model/assignment_team_spec.rb:

<code>
describe "#has_submissions?" do
it "checks if a team has submitted hyperlinks" do
# assignment = build(:assignment)
assign_team = build(:assignment_team)
assign_team.submitted_hyperlinks << "\n- https://www.harrypotter.ncsu.edu"
expect(assign_team.has_submissions?).to be true
end
end
</code>

===Running RSpec===
<code>
$ rspec spec/models/assignment_team_spec.rb
</code>

=== UI Testing ===

Following are some of the steps which we performed to test this code from the User interface.

''Before the test::'' Have two students in an assignment team (say student1 and student2, you can also use the student logins listed in this wiki) for an active assignment. Active assignment simply means that the due date is in future and the assignment is not in "Finish" state. You can use an instructor login (example username: instructor6 password: password) to create a new assignment or update the due dates of some previous assignment and assign the team to it.

''Test1:'' Login as student1. Submit a hyperlink (say : linkA) as student1.

''Expected:'' Submit should go through. The hyperlink should be visible when you login as student2.

''Test2:'' Now as student2, submit another hyperlink (say linkB).

''Expected:'' Submit should go through. Now both hyperlinks should be visible from both student1 and student2.

''Test3:'' As student1, try submitting linkB.

''Expected:'' As linkB already exists in team's hyperlinks the submit should not go through.

''Test4:'' Remove a hyperlink (say linkA).

''Expected:'' Remove goes through. Now only linkB should be visible from both student accounts.

''Test5:'' Remove all hyperlinks.

''Expected:'' Hyperlinks list is empty for team and this can be seen from both student accounts.

''Test6:'' Submit some hyperlink. Try dropping topic after submitting.

''Expected:'' You are not allowed to drop if you have submitted some work. Only if you remove the submitted work it is possible for you to drop.

''Test7:'' Submit some hyperlink. Do a review of the team after logging in as a student in another team. The assignment should be in review stage for this to be possible, you can move the due dates via instructor login to achieve this.

''Expected:'' Peer review should show the links submitted by the other team. Review should be submitted successfully.

CSC/ECE 517 Spring 2016/OSS E1601

2016-04-02T03:41:35Z

Kvysyar: /* RSpec */

=== E1601: Refactor methods related to submitted_hyperlinks. ===

This wiki is for the open source refactoring project E1601: Refactor methods related to submitted_hyperlinks for the course ECE/CSC 517.

== Peer Review Information ==

The reviewers may use the below student and instructor logins for the purpose of reviewing and testing the functionality. These students belong to the same team and work on an assignment created by the below instructor:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student5884, password -> password
* Student login: username -> student6420, password -> password
'''Please note''': Please do not delete the above users or their team. If you wish to do so, please add them back so as to aid other reviewers. Also note that the option to submit hyperlinks in the UI will be displayed to the enrolled students only if assignment deadline is in future and the assignment is not in "Finish" state.

== Expertiza Introduction==

Expertiza is an open source web application which enhances the student's learning through peer reviews. The students can form teams and work on assignments given by course instructors. The peers can review the submissions and give feedback. The students also get to review the reviewers!

== Problem statement ==

'''Before''': When a student submitted files and hyperlinks for an assignment, that content was submitted on behalf of his team, and not individually (even if there is only one person in the team i.e a student working on an assignment cannot be without a team even if he is working alone). Some of the previous contributors had worked on a project which made this change and associated all the submitted content to teams instead of the individual participants: the submitted_hyperlinks and dirctory_num fields were moved to teams table. All the hyperlinks were now recorded in one submitted_hyperlinks field in the team model instead of being present in all the participants of the team. All the submitted files similarly were uploaded to the common file space and in the “course_folder/assignment_folder/team_folder” path.

'''What was wrong with that''': There are multiple methods in assignment_participant.rb which still give the impression that the author is submitting the hyperlinks on behalf of himself and not the team even though the relevant fields have been moved to teams model. These methods are to be moved to assignment_teams.rb for improving the clarity and the efficiency of code.

'''What refactoring we have done to improve the code''':

''Task1::'' Removed “hyperlinks_array” method and “hyperlinks” method from assignment_participant.rb. Created “hyperlinks” method in assignment_team.rb.

''Task2::'' Removed “submit_hyperlink” and “remove_hyperlink” from assignment_pariticpant.rb and created equivalent methods in assignment_team.rb.

''Task3::'' Removed “has_submissions?” from assginment_participant.rb and created the equivalent method in assignment_team.rb.

''Task4::'' All the calls to the above methods were refactored to call the appropriate method's functions.

''Task5::'' Refactored some of the logic of controller methods.

''Task6::'' Tests for submitting and removing submitted hyperlinks and files.

== List of files changed ==

Model Files:

#assignment_participant.rb
#assignment_team.rb
#student_task.rb

View Files:

#submitted_content/_hyperlink.html.erb
#assignments/edit/_calibration.html.erb
#assignments/list_submissions.html.erb
#submitted_content/_main.html.erb

Controller Files:

#sign_up_sheet_controller.rb
#submitted_content_controller.rb

Files with major modifications:

#The model: assignment_participant.rb
#The model: assignment_team.rb
#The controller: submitted_content_controller.rb

== Key changes ==

Four methods <code>hyperlinks</code>, <code>has_submissions?</code>, <code>submit_hyperlink</code> and <code>remove_hyperlink</code> were moved from the assignment_participant.rb model to assignment_team.rb, and the <code>hyperlinks_array</code> method was removed. 

'''Task1::''' The below methods <code>hyperlinks_array</code> and <code>hyperlinks</code> were removed from the assignment_participant.rb. The <code>hyperlinks</code> method which existed here seemed suspect as there was no <code>hyperlinks</code> method in the AssignmentTeam/Team models so the <code>try</code> would always fail and return an empty array. Also the <code>hyperlinks_array</code> method retrieved the hyperlinks from the <code>submitted_hyperlinks</code> field of the team model so in moving to AssignmentTeam model we just had to remove the .team quantifier.

<code>
def hyperlinks
team.try(:hyperlinks) || []
end

def hyperlinks_array
self.team.submitted_hyperlinks.blank? ? [] : YAML::load(self.team.submitted_hyperlinks)
end

</code>

They are both merged into a new single <code>hyperlinks</code> method in assignment_team.rb. All callers of both methods were refactored to use <code>hyperlinks</code> method of AssignmentTeam.

<code>
def hyperlinks
self.submitted_hyperlinks.blank? ? [] : YAML::load(self.submitted_hyperlinks)
end
</code>

'''Task2::''' Earlier, the following method <code>submit_hyperlink</code> was in assignment_participant.rb:

<code>
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks_array
hyperlinks << hyperlink
team_object = self.team
team_object.submitted_hyperlinks = YAML::dump(hyperlinks)
team_object.save
end
</code>

It has been now moved to assignment_team.rb as below. Again, as the method was moved to AssignmentTeam model, it obviated the need to invoke the <code>team</code> method.
<code>
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks
hyperlinks << hyperlink
self.submitted_hyperlinks = YAML::dump(hyperlinks)
self.save
end
</code>
A similar refactoring was done for <code>remove_hyperlinks</code> method in assignment_participant.rb.

'''Task3::''' The methods <code>has_submissions?</code> earlier existed both in AssignmentTeam model and AssignmentParticipant model.

'' In assignment_team.rb''

<code>
def has_submissions?
list_of_users = participants;
list_of_users.each { |participant| return true if participant.has_submissions? }
false
end
</code>

''In assignment_participant.rb''

<code>
def has_submissions?
return ((self.team.submitted_files.length > 0) or (hyperlinks_array.length > 0))
end
</code>

What was happening earlier was that <code>has_submissions?</code> (AssignmentTeam method) used to call <code>has_submissions?</code> for each of the AssignmentParticipants in the team. This was redundant as the <code>submitted_files</code> and <code>submitted_hyperlinks</code> fields had been moved from the participant to the team model in earlier projects. So the participant objects again ended up again going to the team for retrieving these fields.

So we removed <code>has_submissions?</code> from the AssignmentParticipant and refactored the one already present in AssignmentTeam to the simple code below:

''In assignment_team.rb''

<code>
def has_submissions?
return ((self.submitted_files.length > 0) or (hyperlinks.length > 0))
end
</code>

'''Task4::''' Most importantly, at all the places in the Expertiza code, where there were calls to the above mentioned methods in assignment_participant.rb, they were refactored appropriately to call the newly created methods in assignment_team.rb.

''Sample example :''

<code>
if (participant.submitted_at.nil? && participant.hyperlinks.empty?)
</code>

''This was refactored as''

<code>
if (participant.submitted_at.nil? && participant.team.hyperlinks.empty?)
</code>

In refactoring the call points, we used the existing <code>team</code> method of AssignmentParticipant to get the AssignmentTeam object associated with the particular participant and then invoked the equivalent methods on the <code>team</code> object.

'''Task5 ::''' Refactoring some of controller code.

In submitted_content_controller, <code>remove_hyperlink</code> method used to cycle through all the participants belonging to the team of current participant, and for each participant call the <code>remove_hyperlink</code> method of the AssignmentParticipant Model to delete the instance of input hyperlink.

Since all the required hyperlink methods and fields are now in AssignmentTeam this would be redundant and waste cycles. So we refactored it to directly call the AssignmentTeam <code>remove_hyperlink</code> method.

''Earlier in submitted_content_controller.rb''

<code>
def remove_hyperlink
@participant = AssignmentParticipant.find(params[:hyperlinks][:participant_id])

return unless current_user_id?(@participant.user_id)
hyperlink_to_delete = @participant.hyperlinks_array[params['chk_links'].to_i]

team_id = TeamsUser.team_id(@participant.parent_id, @participant.user_id)
team_participants = Array.new
if Team.exists?(team_id)
team_users = TeamsUser.where(team_id: team_id)
team_users.each do |team_user|
team_participants << AssignmentParticipant.where(parent_id: @participant.parent_id, user_id: team_user.user_id).first
end
else
team_participants << @participant
end

team_participants.each do |team_participant|
team_participant.remove_hyperlink(hyperlink_to_delete)
end
</code>

''Refactored:''
<code>
def remove_hyperlink
@participant = AssignmentParticipant.find(params[:hyperlinks][:participant_id])

return unless current_user_id?(@participant.user_id)

team = @participant.team
hyperlink_to_delete = team.hyperlinks[params['chk_links'].to_i]
team.remove_hyperlink(hyperlink_to_delete)
</code>

== Testing Details ==

=== RSpec ===
RSpec is a testing framework for Rails, and is a Behavioral-Driven Development tool. It is a domain specific language(DSL).

There were no existing tests for the hyperlinks related methods. We used RSpec to write test cases using TTD approach. The assignment_team_spec.rb in the spec folder will have these tests. All the tests can be executed by rspec spec command, or can also be executed individually using the command "rspec spec/models/assignment_team_spec.rb".

===Factory Girl===
We have used Factory Girl for creating test Assignment and Team objects. Factory Girl is a replacement for fixtures. Fixtures have to be updated whenever we change a data model whereas adding and removing fields is much easier in Factory Girl. Fixture definitions are global whereas Factories can be local, so isolated cases can be tested. Factories are defined to create objects for testing.

===Setup===
Below code has to be added to the gemfile

<code>
gem 'rspec-rails'
gem "factory_girl_rails", "~> 4.0"
</code>

===Running RSpec===
<code>
$ rspec spec/models/assignment_team_spec.rb
</code>

=== UI Testing ===

Following are some of the steps which we performed to test this code from the User interface.

''Before the test::'' Have two students in an assignment team (say student1 and student2, you can also use the student logins listed in this wiki) for an active assignment. Active assignment simply means that the due date is in future and the assignment is not in "Finish" state. You can use an instructor login (example username: instructor6 password: password) to create a new assignment or update the due dates of some previous assignment and assign the team to it.

''Test1:'' Login as student1. Submit a hyperlink (say : linkA) as student1.

''Expected:'' Submit should go through. The hyperlink should be visible when you login as student2.

''Test2:'' Now as student2, submit another hyperlink (say linkB).

''Expected:'' Submit should go through. Now both hyperlinks should be visible from both student1 and student2.

''Test3:'' As student1, try submitting linkB.

''Expected:'' As linkB already exists in team's hyperlinks the submit should not go through.

''Test4:'' Remove a hyperlink (say linkA).

''Expected:'' Remove goes through. Now only linkB should be visible from both student accounts.

''Test5:'' Remove all hyperlinks.

''Expected:'' Hyperlinks list is empty for team and this can be seen from both student accounts.

''Test6:'' Submit some hyperlink. Try dropping topic after submitting.

''Expected:'' You are not allowed to drop if you have submitted some work. Only if you remove the submitted work it is possible for you to drop.

''Test7:'' Submit some hyperlink. Do a review of the team after logging in as a student in another team. The assignment should be in review stage for this to be possible, you can move the due dates via instructor login to achieve this.

''Expected:'' Peer review should show the links submitted by the other team. Review should be submitted successfully.

CSC/ECE 517 Spring 2016

2016-03-30T16:34:36Z

Kvysyar:

==Writing Assignment 1==
*[[CSC/ECE 517 Spring 2016/Active Job]]

==Calibration Assignment Submissions==
*[[Calibration Assignment Submission (OmniAuth)]]
*[[Calibration Assignment Submission (Patch_verb)]]

==Writing Assignment 2==
*[[CSC/ECE 517 Spring 2016/Mozilla Implement HTML5 form validation]]

*[[CSC/ECE 517 Spring 2016/Functional Tests for Questionnaire Controller]]

*[[CSC/ECE 517 Spring 2016 / Expertiza Self-Review Feature]]

*[[CSC/ECE 517 Spring 2016/Refactor different question types from quiz feature]]

*[[CSC/ECE 517 Spring 2016/Implement private browsing]]

*[[CSC/ECE 517 Spring 2016/Write automated tests for WebDriver]]

*[[CSC/ECE 517 Spring 2016/Refactor sign_up_sheet_controller.rb and sign_up_topic.rb]]

*[[CSC/ECE 517 Spring 2016/Implement Common Parts of the CSSOM API]]

*[[CSC/ECE 517 Spring 2016/Functional tests for assignment creation function]]

*[[CSC/ECE 517 Spring 2016/E1604. Functional tests for Calibration function]]

*[[CSC/ECE 517 Spring 2016/Refactor and write unit tests for question type.rb]]

*[[CSC/ECE 517 Spring 2016/Refactor response controller]]

*[[CSC/ECE_517_Spring_2016_OSS_M1606]]

*[[CSC/ECE_517_Spring_2016_OSS_E1601]]

E1601: Refactor methods related to submitted hyperlinks

2016-03-30T16:33:23Z

Kvysyar: moved E1601: Refactor methods related to submitted hyperlinks to CSC/ECE 517 Spring 2016 OSS E1601: Updated title

#REDIRECT [[CSC/ECE 517 Spring 2016 OSS E1601]]

CSC/ECE 517 Spring 2016/OSS E1601

2016-03-30T16:33:23Z

Kvysyar: moved E1601: Refactor methods related to submitted hyperlinks to CSC/ECE 517 Spring 2016 OSS E1601: Updated title

=== E1601: Refactor methods related to submitted_hyperlinks. ===

This wiki is for the open source refactoring project E1601: Refactor methods related to submitted_hyperlinks for the course ECE/CSC 517.

== Peer Review Information ==

We suggest the reviewers to use the below students and instructor for the purpose of reviewing. These students belong to the same team and work on an assignment given by the below instructor:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student5884, password -> password
* Student login: username -> student6420, password -> password
'''Please note''': Please do not delete the above users or their team. If you wish to do so, please add them back so as to aid other reviewers. Also note that the option to submit hyperlinks in the UI will only be displayed to the enrolled students only before the assignment deadline.

== Expertiza Introduction==

Expertiza is an open source web application which enhances learning through peer reviews. The students can form teams and work on assignments given by course instructors. The peers can review the submissions and give feedback. The students also get to review the reviewers!

== Problem statement ==

'''Before''': When student submitted files and hyperlinks, that content was submitted on behalf of a team, not individuals (even there is only one person in the team). Some of the previous contributors worked on a project which associated all the submitted content to teams: there were submitted_hyperlinks and dirctory_num fields in teams table. All the hyperlinks should ideally be recorded in one submitted_hyperlinks field. All the submitted files were uploaded to the file space and in the “course_folder/assignment_folder/team_folder” path.

'''What was wrong with that''': There are multiple methods in assignment_participant.rb which still look like the author submitted the hyperlinks on behalf of him/herself. They should be moved to assignment_teams.rb.

'''What refactoring we have done to improve''':
* Removed “hyperlinks_array” method and “hyperlinks” method from assignment_participant.rb. Created “hyperlinks” method in assignment_team.rb.
* Removed “submit_hyperlink” and “remove_hyperlink” from assignment_pariticpant.rb and created equivalent methods in assignment_team.rb.

* Removed “has_submissions?” from assginment_participant.rb and created the equivalent method in assignment_team.rb.
* All the calls to the above methods were refactored to call the appropriate method's functions.
* Wrote tests for submitting and removing submitted hyperlinks and files.

== List of files changed ==

Model Files:

#assignment_participant.rb
#assignment_team.rb
#student_task.rb

View Files:

#submitted_content/_hyperlink.html.erb
#assignments/edit/_calibration.html.erb
#assignments/list_submissions.html.erb
#submitted_content/_main.html.erb

Controller Files:

#sign_up_sheet_controller.rb
#submitted_content_controller.rb

Files with major modifications:

#The model: assignment_participant.rb
#The model: assignment_team.rb
#The controller: submitted_content_controller.rb

== Key changes ==

Four methods <code>hyperlinks</code>, <code>has_submissions?</code>, <code>submit_hyperlink</code> and <code>remove_hyperlink</code> were moved from the assignment_participant.rb model to assignment_team.rb, and the <code>hyperlinks_array</code> method was removed. 

1) The below methods <code>hyperlinks_array</code> and <code>hyperlinks</code> were removed from the assignment_participant.rb. The <code>hyperlinks</code> method which existed here seemed suspect as there was no <code>hyperlinks</code> method in the AssignmentTeam/Team models so the <code>try</code> would always fail and return an empty array. Also the <code>hyperlinks_array</code> method retrieved the hyperlinks from the <code>submitted_hyperlinks</code> field of the team model so in moving to AssignmentTeam model we just had to remove the .team quantifier.

<code>
def hyperlinks
team.try(:hyperlinks) || []
end

def hyperlinks_array
self.team.submitted_hyperlinks.blank? ? [] : YAML::load(self.team.submitted_hyperlinks)
end

</code>

They are both merged into a new single <code>hyperlinks</code> method in assignment_team.rb. All callers of both methods were refactored to use <code>hyperlinks</code> method of AssignmentTeam.

<code>
def hyperlinks
self.submitted_hyperlinks.blank? ? [] : YAML::load(self.submitted_hyperlinks)
end
</code>

2) Earlier, the following method <code>submit_hyperlink</code> was in assignment_participant.rb:

<code>
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks_array
hyperlinks << hyperlink
team_object = self.team
team_object.submitted_hyperlinks = YAML::dump(hyperlinks)
team_object.save
end
</code>

It has been now moved to assignment_team.rb as below. Again, as the method was moved to AssignmentTeam model, it obviated the need to invoke the <code>team</code> method.
<code>
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks
hyperlinks << hyperlink
self.submitted_hyperlinks = YAML::dump(hyperlinks)
self.save
end
</code>
A similar refactoring was done for <code>remove_hyperlinks</code> method in assignment_participant.rb.

3) The methods <code>has_submissions?</code> earlier existed both in AssignmentTeam model and AssignmentParticipant model.

'' In assignment_team.rb''

<code>
def has_submissions?
list_of_users = participants;
list_of_users.each { |participant| return true if participant.has_submissions? }
false
end
</code>

''In assignment_participant.rb''

<code>
def has_submissions?
return ((self.team.submitted_files.length > 0) or (hyperlinks_array.length > 0))
end
</code>

What was happening earlier was that <code>has_submissions?</code> (AssignmentTeam method) used to call <code>has_submissions?</code> for each of the AssignmentParticipants in the team. This was redundant as the <code>submitted_files</code> and <code>submitted_hyperlinks</code> fields had been moved from the participant to the team model in earlier projects. So the participant objects again ended up again going to the team for retrieving these fields.

So we removed <code>has_submissions?</code> from the AssignmentParticipant and refactored the one already present in AssignmentTeam to the simple code below:

''In assignment_team.rb''

<code>
def has_submissions?
return ((self.submitted_files.length > 0) or (hyperlinks.length > 0))
end
</code>

4) Most importantly, at all the places in the Expertiza code, where there were calls to the above mentioned methods in assignment_participant.rb, were refactored appropriately to call the newly created methods in assignment_team.rb.

''Sample example :''

<code>
if (participant.submitted_at.nil? && participant.hyperlinks.empty?)
</code>

''This was refactored as''

<code>
if (participant.submitted_at.nil? && participant.team.hyperlinks.empty?)
</code>

In refactoring the call points, we used the existing <code>team</code> method of AssignmentParticipant to get the AssignmentTeam object associated with the particular participant and then invoked the equivalent methods on the <code>team</code> object.

5) Refactoring some of controller code.

In submitted_content_controller, <code>remove_hyperlink</code> method used to cycle through all the participants belonging to the team of current participant, and for each participant call the <code>remove_hyperlink</code> method of the AssignmentParticipant Model to delete the instance of input hyperlink.

Since all the required hyperlink methods and fields are now in AssignmentTeam this would be redundant and waste cycles. So we refactored it to directly call the AssignmentTeam <code>remove_hyperlink</code> method.

''Earlier in submitted_content_controller.rb''

<code>
def remove_hyperlink
@participant = AssignmentParticipant.find(params[:hyperlinks][:participant_id])

return unless current_user_id?(@participant.user_id)
hyperlink_to_delete = @participant.hyperlinks_array[params['chk_links'].to_i]

team_id = TeamsUser.team_id(@participant.parent_id, @participant.user_id)
team_participants = Array.new
if Team.exists?(team_id)
team_users = TeamsUser.where(team_id: team_id)
team_users.each do |team_user|
team_participants << AssignmentParticipant.where(parent_id: @participant.parent_id, user_id: team_user.user_id).first
end
else
team_participants << @participant
end

team_participants.each do |team_participant|
team_participant.remove_hyperlink(hyperlink_to_delete)
end
</code>

''Refactored:''
<code>
def remove_hyperlink
@participant = AssignmentParticipant.find(params[:hyperlinks][:participant_id])

return unless current_user_id?(@participant.user_id)

team = @participant.team
hyperlink_to_delete = team.hyperlinks[params['chk_links'].to_i]
team.remove_hyperlink(hyperlink_to_delete)
</code>

== Testing Details ==

=== RSpec ===
There were no existing tests for the hyperlinks related methods. We used RSpec to write test cases using TTD approach. The assignment_team_spec.rb in the spec folder will have these tests. All the tests can be executed by rspec spec command, or can also be executed individually using the command "rspec spec/models/assignment_team_spec.rb".

=== UI Testing ===

Following are some of the steps which we performed to test this code from the User interface.

''Before the test::'' Have two students in an assignment team (say student1 and student2) for an active assignment. Active assignment simply means that the due date is in future and the assignment is not in "Finish" state. You can use an instructor login (example username: instructor6 password: password) to create a new assignment or update the due dates of some previous assignment and assign the team to it.

''Test1::'' Login as student1. Submit a hyperlink (say : linkA) as student1.

''Expected:'' Submit should go through. The hyperlink should be visible when you login as student2.

''Test2:'' Now as student2, submit another hyperlink (say linkB).

''Expected:'' Submit should go through. Now both hyperlinks should be visible from both student1 and student2.

''Test3:'' As student1, try submitting linkB.

''Expected:'' As linkB already exists in team's hyperlinks the submit should not go through.

''Test4:'' Remove a hyperlink (say linkA).

''Expected:'' Remove goes through. Now only linkB should be visible from both student accounts.

''Test5:'' Remove all hyperlinks.

''Expected:'' Hyperlinks list is empty for team and this can be seen from both student accounts.

''Test6:'' Submit some hyperlink. Try dropping topic after submitting.

''Expected:'' You are not allowed to drop if you have submitted some work. Only if you remove the submitted work it is possible for you to drop.

''Test7:'' Submit some hyperlink. Do a review of the team after logging in as a student in another team. The assignment should be in review stage for this to be possible, you can move the due dates via instructor login to achieve this.

''Expected:'' Peer review should show the links submitted by the other team. Review should be submitted successfully.

User:Kvysyar

2016-03-30T16:26:30Z

Kvysyar: moved User:Kvysyar to E1601: Refactor methods related to submitted hyperlinks: Moved the page and added a new title.

#REDIRECT [[E1601: Refactor methods related to submitted hyperlinks]]

CSC/ECE 517 Spring 2016/OSS E1601

2016-03-30T16:26:30Z

Kvysyar: moved User:Kvysyar to E1601: Refactor methods related to submitted hyperlinks: Moved the page and added a new title.

=== E1601: Refactor methods related to submitted_hyperlinks. ===

This wiki is for the open source refactoring project E1601: Refactor methods related to submitted_hyperlinks for the course ECE/CSC 517.

== Peer Review Information ==

We suggest the reviewers to use the below students and instructor for the purpose of reviewing. These students belong to the same team and work on an assignment given by the below instructor:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student5884, password -> password
* Student login: username -> student6420, password -> password
'''Please note''': Please do not delete the above users or their team. If you wish to do so, please add them back so as to aid other reviewers. Also note that the option to submit hyperlinks in the UI will only be displayed to the enrolled students only before the assignment deadline.

== Expertiza Introduction==

Expertiza is an open source web application which enhances learning through peer reviews. The students can form teams and work on assignments given by course instructors. The peers can review the submissions and give feedback. The students also get to review the reviewers!

== Problem statement ==

'''Before''': When student submitted files and hyperlinks, that content was submitted on behalf of a team, not individuals (even there is only one person in the team). Some of the previous contributors worked on a project which associated all the submitted content to teams: there were submitted_hyperlinks and dirctory_num fields in teams table. All the hyperlinks should ideally be recorded in one submitted_hyperlinks field. All the submitted files were uploaded to the file space and in the “course_folder/assignment_folder/team_folder” path.

'''What was wrong with that''': There are multiple methods in assignment_participant.rb which still look like the author submitted the hyperlinks on behalf of him/herself. They should be moved to assignment_teams.rb.

'''What refactoring we have done to improve''':
* Removed “hyperlinks_array” method and “hyperlinks” method from assignment_participant.rb. Created “hyperlinks” method in assignment_team.rb.
* Removed “submit_hyperlink” and “remove_hyperlink” from assignment_pariticpant.rb and created equivalent methods in assignment_team.rb.

* Removed “has_submissions?” from assginment_participant.rb and created the equivalent method in assignment_team.rb.
* All the calls to the above methods were refactored to call the appropriate method's functions.
* Wrote tests for submitting and removing submitted hyperlinks and files.

== List of files changed ==

Model Files:

#assignment_participant.rb
#assignment_team.rb
#student_task.rb

View Files:

#submitted_content/_hyperlink.html.erb
#assignments/edit/_calibration.html.erb
#assignments/list_submissions.html.erb
#submitted_content/_main.html.erb

Controller Files:

#sign_up_sheet_controller.rb
#submitted_content_controller.rb

Files with major modifications:

#The model: assignment_participant.rb
#The model: assignment_team.rb
#The controller: submitted_content_controller.rb

== Key changes ==

Four methods <code>hyperlinks</code>, <code>has_submissions?</code>, <code>submit_hyperlink</code> and <code>remove_hyperlink</code> were moved from the assignment_participant.rb model to assignment_team.rb, and the <code>hyperlinks_array</code> method was removed. 

1) The below methods <code>hyperlinks_array</code> and <code>hyperlinks</code> were removed from the assignment_participant.rb. The <code>hyperlinks</code> method which existed here seemed suspect as there was no <code>hyperlinks</code> method in the AssignmentTeam/Team models so the <code>try</code> would always fail and return an empty array. Also the <code>hyperlinks_array</code> method retrieved the hyperlinks from the <code>submitted_hyperlinks</code> field of the team model so in moving to AssignmentTeam model we just had to remove the .team quantifier.

<code>
def hyperlinks
team.try(:hyperlinks) || []
end

def hyperlinks_array
self.team.submitted_hyperlinks.blank? ? [] : YAML::load(self.team.submitted_hyperlinks)
end

</code>

They are both merged into a new single <code>hyperlinks</code> method in assignment_team.rb. All callers of both methods were refactored to use <code>hyperlinks</code> method of AssignmentTeam.

<code>
def hyperlinks
self.submitted_hyperlinks.blank? ? [] : YAML::load(self.submitted_hyperlinks)
end
</code>

2) Earlier, the following method <code>submit_hyperlink</code> was in assignment_participant.rb:

<code>
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks_array
hyperlinks << hyperlink
team_object = self.team
team_object.submitted_hyperlinks = YAML::dump(hyperlinks)
team_object.save
end
</code>

It has been now moved to assignment_team.rb as below. Again, as the method was moved to AssignmentTeam model, it obviated the need to invoke the <code>team</code> method.
<code>
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks
hyperlinks << hyperlink
self.submitted_hyperlinks = YAML::dump(hyperlinks)
self.save
end
</code>
A similar refactoring was done for <code>remove_hyperlinks</code> method in assignment_participant.rb.

3) The methods <code>has_submissions?</code> earlier existed both in AssignmentTeam model and AssignmentParticipant model.

'' In assignment_team.rb''

<code>
def has_submissions?
list_of_users = participants;
list_of_users.each { |participant| return true if participant.has_submissions? }
false
end
</code>

''In assignment_participant.rb''

<code>
def has_submissions?
return ((self.team.submitted_files.length > 0) or (hyperlinks_array.length > 0))
end
</code>

What was happening earlier was that <code>has_submissions?</code> (AssignmentTeam method) used to call <code>has_submissions?</code> for each of the AssignmentParticipants in the team. This was redundant as the <code>submitted_files</code> and <code>submitted_hyperlinks</code> fields had been moved from the participant to the team model in earlier projects. So the participant objects again ended up again going to the team for retrieving these fields.

So we removed <code>has_submissions?</code> from the AssignmentParticipant and refactored the one already present in AssignmentTeam to the simple code below:

''In assignment_team.rb''

<code>
def has_submissions?
return ((self.submitted_files.length > 0) or (hyperlinks.length > 0))
end
</code>

4) Most importantly, at all the places in the Expertiza code, where there were calls to the above mentioned methods in assignment_participant.rb, were refactored appropriately to call the newly created methods in assignment_team.rb.

''Sample example :''

<code>
if (participant.submitted_at.nil? && participant.hyperlinks.empty?)
</code>

''This was refactored as''

<code>
if (participant.submitted_at.nil? && participant.team.hyperlinks.empty?)
</code>

In refactoring the call points, we used the existing <code>team</code> method of AssignmentParticipant to get the AssignmentTeam object associated with the particular participant and then invoked the equivalent methods on the <code>team</code> object.

5) Refactoring some of controller code.

In submitted_content_controller, <code>remove_hyperlink</code> method used to cycle through all the participants belonging to the team of current participant, and for each participant call the <code>remove_hyperlink</code> method of the AssignmentParticipant Model to delete the instance of input hyperlink.

Since all the required hyperlink methods and fields are now in AssignmentTeam this would be redundant and waste cycles. So we refactored it to directly call the AssignmentTeam <code>remove_hyperlink</code> method.

''Earlier in submitted_content_controller.rb''

<code>
def remove_hyperlink
@participant = AssignmentParticipant.find(params[:hyperlinks][:participant_id])

return unless current_user_id?(@participant.user_id)
hyperlink_to_delete = @participant.hyperlinks_array[params['chk_links'].to_i]

team_id = TeamsUser.team_id(@participant.parent_id, @participant.user_id)
team_participants = Array.new
if Team.exists?(team_id)
team_users = TeamsUser.where(team_id: team_id)
team_users.each do |team_user|
team_participants << AssignmentParticipant.where(parent_id: @participant.parent_id, user_id: team_user.user_id).first
end
else
team_participants << @participant
end

team_participants.each do |team_participant|
team_participant.remove_hyperlink(hyperlink_to_delete)
end
</code>

''Refactored:''
<code>
def remove_hyperlink
@participant = AssignmentParticipant.find(params[:hyperlinks][:participant_id])

return unless current_user_id?(@participant.user_id)

team = @participant.team
hyperlink_to_delete = team.hyperlinks[params['chk_links'].to_i]
team.remove_hyperlink(hyperlink_to_delete)
</code>

== Testing Details ==

=== RSpec ===
There were no existing tests for the hyperlinks related methods. We used RSpec to write test cases using TTD approach. The assignment_team_spec.rb in the spec folder will have these tests. All the tests can be executed by rspec spec command, or can also be executed individually using the command "rspec spec/models/assignment_team_spec.rb".

=== UI Testing ===

Following are some of the steps which we performed to test this code from the User interface.

''Before the test::'' Have two students in an assignment team (say student1 and student2) for an active assignment. Active assignment simply means that the due date is in future and the assignment is not in "Finish" state. You can use an instructor login (example username: instructor6 password: password) to create a new assignment or update the due dates of some previous assignment and assign the team to it.

''Test1::'' Login as student1. Submit a hyperlink (say : linkA) as student1.

''Expected:'' Submit should go through. The hyperlink should be visible when you login as student2.

''Test2:'' Now as student2, submit another hyperlink (say linkB).

''Expected:'' Submit should go through. Now both hyperlinks should be visible from both student1 and student2.

''Test3:'' As student1, try submitting linkB.

''Expected:'' As linkB already exists in team's hyperlinks the submit should not go through.

''Test4:'' Remove a hyperlink (say linkA).

''Expected:'' Remove goes through. Now only linkB should be visible from both student accounts.

''Test5:'' Remove all hyperlinks.

''Expected:'' Hyperlinks list is empty for team and this can be seen from both student accounts.

''Test6:'' Submit some hyperlink. Try dropping topic after submitting.

''Expected:'' You are not allowed to drop if you have submitted some work. Only if you remove the submitted work it is possible for you to drop.

''Test7:'' Submit some hyperlink. Do a review of the team after logging in as a student in another team. The assignment should be in review stage for this to be possible, you can move the due dates via instructor login to achieve this.

''Expected:'' Peer review should show the links submitted by the other team. Review should be submitted successfully.

CSC/ECE 517 Spring 2016/OSS E1601

2016-03-24T00:42:45Z

Kvysyar: /* Key changes */

This wiki documentation is for the open source refactoring project E1601: Refactor methods related to submitted_hyperlinks for the course ECE/CSC 517.

== Peer Review Information ==

We suggest the reviewers to use the below students and instructor for the purpose of reviewing. These students belong to the same team and work on an assignment given by the below instructor:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student5884, password -> password
* Student login: username -> student6420, password -> password
'''Please note''': Please do not delete the above users or their team. If you wish to do so, please add them back so as to aid other reviewers. Also note that the option to submit hyperlinks in the UI will only be displayed to the enrolled students only before the assignment deadline.

== Expertiza Introduction==

Expertiza is an open source web application which enhances learning through peer reviews. The students can form teams and work on assignments given by course instructors. The peers can review the submissions and give feedback. The students also get to review the reviewers!

== Problem statement ==

'''Before''': When student submitted files and hyperlinks, that content was submitted on behalf of a team, not individuals (even there is only one person in the team). Some of the previous contributors worked on a project which associated all the submitted content to teams: there were submitted_hyperlinks and dirctory_num fields in teams table. All the hyperlinks should ideally be recorded in one submitted_hyperlinks field. All the submitted files were uploaded to the file space and in the “course_folder/assignment_folder/team_folder” path.

'''What was wrong with that''': There are multiple methods in assignment_participant.rb which still look like the author submit the hyperlinks on behalf of him/herself. They should be moved to assignment_teams.rb.

'''What refactoring we have done to improve''':
* Removed “hyperlinks_array” method and “hyperlinks” method from assignment_participant.rb. Created “hyperlinks” method in assignment_team.rb.
* Removed “submit_hyperlink” and “remove_hyperlink” from assignment_pariticpant.rb and created equivalent methods in assignment_team.rb.

* Removed “has_submissions?” from assginment_participant.rb and created equivalent method in assignment_team.rb.
* All the calls to the above methods were refactored to call the appropriate method's functions.
* Wrote tests for submitting and removing submitted hyperlinks and files.

== Key changes ==
Files with major modifications:
# The model: assignment_participant.rb
# The model: assignment_team.rb
Four methods "hyperlinks”, “has_submissions?”, “submit_hyperlink” and “remove_hyperlink” were moved from the assignment_participant.rb model to assignment_team.rb, and the "hyperlinks_array" method removed. 

1.a) The below hyperlinks_array and hyperlinks method are removed from the assignment_participant.rb:
def hyperlinks
team.try(:hyperlinks) || []
end

def hyperlinks_array
self.team.submitted_hyperlinks.blank? ? [] : YAML::load(self.team.submitted_hyperlinks)
end

b) They are both merged into a single method in assignment_team.rb under the hyperlinks method:

def hyperlinks
self.submitted_hyperlinks.blank? ? [] : YAML::load(self.submitted_hyperlinks)
end

2. a) Earlier, the following method submit_hyperlink was in assignment_participant.rb:

def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks_array
hyperlinks << hyperlink
team_object = self.team
team_object.submitted_hyperlinks = YAML::dump(hyperlinks)
team_object.save
end

b) It is now moved to assignment_team.rb:

def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks
hyperlinks << hyperlink
self.submitted_hyperlinks = YAML::dump(hyperlinks)
self.save
end

3) Similarly the methods "has_submissions?", "remove_hyperlink" were moved.

4) Most importantly, at all the places in the Expertiza code, where there were calls to the above methods in assignment_participant.rb, which are now moved to assignment_team.rb, were refactored accordingly to call them appropriately.

== Testing Details ==
=== RSpec ===
There were no existing tests for the hyperlinks related methods. We used RSpec to write test cases using TTD approach. The assignment_team_spec.rb in the spec folder will have these tests. All the tests can be executed by rspec spec command, or can also be executed individually using the command "rspec spec/models/assignment_team_spec.rb".

=== UI Testing ===

Following steps needs to be performed to test this code from UI:
# Login as a student.
# In an assignment, that is still due, add one or more hyperlinks.
# Login as another student who is a teammate of the first student working on the same assignment.
# You should be able to see the newly uploaded link(s) by the other team member.
# Try adding a link that is not a valid http or https link. It will give an error message.
# Try submitting the same link once again. This will give an error message too.

CSC/ECE 517 Spring 2016/OSS E1601

2016-03-24T00:39:01Z

Kvysyar: /* Key changes */

This wiki documentation is for the open source refactoring project E1601: Refactor methods related to submitted_hyperlinks for the course ECE/CSC 517.

== Peer Review Information ==

We suggest the reviewers to use the below students and instructor for the purpose of reviewing. These students belong to the same team and work on an assignment given by the below instructor:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student5884, password -> password
* Student login: username -> student6420, password -> password
'''Please note''': Please do not delete the above users or their team. If you wish to do so, please add them back so as to aid other reviewers. Also note that the option to submit hyperlinks in the UI will only be displayed to the enrolled students only before the assignment deadline.

== Expertiza Introduction==

Expertiza is an open source web application which enhances learning through peer reviews. The students can form teams and work on assignments given by course instructors. The peers can review the submissions and give feedback. The students also get to review the reviewers!

== Problem statement ==

'''Before''': When student submitted files and hyperlinks, that content was submitted on behalf of a team, not individuals (even there is only one person in the team). Some of the previous contributors worked on a project which associated all the submitted content to teams: there were submitted_hyperlinks and dirctory_num fields in teams table. All the hyperlinks should ideally be recorded in one submitted_hyperlinks field. All the submitted files were uploaded to the file space and in the “course_folder/assignment_folder/team_folder” path.

'''What was wrong with that''': There are multiple methods in assignment_participant.rb which still look like the author submit the hyperlinks on behalf of him/herself. They should be moved to assignment_teams.rb.

'''What refactoring we have done to improve''':
* Removed “hyperlinks_array” method and “hyperlinks” method from assignment_participant.rb. Created “hyperlinks” method in assignment_team.rb.
* Removed “submit_hyperlink” and “remove_hyperlink” from assignment_pariticpant.rb and created equivalent methods in assignment_team.rb.

* Removed “has_submissions?” from assginment_participant.rb and created equivalent method in assignment_team.rb.
* All the calls to the above methods were refactored to call the appropriate method's functions.
* Wrote tests for submitting and removing submitted hyperlinks and files.

== Key changes ==
Files with major modifications:
# The model: assignment_participant.rb
# The model: assignment_team.rb
Four methods "hyperlinks”, “has_submissions?”, “submit_hyperlink” and “remove_hyperlink” were moved from the assignment_participant.rb model to assignment_team.rb, and the "hyperlinks_array" method removed. 

1.a) The below hyperlinks_array and hyperlinks method are removed from the assignment_participant.rb:
def hyperlinks
team.try(:hyperlinks) || []
end

def hyperlinks_array
self.team.submitted_hyperlinks.blank? ? [] : YAML::load(self.team.submitted_hyperlinks)
end
b) They are both merged into a single method in assignment_team.rb under the hyperlinks mehtod:
def hyperlinks
self.submitted_hyperlinks.blank? ? [] : YAML::load(self.submitted_hyperlinks)
end

2. a) Earlier, the following method submit_hyperlink was in assignment_participant.rb:
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks_array
hyperlinks << hyperlink
team_object = self.team
team_object.submitted_hyperlinks = YAML::dump(hyperlinks)
team_object.save
end
b) It is now moved to assignment_team.rb:
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks
hyperlinks << hyperlink
self.submitted_hyperlinks = YAML::dump(hyperlinks)
self.save
end
</syntaxhighlight>

3) Similarly the methods "has_submissions?", "remove_hyperlink" were moved.

4) Most importantly, at all the places in the Expertiza code, where there were calls to the above methods in assignment_participant.rb, which are now moved to assignment_team.rb, were refactored accordingly to call them appropriately.

== Testing Details ==
=== RSpec ===
There were no existing tests for the hyperlinks related methods. We used RSpec to write test cases using TTD approach. The assignment_team_spec.rb in the spec folder will have these tests. All the tests can be executed by rspec spec command, or can also be executed individually using the command "rspec spec/models/assignment_team_spec.rb".

=== UI Testing ===

Following steps needs to be performed to test this code from UI:
# Login as a student.
# In an assignment, that is still due, add one or more hyperlinks.
# Login as another student who is a teammate of the first student working on the same assignment.
# You should be able to see the newly uploaded link(s) by the other team member.
# Try adding a link that is not a valid http or https link. It will give an error message.
# Try submitting the same link once again. This will give an error message too.

CSC/ECE 517 Spring 2016/OSS E1601

2016-03-24T00:36:53Z

Kvysyar: Moved the entire content from sandbox to ncsu wiki

This wiki documentation is for the open source refactoring project E1601: Refactor methods related to submitted_hyperlinks for the course ECE/CSC 517.

== Peer Review Information ==

We suggest the reviewers to use the below students and instructor for the purpose of reviewing. These students belong to the same team and work on an assignment given by the below instructor:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student5884, password -> password
* Student login: username -> student6420, password -> password
'''Please note''': Please do not delete the above users or their team. If you wish to do so, please add them back so as to aid other reviewers. Also note that the option to submit hyperlinks in the UI will only be displayed to the enrolled students only before the assignment deadline.

== Expertiza Introduction==

Expertiza is an open source web application which enhances learning through peer reviews. The students can form teams and work on assignments given by course instructors. The peers can review the submissions and give feedback. The students also get to review the reviewers!

== Problem statement ==

'''Before''': When student submitted files and hyperlinks, that content was submitted on behalf of a team, not individuals (even there is only one person in the team). Some of the previous contributors worked on a project which associated all the submitted content to teams: there were submitted_hyperlinks and dirctory_num fields in teams table. All the hyperlinks should ideally be recorded in one submitted_hyperlinks field. All the submitted files were uploaded to the file space and in the “course_folder/assignment_folder/team_folder” path.

'''What was wrong with that''': There are multiple methods in assignment_participant.rb which still look like the author submit the hyperlinks on behalf of him/herself. They should be moved to assignment_teams.rb.

'''What refactoring we have done to improve''':
* Removed “hyperlinks_array” method and “hyperlinks” method from assignment_participant.rb. Created “hyperlinks” method in assignment_team.rb.
* Removed “submit_hyperlink” and “remove_hyperlink” from assignment_pariticpant.rb and created equivalent methods in assignment_team.rb.

* Removed “has_submissions?” from assginment_participant.rb and created equivalent method in assignment_team.rb.
* All the calls to the above methods were refactored to call the appropriate method's functions.
* Wrote tests for submitting and removing submitted hyperlinks and files.

== Key changes ==
Files with major modifications:
# The model: assignment_participant.rb
# The model: assignment_team.rb
Four methods "hyperlinks”, “has_submissions?”, “submit_hyperlink” and “remove_hyperlink” were moved from the assignment_participant.rb model to assignment_team.rb, and the "hyperlinks_array" method removed. 

1.a) The below hyperlinks_array and hyperlinks method are removed from the assignment_participant.rb:<syntaxhighlight lang="ruby">
def hyperlinks
team.try(:hyperlinks) || []
end

def hyperlinks_array
self.team.submitted_hyperlinks.blank? ? [] : YAML::load(self.team.submitted_hyperlinks)
end
</syntaxhighlight>b) They are both merged into a single method in assignment_team.rb under the hyperlinks mehtod:<syntaxhighlight lang="ruby">
def hyperlinks
self.submitted_hyperlinks.blank? ? [] : YAML::load(self.submitted_hyperlinks)
end
</syntaxhighlight>

2. a) Earlier, the following method submit_hyperlink was in assignment_participant.rb:<syntaxhighlight lang="ruby">
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks_array
hyperlinks << hyperlink
team_object = self.team
team_object.submitted_hyperlinks = YAML::dump(hyperlinks)
team_object.save
end
</syntaxhighlight>b) It is now moved to assignment_team.rb:<syntaxhighlight lang="ruby">
def submit_hyperlink(hyperlink)
hyperlink.strip!
raise "The hyperlink cannot be empty" if hyperlink.empty?
url = URI.parse(hyperlink)
# If not a valid URL, it will throw an exception
Net::HTTP.start(url.host, url.port)
hyperlinks = self.hyperlinks
hyperlinks << hyperlink
self.submitted_hyperlinks = YAML::dump(hyperlinks)
self.save
end
</syntaxhighlight>

3) Similarly the methods "has_submissions?", "remove_hyperlink" were moved.

4) Most importantly, at all the places in the Expertiza code, where there were calls to the above methods in assignment_participant.rb, which are now moved to assignment_team.rb, were refactored accordingly to call them appropriately.

== Testing Details ==
=== RSpec ===
There were no existing tests for the hyperlinks related methods. We used RSpec to write test cases using TTD approach. The assignment_team_spec.rb in the spec folder will have these tests. All the tests can be executed by rspec spec command, or can also be executed individually using the command "rspec spec/models/assignment_team_spec.rb".

=== UI Testing ===

Following steps needs to be performed to test this code from UI:
# Login as a student.
# In an assignment, that is still due, add one or more hyperlinks.
# Login as another student who is a teammate of the first student working on the same assignment.
# You should be able to see the newly uploaded link(s) by the other team member.
# Try adding a link that is not a valid http or https link. It will give an error message.
# Try submitting the same link once again. This will give an error message too.

CSC/ECE 517 Spring 2016/OSS E1601

2016-03-23T18:07:38Z

Kvysyar: testing a template

Testing a template. This wiki page is for the description of changes made under E1555 OSS assignment for Fall 2015, CSC/ECE 517.

== Peer Review Information ==

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student4340, password -> password
* Student login: username -> student4405, password -> password

== Expertiza Background==

Expertiza is an educational web application created and maintained by the joint efforts of the students and the faculty at NCSU. It’s an open source project developed on Ruby on Rails platform and it’s code is available on Github. It allows students to review each other’s work and improve their work upon this feedback.

== Description of the current project ==

The following is an Expertiza based OSS project which deals primarily with the GradesController and GradesHelper. It focusses on refactoring some of the more complex methods, modifying some of the language to make it more Ruby friendly, removing some redundant code. The goal of this project is to attempt to make this part of the application easier to read and maintain.

== Files modified in current project ==

A controller and a helper file were modified for this project namely: 
1. GradesController 
2. GradesHelper 

=== GradesController ===

This is a controller that helps students and instructors view grades and reviews, update scores, check for grading conflicts and calculate penalties. A couple of long and complex methods were refactored from this controller along with removal of some non-functional code and a few language changes to make it Ruby style.
Three methods in particular, namely conflict_notification ,calculate_all_penalties and edit were found to be too long and were in need of refactoring into smaller, easier to manage methods. Few more compact methods were created for this purpose.

There were no existing test cases for the controller. We have added a spec file named 'grades_spec.rb' under the spec folder. As no changes were done for the model, no tests for the model were included.

=== GradesHelper ===

This is a helper class which contains methods for constructing a table(construct_table) and to check whether an assignment has a team and metareveiw(has_team_and_metareview)

== List of changes ==
We worked on the following work items(WIs) 
WI1 : Refactor calculate_all_penalties method into smaller methods 
WI2 : Move the repeated code in conflict_notification & edit methods to a separate method list_questions. 
WI3 : Refactor the code as per the Ruby style guidelines and incorporate the good practices 
WI4 : Test the conflict_notification method to test the changes made. 
WI5 : Move the repeated code in view and view_my_scores methods to a separate method retrieve_questions

=== Solutions Implemented and Delivered ===

*Refactoring calculate_all_penalties method

This is used to calculate various penalty values for each assignment if penalty is applicable.

The following changes were made:

1. This method was very complex, performing too many functions within a single method and had to be broken into 3 smaller methods each having a more well defined function.
2. The following 3 methods were created after splitting the first method 
i. calculate_all_penalties 
ii. calculate_penatly_attributes 
iii. assign_all_penalties 
3. Changes were also made to make the code follow ruby style.The language was made more ruby friendly.
4. Finally some redundant code was commented out as it was non-functional.

Refactoring into smaller more specific methods:

[[File:Change6_new.png]]

Removal of non-functional code :

[[File:Change5_new.png]]

Change of language to make it more Ruby friendly:

[[File:Change1_new.png]]

*Move the redundant piece of code from conflict_notification & edit methods to a new method list_questions

The conflict_notification method is used to help the instructors decide if one of the reviews are unfair or inaccurate.
This was again split into 2 methods with some part of the code which is repeated in another method refactored into a new method.

[[File:Change3_new.png]]

Refactored #Created a method which was a duplicate in conflict_notification and edit methods

[[File:Change4_new.png]]

edit method:

This method is used to edit the questionnaires. This method again has code which is repeated in the conflict_notification method and thus the repeated section was split into a new method.

[[File:Change2_new.png]]

New method:
Refactored #Created a method which was a duplicate in conflict_notification and edit methods

[[File:Change4_new.png]]

Similar refactoring was performed to obtain the retrieve_questions method:

[[File:Latest1.png]]

This is the new method created after the above refactoring:

[[File:Latest2.png]]

== Testing Details==

=== RSpec ===
There were no existing test cases for the GradesController. We have added a new spec file 'grades_spec.rb' which covers testing scenario for the newly added method. The specs were run on the previous and current files and they return the same results implying that the refactored code does not break anything.
As the model was not changed, no test cases were added for the model.

=== UI Testing ===

Following steps needs to be performed to test this code from UI: 
1. Login as instructor. Create a course and an assignment under that course. 
2. Keep the has team checkbox checked while creating the assignment. Add a grading rubric to it. Add at least two students as participants to the assignment. 
3. Create topics for the assignment. 
4. Sign in as one of the students who were added to the assignment. 
5. Go to the assignment and sign up for a topic. 
6. Submit student's work by clicking 'Your work' under that assignment. 
7. Sign in as a different student which is participant of the assignment. 
8. Go to Assignments--><assignment name>-->Others' work (If the link is disabled, login as instructor and change the due date of the assignment to current time). 
9. Give reviews on first student's work. 
10. Login as instructor or first student to look at the review grades. 

== Scope for future improvement ==
1. The construct_table method in GradesHelper is not used anywhere. It has no reference in the project. So we feel it can be safely removed. 
2. The has_team_and_metareview? method in GradesHelper can be broken down into separate methods, one each for team and metareview. This will provide improved flexibility. It needs some analysis though, as both the entities(team & metareview) are currently checked in conjuction from all the views they are referenced from.

CSC/ECE 517 Spring 2016/OSS E1601

2016-03-23T18:02:31Z

Kvysyar: testing

testing