Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Fall 2015 E1591 Integration testing for peer review

2015-12-04T21:44:43Z

Athakur3:

==Background==
Expertiza<ref>[https://github.com/expertiza/expertiza Expertiza's Github Site]</ref> is an open-source web tool that is used for online assignment completion purpose for different courses. The software enables the students to form online teams and choose different assignment topics. Each topic has a completion deadline and the students can submit the links to their work or assignments through expertiza.

One of the key features of expertiza is the online peer review process. Students who are enrolled in the same courses can review the works or the assignments of the fellow students. Typically for each assignment there is a link called "Other’s Work" where a student is prompted to give feedback to the works of their fellow classmates. This link becomes active after a certain date that is determined by the instructor or the administrator. After the date, a student can choose one of the assignments that are available for review through the activated link.

After an assignment has been chosen for review, the reviewer will be presented with a review sheet that has a number of parameters based upon which the review should be done. The reviewer chooses a rating for each parameter, typically being the lowest and 5 as the highest. He can also leave a comment justifying his rating and stating the scopes of improvements for the project. The team members whose assignment has been reviewed can also review the review, i.e. provide an explanation about their work or give comments with respect to the feedback provided by the reviewer.

For some assignments there is a scope for a re submission of an improved version of the work. After such a re-submission the reviewer can again review the assignment editing and updating his previous review based upon the resubmitted work.

==Purpose==
The purpose of the project is to perform an integration testing for the peer review of a submitted assignment. We aim to write test cases that test the different positive test scenarios for which the reviewer should be able to successfully submit the review to an assignment. The review process has certain minimum guidelines which need to be adhered to. We also plan to test the negative scenarios for which the reviewer doesn’t adhere to these guidelines and hence should not be able to submit the review to the assignment.
The Expertiza project has been developed using Ruby on Rails. The technologies to be used for the project are RSpec<ref>[http://rspec.info/documentation/ Rspec].''Rspec Documentation''</ref> and Capybara<ref>[https://github.com/jnicklas/capybara capybara]</ref> using Ruby on Rails as an underlying framework. We will write the spec methods using RSpec. The various entities like users, assignment, questionnaire and instructor will be simulated using Capybara.

==Technologies to be used==
===RSpec===

Rspec is Behavior Driven Development<ref>[https://en.wikipedia.org/wiki/Behavior-driven_development Behavior Driven Development]</ref> framework for ruby, it is a meta gem composed of different libraries designed to work together or with other testing tools like Cucumber<ref>[https://cucumber.io/ Cucumber]</ref>. It has dependencies on Rspec core, Rspec expectations, Rspec mocks gems which can be loaded by using 'require'.

Installation-
gem install rspec

===Capybara===

Capybara<ref>[https://github.com/jnicklas/capybara Capybara github link]</ref> is a library written in ruby, it helps you to test your web applications by pretending to be a user and following the user story, it simulates the browser to interact with app to receive pages parse the HTML and submit forms just as an actual user would do. Capybara can interact with many different drivers which execute tests through the same clean and simple interface. One can choose between Selenium, Webkit or pure Ruby drivers.

Installation-

Capybara requires ruby 1.9.3 or later,to install add following line to your gem file and run bundle install
gem 'capybara'
If testing a rails app add this to your test helper file
require 'capybara/rails'

===Integration Testing===

Integration testing is a phase of software testing where units which have already been tested are combined into a component and the interface between them is tested<ref>[https://msdn.microsoft.com/en-us/library/aa292128(v=vs.71).aspx Integration Testing - Microsoft Developer Network]</ref>. The idea is to test combination of pieces and eventually expand the process to test the modules with those of the other group. Eventually, all the modules making up a process are tested together.
Integration testing identifies the problem that occurs when units are combined. Some different types of integration testing are big bang, top- down, bottom-up<ref>[https://en.wikipedia.org/wiki/Integration_testing Integration Testing - Wikipedia]</ref>.
*Big Bang Integration Testing – All modules are integrated simultaneously, after which everything is tested as a whole. It is very effective for saving time in the integration testing process.
*The top down approach – The highest level modules are tested first. This allows high level logic and data flow to be integrated early in the process.
*The bottom up approach - In this approach, lowest level components are tested first, and then used to facilitate the testing of higher level components.

==Test Pattern and Approach==

We are required to write integration tests for the peer review process. Since we are not adding any new functionalities or amending any existing ones we don’t need to follow any specific design pattern. However we need to follow a specific integration testing approach to conduct the integration testing. The testing approach that we have decided to follow is '''Top-Down'''.
We are first going to create the following entities
* '''User or Reviewer'''
* '''Assignment Questionnaire or Rubric'''
* '''Assignment'''
Then we plan to incrementally test the interaction of these entities in a bottom-up manner.
We first test whether the user is able to successfully login.
If he is able to successfully login, we test whether he is able to successfully select an assignment for review.
If he is able to successfully select an assignment, we test whether he is able to able to submit a review successfully based on whether it is a text-based, point-based or both and whether he chooses to answer all, some or none of the questions of the rubric.

==Flowchart for the Peer Review Testing Approach==
[[File:FlowchartE1591 2.png]]

==Use Case Diagram==

[[File: UseCaseE1591.png]]

==Test Plan==
The following section describes how we plan on testing the application based requirements found on the Project Description<ref>[https://docs.google.com/document/d/1t0keeNQ2kP0NmcgIrtQXGi5K_GhQYcKzEVWJPPRBOHE/edit Project Description]</ref>. We wish to create Integration tests that thoroughly test a Student user reviewing an assignment submission. We will create a file called spec/features/peer_review.rb to run our integration tests.

=== Test Cases Prerequisites ===
To create and run our test cases we require some setup in Expertiza. The following list describes the information that we will enter into the system before running any of our test cases.

# We will use the Student with the username 'student13' and password 'password' to login as a Student.
# We will enroll Student into a specific course.
# We will create an Assignment per individual test case that is customized for our test case.
# We will submit an Assignment from a student other than 'student13' for 'student13' to review

=== Test Cases ===
The following list describes the individual test cases we will creating for Expertiza.

T1: Login as a Student user and select a submission for review. The submission will only contain points based review choices. The user will successfully submit the review answering the points based review choices.

T2: Login as a Student user and select a submission for review. The submission will only contain points based review choices. The user will unsuccessfully submit the review by only answering some of the points based review choices.

T3: Login as a Student user and select a submission for review. The submission will only contain points based review choices. The user will unsuccessfully submit the review by answering none of the points based review choices.

T4: Login as a Student user and select a submission for review. The submission will only contain text based review choices. The user will successfully submit the review answering the text based review choices.

T5: Login as a Student user and select a submission for review. The submission will only contain text based review choices. The user will unsuccessfully submit the review by only answering some of the text based review choices.

T6: Login as a Student user and select a submission for review. The submission will only contain text based review choices. The user will unsuccessfully submit the review by answering none of the text based review choices.

T7: Login as a Student user and select a submission for review. The submission will only contain text based review choices. The user will submit the review by answering all of the text based review choices with single word answers.

T8: Login as a Student user and select a submission for review. The submission will only contain both points based and text based review choices. The user will successfully submit the review answering all of the review choices.

T9: Login as a Student user and select a submission for review. The submission will only contain both points based and text based review choices. The user will unsuccessfully submit the review answering only some of the review choices.

T10: Login as a Student user and select a submission for review. The submission will only contain both points based and text based review choices. The user will unsuccessfully submit the review answering none of the review choices.

==Scenarios Tested==
Six major scenarios were tested. They are as follows:<br>
====T1: Successfully submitting a review by filling in both points and comments.====
This scenario tests if a student can successfully submit a review by filling in comments in a text box and selecting a value from the drop down list.<br>The detailed steps are as follows:
1. Login as a student
2. Select the test assignment for review
3. Click on link "Others Work"
4. Click on button "Request a new submission to Review"
5. Click on the link "Begin"
6. Fill in the text box with comment "Hello World".
7. Select the value 5 from drop down list.
8. Click on the button "Submit Review".
9. "Your response was successfully saved" should be displayed.
===T2: Successfully submitting a review by filling in multi word comments.====
This scenario tests if a student can successfully submit a review by filling in multi word comments in a text box .<br>The detailed steps are as follows:
1. Login as a student
2. Select the test assignment for review
3. Click on link "Others Work"
4. Click on button "Request a new submission to Review"
5. Click on the link "Begin"
6. Fill in the text box with comment "Excellent Work".
7. Click on the button "Submit Review".
8. "Your response was successfully saved" should be displayed.
===T3: Successfully submitting a review by filling a single comment with single word.===
This scenario tests if a student can successfully submit a review by filling in single comment with single word in a text box.<br>The detailed steps are as follows:
1. Login as a student
2. Select the test assignment for review
3. Click on link "Others Work"
4. Click on button "Request a new submission to Review"
5. Click on the link "Begin"
6. Fill in the text box with comment "Excellent".
7. Click on the button "Submit Review".
8. "Your response was successfully saved" should be displayed.
===T4: Successfully submitting a review by filling in only points.===
This scenario tests if a student can successfully submit a review by selecting a value from the drop down list.<br>The detailed steps are as follows:
1. Login as a student
2. Select the test assignment for review
3. Click on link "Others Work"
4. Click on button "Request a new submission to Review"
5. Click on the link "Begin"
6. Select the value 5 from drop down list.
7. Click on the button "Submit Review".
8. "Your response was successfully saved" should be displayed.
===T5: Successfully submitting an empty review.===
This scenario tests if a student can successfully submit an empty review.<br>The detailed steps are as follows:
1. Login as a student
2. Select the test assignment for review
3. Click on link "Others Work"
4. Click on button "Request a new submission to Review"
5. Click on the link "Begin"
6. Click on the button "Submit Review".
7. "Your response was successfully saved" should be displayed.
===T6: Successfully submitting an empty review with additional comments only.===
This scenario tests if a student can successfully submit an empty review by filling in additional comments in a text box.<br>The detailed steps are as follows-
1. Login as a student
2. Select the test assignment for review
3. Click on link "Others Work"
4. Click on button "Request a new submission to Review"
5. Click on the link "Begin"
6. Fill in the additional comments text box with comment "Excellent work done".
7. Click on the button "Submit Review".
8. "Your response was successfully saved" should be displayed.

==Execution==
The tests were executed by submitting the following command from the terminal. <br>
<pre>bundle exec rspec spec/features/peer_review_spec.rb</pre>
[[File: Screenshot.JPG]]
==References==
<references></references>

CSC/ECE 517 Fall 2015 E1591 Integration testing for peer review

2015-12-04T21:41:17Z

Athakur3:

==Background==
Expertiza<ref>[https://github.com/expertiza/expertiza Expertiza's Github Site]</ref> is an open-source web tool that is used for online assignment completion purpose for different courses. The software enables the students to form online teams and choose different assignment topics. Each topic has a completion deadline and the students can submit the links to their work or assignments through expertiza.

One of the key features of expertiza is the online peer review process. Students who are enrolled in the same courses can review the works or the assignments of the fellow students. Typically for each assignment there is a link called "Other’s Work" where a student is prompted to give feedback to the works of their fellow classmates. This link becomes active after a certain date that is determined by the instructor or the administrator. After the date, a student can choose one of the assignments that are available for review through the activated link.

After an assignment has been chosen for review, the reviewer will be presented with a review sheet that has a number of parameters based upon which the review should be done. The reviewer chooses a rating for each parameter, typically being the lowest and 5 as the highest. He can also leave a comment justifying his rating and stating the scopes of improvements for the project. The team members whose assignment has been reviewed can also review the review, i.e. provide an explanation about their work or give comments with respect to the feedback provided by the reviewer.

For some assignments there is a scope for a re submission of an improved version of the work. After such a re-submission the reviewer can again review the assignment editing and updating his previous review based upon the resubmitted work.

==Purpose==
The purpose of the project is to perform an integration testing for the peer review of a submitted assignment. We aim to write test cases that test the different positive test scenarios for which the reviewer should be able to successfully submit the review to an assignment. The review process has certain minimum guidelines which need to be adhered to. We also plan to test the negative scenarios for which the reviewer doesn’t adhere to these guidelines and hence should not be able to submit the review to the assignment.
The Expertiza project has been developed using Ruby on Rails. The technologies to be used for the project are RSpec<ref>[http://rspec.info/documentation/ Rspec].''Rspec Documentation''</ref> and Capybara<ref>[https://github.com/jnicklas/capybara capybara]</ref> using Ruby on Rails as an underlying framework. We will write the spec methods using RSpec. The various entities like users, assignment, questionnaire and instructor will be simulated using Capybara.

==Technologies to be used==
===RSpec===

Rspec is Behavior Driven Development<ref>[https://en.wikipedia.org/wiki/Behavior-driven_development Behavior Driven Development]</ref> framework for ruby, it is a meta gem composed of different libraries designed to work together or with other testing tools like Cucumber<ref>[https://cucumber.io/ Cucumber]</ref>. It has dependencies on Rspec core, Rspec expectations, Rspec mocks gems which can be loaded by using 'require'.

Installation-
gem install rspec

===Capybara===

Capybara<ref>[https://github.com/jnicklas/capybara Capybara github link]</ref> is a library written in ruby, it helps you to test your web applications by pretending to be a user and following the user story, it simulates the browser to interact with app to receive pages parse the HTML and submit forms just as an actual user would do. Capybara can interact with many different drivers which execute tests through the same clean and simple interface. One can choose between Selenium, Webkit or pure Ruby drivers.

Installation-

Capybara requires ruby 1.9.3 or later,to install add following line to your gem file and run bundle install
gem 'capybara'
If testing a rails app add this to your test helper file
require 'capybara/rails'

===Integration Testing===

Integration testing is a phase of software testing where units which have already been tested are combined into a component and the interface between them is tested<ref>[https://msdn.microsoft.com/en-us/library/aa292128(v=vs.71).aspx Integration Testing - Microsoft Developer Network]</ref>. The idea is to test combination of pieces and eventually expand the process to test the modules with those of the other group. Eventually, all the modules making up a process are tested together.
Integration testing identifies the problem that occurs when units are combined. Some different types of integration testing are big bang, top- down, bottom-up<ref>[https://en.wikipedia.org/wiki/Integration_testing Integration Testing - Wikipedia]</ref>.
*Big Bang Integration Testing – All modules are integrated simultaneously, after which everything is tested as a whole. It is very effective for saving time in the integration testing process.
*The top down approach – The highest level modules are tested first. This allows high level logic and data flow to be integrated early in the process.
*The bottom up approach - In this approach, lowest level components are tested first, and then used to facilitate the testing of higher level components.

==Test Pattern and Approach==

We are required to write integration tests for the peer review process. Since we are not adding any new functionalities or amending any existing ones we don’t need to follow any specific design pattern. However we need to follow a specific integration testing approach to conduct the integration testing. The testing approach that we have decided to follow is '''Top-Down'''.
We are first going to create the following entities
* '''User or Reviewer'''
* '''Assignment Questionnaire or Rubric'''
* '''Assignment'''
Then we plan to incrementally test the interaction of these entities in a bottom-up manner.
We first test whether the user is able to successfully login.
If he is able to successfully login, we test whether he is able to successfully select an assignment for review.
If he is able to successfully select an assignment, we test whether he is able to able to submit a review successfully based on whether it is a text-based, point-based or both and whether he chooses to answer all, some or none of the questions of the rubric.

==Flowchart for the Peer Review Testing Approach==
[[File:FlowchartE1591 2.png]]

==Use Case Diagram==

[[File: UseCaseE1591.png]]

==Test Plan==
The following section describes how we plan on testing the application based requirements found on the Project Description<ref>[https://docs.google.com/document/d/1t0keeNQ2kP0NmcgIrtQXGi5K_GhQYcKzEVWJPPRBOHE/edit Project Description]</ref>. We wish to create Integration tests that thoroughly test a Student user reviewing an assignment submission. We will create a file called spec/features/peer_review.rb to run our integration tests.

=== Test Cases Prerequisites ===
To create and run our test cases we require some setup in Expertiza. The following list describes the information that we will enter into the system before running any of our test cases.

# We will use the Student with the username 'student13' and password 'password' to login as a Student.
# We will enroll Student into a specific course.
# We will create an Assignment per individual test case that is customized for our test case.
# We will submit an Assignment from a student other than 'student13' for 'student13' to review

=== Test Cases ===
The following list describes the individual test cases we will creating for Expertiza.

T1: Login as a Student user and select a submission for review. The submission will only contain points based review choices. The user will successfully submit the review answering the points based review choices.

T2: Login as a Student user and select a submission for review. The submission will only contain points based review choices. The user will unsuccessfully submit the review by only answering some of the points based review choices.

T3: Login as a Student user and select a submission for review. The submission will only contain points based review choices. The user will unsuccessfully submit the review by answering none of the points based review choices.

T4: Login as a Student user and select a submission for review. The submission will only contain text based review choices. The user will successfully submit the review answering the text based review choices.

T5: Login as a Student user and select a submission for review. The submission will only contain text based review choices. The user will unsuccessfully submit the review by only answering some of the text based review choices.

T6: Login as a Student user and select a submission for review. The submission will only contain text based review choices. The user will unsuccessfully submit the review by answering none of the text based review choices.

T7: Login as a Student user and select a submission for review. The submission will only contain text based review choices. The user will submit the review by answering all of the text based review choices with single word answers.

T8: Login as a Student user and select a submission for review. The submission will only contain both points based and text based review choices. The user will successfully submit the review answering all of the review choices.

T9: Login as a Student user and select a submission for review. The submission will only contain both points based and text based review choices. The user will unsuccessfully submit the review answering only some of the review choices.

T10: Login as a Student user and select a submission for review. The submission will only contain both points based and text based review choices. The user will unsuccessfully submit the review answering none of the review choices.

==Scenarios Tested==
Six major scenarios were tested. They are as follows:<br>
T1: Successfully submitting a review by filling in both points and comments. <br>This scenario tests if a student can successfully submit a review by filling in comments in a text box and selecting a value from the drop down list.<br>The detailed steps are as follows:
1. Login as a student
2. Select the test assignment for review
3. Click on link "Others Work"
4. Click on button "Request a new submission to Review"
5. Click on the link "Begin"
6. Fill in the text box with comment "Hello World".
7. Select the value 5 from drop down list.
8. Click on the button "Submit Review".
9. "Your response was successfully saved" should be displayed.
T2: Successfully submitting a review by filling in multi word comments. <br>This scenario tests if a student can successfully submit a review by filling in multi word comments in a text box .<br>The detailed steps are as follows:
1. Login as a student
2. Select the test assignment for review
3. Click on link "Others Work"
4. Click on button "Request a new submission to Review"
5. Click on the link "Begin"
6. Fill in the text box with comment "Excellent Work".
7. Click on the button "Submit Review".
8. "Your response was successfully saved" should be displayed.
T3: Successfully submitting a review by filling a single comment with single word. <br>This scenario tests if a student can successfully submit a review by filling in single comment with single word in a text box.<br>The detailed steps are as follows:
1. Login as a student
2. Select the test assignment for review
3. Click on link "Others Work"
4. Click on button "Request a new submission to Review"
5. Click on the link "Begin"
6. Fill in the text box with comment "Excellent".
7. Click on the button "Submit Review".
8. "Your response was successfully saved" should be displayed.
T4: Successfully submitting a review by filling in only points. <br>This scenario tests if a student can successfully submit a review by selecting a value from the drop down list.<br>The detailed steps are as follows:
1. Login as a student
2. Select the test assignment for review
3. Click on link "Others Work"
4. Click on button "Request a new submission to Review"
5. Click on the link "Begin"
6. Select the value 5 from drop down list.
7. Click on the button "Submit Review".
8. "Your response was successfully saved" should be displayed.
T5: Successfully submitting an empty review. <br>This scenario tests if a student can successfully submit an empty review.<br>The detailed steps are as follows:
1. Login as a student
2. Select the test assignment for review
3. Click on link "Others Work"
4. Click on button "Request a new submission to Review"
5. Click on the link "Begin"
6. Click on the button "Submit Review".
7. "Your response was successfully saved" should be displayed.
T6: Successfully submitting an empty review with additional comments only. <br>This scenario tests if a student can successfully submit an empty review by filling in additional comments in a text box.<br>The detailed steps are as follows-
# Login as a student
2. Select the test assignment for review
3. Click on link "Others Work"
4. Click on button "Request a new submission to Review"
5. Click on the link "Begin"
6. Fill in the additional comments text box with comment "Excellent work done".
7. Click on the button "Submit Review".
8. "Your response was successfully saved" should be displayed.

==Execution==
The tests were executed by submitting the following command from the terminal. <br>
<pre>bundle exec rspec spec/features/peer_review_spec.rb</pre>
[[File: Screenshot.JPG]]
==References==
<references></references>

File:Screenshot.JPG

2015-12-04T21:36:05Z

Athakur3:

CSC/ECE 517 Fall 2015 E1591 Integration testing for peer review

2015-11-07T19:27:58Z

Athakur3:

CSC/ECE 517 Fall 2015 E1591 Integration testing for peer review

2015-11-07T19:23:48Z

Athakur3:

CSC/ECE 517 Fall 2015 E1591 Integration testing for peer review

2015-11-07T19:19:04Z

Athakur3:

CSC/ECE 517 Fall 2015 E1591 Integration testing for peer review

2015-11-07T19:18:27Z

Athakur3:

CSC/ECE 517 Fall 2015 E1591 Integration testing for peer review

2015-11-07T19:16:56Z

Athakur3: /* Technologies to be used */

==Background==
Expertiza<ref>[https://github.com/expertiza/expertiza]</ref> is an open-source web tool that is used for online assignment completion purpose for different courses. The software enables the students to form online teams and choose different assignment topics. Each topic has a completion deadline and the students can submit the links to their work or assignments through expertiza.

One of the key features of expertiza is the online peer review process. Students who are enrolled in the same courses can review the works or the assignments of the fellow students. Typically for each assignment there is a link called "Other’s Work" where a student is prompted to give feedback to the works of their fellow classmates. This link becomes active after a certain date that is determined by the instructor or the administrator. After the date, a student can choose one of the assignments that are available for review through the activated link.

After an assignment has been chosen for review, the reviewer will be presented with a review sheet that has a number of parameters based upon which the review should be done. The reviewer chooses a rating for each parameter, typically being the lowest and 5 as the highest. He can also leave a comment justifying his rating and stating the scopes of improvements for the project. The team members whose assignment has been reviewed can also review the review, i.e. provide an explanation about their work or give comments with respect to the feedback provided by the reviewer.

For some assignments there is a scope for a re submission of an improved version of the work. After such a re-submission the reviewer can again review the assignment editing and updating his previous review based upon the resubmitted work.

==Purpose==
The purpose of the project is to perform an integration testing for the peer review of a submitted assignment. We aim to write test cases that test the different positive test scenarios for which the reviewer should be able to successfully submit the review to an assignment. The review process has certain minimum guidelines which need to be adhered to. We also plan to test the negative scenarios for which the reviewer doesn’t adhere to these guidelines and hence should not be able to submit the review to the assignment.
The Expertiza project has been developed using Ruby on Rails. The technologies to be used for the project are '''RSpec''' and '''Capybara''' using Ruby on Rails as an underlying framework. We will write the spec methods using RSpec. The various entities like '''users, assignment''' '''questionnaire''' and '''instructor''' will be simulated using Capybara.

==Technologies to be used==
'''RSpec'''

Rspec is Behavior Driven Development framework for ruby, it is a meta gem composed of different libraries designed to work together or with other testing tools like Cucumber. It has dependencies on Rspec core, Rspec expectations, Rspec mocks gems which can be loaded by using 'require'.

Installation-
gem install rspec

'''Capybara'''

Capybara is a library written in ruby, it helps you to test your web applications by pretending to be a user and following the user story, it simulates the browser to interact with app to receive pages parse the HTML and submit forms just as an actual user would do. Capybara can interact with many different drivers which execute tests through the same clean and simple interface. One can choose between Selenium, Webkit or pure Ruby drivers.

Installation-

Capybara requires ruby 1.9.3 or later,to install add following line to your gem file and run bundle install
gem 'capybara'
If testing a rails app add this to your test helper file
require 'capybara/rails'

'''Integration Testing'''

Integration testing is a phase of software testing where units which have already been tested are combined into a component and the interface between them is tested. The idea is to test combination of pieces and eventually expand the process to test the modules with those of the other group. Eventually, all the modules making up a process are tested together.
Integration testing identifies the problem that occurs when units are combined. Some different types of integration testing are big bang, top- down, bottom-up.
*Big Bang Integration Testing – All modules are integrated simultaneously, after which everything is tested as a whole. It is very effective for saving time in the integration testing process.
*The top down approach – The highest level modules are tested first. This allows high level logic and data flow to be integrated early in the process.
*The bottom up approach - In this approach, lowest level components are tested first, and then used to facilitate the testing of higher level components.

==Test Scenarios==
Using the testing requirements found on the Project Description:

T1: Login as a Student user and select a submission for review. The submission will only contain points based review choices. The user will successfully submit the review answering the points based review choices.

T2: Login as a Student user and select a submission for review. The submission will only contain points based review choices. The user will unsuccessfully submit the review by only answering some of the points based review choices.

T3: Login as a Student user and select a submission for review. The submission will only contain points based review choices. The user will unsuccessfully submit the review by answering none of the points based review choices.

T4: Login as a Student user and select a submission for review. The submission will only contain text based review choices. The user will successfully submit the review answering the text based review choices.

T5: Login as a Student user and select a submission for review. The submission will only contain text based review choices. The user will unsuccessfully submit the review by only answering some of the text based review choices.

T6: Login as a Student user and select a submission for review. The submission will only contain text based review choices. The user will unsuccessfully submit the review by answering none of the text based review choices.

T7: Login as a Student user and select a submission for review. The submission will only contain text based review choices. The user will submit the review by answering all of the text based review choices with single word answers.

T8: Login as a Student user and select a submission for review. The submission will only contain both points based and text based review choices. The user will successfully submit the review answering all of the review choices.

T9: Login as a Student user and select a submission for review. The submission will only contain both points based and text based review choices. The user will unsuccessfully submit the review answering only some of the review choices.

T10: Login as a Student user and select a submission for review. The submission will only contain both points based and text based review choices. The user will unsuccessfully submit the review answering none of the review choices.

==References==
<references></references>

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-06T22:04:41Z

Athakur3: /* Steps to verify changes manually */

'''E1565: Refactoring Admin Controller and Course Controller'''

This page is about an open source project based on Expertiza OSS. This project aims to refactor Admin Controller and Course Controller. It also aims to refactor other snippets which does not follow the Ruby style guidelines.
Refactoring
Refactoring[https://en.wikipedia.org/wiki/Code_refactoring] is a technique to alter the internal structure of the code without affecting the external behaviour. It is used for:
Programs that are difficult to read;
Programs that does not follow the language specific guidelines;
Programs that has high coupling;

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
[Expertiza][https://expertiza.ncsu.edu/] is an open source project developed using the Ruby on Rails platform. It provides features like team assignments, peer review, submission of projects, grading etc. The code can be cloned and modified from GitHub. This application provides an efficient way to manage assignments, grades and reviews.
Assignment.rb is the largest class in Expertiza, comprising 1105 lines of code which contains the assignment model and the related functionalities. It also contains the score module which is refactored to make the file more clean and easy to read.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly. The functionality to view the list of super-admins, admins and the instructors was added. Also changes were made to add_adminstrator method for adding a new administrator in a clean way.
===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

The user_param() method in user_controller did not have any facility to include the institution of the user. After doing a db migration and creating an institution_id in the user table, an institute can be selected while creating a new user. A mailer error is caused when a user/admin/instructor is created and the application crashes. But this is not because of an error with the functionality. Although, despite the error, one can check if the user has been created by going to Manage -> Users page in the user interface.

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>

== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

==Case 7: Addition of the institution id in the view==

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

==Case 8: Adding the institution feature to user view==

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

==Case 9:Routes added ==

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
=Steps to verify changes manually=
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

===Removing the text field from New Administrator===
[[File:List admin.png]]
===Administrator as the default role===
[[File:New admin.png]]
===Adding Institutions to User creation forms and having student as the default role===
[[File:Student.png]]

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-06T22:04:11Z

Athakur3: /* Case 3 : Refactoring user_controller */

'''E1565: Refactoring Admin Controller and Course Controller'''

This page is about an open source project based on Expertiza OSS. This project aims to refactor Admin Controller and Course Controller. It also aims to refactor other snippets which does not follow the Ruby style guidelines.
Refactoring
Refactoring[https://en.wikipedia.org/wiki/Code_refactoring] is a technique to alter the internal structure of the code without affecting the external behaviour. It is used for:
Programs that are difficult to read;
Programs that does not follow the language specific guidelines;
Programs that has high coupling;

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
[Expertiza][https://expertiza.ncsu.edu/] is an open source project developed using the Ruby on Rails platform. It provides features like team assignments, peer review, submission of projects, grading etc. The code can be cloned and modified from GitHub. This application provides an efficient way to manage assignments, grades and reviews.
Assignment.rb is the largest class in Expertiza, comprising 1105 lines of code which contains the assignment model and the related functionalities. It also contains the score module which is refactored to make the file more clean and easy to read.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly. The functionality to view the list of super-admins, admins and the instructors was added. Also changes were made to add_adminstrator method for adding a new administrator in a clean way.
===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

The user_param() method in user_controller did not have any facility to include the institution of the user. After doing a db migration and creating an institution_id in the user table, an institute can be selected while creating a new user. A mailer error is caused when a user/admin/instructor is created and the application crashes. But this is not because of an error with the functionality. Although, despite the error, one can check if the user has been created by going to Manage -> Users page in the user interface.

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>

== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

==Case 7: Addition of the institution id in the view==

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

==Case 8: Adding the institution feature to user view==

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

==Case 9:Routes added ==

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
==Steps to verify changes manually==
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

===Removing the text field from New Administrator===
[[File:List admin.png]]
===Administrator as the default role===
[[File:New admin.png]]
===Adding Institutions to User creation forms and having student as the default role===
[[File:Student.png]]

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-06T22:03:56Z

Athakur3: /* Case 3 : Refactoring user_controller */

'''E1565: Refactoring Admin Controller and Course Controller'''

This page is about an open source project based on Expertiza OSS. This project aims to refactor Admin Controller and Course Controller. It also aims to refactor other snippets which does not follow the Ruby style guidelines.
Refactoring
Refactoring[https://en.wikipedia.org/wiki/Code_refactoring] is a technique to alter the internal structure of the code without affecting the external behaviour. It is used for:
Programs that are difficult to read;
Programs that does not follow the language specific guidelines;
Programs that has high coupling;

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
[Expertiza][https://expertiza.ncsu.edu/] is an open source project developed using the Ruby on Rails platform. It provides features like team assignments, peer review, submission of projects, grading etc. The code can be cloned and modified from GitHub. This application provides an efficient way to manage assignments, grades and reviews.
Assignment.rb is the largest class in Expertiza, comprising 1105 lines of code which contains the assignment model and the related functionalities. It also contains the score module which is refactored to make the file more clean and easy to read.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly. The functionality to view the list of super-admins, admins and the instructors was added. Also changes were made to add_adminstrator method for adding a new administrator in a clean way.
===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

The user_param() method in user_controller did not have any facility to include the institution of the user. After doing a db migration and creating an institution_id in the user table, an institute can be selected while creating a new user. A mailer error is caused when a user/admin/instructor is created and the application crashes. But this is not because of an error with the functionality. Although, despite the error, one can check if the user has been created by going to Manage->Users page in the user interface.

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>

== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

==Case 7: Addition of the institution id in the view==

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

==Case 8: Adding the institution feature to user view==

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

==Case 9:Routes added ==

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
==Steps to verify changes manually==
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

===Removing the text field from New Administrator===
[[File:List admin.png]]
===Administrator as the default role===
[[File:New admin.png]]
===Adding Institutions to User creation forms and having student as the default role===
[[File:Student.png]]

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-06T22:03:35Z

Athakur3: /* Case 1 : Refactoring admin_controller */

'''E1565: Refactoring Admin Controller and Course Controller'''

This page is about an open source project based on Expertiza OSS. This project aims to refactor Admin Controller and Course Controller. It also aims to refactor other snippets which does not follow the Ruby style guidelines.
Refactoring
Refactoring[https://en.wikipedia.org/wiki/Code_refactoring] is a technique to alter the internal structure of the code without affecting the external behaviour. It is used for:
Programs that are difficult to read;
Programs that does not follow the language specific guidelines;
Programs that has high coupling;

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
[Expertiza][https://expertiza.ncsu.edu/] is an open source project developed using the Ruby on Rails platform. It provides features like team assignments, peer review, submission of projects, grading etc. The code can be cloned and modified from GitHub. This application provides an efficient way to manage assignments, grades and reviews.
Assignment.rb is the largest class in Expertiza, comprising 1105 lines of code which contains the assignment model and the related functionalities. It also contains the score module which is refactored to make the file more clean and easy to read.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly. The functionality to view the list of super-admins, admins and the instructors was added. Also changes were made to add_adminstrator method for adding a new administrator in a clean way.
===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

The user_param() method in user_controller did not have any facility to include the institution of the user. After doing a db migration and creating an institution_id in the user table, an institute can be selected while creating a new user.

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>

== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

==Case 7: Addition of the institution id in the view==

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

==Case 8: Adding the institution feature to user view==

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

==Case 9:Routes added ==

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
==Steps to verify changes manually==
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

===Removing the text field from New Administrator===
[[File:List admin.png]]
===Administrator as the default role===
[[File:New admin.png]]
===Adding Institutions to User creation forms and having student as the default role===
[[File:Student.png]]

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-06T22:03:05Z

Athakur3: /* Case 1 : Refactoring admin_controller */

'''E1565: Refactoring Admin Controller and Course Controller'''

This page is about an open source project based on Expertiza OSS. This project aims to refactor Admin Controller and Course Controller. It also aims to refactor other snippets which does not follow the Ruby style guidelines.
Refactoring
Refactoring[https://en.wikipedia.org/wiki/Code_refactoring] is a technique to alter the internal structure of the code without affecting the external behaviour. It is used for:
Programs that are difficult to read;
Programs that does not follow the language specific guidelines;
Programs that has high coupling;

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
[Expertiza][https://expertiza.ncsu.edu/] is an open source project developed using the Ruby on Rails platform. It provides features like team assignments, peer review, submission of projects, grading etc. The code can be cloned and modified from GitHub. This application provides an efficient way to manage assignments, grades and reviews.
Assignment.rb is the largest class in Expertiza, comprising 1105 lines of code which contains the assignment model and the related functionalities. It also contains the score module which is refactored to make the file more clean and easy to read.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly. The functionality to view the list of super-admins, admins and the instructors was added. Also changes were made to add_adminstrator method for adding a new administrator in a clean way. A mailer error is caused when a user/admin/instructor is created and the application crashes. But this is not because of an error with the functionality. Although, despite the error, one can check if the user has been created by going to Manage->Users page in the user interface.
===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

The user_param() method in user_controller did not have any facility to include the institution of the user. After doing a db migration and creating an institution_id in the user table, an institute can be selected while creating a new user.

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>

== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

==Case 7: Addition of the institution id in the view==

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

==Case 8: Adding the institution feature to user view==

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

==Case 9:Routes added ==

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
==Steps to verify changes manually==
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

===Removing the text field from New Administrator===
[[File:List admin.png]]
===Administrator as the default role===
[[File:New admin.png]]
===Adding Institutions to User creation forms and having student as the default role===
[[File:Student.png]]

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-06T21:57:58Z

Athakur3: /* Case 9:Routes added */

'''E1565: Refactoring Admin Controller and Course Controller'''

This page is about an open source project based on Expertiza OSS. This project aims to refactor Admin Controller and Course Controller. It also aims to refactor other snippets which does not follow the Ruby style guidelines.
Refactoring
Refactoring[https://en.wikipedia.org/wiki/Code_refactoring] is a technique to alter the internal structure of the code without affecting the external behaviour. It is used for:
Programs that are difficult to read;
Programs that does not follow the language specific guidelines;
Programs that has high coupling;

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
[Expertiza][https://expertiza.ncsu.edu/] is an open source project developed using the Ruby on Rails platform. It provides features like team assignments, peer review, submission of projects, grading etc. The code can be cloned and modified from GitHub. This application provides an efficient way to manage assignments, grades and reviews.
Assignment.rb is the largest class in Expertiza, comprising 1105 lines of code which contains the assignment model and the related functionalities. It also contains the score module which is refactored to make the file more clean and easy to read.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly. The functionality to view the list of super-admins, admins and the instructors was added. Also changes were made to add_adminstrator method for adding a new administrator in a clean way.
===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

The user_param() method in user_controller did not have any facility to include the institution of the user. After doing a db migration and creating an institution_id in the user table, an institute can be selected while creating a new user.

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>

== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

==Case 7: Addition of the institution id in the view==

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

==Case 8: Adding the institution feature to user view==

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

==Case 9:Routes added ==

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
==Steps to verify changes manually==
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

===Removing the text field from New Administrator===
[[File:List admin.png]]
===Administrator as the default role===
[[File:New admin.png]]
===Adding Institutions to User creation forms and having student as the default role===
[[File:Student.png]]

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-06T21:57:21Z

Athakur3: /* Case 8: Adding the institution feature to user view */

'''E1565: Refactoring Admin Controller and Course Controller'''

This page is about an open source project based on Expertiza OSS. This project aims to refactor Admin Controller and Course Controller. It also aims to refactor other snippets which does not follow the Ruby style guidelines.
Refactoring
Refactoring[https://en.wikipedia.org/wiki/Code_refactoring] is a technique to alter the internal structure of the code without affecting the external behaviour. It is used for:
Programs that are difficult to read;
Programs that does not follow the language specific guidelines;
Programs that has high coupling;

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
[Expertiza][https://expertiza.ncsu.edu/] is an open source project developed using the Ruby on Rails platform. It provides features like team assignments, peer review, submission of projects, grading etc. The code can be cloned and modified from GitHub. This application provides an efficient way to manage assignments, grades and reviews.
Assignment.rb is the largest class in Expertiza, comprising 1105 lines of code which contains the assignment model and the related functionalities. It also contains the score module which is refactored to make the file more clean and easy to read.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly. The functionality to view the list of super-admins, admins and the instructors was added. Also changes were made to add_adminstrator method for adding a new administrator in a clean way.
===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

The user_param() method in user_controller did not have any facility to include the institution of the user. After doing a db migration and creating an institution_id in the user table, an institute can be selected while creating a new user.

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>

== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

==Case 7: Addition of the institution id in the view==

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

==Case 8: Adding the institution feature to user view==

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

===Case 9:Routes added ===

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
==Steps to verify changes manually==
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

===Removing the text field from New Administrator===
[[File:List admin.png]]
===Administrator as the default role===
[[File:New admin.png]]
===Adding Institutions to User creation forms and having student as the default role===
[[File:Student.png]]

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-06T21:57:10Z

Athakur3: /* Case 7: Addition of the institution id in the view */

'''E1565: Refactoring Admin Controller and Course Controller'''

This page is about an open source project based on Expertiza OSS. This project aims to refactor Admin Controller and Course Controller. It also aims to refactor other snippets which does not follow the Ruby style guidelines.
Refactoring
Refactoring[https://en.wikipedia.org/wiki/Code_refactoring] is a technique to alter the internal structure of the code without affecting the external behaviour. It is used for:
Programs that are difficult to read;
Programs that does not follow the language specific guidelines;
Programs that has high coupling;

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
[Expertiza][https://expertiza.ncsu.edu/] is an open source project developed using the Ruby on Rails platform. It provides features like team assignments, peer review, submission of projects, grading etc. The code can be cloned and modified from GitHub. This application provides an efficient way to manage assignments, grades and reviews.
Assignment.rb is the largest class in Expertiza, comprising 1105 lines of code which contains the assignment model and the related functionalities. It also contains the score module which is refactored to make the file more clean and easy to read.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly. The functionality to view the list of super-admins, admins and the instructors was added. Also changes were made to add_adminstrator method for adding a new administrator in a clean way.
===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

The user_param() method in user_controller did not have any facility to include the institution of the user. After doing a db migration and creating an institution_id in the user table, an institute can be selected while creating a new user.

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>

== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

==Case 7: Addition of the institution id in the view==

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

===Case 8: Adding the institution feature to user view===

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

===Case 9:Routes added ===

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
==Steps to verify changes manually==
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

===Removing the text field from New Administrator===
[[File:List admin.png]]
===Administrator as the default role===
[[File:New admin.png]]
===Adding Institutions to User creation forms and having student as the default role===
[[File:Student.png]]

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-06T21:52:06Z

Athakur3: /* Case 3 : Refactoring user_controller */

'''E1565: Refactoring Admin Controller and Course Controller'''

This page is about an open source project based on Expertiza OSS. This project aims to refactor Admin Controller and Course Controller. It also aims to refactor other snippets which does not follow the Ruby style guidelines.
Refactoring
Refactoring[https://en.wikipedia.org/wiki/Code_refactoring] is a technique to alter the internal structure of the code without affecting the external behaviour. It is used for:
Programs that are difficult to read;
Programs that does not follow the language specific guidelines;
Programs that has high coupling;

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
[Expertiza][https://expertiza.ncsu.edu/] is an open source project developed using the Ruby on Rails platform. It provides features like team assignments, peer review, submission of projects, grading etc. The code can be cloned and modified from GitHub. This application provides an efficient way to manage assignments, grades and reviews.
Assignment.rb is the largest class in Expertiza, comprising 1105 lines of code which contains the assignment model and the related functionalities. It also contains the score module which is refactored to make the file more clean and easy to read.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly. The functionality to view the list of super-admins, admins and the instructors was added. Also changes were made to add_adminstrator method for adding a new administrator in a clean way.
===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

The user_param() method in user_controller did not have any facility to include the institution of the user. After doing a db migration and creating an institution_id in the user table, an institute can be selected while creating a new user.

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>

== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

===Case 7: Addition of the institution id in the view===

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

===Case 8: Adding the institution feature to user view===

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

===Case 9:Routes added ===

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
==Steps to verify changes manually==
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

===Removing the text field from New Administrator===
[[File:List admin.png]]
===Administrator as the default role===
[[File:New admin.png]]
===Adding Institutions to User creation forms and having student as the default role===
[[File:Student.png]]

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-06T21:42:03Z

Athakur3: /* Case 1 : Refactoring admin_controller */

'''E1565: Refactoring Admin Controller and Course Controller'''

This page is about an open source project based on Expertiza OSS. This project aims to refactor Admin Controller and Course Controller. It also aims to refactor other snippets which does not follow the Ruby style guidelines.
Refactoring
Refactoring[https://en.wikipedia.org/wiki/Code_refactoring] is a technique to alter the internal structure of the code without affecting the external behaviour. It is used for:
Programs that are difficult to read;
Programs that does not follow the language specific guidelines;
Programs that has high coupling;

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
[Expertiza][https://expertiza.ncsu.edu/] is an open source project developed using the Ruby on Rails platform. It provides features like team assignments, peer review, submission of projects, grading etc. The code can be cloned and modified from GitHub. This application provides an efficient way to manage assignments, grades and reviews.
Assignment.rb is the largest class in Expertiza, comprising 1105 lines of code which contains the assignment model and the related functionalities. It also contains the score module which is refactored to make the file more clean and easy to read.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly. The functionality to view the list of super-admins, admins and the instructors was added. Also changes were made to add_adminstrator method for adding a new administrator in a clean way.
===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>
== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

===Case 7: Addition of the institution id in the view===

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

===Case 8: Adding the institution feature to user view===

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

===Case 9:Routes added ===

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
==Steps to verify changes manually==
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

===Removing the text field from New Administrator===
[[File:List admin.png]]
===Administrator as the default role===
[[File:New admin.png]]
===Adding Institutions to User creation forms and having student as the default role===
[[File:Student.png]]

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T03:05:34Z

Athakur3:

'''E1565: Refactoring Admin Controller and Course Controller'''

This page provides a description of the Expertiza based OSS project aimed at refactoring Admin Controller and Course Controller.

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
Expertiza is a web application where students can submit and peer-review learning objects (articles, code, web sites, etc). It is used in select courses at NC State and by professors at several other colleges and universities.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The consistency of these two methods shows plenty of repetitive code. In order to effectively refactor these methods and reuse the code they both need, our team merged the two methods together by enabling add_this_bookmark method to handle creation of a bookmark when a topic id is provided (main functionality of add_topic_bookmark method). Once we implemented this change on add_this_bookmark method, we deleted add_topic_bookmark method as it became obsolete. Therefore, we retained the same functionality while reducing the method by 7 repetitive lines of code.

===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>
== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

===Case 7: Addition of the institution id in the view===

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

===Case 8: Adding the institution feature to user view===

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

===Case 9:Routes added ===

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
==Steps to verify changes manually==
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

===Removing the text field from New Administrator===
[[File:List admin.png]]
===Administrator as the default role===
[[File:New admin.png]]
===Adding Institutions to User creation forms and having student as the default role===
[[File:Student.png]]

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T03:05:20Z

Athakur3: /* Steps to verify changes manually */

File:Student.png

2015-11-01T03:02:01Z

Athakur3: uploaded a new version of "File:Student.png"

File:Student.png

2015-11-01T03:00:34Z

Athakur3:

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T02:54:24Z

Athakur3: /* Administrator as the default role */

'''E1565: Refactoring Admin Controller and Course Controller'''

This page provides a description of the Expertiza based OSS project aimed at refactoring Admin Controller and Course Controller.

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
Expertiza is a web application where students can submit and peer-review learning objects (articles, code, web sites, etc). It is used in select courses at NC State and by professors at several other colleges and universities.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The consistency of these two methods shows plenty of repetitive code. In order to effectively refactor these methods and reuse the code they both need, our team merged the two methods together by enabling add_this_bookmark method to handle creation of a bookmark when a topic id is provided (main functionality of add_topic_bookmark method). Once we implemented this change on add_this_bookmark method, we deleted add_topic_bookmark method as it became obsolete. Therefore, we retained the same functionality while reducing the method by 7 repetitive lines of code.

===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>
== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

===Case 7: Addition of the institution id in the view===

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

===Case 8: Adding the institution feature to user view===

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

===Case 9:Routes added ===

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
==Steps to verify changes manually==
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

===Removing the text field from New Administrator===
<p>'''Visit the url [http://152.46.16.123:3000/admin/list_administrators http://152.46.16.123:3000/admin/list_administrators]'''</p>
[[File:List admin.png]]
===Administrator as the default role===
<p>'''Visit the url [http://152.46.16.123:3000/users/new?utf8=%E2%9C%93&role=Administrator&commit=New+Administrator]'''</p>
[[File:New admin.png]]

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T02:54:09Z

Athakur3: /* Removing the text field from New Administrator */

'''E1565: Refactoring Admin Controller and Course Controller'''

This page provides a description of the Expertiza based OSS project aimed at refactoring Admin Controller and Course Controller.

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
Expertiza is a web application where students can submit and peer-review learning objects (articles, code, web sites, etc). It is used in select courses at NC State and by professors at several other colleges and universities.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The consistency of these two methods shows plenty of repetitive code. In order to effectively refactor these methods and reuse the code they both need, our team merged the two methods together by enabling add_this_bookmark method to handle creation of a bookmark when a topic id is provided (main functionality of add_topic_bookmark method). Once we implemented this change on add_this_bookmark method, we deleted add_topic_bookmark method as it became obsolete. Therefore, we retained the same functionality while reducing the method by 7 repetitive lines of code.

===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>
== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

===Case 7: Addition of the institution id in the view===

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

===Case 8: Adding the institution feature to user view===

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

===Case 9:Routes added ===

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
==Steps to verify changes manually==
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

===Removing the text field from New Administrator===
<p>'''Visit the url [http://152.46.16.123:3000/admin/list_administrators http://152.46.16.123:3000/admin/list_administrators]'''</p>
[[File:List admin.png]]
===Administrator as the default role===
<p>'''Visit the url [http://152.46.16.123:3000/users/new?utf8=%E2%9C%93&role=Administrator&commit=New+Administrator]'''</p>
[[File:List admin.png]]

File:New admin.png

2015-11-01T02:53:39Z

Athakur3:

File:List admin.png

2015-11-01T02:47:56Z

Athakur3: uploaded a new version of "File:List admin.png"

File:List admin.png

2015-11-01T02:47:15Z

Athakur3: uploaded a new version of "File:List admin.png"

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T02:46:43Z

Athakur3: /* Removing the text field from New Administrator */

'''E1565: Refactoring Admin Controller and Course Controller'''

This page provides a description of the Expertiza based OSS project aimed at refactoring Admin Controller and Course Controller.

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
Expertiza is a web application where students can submit and peer-review learning objects (articles, code, web sites, etc). It is used in select courses at NC State and by professors at several other colleges and universities.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The consistency of these two methods shows plenty of repetitive code. In order to effectively refactor these methods and reuse the code they both need, our team merged the two methods together by enabling add_this_bookmark method to handle creation of a bookmark when a topic id is provided (main functionality of add_topic_bookmark method). Once we implemented this change on add_this_bookmark method, we deleted add_topic_bookmark method as it became obsolete. Therefore, we retained the same functionality while reducing the method by 7 repetitive lines of code.

===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>
== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

===Case 7: Addition of the institution id in the view===

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

===Case 8: Adding the institution feature to user view===

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

===Case 9:Routes added ===

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
==Steps to verify changes manually==
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

==='''Removing the text field from New Administrator'''===
<p>'''Visit the url [http://152.46.16.123:3000/admin/list_administrators http://152.46.16.123:3000/admin/list_administrators]'''</p>
[[File:List admin.png]]

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T02:46:19Z

Athakur3: /* Removing the text field from New Administrator */

'''E1565: Refactoring Admin Controller and Course Controller'''

This page provides a description of the Expertiza based OSS project aimed at refactoring Admin Controller and Course Controller.

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
Expertiza is a web application where students can submit and peer-review learning objects (articles, code, web sites, etc). It is used in select courses at NC State and by professors at several other colleges and universities.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The consistency of these two methods shows plenty of repetitive code. In order to effectively refactor these methods and reuse the code they both need, our team merged the two methods together by enabling add_this_bookmark method to handle creation of a bookmark when a topic id is provided (main functionality of add_topic_bookmark method). Once we implemented this change on add_this_bookmark method, we deleted add_topic_bookmark method as it became obsolete. Therefore, we retained the same functionality while reducing the method by 7 repetitive lines of code.

===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>
== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

===Case 7: Addition of the institution id in the view===

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

===Case 8: Adding the institution feature to user view===

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

===Case 9:Routes added ===

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
==Steps to verify changes manually==
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

==='''Removing the text field from New Administrator'''===
<p>'''Visit the url [http://152.46.16.123:3000/admin/list_administrators http://152.46.16.123:3000/admin/list_administrators]'''</p>
[[File:List_admin.png]]

File:List admin.png

2015-11-01T02:46:01Z

Athakur3: uploaded a new version of "File:List admin.png"

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T02:45:02Z

Athakur3: /* Steps to verify changes manually */

'''E1565: Refactoring Admin Controller and Course Controller'''

This page provides a description of the Expertiza based OSS project aimed at refactoring Admin Controller and Course Controller.

'''In order to run our code visit the link 152.46.16.123:3000 and use the following credentials - username: admin, password: admin.'''

==Introduction to Expertiza==
Expertiza is a web application where students can submit and peer-review learning objects (articles, code, web sites, etc). It is used in select courses at NC State and by professors at several other colleges and universities.

==Problem Statement==
<p>'''Files involved'''</p>
<pre>
admin_controller.rb
views in app/views/admin folder
user_controller</pre>

<p>'''What they do: '''</p>
The admin controller defines the changes that can be done on other types of users by super-admin and provides the view accordingly.

<p>'''What needs to be done: ''' </p>
* The text field for adding a new admin/instructor has to be removed, leaving only the button.
* When the button is clicked, it should be redirected to new user creation view making the role (super-admin, admin or instructor) selected as default.
* Make sure that user creation works for super-admin/admin/instructor.
* Associate the users with the institution table
* Add a drop down to the view of creating users so that new account creator can select the institute and save it.

==Changes Made==
===Admin Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| show_super_admin
| Added this method to controller
| This method helps in displaying the specified super_admin
|-
| show_admin
| Added this method to Admin Controller
| This method helps in displaying the specified admin
|-
| remove_instructor
| Added this method to Admin Controller
| This method helps in destroying an instructor.
|-
| remove_administrator
| Added this method to Admin Controller
| This method helps in destroying an administrator.
|-
| remove_super_administrator
| Added this method to Admin Controller.
| This method helps in destroying a super_admin.
|}

=== Institution Controller ===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Changed line @institution = Institution.new(params[:institution]) to @institution = Institution.new(:name => params[:institution][:name])
| Was throwing error
|}
===User Controller===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| create
| Added the line @user.institutions_id=params[:users][:institutions_id]
| To include institution for newly created users
|-
| user_params
| Added the parameter institutions ID.
| To include institution for newly created users.
|}
===Views===
{| class="wikitable"
|-
! style="width:13%;"|Method Name
! style="width:33%;"|Changes Made
! style="width:43%;"|Reason For Change
|- style="vertical-align:top;"
| views/admin/list_administrators.html.erb
| Removed the text field next to New Administrator button
| The button redirects to a new user page and passes a hidden field with role:administrator
|-
| views/admin/list_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_admin',:id => user.id %>
| There is no show method in Admin Controller
|-
| views/admin/list_instructors.html.erb
| Removed the text field next to New Instructor button
| The button redirects to a new user page and passes a hidden field with role:Instructor
|-
| views/admin/list_super_administrators.html.erb
| Changed line 7 to <%= link_to user.name, :action => 'show_super_admin',:id => user.id %>
| There is no show method in Admin Controller.
|-
| views/admin/list_super_administrators.html.erb
| Removed the Parent column from line 4
| Super admins don't have any parents.
|-
| views/admin/list_super_administrators.html.erb
| Removed <%= link_to 'New Super administrator', :action => 'new' %>
| There can be only one super admin.
|-
| views/admin/show_admin.html.erb
| Added this file.
| show_admin lists out all the administrator.
|-
| views/admin/show_super_admin.html.erb
| Added this file.
| Lists out the super administrator.
|-
| views/users/_institutions.html.erb
| Added this file
| To add institution drop down menu in the new user creation form.
|-
| views/users/_user.html.erb
| Added line <%= render :partial => 'users/institutions' %>
| To select the institution.
|-
| views/users/list.html.erb
| Sending role:student as the default role for new user creation form
| Was throwing an error
|-
| views/users/new.html
| Default role is selected in new user creation form.
| So that if new administrator is clicked the user creation form will have admin as the pre selected role.
|}

= Re-factored Code Cases =

== Case 1 : Refactoring admin_controller ==

The consistency of these two methods shows plenty of repetitive code. In order to effectively refactor these methods and reuse the code they both need, our team merged the two methods together by enabling add_this_bookmark method to handle creation of a bookmark when a topic id is provided (main functionality of add_topic_bookmark method). Once we implemented this change on add_this_bookmark method, we deleted add_topic_bookmark method as it became obsolete. Therefore, we retained the same functionality while reducing the method by 7 repetitive lines of code.

===Before Changes===

<pre>
def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
end

def list_administrators
</pre>

===After Changes===
<pre>
+ def show
+ redirect_to url_for(:controller => :users, :action => :new)
+ end

def list_instructors
@users = User.
instructors.
order(:name).
where("parent_id = ?", current_user.id).
paginate(:page => params[:page], :per_page => 50)
end

def add_administrator
@user = User.new
+ redirect_to 'users#new'
end

def new_administrator
add_administrator
end

def save_administrator # saves newly created administrator to database
PgUsersController.create(Role.administrator.id, :admin_controller, :list_administrators, :add_administrator)
end

def create_administrator
save_administrator
+ redirect_to "users#new"
end

def list_administrators
</pre>

== Case 2 : Refactoring institution_controller ==

=== Before changes ===

<pre>

def index
list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
@institutions = Institution.all
end

</pre>

=== After Changes ===

<pre>
def index
- list
+ # @institutions=Institution.All
+ list
render :action => 'list'
end

# GETs should be safe (see http://www.w3.org/2001/tag/doc/whenToUseGet.html)
verify :method => :post, :only => [ :destroy, :create, :update ],
:redirect_to => { :action => :list }

def list
- @institutions = Institution.all
+ @institutions = Institution.all
+
end
</pre>

== Case 3 : Refactoring user_controller ==

===Before Changes===

<pre>

@user = User.new(user_params)
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's
@user.timezonepref = User.find(@user.parent_id).timezonepref

</pre>

===After Changes===

<pre>
@user = User.new(user_params)
+ @user.institutions_id=params[:users][:institutions_id]
# record the person who created this new user
@user.parent_id = (session[:user]).id
# set the user's timezone to its parent's

</pre>
== Case 4 : Refactoring list_administrators ==
===Before Changes===

<pre>

<h1>Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Administrator'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Administrators</h1>

<table>
- <tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <tr><th>Name</th><th></th><th>Full name</th><th></th><th>E-mail address</th><th></th><th>Parent</th><th></th></tr>
+ <% for user in User.all %>
+ <% if user.role_id == 3%>
<tr>
<td><%= link_to user.name, :action => 'show',
- :id => user.id %></td>
- <td><%= user.fullname %></td>
- <td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ :id => user.id %></td><td> </td>
+ <td><%= user.fullname %></td><td> </td>
+ <td><%= user.email %></td><td> </td>
+ <td><%= (User.find(user.parent_id)).name %></td><td> </td>
</tr>
<% end -%>
+ <% end -%>
</table>
-
-
<hr/>
-<%= form_tag({:action => 'new_administrator'}, {:method => 'post'}) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Administrator'/>
-<% end %>
+<%= form_tag(new_admin_path, method: :get) do %>
+<input type='submit' value='New Administrator'/>
+<% end %>

</pre>

== Case 5 : Refactoring list_instructors.html.erb ==

===Before Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<%= form_tag('new_instructor', method: :get) do %>

<br>
<%= text_field_tag('name', '', {'size' => 30}) %>
 
<input type='submit' value='New Instructor'/>
<% end %>
</pre>

===After Changes===

<pre>

<h1>Instructors</h1>

<%= will_paginate @users %>

<table>
<tr><th>Name</th><th>Full Name</th><th>Email Address</th><th>Parent</th></tr>
- <% for user in @users %>
+ <% for user in User.all %>
+ <% if user.role_id == 2%>
<tr>
<td><%= link_to user.name, :action => 'show_instructor',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
+ <% end -%>
</table>

<hr/>
-<%= form_tag('new_instructor', method: :get) do %>
-
- <br>
- <%= text_field_tag('name', '', {'size' => 30}) %>
-  
- <input type='submit' value='New Instructor'/>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Instructor'/>
<% end %>
</pre>

== Case 6: Refactoring list_super_administrators.html.erb ==
===Before Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
<td><%= (User.find(user.parent_id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
<p><%= link_to 'New Super administrator', :action => 'new' %></p>

</pre>

===After Changes===

<pre>
<h1>Super-Administrators</h1>

<table>
<tr><th>Name</th><th>Full name</th><th>E-mail address</th><th>Parent</th></tr>
<% for user in @users %>
<tr>
<td><%= link_to user.name, :action => 'show',
:id => user.id %></td>
<td><%= user.fullname %></td>
<td><%= user.email %></td>
- <td><%= (User.find(user.parent_id)).name %></td>
+ <td><%= (User.find(user.id)).name %></td>
</tr>
<% end -%>
</table>

<hr/>
-<p><%= link_to 'New Super administrator', :action => 'new' %></p>
+<%= form_tag(new_admin_path, method: :get) do %>
+ <input type='submit' value='New Super Administrator'/>
+<% end %>
</pre>

===Case 7: Addition of the institution id in the view===

<pre>

<p><label for="user_institutions">Institution</label><br/>
+ <%= collection_select('users',:institutions_id, Institution.all, :id, :name) %></p>
</pre>

===Case 8: Adding the institution feature to user view===

===Before Changes===
<pre>
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>

<br/>
</pre>

===After Changes===
<pre>
@@ -1,13 +1,14 @@
<%= error_messages_for 'user' %>


<p><label for="user_name">Username</label><br/>
<%= text_field 'user', 'name' %></p>

<% new_user ||= false %>
<%= render :partial => 'users/name' %>
<%= render :partial => 'users/email' %>
<%= render :partial => 'users/password', :locals => { :new_user => new_user } %>
<%= render :partial => 'users/prefs' %>
+<%= render :partial => 'users/institutions' %>

<br/>
</pre>

===Case 9:Routes added ===

===Before Changes===
<pre>
resources :admin do
collection do
get :list_super_administrators
get :list_administrators
get :list_instructors
get :new_administrator
</pre>

===After Changes===
<pre>
resources :admin do
collection do
+ get :create_any_user
get :list_super_administrators
get :list_administrators
get :list_instructors
</pre>
==Steps to verify changes manually==
In order to inspect the changes made, you can visit our deployed [http://152.46.16.123:3000/admin/list_administrators Expertiza] and use the credentials mentioned at the top of the page.

==='''Removing the text field from New Administrator'''===
<p>'''Visit the url [http://152.46.16.123:3000/admin/list_administrators http://152.46.16.123:3000/admin/list_administrators]'''</p>
[[File:List admin.png]]

File:List admin.png

2015-11-01T02:39:46Z

Athakur3: uploaded a new version of "File:List admin.png"

File:List admin.png

2015-11-01T02:32:25Z

Athakur3: uploaded a new version of "File:List admin.png"

File:List admin.png

2015-11-01T02:30:30Z

Athakur3:

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T02:13:46Z

Athakur3:

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T01:49:44Z

Athakur3: /* Changes Made */

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T01:48:32Z

Athakur3: /* Views/admin */

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T01:28:00Z

Athakur3: /* Changes Made */

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T01:26:11Z

Athakur3: /* Changes Made */

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T01:25:44Z

Athakur3: /* Views/admin */

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T01:25:30Z

Athakur3: /* Views/admin */

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T01:25:12Z

Athakur3: /* Views */

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T01:21:47Z

Athakur3: /* Views */

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T01:08:01Z

Athakur3: /* Changes Made */

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T01:07:35Z

Athakur3: /* User Controller */

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T01:02:36Z

Athakur3: /* Changes Made */

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T01:00:35Z

Athakur3: /* Modified the methods add_administrator and create_administrator */

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T00:41:14Z

Athakur3: /* Admin Controller */

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T00:21:40Z

Athakur3:

CSC/ECE 517 Fall 2015/oss E1565 AAJ

2015-11-01T00:19:39Z

Athakur3: