Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Fall 2015 E1583 Fix the CSS used for Menu Item

2015-12-05T00:57:07Z

Sbalasu7: /* Implementation */

== Purpose ==

This project aims to fix the problem of the Expertiza Menu layout for smaller devices such as mobiles and tablets. There are many problems currently existing for devices with smaller screen size such as, 

* The Menu layout takes almost whole screen space, which is not suitable from user experience point of view. 
* The submenu items cannot be selected smoothly. 
* User can neither see the content nor scroll it down properly when the menu is open, as it stays behind the menu. 

The above mentioned issues can be seen in the screenshot below. 

[[File:Screen_Shot_2015-11-08_at_10.33.15_PM.png‎]] 

== Planned Changes ==

The aforementioned issues will be addressed as explained below: 

* Reduce the height of the Menu items. As seen in the screenshot above, there is a lot of space between two menu items. Reducing the space in between will reduce the total screen space consumed by the Menu. 
* Show submenu items on the right side of respective menu item, so that they don't overlap with other menu items (Right now, they are shown below the main menu item). 
* Show the Menu selection button on the left side instead of right side. 
* Start the content below the Menu when Menu is in expanded mode. 

Below is the screenshot of the proposed CSS changes which will be reflected as below. 

[[File:WikiImage.png]] 

Note: This is for reduced screen sizes, and not for normal desktop size. Normal desktop layout will remain the same. 
As seen in the screenshot, 
* Menu item height is reduced and now it is not taking all the screen space. 
* Submenu items are displayed on the right side of the main menu item. 
* Content is displayed below the Menu, and it is not behind the Menu. It can be easily scrolled down/viewed even with Menu open. 
* Menu button is on the left side instead of right. 

== Testing ==

We are using the browser extension "Responsive Web Design Tester". This extension allows to test a web page for variety of devices with different screen sizes, such as (but not limited to), iPad, iPadMini, iPhone, Nexus, HTC-One etc. 
With the help of this tool, we will be easily able to test that our changes are working fine for different screen sizes.
 
Responsive Web Design Tool - Settings 

[[File:Screen_Shot_2015-11-08_at_10.21.31_PM.png]] 

Responsive Web Design Tool - Devices list 

[[File: Screen_Shot_2015-11-08_at_10.22.05_PM.png]] 

== Implementation ==

The files listed below need to be changed to make proposed changes. 

* /config/vendor/assets/components/bootstrap/less/navbar.less 
The navbar-collapse less style needs to be modified here to enable seamless use and to handle overflows in case there is too much content for the user's view port on mobile devices.
* /app/assets/stylesheets/navbar.scss
The height difference between different menu options needs to be changed by modifying the number of pixels in the height attribute of the style in the following code
<pre>
ul.nav li.first-level {
font-size: 100%;
height: 66px;
}
</pre>
* /app/views/shared/_navigation.html.erb 
The icon for the menu selection currently is placed on the right and the positioning of this now needs to be moved to the far left. This shall be done by moving the 'icon-bar' around.
* /app/assets/stylesheets/layout.scss
This file will contain the changes where we intend to make the position of the content relative to the header. This will ensure that even upon re-sizing the window the content will start below the header and would not be covered underneath the header.

=== Sub-Menu Overlapping Main-Menu Problem ===

On hovering over the menu items on the navigation bar shows the sub-menu. But, the sub-menu position on the screen hides the Main menu. In order to resolve the problem, we have shifted the sub-menu to the right from its actual position such that the main-menu is visible.

app/assets/stylesheets/navbar.scss
.dropdown-menu.pull-left{
top: 25%;
left: 15%;
}

== Plan of Work ==

Following milestones have been identified to ensure the delivery of the project 1 week prior to the final deadline. 
* Reducing Height of Menu items and pushing submenu to right side of main Menu item. - 15th November, 2015 
* Pushing the content below the menu - 22nd November, 2015 
* Changing position of menu button to left side, and changing transition from left to right. - 29th November, 2015

CSC/ECE 517 Fall 2015/oss E1571

2015-11-07T04:53:14Z

Sbalasu7: /* Running the Project Locally */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.

bundle install
rake db:create:all
rake db:migrate
rails s

==Testing using Selenium IDE==
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here].

===Testing Instructions===
Follow the instructions below to test the application using selenium IDE
<ul>
<li> Install Selenium IDE plugin for Firefox.
<li> Login to Expertiza using username: instructor6 & password: password.
<li> Download the test scripts from [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]
<li> Open the test script from Selenium IDE. File -> open -> select test script html file
<li> Set the testing speed to slow on the speed slidebar, otherwise test may fail due to slow page response
<li> Run the script from Selenium IDE using 'Play current test case' icon.
</ul>

To add a new assignment or topic and to remove an assignment or topic, we need to change the script every time to avoid duplicate record IDs. When we run the same script more than once, the test case will fail. Thus, we haven't provided test case for those scenarios.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

CSC/ECE 517 Fall 2015/oss E1571

2015-11-07T04:52:39Z

Sbalasu7: /* Testing using Selenium IDE */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

==Testing using Selenium IDE==
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here].

===Testing Instructions===
Follow the instructions below to test the application using selenium IDE
<ul>
<li> Install Selenium IDE plugin for Firefox.
<li> Login to Expertiza using username: instructor6 & password: password.
<li> Download the test scripts from [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]
<li> Open the test script from Selenium IDE. File -> open -> select test script html file
<li> Set the testing speed to slow on the speed slidebar, otherwise test may fail due to slow page response
<li> Run the script from Selenium IDE using 'Play current test case' icon.
</ul>

To add a new assignment or topic and to remove an assignment or topic, we need to change the script every time to avoid duplicate record IDs. When we run the same script more than once, the test case will fail. Thus, we haven't provided test case for those scenarios.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

CSC/ECE 517 Fall 2015 E1583 Fix the CSS used for Menu Item

2015-11-07T00:14:26Z

Sbalasu7:

== Introduction ==

== Purpose ==

== Testing ==

== Implementation ==

==References==
{{reflist}}

CSC/ECE 517 Fall 2015 E1583 Fix the CSS used for Menu Item

2015-11-07T00:14:09Z

Sbalasu7:

== Introduction ==

== Purpose ===

== Testing ===

== Implementation ==

==References==
{{reflist}}

CSC/ECE 517 Fall 2015 E1583 Fix the CSS used for Menu Item

2015-11-07T00:13:53Z

Sbalasu7:

== Introduction ==

=== Purpose ===

=== Testing ===

== Implementation ==

==References==
{{reflist}}

CSC/ECE 517 Fall 2015

2015-11-07T00:07:41Z

Sbalasu7: /* Final Project Design Document */

==Writing Assignment 2==
*[[CSC/ECE_517_Fall_2015/sample_page]]
*[[CSC/ECE_517_Fall_2015/ossE1558BGJ]]
*[[CSC/ECE 517 Fall 2015/oss/M1502/AAAASS]]
*[[CSC/ECE 517 Fall 2015/oss/M1503/IntegrateXMLParser]]
*[[CSC/ECE_517_Fall_2015/ossE1568BZHXJS]]
*[[CSC/ECE_517_Fall_2015/ossE1572VGA]]
*[[CSC/ECE_517_Fall_2015/oss_E1573_sap]]
*[[CSC/ECE 517 Fall 2015/oss E1559 rrz]]
*[[CSC/ECE 517 Fall 2015/oss E1570 avr]]
*[[CSC/ECE 517 Fall 2015/oss E1556 CHM]]
*[[CSC/ECE 517 Fall 2015/oss M1504 JJD]]
*[[CSC/ECE 517 Fall 2015/oss E1562 APS]]
*[[CSC/ECE 517 Fall 2015/oss M1501 GSN]]
*[[CSC/ECE 517 Fall 2015/oss M1501 GSN]]
*[[CSC/ECE 517 Fall 2015/oss E1550 KMM]]
*[[CSC/ECE 517 Fall 2015/oss E1551 RGS]]
*[[CSC/ECE 517 Fall 2015/oss E1555 GMR]]
*[[CSC/ECE 517 Fall 2015/oss E1552 NFR]]
*[[CSC/ECE 517 Fall 2015/oss E1565 AAJ]]
*[[CSC/ECE 517 Fall 2015/oss E1561 WZL]]
*[[CSC/ECE 517 Fall 2015/oss E1553 AAJ]]
*[[CSC/ECE 517 Fall 2015/oss E1554 AAR]]
*[[CSC/ECE 517 Fall 2015/oss E1569 JNR]]
*[[CSC/ECE 517 Fall 2015/oss E1560 PSV]]
*[[CSC/ECE 517 Fall 2015/oss M1505 MSV]]
*[[CSC/ECE 517 Fall 2015/oss E1557 GXM]]
*[[CSC/ECE 517 Fall 2015/oss E1566 ARB]]
*[[CSC/ECE 517 Fall 2015/oss E1567 APT]]
*[[CSC/ECE 517 Fall 2015/oss E1574 BKS]]
*[[CSC/ECE 517 Fall 2015/ossA1550RAN]]
*[[CSC/ECE 517 Fall 2015/oss E1571]]

==Final Project Design Document==
*[[CSC/ECE 517 Fall 2015 E1577 MayYellowRoverJump]]
*[[CSC/ECE 517 Fall 2015 E1586 AnonymousChatBetweenAuthorAndReviewer]]
*[[CSC/ECE 517 Fall 2015 E1582 Create integration tests for the instructor interface using capybara and rspec]]
*[[CSC/ECE 517 Fall 2015 E1576 Refactoring submitted content (hyperlinks and files)]]
*[[CSC/ECE 517 Fall 2015 E1590 Integration testing for Team creation]]
*[[CSC/ECE 517 Fall 2015 E1585 Use Ajax for Add Participants, Add TA ,Edit Questionnaires Screens]]
*[[CSC/ECE 517 Fall 2015 E1581 Integration testing for student interface]]
*[[CSC/ECE 517 Fall 2015 E1583 Fix the CSS used for Menu Item]]

CSC/ECE 517 Fall 2015 E1583 Fix the CSS used for Menu Item

2015-11-07T00:06:48Z

Sbalasu7: Created page with "==Introduction=="

==Introduction==

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T22:38:39Z

Sbalasu7: /* Testing Instructions */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here].

===Testing Instructions===
Follow the instructions below to test the application using selenium IDE
<ul>
<li> Install Selenium IDE plugin for Firefox.
<li> Login to Expertiza using username: instructor6 & password: password.
<li> Download the test scripts from [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]
<li> Open the test script from Selenium IDE. File -> open -> select test script html file
<li> Set the testing speed to slow on the speed slidebar, otherwise test may fail due to slow page response
<li> Run the script from Selenium IDE using 'Play current test case' icon.
</ul>

To add a new assignment or topic and to remove an assignment or topic, we need to change the script every time to avoid duplicate record IDs. When we run the same script more than once, the test case will fail. Thus, we haven't provided test case for those scenarios.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T22:37:45Z

Sbalasu7: /* Testing Instructions */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here].

===Testing Instructions===
Follow the instructions below to test the application using selenium IDE

<li> Install Selenium IDE plugin for Firefox.
<li> Login to Expertiza using username: instructor6 & password: password.
<li> Download the test scripts from [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]
<li> Open the test script from Selenium IDE. File -> open -> select test script html file
<li> Set the testing speed to slow on the speed slidebar, otherwise test may fail due to slow page response
<li> Run the script from Selenium IDE using 'Play current test case' icon.

To add a new assignment or topic and to remove an assignment or topic, we need to change the script every time to avoid duplicate record IDs. When we run the same script more than once, the test case will fail. Thus, we haven't provided test case for those scenarios.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T22:37:15Z

Sbalasu7: /* Testing Instructions */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here].

===Testing Instructions===
Follow the instructions below to test the application using selenium IDE

<li> Install Selenium IDE plugin for Firefox.
<li> Login to Expertiza using username: instructor6 & password: password.
<li> Download the test scripts from [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]
<li> Open the test script from Selenium IDE. File -> open -> select test script html file
<li> Set the testing speed to slow on the speed slidebar, otherwise test may fail due to slow page response
<li> Run the script from Selenium IDE using 'Play current test case' icon

To add a new assignment or topic and to remove an assignment or topic, we need to change the script every time to avoid duplicate record IDs. When we run the same script more than once, the test case will fail. Thus, we haven't provided test case for those scenarios.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T22:37:06Z

Sbalasu7: /* Testing using Selenium IDE */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here].

===Testing Instructions===
Follow the instructions below to test the application using selenium IDE

<li> Install Selenium IDE plugin for Firefox.
<li> Login to Expertiza using username: instructor6 & password: password.
<li> Download the test scripts from [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]
<li> Open the test script from Selenium IDE. File -> open -> select test script html file
<li> Set the testing speed to slow on the speed slidebar, otherwise test may fail due to slow page response
<li> Run the script from Selenium IDE using 'Play current test case' icon

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T22:36:30Z

Sbalasu7: /* Testing using Selenium IDE */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here].
To add a new assignment or topic and to remove an assignment or topic, we need to change the script every time to avoid duplicate record IDs. When we run the same script more than once, the test case will fail. Thus, we haven't provided test case for those scenarios.

===Testing Instructions===
Follow the instructions below to test the application using selenium IDE

<li> Install Selenium IDE plugin for Firefox.
<li> Login to Expertiza using username: instructor6 & password: password.
<li> Download the test scripts from [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]
<li> Open the test script from Selenium IDE. File -> open -> select test script html file
<li> Set the testing speed to slow on the speed slidebar, otherwise test may fail due to slow page response
<li> Run the script from Selenium IDE using 'Play current test case' icon

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T22:36:14Z

Sbalasu7: /* Testing using Selenium IDE */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]
To add a new assignment or topic and to remove an assignment or topic, we need to change the script every time to avoid duplicate record IDs. When we run the same script more than once, the test case will fail. Thus, we haven't provided test case for those scenarios.

===Testing Instructions===
Follow the instructions below to test the application using selenium IDE

<li> Install Selenium IDE plugin for Firefox.
<li> Login to Expertiza using username: instructor6 & password: password.
<li> Download the test scripts from [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]
<li> Open the test script from Selenium IDE. File -> open -> select test script html file
<li> Set the testing speed to slow on the speed slidebar, otherwise test may fail due to slow page response
<li> Run the script from Selenium IDE using 'Play current test case' icon

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T21:48:19Z

Sbalasu7: /* References */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]

===Testing Instructions===
Follow the instructions below to test the application using selenium IDE

<li> Install Selenium IDE plugin for Firefox.
<li> Login to Expertiza using username: instructor6 & password: password.
<li> Download the test scripts from [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]
<li> Open the test script from Selenium IDE. File -> open -> select test script html file
<li> Set the testing speed to slow on the speed slidebar, otherwise test may fail due to slow page response
<li> Run the script from Selenium IDE using 'Play current test case' icon

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T21:47:53Z

Sbalasu7: /* Testing Instructions */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]

===Testing Instructions===
Follow the instructions below to test the application using selenium IDE

<li> Install Selenium IDE plugin for Firefox.
<li> Login to Expertiza using username: instructor6 & password: password.
<li> Download the test scripts from [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]
<li> Open the test script from Selenium IDE. File -> open -> select test script html file
<li> Set the testing speed to slow on the speed slidebar, otherwise test may fail due to slow page response
<li> Run the script from Selenium IDE using 'Play current test case' icon

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T21:44:56Z

Sbalasu7: /* Testing Instructions */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]

===Testing Instructions===

<li> Install Selenium IDE plugin for Firefox.
<li>2 Login to Expertiza using username: instructor6 & password: password.
<li> Download the test scripts from [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]</li>
<li> Open the test script from Selenium IDE. File -> open -> select test script html file.</li>
<li> Set the testing speed to slow on the speed slidebar, otherwise test may fail due to slow page loading.</li>
<li> Run the script from Selenium IDE using 'Play current test case' icon.</li>

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T21:44:22Z

Sbalasu7: /* Testing Instructions */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]

===Testing Instructions===

<li> Install Selenium IDE plugin for Firefox.</li>
<li>2 Login to Expertiza using username: instructor6 & password: password.</li>
<li> Download the test scripts from [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]</li>
<li> Open the test script from Selenium IDE. File -> open -> select test script html file.</li>
<li> Set the testing speed to slow on the speed slidebar, otherwise test may fail due to slow page loading.</li>
<li> Run the script from Selenium IDE using 'Play current test case' icon.</li>

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T21:44:00Z

Sbalasu7: /* Testing Instructions */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]

===Testing Instructions===

<li>1. Install Selenium IDE plugin for Firefox.</li>
<li>2. Login to Expertiza using username: instructor6 & password: password.</li>
<li>3. Download the test scripts from [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]</li>
<li>4. Open the test script from Selenium IDE. File -> open -> select test script html file.</li>
<li>5. Set the testing speed to slow on the speed slidebar, otherwise test may fail due to slow page loading.</li>
<li>6. Run the script from Selenium IDE using 'Play current test case' icon.</li>

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T21:43:10Z

Sbalasu7: /* Testing from UI Instructions */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]

===Testing Instructions===

1. Install Selenium IDE plugin for Firefox.
2. Login to Expertiza using username: instructor6 & password: password.
3. Download the test scripts from [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]
4. Open the test script from Selenium IDE. File -> open -> select test script html file.
5. Set the testing speed to slow on the speed slidebar, otherwise test may fail due to slow page loading.
6. Run the script from Selenium IDE using 'Play current test case' icon.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T21:42:34Z

Sbalasu7: /* Testing using Selenium IDE */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]

==Testing from UI Instructions==

1. Install Selenium IDE plugin for Firefox.
2. Login to Expertiza using username: instructor6 & password: password.
3. Download the test scripts from [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]
4. Open the test script from Selenium IDE. File -> open -> select test script html file.
5. Set the testing speed to slow on the speed slidebar, otherwise test may fail due to slow page loading.
6. Run the script from Selenium IDE using 'Play current test case' icon.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T21:32:05Z

Sbalasu7: /* Testing using Selenium IDE */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T21:31:42Z

Sbalasu7: /* Testing using Selenium IDE */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]<reference name=selenium>{{cite web|title=Selenium IDE Patch|url=http://www.seleniumhq.org/projects/ide/ Selenium IDE}}</reference> is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application. Test scripts can be found [https://drive.google.com/drive/u/2/folders/0B490tipwiqqMVnVjc0FjcDdIZ2c here]

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T20:51:56Z

Sbalasu7: /* Testing using Selenium IDE */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]<reference name=selenium>{{cite web|title=Selenium IDE Patch|url=http://www.seleniumhq.org/projects/ide/ Selenium IDE}}</reference> is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T20:51:27Z

Sbalasu7: /* References */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]<ref name=selenium>{{cite web|title=Selenium IDE Patch|url=http://www.seleniumhq.org/projects/ide/ Selenium IDE}}</ref> is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T20:51:08Z

Sbalasu7: /* References */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]<ref name=selenium>{{cite web|title=Selenium IDE Patch|url=http://www.seleniumhq.org/projects/ide/ Selenium IDE}}</ref> is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T20:50:56Z

Sbalasu7: /* References */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]<ref name=selenium>{{cite web|title=Selenium IDE Patch|url=http://www.seleniumhq.org/projects/ide/ Selenium IDE}}</ref> is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<reflist/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T20:50:17Z

Sbalasu7: /* Testing using Selenium IDE */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE]<ref name=selenium>{{cite web|title=Selenium IDE Patch|url=http://www.seleniumhq.org/projects/ide/ Selenium IDE}}</ref> is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed. The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T20:46:02Z

Sbalasu7: /* Change to new redirect method rather using controller and action explicitly. */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as follows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE] is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed.<ref>{{cite web|title=Selenium IDE|url=http://www.seleniumhq.org/projects/ide/|accessdate=31 October 2015}}</ref> The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T20:45:19Z

Sbalasu7: /* Change to new redirect method rather using controller and action explicitly. */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as foloows

def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE] is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed.<ref>{{cite web|title=Selenium IDE|url=http://www.seleniumhq.org/projects/ide/|accessdate=31 October 2015}}</ref> The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T20:45:03Z

Sbalasu7: /* remove_assignment_from_course moved to model */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,

def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end

Inside model,

def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as foloows
<source lang="ruby">
def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end
</source>

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE] is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed.<ref>{{cite web|title=Selenium IDE|url=http://www.seleniumhq.org/projects/ide/|accessdate=31 October 2015}}</ref> The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T20:44:30Z

Sbalasu7: /* associate_assignment_with_course moved to model */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end

Inside model,

def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,
<source lang="ruby">
def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end
</source>
Inside model,
<source lang="ruby">
def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end
</source>

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as foloows
<source lang="ruby">
def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end
</source>

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE] is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed.<ref>{{cite web|title=Selenium IDE|url=http://www.seleniumhq.org/projects/ide/|accessdate=31 October 2015}}</ref> The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T20:44:03Z

Sbalasu7: /* Refactoring Update function */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.

session[:user]

The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.

redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
<source lang="ruby"> def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end
</source>

Inside model,
<source lang="ruby">
def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end
</source>

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,
<source lang="ruby">
def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end
</source>
Inside model,
<source lang="ruby">
def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end
</source>

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as foloows
<source lang="ruby">
def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end
</source>

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE] is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed.<ref>{{cite web|title=Selenium IDE|url=http://www.seleniumhq.org/projects/ide/|accessdate=31 October 2015}}</ref> The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T20:43:26Z

Sbalasu7: /* Refactoring Edit function */

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.

@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end

This part of the code is moved into a separate function.

def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.

if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end

Redundant code is removed and made it more efficient and understandable.

@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.

if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.

def needed_rubrics(empty_rubrics_list)
...
end

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.
<source lang="ruby">
session[:user]
</Source>
The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.
<source lang="ruby">
redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct
</Source>

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
<source lang="ruby"> def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end
</source>

Inside model,
<source lang="ruby">
def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end
</source>

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,
<source lang="ruby">
def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end
</source>
Inside model,
<source lang="ruby">
def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end
</source>

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as foloows
<source lang="ruby">
def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end
</source>

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE] is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed.<ref>{{cite web|title=Selenium IDE|url=http://www.seleniumhq.org/projects/ide/|accessdate=31 October 2015}}</ref> The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>

CSC/ECE 517 Fall 2015

2015-11-06T20:37:55Z

Sbalasu7: /* Writing Assignment 2 */

CSC/ECE 517 Fall 2015/oss E1571

2015-11-06T20:33:18Z

Sbalasu7: OSS Project - E1571 Moving wiki to valid page

==Introduction==
===Background===
Expertiza provides a dashboard for all the Assignments corresponding to a course and provides absolute control to the Instructors and Teaching Assistant. In addition to Assignments, it encompasses peer reviews where in participants are allowed to provide feedback anonymously about each other's work thereby providing scope for better outcome.

Assignments in Expertiza have their association with many other components like Courses,Reviews,Participants and Scores. The Assignment controller in specific performs various other actions in addition to the CRUD operations applied on its model. These actions sew up the association of an assignment with others like courses etc. There exists an AssignmentForm model which acts as a wrapper for the Assignment and provides further control.

===Motivation===
This project in particular intends that the students collaborate with each other and work on making enhancements to the code base by applying the concepts of Rails,RSpec, DRY code,Test driven development etc. This provides an opportunity for students to contribute to an open source project and learn further about software deployment etc.

Currently, the Assignment controller holds methods which are deprecated and also has a few lines code in its Edit and Update methods which could be removed to make the code DRY. In addition to that, it houses a few actions where the code could be refactored to make it DRY and follow the Ruby style guide and Rails 4 syntax.

===Tasks Identified===
* Refactor edit and update method
* Remove irrelevant comments from the create action.
* associate_assignment_with_course and remove_assignment_from_course logic should be moved to model.
* Change to new redirect method rather using controller and action explicitly.
* Refactor the update_due_dates from AssignmentForm. using the DRY principle
* Refactor code to follow ruby style guide and Rails4 syntax.

====Classes====
* controllers/assignments_controller.rb
* models/Assignment.rb

==== Refactoring Edit function ====
Edit function contains many manipulations that need be refactored into separate functions.
The following block contains manipulations such as checking if due_date name or description is not defined.
<source lang="Ruby">
@due_date_all.each do |dd|
if((!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?))
@due_date_nameurl_notempty = true
@due_date_nameurl_notempty_checkbox = true
end
....
end</source>

This part of the code is moved into a separate function.
<source lang="Ruby">
def is_due_date_nameurl_notempty(dd)
(!dd.deadline_name.nil?&&!dd.deadline_name.empty?)||(!dd.description_url.nil?&&!dd.description_url.empty?)
end
</source>

There are lots of redundant code in the following snippet.And, constants are hard coded inside the code rather than defining constants.
<source lang="Ruby">
if dd.deadline_type_id==5
@metareview_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @metareview_allowed
break
end
if dd.deadline_type_id==6
@drop_topic_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @drop_topic_allowed
break
end
if dd.deadline_type_id==7
@signup_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @signup_allowed
break
end
if dd.deadline_type_id==8
@team_formation_allowed = true
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox && @team_formation_allowed
break
end
end
</source>

Redundant code is removed and made it more efficient and understandable.
<source lang="Ruby">
@metareview_allowed = is_meta_review_allowed?(dd);
@drop_topic_allowed = is_drop_topic_allowed?(dd);
@signup_allowed = is_signup_allowed?(dd);
@team_formation_allowed = is_team_formation_allowed?(dd);

if dd.due_at.present?
dd.due_at = dd.due_at.to_s.in_time_zone(current_user.timezonepref)
end
if @due_date_nameurl_notempty && @due_date_nameurl_notempty_checkbox &&
(@metareview_allowed || @drop_topic_allowed || @signup_allowed || @team_formation_allowed)
break
end
</source>

Hard coded constants are removed and necessary constants are defined in the deadline_helper.rb helper file.

<source lang="Ruby">
DEADLINE_TYPE_METAREVIEW = 5
DEADLINE_TYPE_DROP_TOPIC = 6
DEADLINE_TYPE_SIGN_UP = 7
DEADLINE_TYPE_TEAM_FORMATION = 8
</source>

And, manipulations such as checking if metareview, drop_topic,singn_up and team_formation are allowed need to be refactored into separate functions.

<source lang="Ruby">
def is_meta_review_allowed?(dd)
status = false
if dd.deadline_type_id== DeadlineHelper::DEADLINE_TYPE_METAREVIEW
status = true
end
status
end

def is_drop_topic_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_DROP_TOPIC
status = true
end
status
end

def is_signup_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_SIGN_UP
status = true
end
status
end

def is_team_formation_allowed?(dd)
status = false
if dd.deadline_type_id==DeadlineHelper::DEADLINE_TYPE_TEAM_FORMATION
status = true
end
status
end
</source>

The following snippet constructs a string that need to be shown to the user when rubrics are not set by the instructor.
<source lang="Ruby">
if !empty_rubrics_list.empty? and request.original_fullpath == "/assignments/#{@assignment_form.assignment.id}/edit"
empty_rubrics = "["
empty_rubrics_list.each do |item|
empty_rubrics += item[0...-13] + ", "
end
empty_rubrics = empty_rubrics[0...-2]
empty_rubrics += "] "
flash.now[:error] = "You did not specify all necessary rubrics: " +empty_rubrics+" of assignment #{@assignment_form.assignment.name} before saving the assignment. You can assign rubrics <a id='go_to_tabs2' style='color: blue;'>here</a>."
end
</source>

Adding code such as above directly into the controller action made the edit function bulkier. The above snippet is also refactored into a separate function.
<source lang="Ruby">
def needed_rubrics(empty_rubrics_list)
...
end
</source>

Refactoring out into separate functions makes the code more readable and understandable.

==== Refactoring Update function ====

Update function of the Assignment controller helps in updating the resources after editing it.
The following code retrieves the data of the user logged in.
<source lang="ruby">
session[:user]
</Source>
The above statement checks the cookies every time, when we want to retrieve information about current user. As it is inefficient and wrong, we need to use helper functions to retrieve such information.

Explicit URL creation using controller name and controller action is refactored in such a way that helper functions create the URL when required.
<source lang="ruby">
redirect_to :action => 'edit', :id => @assignment.id # Incorrect
redirect_to edit_assignment_path @assignment.id # Correct
</Source>

==== Remove irrelevant comments from the create action ====
The Create action has a few comments which are irrelevant and make the method bulky. These could be safely removed from the code.

==== associate_assignment_with_course moved to model ====
This is currently defined as an action in AssignmentsController. The action logic could be moved to Model by passing the current user object to the corresponding method we create in the Model. The method in the model would then return a list of courses which would be used in the View corresponding to this action.

Inside controller action,
<source lang="ruby"> def associate_assignment_with_course
@assignment = Assignment.find(params[:id])
@courses = Assignment.set_courses_to_assignment(current_user)
end
</source>

Inside model,
<source lang="ruby">
def self.set_courses_to_assignment(user)
@courses=Course.where(instructor_id: user.id).order(:name)
end
</source>

==== remove_assignment_from_course moved to model ====
All the current logic here, except for the "save" and "redirect_to" calls could be moved to the model. There is a need to create a method in the Model with the same name and then pass an assignment object. All operations can be performed inside this method and then the user can be redirected back to the assignment list page.
Inside controller action,
<source lang="ruby">
def remove_assignment_from_course
assignment = Assignment.find(params[:id])
Assignment.remove_assignment_from_course(assignment)
redirect_to controller: 'tree_display', action: 'list'
end
</source>
Inside model,
<source lang="ruby">
def self.remove_assignment_from_course(assignment)
oldpath = assignment.path rescue nil
assignment.course_id = nil
assignment.save
newpath = assignment.path rescue nil
FileHelper.update_file_location(oldpath, newpath)
end
</source>

==== Change to new redirect method rather using controller and action explicitly. ====
According to Ruby style guidelines explicit use of action and controller in redirect calls should be avoided. This helps in easy refactoring in case action names are changed in future. All the redirect calls in assignment controller have been modified to use url path helpers. In controller assignment and action toggle_access, redirect call has been changes as foloows
<source lang="ruby">
def toggle_access
assignment = Assignment.find(params[:id])
assignment.private = !assignment.private
assignment.save
redirect_to list_tree_display_index_path
end
</source>

==== Refactor the update_due_dates from AssignmentForm using the DRY principle ====
There exist variables which are declared and initialized but are not used in any part of the code. Such variables could be safely removed without impacting the functionality of the action.
Conditional statements which check whether due_date object is present can be avoided. The same piece of code is used at various points of code making it redundant and hence can be converted to a method.
There exist dead code which tries to remove due_dates not in update or new list from database. There can never be such a case, so the code can be safely removed.

===Changes to the View===

==== Impact Analysis ====
* Tested using [http://www.seleniumhq.org/projects/ide/ Selenium IDE], the associated objects , assignment and course before and after the change. Testing confirms that the objects and their interactions have not changed.
* Addition of new methods do not coincide with the performance of currently existent methods.
* Removal of existing code does not lead to reduced/loss of functionality.

===Affected Classes===
Changed existent classes
* controllers/assignments_controller.rb
* controllers/application_controller.rb
* models/Assignment.rb
* models/User.rb
* views/tree_display/actions/_assignments_actions.html.erb

==Running the Project Locally==
The project could be run locally by cloning the Github repository [https://github.com/codemonkey19/expertiza expertiza] and then running the following commands sequentially.
<source lang="unix">
bundle install
rake db:create:all
rake db:migrate
rails s
</source>

===Testing using Selenium IDE===
[http://www.seleniumhq.org/projects/ide/ Selenium IDE] is implemented as a [[Firefox]] extension. It allows user to record edit and debug test cases. It has a recording feature, which records user actions as they are performed and then exports them as a reusable script in one of many programming languages that can be later executed.<ref>{{cite web|title=Selenium IDE|url=http://www.seleniumhq.org/projects/ide/|accessdate=31 October 2015}}</ref> The UI automation helps in testing quickly yet rigorously. Major test cases identified are following
<ul>
<li>Loading assignments</li>
<li>Creating assignments</li>
<li>Editing assignments</li>
<li>Updating due dates for assignments</li>
<li>Associating assignments to a course</li>
<li>Deleting assignments</li>
</ul>
The test cases were manually run and recorded once. Then the test cases were re run with changed parameters on the deployed version of the application.

==Future Work==
The tasks accomplish only a part of the refactoring and as always with any software application there is scope for even more. The following are a few which were identified as part of this project.
* There exists lot of redundant code which code be refactored into methods.
* Unused variables and deprecated methods could be ridden off from the code base.
* The test cases are written only for the methods that came under the scope of this project. But there is room for implementing test cases for the other actions too.
* In an MVC framework, most of the business logic should be within the Model. Work needs to be done on this front to make it more close to this convention of Rails.

==References==
<references/>