Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Fall 2017/E17A6. Fix account creation over web to work reasonably

2017-11-16T14:19:05Z

Hren3:

=='''Introduction to Expertise'''==

Expertiza is a educational website that allows school users to submit peer reviews on assignments. To register to use this website, one needs to be either a course TA or Instructor and send a request to expertiza administrator to get approved. The request function only serves TAs and Instructors, so students won't be able to register. A student account is created by instructors or TAs. The whole purpose of this approval process is because students have no point of using this website without enrolling to an instructors course first, so they shouldn't request accounts from expertiza but from their instructors and TAs.

A pending request page that displays all the requests should also be able to perform actions like approve or reject with an note.

However, currently, all of these are done manually, which means no pending request page currently exists, no simple approve or reject action, and no user experience, and all users are added into the database through command line operations in the database(only instructor and TA roles which is acceptable currently but not when more other institutions try to user Expertiza).

In addition, we also want users be able to add their institution if it does not show under the list provided by us.

=='''Problem Statement'''==
'''Overall Introduction'''

Request account function is currently not working properly. A new user who tries to request an expertiza account can do so, but the instructor can see yet has no power to approve any request. The approval process is currently done manually, which is cumbersome.
Our mission is to fix the request account functionality so that whoever requests for a new accounts can be approved by instructor clicking a button, and secondly, optimize the functionality to improve user experience.

===Detailed Requirements and Plan===
'''1. Add institution''' <br\>
-A registering user should be able to choose their institution from a drop-down menu, but they should also be able to add an institution if they couldn't find one. <br\>

-The new institution should be saved as a new record into the ''institution table''<br\>

-New account request should be saved to ''requested_users table'' with correct institution id.<br\>

'''2.superadmin, admin should be able to approve/decline requests'''<br\>

-add a new drop-down item under “Administration > Show…” so requests can be visually displayed and handled<br\>

-enable the "approve" function for admin and superadmin<br\>

-make clickable user email address so a conversation is possible<br\>

'''3. The record retain after admin's approval/rejection and an email will be sent to remind'''<br\>

-an email should be sent when a request is approved or declined<br\>

-all record should be kept on the page even it has been handled<br\>

===Use Case Diagram ===

The following is the use case diagram for the feature that we are going to implement.

[[File:Fix request.png]]

===Fix 1. Add institution ===
- redesign the view shown in the previous section to include a field named institution in the current form to allow user to submit their institution if they couldn't find one in the list, and save it into the table corresponding to the form it submits to.

- design a requested_user table to save all account requests

===Fix 2. Add new Page and Action===
- to create an new view that can show all pending account requests and include actions like approve and reject to allow instructors to process requests on the same page

- create corresponding new controller with methods corresponding to actions like reject, approve and index.

[[File:Rel.jpg]]

===Fix 3. The record retains after admin's approval and email reminder ===
- redesign the view and model to retain the request record after super-admin or admin approves the request.

- add an email tag in the corresponding view, so there will be an email send to email address offered by requester after approval/rejection.

=='''Mockup Screens'''==

1. First of all, a new user need to click 'REQUEST ACCOUNT' to create a new account.

[[File:cs.png]]

2. In the 'request new' page, in the original version, the user can only choose an institution from the drop-down menu. After our modification, the user can choose the 'Not List' choice in the drop-down menu and type in an institution when (s)he is not able to find his(her) institution in the drop-down menu.

[[File:Screen_Shot_2017-11-06_at_10.17.01_PM.png]]

Add a picture after changing.

3. After log in as an admin/super admin, the user can see a drop-down item under 'Administration->Show requests' to see all the new account requests.

[[File:Administrator.png]] ‎

4. After clicking the 'Show requests', the admin can see a list of new account requests and the emails are clickable to send email to the requesting user. Also, after the admin approves the request, the record should retain in the view and an email informing the requestor will be sent to the requestors' given email address.

[[File:Requests.png]]

5. After clicking the email address, the email editor will pop up and admin can converse requestors.

[[File:Emails.png]]

=='''Design Pattern'''==

==='''1. MVC'''===
The project is implemented in Ruby on Rails that uses MVC architecture. It separates an application’s data model, user interface, and control logic into three distinct components (model, view and controller, respectively).

==='''2. Dry Principle'''===
We are trying to reuse the existing functionalities in Expertiza, thus avoiding code duplication. Whenever possible, code modification based on the existing classes, controllers, or tables will be done instead of creating the new one.

=='''Test Plan'''==
Both manual and RSpec test will be performed on the following items.

1. a proper error message for improper information filled in, such as wrong email address format or existing account.

2. When a pending account gets approved or rejected, an email will be sent to the submitted email address.

3. A new item "show" should be added under administration and direct instructor to pending requests page when clicked

4. pending requests will have an email column that is clickable which allows instructor to send emails to applicants before a decision is made.

5. when a request is processed, its record is still kept with the status of processed.

== Team Informantion ==
Guiyang Fan(gfan@ncsu.edu)

Haoxu Ren(hren3@ncsu.edu)

Dongyu Wang(dwang32@ncsu.edu)

Yichi Zhang(yzhan222@ncsu.edu)

File:Fix request.png

2017-11-16T14:18:43Z

Hren3: request

request

CSC/ECE 517 Fall 2017/E17A6. Fix account creation over web to work reasonably

2017-11-07T16:10:51Z

Hren3: /* Problem Statement */

==Introduction==

===Problem Statement===
Request account function is currently not working properly. A new user who tries to request an expertiza account can do so, but the instructor can see yet has no power to approve any request. The approval process is currently done manually, which is cumbersome.
Our mission is to fix the request account functionality so that whoever request for a new accounts can be approved by instructor clicking an button and optimize the functionality to improve user experience.

[[File:cs.png]]

[[File:Screen_Shot_2017-11-06_at_10.17.01_PM.png]]

===Task Specifications===
'''1. Add institution''' <br\>
-A registering user should be able to choose their institution from a drop-down menu, but they should also be able to add an institution if they couldn't find one. <br\>
-The new institution should be saved as a new record into the ''institution table''<br\>
-New account request should be saved to ''requested_users table'' with correct institution id.<br\>
'''2.superadmin, admin should be able to approve/decline requests'''<br\>
-add a new drop-down item under “Administration > Show…” so requests can be visually displayed and handled<br\>
-enable the "approve" function for admin and superadmin<br\>
-make clickable user email address so a conversation is possible<br\>
-an email should be sent when a request is approved or declined<br\>
-all record should be kept on the page even it has been handled<br\>

== Team Informantion ==
Guiyang Fan(gfan@ncsu.edu)

Haoxu Ren(hren3@ncsu.edu)

File:Cs.png

2017-11-07T16:10:29Z

Hren3: screen shot

screen shot

File:As.png

2017-11-07T16:07:46Z

Hren3: screen

screen

CSC/ECE 517 Fall 2017/E17A6. Fix account creation over web to work reasonably

2017-11-07T15:53:19Z

Hren3: /* Team Informantion */

==Introduction==

===Problem Statement===
Request account function is currently not working properly. A new user who tries to request an expertiza account can do so, but the instructor can see yet has no power to approve any request. The approval process is currently done manually, which is cumbersome.
Our mission is to fix the request account functionality so that whoever request for a new accounts can be approved by instructor clicking an button and optimize the functionality to improve user experience.

[[File:Screen_Shot_2017-11-06_at_10.17.01_PM.png]]

===Task Specifications===
'''1. Add institution''' <br\>
-A registering user should be able to choose their institution from a drop-down menu, but they should also be able to add an institution if they couldn't find one. <br\>
-The new institution should be saved as a new record into the ''institution table''<br\>
-New account request should be saved to ''requested_users table'' with correct institution id.<br\>
'''2.superadmin, admin should be able to approve/decline requests'''<br\>
-add a new drop-down item under “Administration > Show…” so requests can be visually displayed and handled<br\>
-enable the "approve" function for admin and superadmin<br\>
-make clickable user email address so a conversation is possible<br\>
-an email should be sent when a request is approved or declined<br\>
-all record should be kept on the page even it has been handled<br\>

== Team Informantion ==
Guiyang Fan(gfan@ncsu.edu)

Haoxu Ren(hren3@ncsu.edu)

CSC/ECE 517 Fall 2017/E17A6. Fix account creation over web to work reasonably

2017-11-07T15:53:12Z

Hren3: /* Team Informantion */

==Introduction==

===Problem Statement===
Request account function is currently not working properly. A new user who tries to request an expertiza account can do so, but the instructor can see yet has no power to approve any request. The approval process is currently done manually, which is cumbersome.
Our mission is to fix the request account functionality so that whoever request for a new accounts can be approved by instructor clicking an button and optimize the functionality to improve user experience.

[[File:Screen_Shot_2017-11-06_at_10.17.01_PM.png]]

===Task Specifications===
'''1. Add institution''' <br\>
-A registering user should be able to choose their institution from a drop-down menu, but they should also be able to add an institution if they couldn't find one. <br\>
-The new institution should be saved as a new record into the ''institution table''<br\>
-New account request should be saved to ''requested_users table'' with correct institution id.<br\>
'''2.superadmin, admin should be able to approve/decline requests'''<br\>
-add a new drop-down item under “Administration > Show…” so requests can be visually displayed and handled<br\>
-enable the "approve" function for admin and superadmin<br\>
-make clickable user email address so a conversation is possible<br\>
-an email should be sent when a request is approved or declined<br\>
-all record should be kept on the page even it has been handled<br\>

== Team Informantion ==
Guiyang Fan(gfan@ncsu.edu)
Haoxu Ren(hren3@ncsu.edu)

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-11-06T00:47:30Z

Hren3:

=='''Introduction'''==
==='''Introduction to Expertiza'''===
[http://expertiza.ncsu.edu/ Expertiza] is a peer review based system which provides incremental learning from the class. This project has been developed together by faculty and students using [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create, edit and delete assignments, create new assignment topics, assign them to a particular class or selected students, have students work on teams and then review each other's assignments at the end. For the students, they can signup for topics, form teams, and submit their projects and assignments.
Students then review the work done by other students and give suggestions to improve. Teams after reviews are allotted scores and they can refer to the peer comments to further improve their work. It also supports submission of different file types for assignments, including the URLs and wiki pages.
==='''Problem Statement'''===
Online peer-review systems are now in common use in higher education. They free the instructor and course staff from having to provide personally all the feedback that students receive on their work. However, if we want to assure that all students receive competent feedback, or even use peer assigned grades, we need a way to judge which peer reviewers are most credible. The solution is the reputation system. Reputation systems have been deployed as web services, peer-review researchers will be able to use them to calculate scores on assignments, both past and present (past data can be used to tune the algorithms).

For this project, our team's job is to refactor the file: reputation_web_service_controller.rb. This file is the controller to calculate the reputation scores. A “reputation” measures how close a reviewer’s scores are to other reviewers scores. This controller is the sub-class of Application_Controller, in this controller, it implements the calculation and query of reputation scores.
----

=='''Issues to be fixed'''==

Reputation_web_service_controller.rb is a fairly complex file. It contains a lot of methods that are long and hard to understand. These methods need to be broken down into simpler and more specific methods that are easier to read/understand. Also, the few instances of code duplication that exist should be removed.

#Replace class variable with a class instance var, and change all other places using these variables.
#Refactor method db_query
#Use zero? method instead of == 0
#Use find_by instead of dynamic method
#Delete lines which is wrong or useless.
#Comment lines

----

=='''Modified Files'''==

reputation_web_service_controller.rb

----

=='''NewFiles'''==

reputation_web_service_controller_spec.rb

----
=='''Important links'''==
[https://www.youtube.com/watch?v=jdL-L-UZp1A&t=20s Demo Video Link]<br>

[https://github.com/yuxuyang/expertiza Project Repo]<br>

[https://github.com/expertiza/expertiza/pull/1027 Github Pull Request]

----
=='''Approach taken to resolve the issues'''==
==='''Replace class variable with a class instance var, and change all other places using these variables.'''===
In ruby, class variables are started with '@@'and this class variable are shared in the whole inherit chain, and class variables are shared by all objects of a class. For instance variables, in Ruby, instance variables are started with '@' and instance variables belong to one object, can not be used by sub-classes.

In the project, we are asked to replace the class variable with a class instance variables.
So, as the figure 1 shown, in variables declaration part, we changed all class variables to instance variables.

[[File:Reputation code1.png]]

Next, we need to change all other places using these variables.

In action '''send_post_request'''

[[File:Reputaion 2.png]]

[[File:Reputaion3.png]]

[[File:Reputation4.png]]

[[File:Reputation5.png]]

[[File:Reputation6.png]]

----

==='''Refactor method db_query'''===
In the Reputation_web_service controller, the action db_query is important, because it implements the query of peer review grade.To refactor the db_query method, we need to firstly focus on the input variables of this method, there are 4 variables need inputs.
The db_query method is shown as follow:
<pre>
def db_query(assignment_id, another_assignment_id = 0, round_num, hasTopic)
...
end
</pre>
There is a problem: Optional arguments should appear at the end of the argument list, in this method, '''''another_assignment_id''''' is a optional arguments, but in the original version, the optional arguments -- '''''another_assignment_id''''' just appear right behind the argument--'''''assignment_id''''', so we need to refactor it.

[[File:Reputation8.png]]

And the same problems occurs when we assigns value to variable '''''@results'''''. We also need to change it.

[[File:Reputation10.png]]

There is another problem in the db_query method, we can see there is a input variable named '''''hasTopic''''', but actually that's not follow the '''"good Ruby and Rails coding practices"''', so we need to refactor it which shown as follow:

[[File:Reputation9.png]]

Finally, The project asked us'' 'Move Line 26-50 before this method''', it's important for code refactor, because it's very inelegant if you put the comments inside the method. If others want to read your codes, it will cost their much more time so that it needs to be changed. Simply, we can just move the whole comments before the method, and that will make the codes easily for reading.

[[File:Reputation11.png]]

==='''Use zero? method instead of == 0.'''===

In the original version of project, every time there is a zero judgement occurs, it use ''''''==0'''''' or ''''''!=0'''''', of course it can achieve the judgement of zero, but according to the security consideration in ruby on rails development, we need to avoid the use of ''''''==0'''''' or ''''''!=0''''''. So the best way to solve it is using the '''''zero?''''' instead of ''''''==0'''''' or ''''''!=0'''''', which shown as follow:
[[File:Reputation12.png]]

[[File:Reputation13.png]]

==='''Use find_by instead of dynamic method.'''===
This issue is replacing the find_by_id to find_by(id: id). The only difference of this change is syntax and their function worked in the same way. But if the customer want to change the way to find assignment, find_by(id: id) would be easier to change because programmer only need to change the parameter, while find_by_id need to change the whole method and its parameter. Therefore, in this project, using find_by_id is more efficient.
[[File:refactor15.png]]
[[File:refactor16.png]]

==='''Delete lines which is wrong or useless.'''===
In Line 111, the variable inner_msg has never been used, so it can be deleted
[[File:refactor17.png]]

From line 164 to 201 and line 204 to 241, cases for reputation value and do not need to handle the case for assign id. req body would change only in the case for 'Add expert grades'. If these two part failed to be deleted, the body would append three times and output wrong result.
[[File:refactor21.png]]

[[File:refactor19.png]]

==='''Comment lines.'''===
The function for controller is not for output but just control and update the change of view. So puts should not appear in controller

[[File:refactor22.png]]
[[File:refactor23.png]]

==='''RSpec Test'''===
Our test plan is to test if db_query(assignment_id, round_num, has_topic, another_assignment_id = 0)(in line 51), db_query_with_quiz_score(assignment_id, another_assignment_id = 0)(int line 82), json_generator(assignment_id, another_assignment_id = 0, round_num = 2, type = 'peer review grades')(in line 101) work in the right way. These are in controller/reputation_web_service_controller.rb
<pre>
require 'rspec'

describe ReputationWebServiceController do

before(:each) do
user = build(:instructor)
stub_current_user(user, user.role.name, user.role)
end

it 'should do execute query' do
#controller.json_generator(41,0,2,'peer review grades').should != nil
has_topic = !SignUpTopic.where(41).empty?
raw_data_array = []
raw_data_array = controller.db_query(41,2,has_topic,0)
expect(controller.db_query(41,2,has_topic,0)).to be_an_instance_of(Array)
expect(controller.db_query(41,2,has_topic,0)).should_not be(nil)
expect(raw_data_array).should_not be(nil)
end
</pre>
The above codes is to test if get query can parse id and some other variables and return the correct review grade. Our test make sure the review grade are array and array contains data which is not null.

<pre>
it 'should execute query for quiz score' do
result = controller.db_query_with_quiz_score(55,0)
expect(result).to be_an_instance_of(Array)
expect(result).should_not be(nil)
#puts result
end
</pre>
The above code is to test if we can query quiz score by given assignment id and another assignment id and return correct quiz score array. Our test make sure that the the result is array and not a null array

But when we try to test json_generator(assignment_id, another_assignment_id = 0, round_num = 2, type = 'peer review grades'). There is a wrong report which is no method. the line 106 that is no method is:
@results = db_query(assignment.id, round_num, has_topic, another_assignment_id)
And the report is that:
[[File:24.jpeg]]

Because this method can't be called for some reason and this is not the part that we need to refactor, we can't test this part.

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-11-06T00:44:29Z

Hren3:

=='''Introduction'''==
==='''Introduction to Expertiza'''===
[http://expertiza.ncsu.edu/ Expertiza] is a peer review based system which provides incremental learning from the class. This project has been developed together by faculty and students using [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create, edit and delete assignments, create new assignment topics, assign them to a particular class or selected students, have students work on teams and then review each other's assignments at the end. For the students, they can signup for topics, form teams, and submit their projects and assignments.
Students then review the work done by other students and give suggestions to improve. Teams after reviews are allotted scores and they can refer to the peer comments to further improve their work. It also supports submission of different file types for assignments, including the URLs and wiki pages.
==='''Problem Statement'''===
Online peer-review systems are now in common use in higher education. They free the instructor and course staff from having to provide personally all the feedback that students receive on their work. However, if we want to assure that all students receive competent feedback, or even use peer assigned grades, we need a way to judge which peer reviewers are most credible. The solution is the reputation system. Reputation systems have been deployed as web services, peer-review researchers will be able to use them to calculate scores on assignments, both past and present (past data can be used to tune the algorithms).

For this project, our team's job is to refactor the file: reputation_web_service_controller.rb. This file is the controller to calculate the reputation scores. A “reputation” measures how close a reviewer’s scores are to other reviewers scores. This controller is the sub-class of Application_Controller, in this controller, it implements the calculation and query of reputation scores.
----

=='''Issues to be fixed'''==

Reputation_web_service_controller.rb is a fairly complex file. It contains a lot of methods that are long and hard to understand. These methods need to be broken down into simpler and more specific methods that are easier to read/understand. Also, the few instances of code duplication that exist should be removed.

#Replace class variable with a class instance var, and change all other places using these variables.
#Refactor method db_query
#Use zero? method instead of == 0
#Use find_by instead of dynamic method
#Delete lines which is wrong or useless.
#Comment lines

----

=='''Modified Files'''==

reputation_web_service_controller.rb

----

=='''NewFiles'''==

reputation_web_service_controller_spec.rb

----
=='''NewFiles'''==

----
=='''Approach taken to resolve the issues'''==
==='''Replace class variable with a class instance var, and change all other places using these variables.'''===
In ruby, class variables are started with '@@'and this class variable are shared in the whole inherit chain, and class variables are shared by all objects of a class. For instance variables, in Ruby, instance variables are started with '@' and instance variables belong to one object, can not be used by sub-classes.

In the project, we are asked to replace the class variable with a class instance variables.
So, as the figure 1 shown, in variables declaration part, we changed all class variables to instance variables.

[[File:Reputation code1.png]]

Next, we need to change all other places using these variables.

In action '''send_post_request'''

[[File:Reputaion 2.png]]

[[File:Reputaion3.png]]

[[File:Reputation4.png]]

[[File:Reputation5.png]]

[[File:Reputation6.png]]

----

==='''Refactor method db_query'''===
In the Reputation_web_service controller, the action db_query is important, because it implements the query of peer review grade.To refactor the db_query method, we need to firstly focus on the input variables of this method, there are 4 variables need inputs.
The db_query method is shown as follow:
<pre>
def db_query(assignment_id, another_assignment_id = 0, round_num, hasTopic)
...
end
</pre>
There is a problem: Optional arguments should appear at the end of the argument list, in this method, '''''another_assignment_id''''' is a optional arguments, but in the original version, the optional arguments -- '''''another_assignment_id''''' just appear right behind the argument--'''''assignment_id''''', so we need to refactor it.

[[File:Reputation8.png]]

And the same problems occurs when we assigns value to variable '''''@results'''''. We also need to change it.

[[File:Reputation10.png]]

There is another problem in the db_query method, we can see there is a input variable named '''''hasTopic''''', but actually that's not follow the '''"good Ruby and Rails coding practices"''', so we need to refactor it which shown as follow:

[[File:Reputation9.png]]

Finally, The project asked us'' 'Move Line 26-50 before this method''', it's important for code refactor, because it's very inelegant if you put the comments inside the method. If others want to read your codes, it will cost their much more time so that it needs to be changed. Simply, we can just move the whole comments before the method, and that will make the codes easily for reading.

[[File:Reputation11.png]]

==='''Use zero? method instead of == 0.'''===

In the original version of project, every time there is a zero judgement occurs, it use ''''''==0'''''' or ''''''!=0'''''', of course it can achieve the judgement of zero, but according to the security consideration in ruby on rails development, we need to avoid the use of ''''''==0'''''' or ''''''!=0''''''. So the best way to solve it is using the '''''zero?''''' instead of ''''''==0'''''' or ''''''!=0'''''', which shown as follow:
[[File:Reputation12.png]]

[[File:Reputation13.png]]

==='''Use find_by instead of dynamic method.'''===
This issue is replacing the find_by_id to find_by(id: id). The only difference of this change is syntax and their function worked in the same way. But if the customer want to change the way to find assignment, find_by(id: id) would be easier to change because programmer only need to change the parameter, while find_by_id need to change the whole method and its parameter. Therefore, in this project, using find_by_id is more efficient.
[[File:refactor15.png]]
[[File:refactor16.png]]

==='''Delete lines which is wrong or useless.'''===
In Line 111, the variable inner_msg has never been used, so it can be deleted
[[File:refactor17.png]]

From line 164 to 201 and line 204 to 241, cases for reputation value and do not need to handle the case for assign id. req body would change only in the case for 'Add expert grades'. If these two part failed to be deleted, the body would append three times and output wrong result.
[[File:refactor21.png]]

[[File:refactor19.png]]

==='''Comment lines.'''===
The function for controller is not for output but just control and update the change of view. So puts should not appear in controller

[[File:refactor22.png]]
[[File:refactor23.png]]

==='''RSpec Test'''===
Our test plan is to test if db_query(assignment_id, round_num, has_topic, another_assignment_id = 0)(in line 51), db_query_with_quiz_score(assignment_id, another_assignment_id = 0)(int line 82), json_generator(assignment_id, another_assignment_id = 0, round_num = 2, type = 'peer review grades')(in line 101) work in the right way. These are in controller/reputation_web_service_controller.rb
<pre>
require 'rspec'

describe ReputationWebServiceController do

before(:each) do
user = build(:instructor)
stub_current_user(user, user.role.name, user.role)
end

it 'should do execute query' do
#controller.json_generator(41,0,2,'peer review grades').should != nil
has_topic = !SignUpTopic.where(41).empty?
raw_data_array = []
raw_data_array = controller.db_query(41,2,has_topic,0)
expect(controller.db_query(41,2,has_topic,0)).to be_an_instance_of(Array)
expect(controller.db_query(41,2,has_topic,0)).should_not be(nil)
expect(raw_data_array).should_not be(nil)
end
</pre>
The above codes is to test if get query can parse id and some other variables and return the correct review grade. Our test make sure the review grade are array and array contains data which is not null.

<pre>
it 'should execute query for quiz score' do
result = controller.db_query_with_quiz_score(55,0)
expect(result).to be_an_instance_of(Array)
expect(result).should_not be(nil)
#puts result
end
</pre>
The above code is to test if we can query quiz score by given assignment id and another assignment id and return correct quiz score array. Our test make sure that the the result is array and not a null array

But when we try to test json_generator(assignment_id, another_assignment_id = 0, round_num = 2, type = 'peer review grades'). There is a wrong report which is no method. the line 106 that is no method is:
@results = db_query(assignment.id, round_num, has_topic, another_assignment_id)
And the report is that:
[[File:24.jpeg]]

Because this method can't be called for some reason and this is not the part that we need to refactor, we can't test this part.

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-11-06T00:43:25Z

Hren3:

=='''Introduction'''==
==='''Introduction to Expertiza'''===
[http://expertiza.ncsu.edu/ Expertiza] is a peer review based system which provides incremental learning from the class. This project has been developed together by faculty and students using [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create, edit and delete assignments, create new assignment topics, assign them to a particular class or selected students, have students work on teams and then review each other's assignments at the end. For the students, they can signup for topics, form teams, and submit their projects and assignments.
Students then review the work done by other students and give suggestions to improve. Teams after reviews are allotted scores and they can refer to the peer comments to further improve their work. It also supports submission of different file types for assignments, including the URLs and wiki pages.
==='''Problem Statement'''===
Online peer-review systems are now in common use in higher education. They free the instructor and course staff from having to provide personally all the feedback that students receive on their work. However, if we want to assure that all students receive competent feedback, or even use peer assigned grades, we need a way to judge which peer reviewers are most credible. The solution is the reputation system. Reputation systems have been deployed as web services, peer-review researchers will be able to use them to calculate scores on assignments, both past and present (past data can be used to tune the algorithms).

For this project, our team's job is to refactor the file: reputation_web_service_controller.rb. This file is the controller to calculate the reputation scores. A “reputation” measures how close a reviewer’s scores are to other reviewers scores. This controller is the sub-class of Application_Controller, in this controller, it implements the calculation and query of reputation scores.
----

=='''Issues to be fixed'''==

Reputation_web_service_controller.rb is a fairly complex file. It contains a lot of methods that are long and hard to understand. These methods need to be broken down into simpler and more specific methods that are easier to read/understand. Also, the few instances of code duplication that exist should be removed.

#Replace class variable with a class instance var, and change all other places using these variables.
#Refactor method db_query
#Use zero? method instead of == 0
#Use find_by instead of dynamic method
#Delete lines which is wrong or useless.
#Comment lines

----

=='''Modified Files'''==

reputation_web_service_controller.rb

----

=='''NewFiles'''==

reputation_web_service_controller_spec.rb

----
=='''Approach taken to resolve the issues'''==
==='''Replace class variable with a class instance var, and change all other places using these variables.'''===
In ruby, class variables are started with '@@'and this class variable are shared in the whole inherit chain, and class variables are shared by all objects of a class. For instance variables, in Ruby, instance variables are started with '@' and instance variables belong to one object, can not be used by sub-classes.

In the project, we are asked to replace the class variable with a class instance variables.
So, as the figure 1 shown, in variables declaration part, we changed all class variables to instance variables.

[[File:Reputation code1.png]]

Next, we need to change all other places using these variables.

In action '''send_post_request'''

[[File:Reputaion 2.png]]

[[File:Reputaion3.png]]

[[File:Reputation4.png]]

[[File:Reputation5.png]]

[[File:Reputation6.png]]

----

==='''Refactor method db_query'''===
In the Reputation_web_service controller, the action db_query is important, because it implements the query of peer review grade.To refactor the db_query method, we need to firstly focus on the input variables of this method, there are 4 variables need inputs.
The db_query method is shown as follow:
<pre>
def db_query(assignment_id, another_assignment_id = 0, round_num, hasTopic)
...
end
</pre>
There is a problem: Optional arguments should appear at the end of the argument list, in this method, '''''another_assignment_id''''' is a optional arguments, but in the original version, the optional arguments -- '''''another_assignment_id''''' just appear right behind the argument--'''''assignment_id''''', so we need to refactor it.

[[File:Reputation8.png]]

And the same problems occurs when we assigns value to variable '''''@results'''''. We also need to change it.

[[File:Reputation10.png]]

There is another problem in the db_query method, we can see there is a input variable named '''''hasTopic''''', but actually that's not follow the '''"good Ruby and Rails coding practices"''', so we need to refactor it which shown as follow:

[[File:Reputation9.png]]

Finally, The project asked us'' 'Move Line 26-50 before this method''', it's important for code refactor, because it's very inelegant if you put the comments inside the method. If others want to read your codes, it will cost their much more time so that it needs to be changed. Simply, we can just move the whole comments before the method, and that will make the codes easily for reading.

[[File:Reputation11.png]]

==='''Use zero? method instead of == 0.'''===

In the original version of project, every time there is a zero judgement occurs, it use ''''''==0'''''' or ''''''!=0'''''', of course it can achieve the judgement of zero, but according to the security consideration in ruby on rails development, we need to avoid the use of ''''''==0'''''' or ''''''!=0''''''. So the best way to solve it is using the '''''zero?''''' instead of ''''''==0'''''' or ''''''!=0'''''', which shown as follow:
[[File:Reputation12.png]]

[[File:Reputation13.png]]

==='''Use find_by instead of dynamic method.'''===
This issue is replacing the find_by_id to find_by(id: id). The only difference of this change is syntax and their function worked in the same way. But if the customer want to change the way to find assignment, find_by(id: id) would be easier to change because programmer only need to change the parameter, while find_by_id need to change the whole method and its parameter. Therefore, in this project, using find_by_id is more efficient.
[[File:refactor15.png]]
[[File:refactor16.png]]

==='''Delete lines which is wrong or useless.'''===
In Line 111, the variable inner_msg has never been used, so it can be deleted
[[File:refactor17.png]]

From line 164 to 201 and line 204 to 241, cases for reputation value and do not need to handle the case for assign id. req body would change only in the case for 'Add expert grades'. If these two part failed to be deleted, the body would append three times and output wrong result.
[[File:refactor21.png]]

[[File:refactor19.png]]

==='''Comment lines.'''===
The function for controller is not for output but just control and update the change of view. So puts should not appear in controller

[[File:refactor22.png]]
[[File:refactor23.png]]

==='''RSpec Test'''===
Our test plan is to test if db_query(assignment_id, round_num, has_topic, another_assignment_id = 0)(in line 51), db_query_with_quiz_score(assignment_id, another_assignment_id = 0)(int line 82), json_generator(assignment_id, another_assignment_id = 0, round_num = 2, type = 'peer review grades')(in line 101) work in the right way. These are in controller/reputation_web_service_controller.rb
<pre>
require 'rspec'

describe ReputationWebServiceController do

before(:each) do
user = build(:instructor)
stub_current_user(user, user.role.name, user.role)
end

it 'should do execute query' do
#controller.json_generator(41,0,2,'peer review grades').should != nil
has_topic = !SignUpTopic.where(41).empty?
raw_data_array = []
raw_data_array = controller.db_query(41,2,has_topic,0)
expect(controller.db_query(41,2,has_topic,0)).to be_an_instance_of(Array)
expect(controller.db_query(41,2,has_topic,0)).should_not be(nil)
expect(raw_data_array).should_not be(nil)
end
</pre>
The above codes is to test if get query can parse id and some other variables and return the correct review grade. Our test make sure the review grade are array and array contains data which is not null.

<pre>
it 'should execute query for quiz score' do
result = controller.db_query_with_quiz_score(55,0)
expect(result).to be_an_instance_of(Array)
expect(result).should_not be(nil)
#puts result
end
</pre>
The above code is to test if we can query quiz score by given assignment id and another assignment id and return correct quiz score array. Our test make sure that the the result is array and not a null array

But when we try to test json_generator(assignment_id, another_assignment_id = 0, round_num = 2, type = 'peer review grades'). There is a wrong report which is no method. the line 106 that is no method is:
@results = db_query(assignment.id, round_num, has_topic, another_assignment_id)
And the report is that:
[[File:24.jpeg]]

Because this method can't be called for some reason and this is not the part that we need to refactor, we can't test this part.

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-11-03T01:43:46Z

Hren3: /* Delete lines which is wrong or useless. */

==Introduction==
==='''Introduction to Expertiza'''===
[http://expertiza.ncsu.edu/ Expertiza] is a peer review based system which provides incremental learning from the class. This project has been developed together by faculty and students using [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create, edit and delete assignments, create new assignment topics, assign them to a particular class or selected students, have students work on teams and then review each other's assignments at the end. For the students, they can signup for topics, form teams, and submit their projects and assignments.
Students then review the work done by other students and give suggestions to improve. Teams after reviews are allotted scores and they can refer to the peer comments to further improve their work. It also supports submission of different file types for assignments, including the URLs and wiki pages.
==='''Problem Statement'''===
Online peer-review systems are now in common use in higher education. They free the instructor and course staff from having to provide personally all the feedback that students receive on their work. However, if we want to assure that all students receive competent feedback, or even use peer assigned grades, we need a way to judge which peer reviewers are most credible. The solution is the reputation system. Reputation systems have been deployed as web services, peer-review researchers will be able to use them to calculate scores on assignments, both past and present (past data can be used to tune the algorithms).

For this project, our team's job is to refactor the file: reputation_web_service_controller.rb. This file is the controller to calculate the reputation scores. A “reputation” measures how close a reviewer’s scores are to other reviewers scores. This controller is the sub-class of Application_Controller, in this controller, it implements the calculation and query of reputation scores.
----

=='''Issues to be fixed'''==

Reputation_web_service_controller.rb is a fairly complex file. It contains a lot of methods that are long and hard to understand. These methods need to be broken down into simpler and more specific methods that are easier to read/understand. Also, the few instances of code duplication that exist should be removed.

#Replace class variable with a class instance var, and change all other places using these variables.
#Refactor method db_query
#Use zero? method instead of == 0
#Use find_by instead of dynamic method
#Delete lines which is wrong or useless.
#Comment lines

----

=='''Modified Files'''==

reputation_web_service_controller.rb

----
=='''Approach taken to resolve the issues'''==
==='''Replace class variable with a class instance var, and change all other places using these variables.'''===
In ruby, class variables are started with '@@'and this class variable are shared in the whole inherit chain, and class variables are shared by all objects of a class. For instance variables, in Ruby, instance variables are started with '@' and instance variables belong to one object, can not be used by sub-classes.

In the project, we are asked to replace the class variable with a class instance variables.
So, as the figure 1 shown, in variables declaration part, we changed all class variables to instance variables.

[[File:Reputation code1.png]]

Next, we need to change all other places using these variables.

In action '''send_post_request'''

[[File:Reputaion 2.png]]

[[File:Reputaion3.png]]

[[File:Reputation4.png]]

[[File:Reputation5.png]]

[[File:Reputation6.png]]

----

==='''Refactor method db_query'''===
In the Reputation_web_service controller, the action db_query is important, because it implements the query of peer review grade.To refactor the db_query method, we need to firstly focus on the input variables of this method, there are 4 variables need inputs.
The db_query method is shown as follow:
<pre>
def db_query(assignment_id, another_assignment_id = 0, round_num, hasTopic)
...
end
</pre>
There is a problem: Optional arguments should appear at the end of the argument list, in this method, '''''another_assignment_id''''' is a optional arguments, but in the original version, the optional arguments -- '''''another_assignment_id''''' just appear right behind the argument--'''''assignment_id''''', so we need to refactor it.

[[File:Reputation8.png]]

And the same problems occurs when we assigns value to variable '''''@results'''''. We also need to change it.

[[File:Reputation10.png]]

There is another problem in the db_query method, we can see there is a input variable named '''''hasTopic''''', but actually that's not follow the '''"good Ruby and Rails coding practices"''', so we need to refactor it which shown as follow:

[[File:Reputation9.png]]

Finally, The project asked us'' 'Move Line 26-50 before this method''', it's important for code refactor, because it's very inelegant if you put the comments inside the method. If others want to read your codes, it will cost their much more time so that it needs to be changed. Simply, we can just move the whole comments before the method, and that will make the codes easily for reading.

[[File:Reputation11.png]]

==='''Use zero? method instead of == 0.'''===

In the original version of project, every time there is a zero judgement occurs, it use ''''''==0'''''' or ''''''!=0'''''', of course it can achieve the judgement of zero, but according to the security consideration in ruby on rails development, we need to avoid the use of ''''''==0'''''' or ''''''!=0''''''. So the best way to solve it is using the '''''zero?''''' instead of ''''''==0'''''' or ''''''!=0'''''', which shown as follow:
[[File:Reputation12.png]]

[[File:Reputation13.png]]

==='''Use find_by instead of dynamic method.'''===
This issue is replacing the find_by_id to find_by(id: id). The only difference of this change is syntax and their function worked in the same way. But if the customer want to change the way to find assignment, find_by(id: id) would be easier to change because programmer only need to change the parameter, while find_by_id need to change the whole method and its parameter. Therefore, in this project, using find_by_id is more efficient.
[[File:refactor15.png]]
[[File:refactor16.png]]

==='''Delete lines which is wrong or useless.'''===
In Line 111, the variable inner_msg has never been used, so it can be deleted
[[File:refactor17.png]]

From line 164 to 201 and line 204 to 241, cases for reputation value and do not need to handle the case for assign id. req body would change only in the case for 'Add expert grades'. If these two part failed to be deleted, the body would append three times and output wrong result.
[[File:refactor21.png]]

[[File:refactor19.png]]

==='''Comment lines.'''===
The function for controller is not for output but just control and update the change of view. So puts should not appear in controller

[[File:refactor22.png]]
[[File:refactor23.png]]

==='''RSpec Test'''===
To finish the Test part, we write some test to ensure the functionality works and also to make sure the refactoring does not break any of the existing functionality. These tests check for the basic functionality of the reputation_web_service_controller.
<pre>
describe ReputationWebServiceController do

before(:each) do
user = build(:instructor)
stub_current_user(user, user.role.name, user.role)
request.env['HTTP_REFERER'] = 'www.google.com'
end

it 'should do execute query' do
#controller.json_generator(41,0,2,'peer review grades').should != nil
has_topic = !SignUpTopic.where(41).empty?
raw_data_array = []
raw_data_array = controller.db_query(41,2,has_topic,0)
expect(controller.db_query(41,2,has_topic,0)).to be_an_instance_of(Array)
expect(controller.db_query(41,2,has_topic,0)).should_not be(nil)
expect(raw_data_array).should_not be(nil)
end

it 'should do special query with quiz score' do
#controller.json_generator(41,0,2,'peer review grades').should != nil
raw_data_array = []
raw_data_array = controller.db_query_with_quiz_score(41,0)
expect(controller.db_query_with_quiz_score(41,0)).to be_an_instance_of(Array)
expect(controller.db_query_with_quiz_score(41,0)).should_not be(nil)
expect(raw_data_array).should_not be(nil)
end

it 'should query the QuizQuestionnaire with team_ids' do
team_ids = 1
quiz_questionnnaire_ids = []
allow( QuizQuestionnaire).to where(:team_id).and_return(quiz_questionnnaires)
expect(quiz_questionnnaires).should_not be(nil)
end

it 'should get the quiz score' do
allow(Participant).to find(:reviewer_id).and_return(participant)
expect(participant).should_not be(nil)
allow(Participant).to find(:response_map).and_return(quiz_score)
expect(quiz_score).should_not be(nil)
end

end
</pre>
The above codes achieve the test part of this controller——'''''Reputation_Web_Service_Controller''''', it this controller, there are some methods and we choose two methods do the test: '''''db_query''''' and '''''db_query_with_quiz_score'''''.

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-11-03T01:43:26Z

Hren3: /* Delete lines which is wrong or useless. */

==Introduction==
==='''Introduction to Expertiza'''===
[http://expertiza.ncsu.edu/ Expertiza] is a peer review based system which provides incremental learning from the class. This project has been developed together by faculty and students using [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create, edit and delete assignments, create new assignment topics, assign them to a particular class or selected students, have students work on teams and then review each other's assignments at the end. For the students, they can signup for topics, form teams, and submit their projects and assignments.
Students then review the work done by other students and give suggestions to improve. Teams after reviews are allotted scores and they can refer to the peer comments to further improve their work. It also supports submission of different file types for assignments, including the URLs and wiki pages.
==='''Problem Statement'''===
Online peer-review systems are now in common use in higher education. They free the instructor and course staff from having to provide personally all the feedback that students receive on their work. However, if we want to assure that all students receive competent feedback, or even use peer assigned grades, we need a way to judge which peer reviewers are most credible. The solution is the reputation system. Reputation systems have been deployed as web services, peer-review researchers will be able to use them to calculate scores on assignments, both past and present (past data can be used to tune the algorithms).

For this project, our team's job is to refactor the file: reputation_web_service_controller.rb. This file is the controller to calculate the reputation scores. A “reputation” measures how close a reviewer’s scores are to other reviewers scores. This controller is the sub-class of Application_Controller, in this controller, it implements the calculation and query of reputation scores.
----

=='''Issues to be fixed'''==

Reputation_web_service_controller.rb is a fairly complex file. It contains a lot of methods that are long and hard to understand. These methods need to be broken down into simpler and more specific methods that are easier to read/understand. Also, the few instances of code duplication that exist should be removed.

#Replace class variable with a class instance var, and change all other places using these variables.
#Refactor method db_query
#Use zero? method instead of == 0
#Use find_by instead of dynamic method
#Delete lines which is wrong or useless.
#Comment lines

----

=='''Modified Files'''==

reputation_web_service_controller.rb

----
=='''Approach taken to resolve the issues'''==
==='''Replace class variable with a class instance var, and change all other places using these variables.'''===
In ruby, class variables are started with '@@'and this class variable are shared in the whole inherit chain, and class variables are shared by all objects of a class. For instance variables, in Ruby, instance variables are started with '@' and instance variables belong to one object, can not be used by sub-classes.

In the project, we are asked to replace the class variable with a class instance variables.
So, as the figure 1 shown, in variables declaration part, we changed all class variables to instance variables.

[[File:Reputation code1.png]]

Next, we need to change all other places using these variables.

In action '''send_post_request'''

[[File:Reputaion 2.png]]

[[File:Reputaion3.png]]

[[File:Reputation4.png]]

[[File:Reputation5.png]]

[[File:Reputation6.png]]

----

==='''Refactor method db_query'''===
In the Reputation_web_service controller, the action db_query is important, because it implements the query of peer review grade.To refactor the db_query method, we need to firstly focus on the input variables of this method, there are 4 variables need inputs.
The db_query method is shown as follow:
<pre>
def db_query(assignment_id, another_assignment_id = 0, round_num, hasTopic)
...
end
</pre>
There is a problem: Optional arguments should appear at the end of the argument list, in this method, '''''another_assignment_id''''' is a optional arguments, but in the original version, the optional arguments -- '''''another_assignment_id''''' just appear right behind the argument--'''''assignment_id''''', so we need to refactor it.

[[File:Reputation8.png]]

And the same problems occurs when we assigns value to variable '''''@results'''''. We also need to change it.

[[File:Reputation10.png]]

There is another problem in the db_query method, we can see there is a input variable named '''''hasTopic''''', but actually that's not follow the '''"good Ruby and Rails coding practices"''', so we need to refactor it which shown as follow:

[[File:Reputation9.png]]

Finally, The project asked us'' 'Move Line 26-50 before this method''', it's important for code refactor, because it's very inelegant if you put the comments inside the method. If others want to read your codes, it will cost their much more time so that it needs to be changed. Simply, we can just move the whole comments before the method, and that will make the codes easily for reading.

[[File:Reputation11.png]]

==='''Use zero? method instead of == 0.'''===

In the original version of project, every time there is a zero judgement occurs, it use ''''''==0'''''' or ''''''!=0'''''', of course it can achieve the judgement of zero, but according to the security consideration in ruby on rails development, we need to avoid the use of ''''''==0'''''' or ''''''!=0''''''. So the best way to solve it is using the '''''zero?''''' instead of ''''''==0'''''' or ''''''!=0'''''', which shown as follow:
[[File:Reputation12.png]]

[[File:Reputation13.png]]

==='''Use find_by instead of dynamic method.'''===
This issue is replacing the find_by_id to find_by(id: id). The only difference of this change is syntax and their function worked in the same way. But if the customer want to change the way to find assignment, find_by(id: id) would be easier to change because programmer only need to change the parameter, while find_by_id need to change the whole method and its parameter. Therefore, in this project, using find_by_id is more efficient.
[[File:refactor15.png]]
[[File:refactor16.png]]

==='''Delete lines which is wrong or useless.'''===
In Line 111, the variable inner_msg has never been used, so it can be deleted
[[File:refactor17.png]]
From line 164 to 201 and line 204 to 241, cases for reputation value and do not need to handle the case for assign id. req body would change only in the case for 'Add expert grades'. If these two part failed to be deleted, the body would append three times and output wrong result.
[[File:refactor21.png]]

[[File:refactor19.png]]

==='''Comment lines.'''===
The function for controller is not for output but just control and update the change of view. So puts should not appear in controller

[[File:refactor22.png]]
[[File:refactor23.png]]

==='''RSpec Test'''===
To finish the Test part, we write some test to ensure the functionality works and also to make sure the refactoring does not break any of the existing functionality. These tests check for the basic functionality of the reputation_web_service_controller.
<pre>
describe ReputationWebServiceController do

before(:each) do
user = build(:instructor)
stub_current_user(user, user.role.name, user.role)
request.env['HTTP_REFERER'] = 'www.google.com'
end

it 'should do execute query' do
#controller.json_generator(41,0,2,'peer review grades').should != nil
has_topic = !SignUpTopic.where(41).empty?
raw_data_array = []
raw_data_array = controller.db_query(41,2,has_topic,0)
expect(controller.db_query(41,2,has_topic,0)).to be_an_instance_of(Array)
expect(controller.db_query(41,2,has_topic,0)).should_not be(nil)
expect(raw_data_array).should_not be(nil)
end

it 'should do special query with quiz score' do
#controller.json_generator(41,0,2,'peer review grades').should != nil
raw_data_array = []
raw_data_array = controller.db_query_with_quiz_score(41,0)
expect(controller.db_query_with_quiz_score(41,0)).to be_an_instance_of(Array)
expect(controller.db_query_with_quiz_score(41,0)).should_not be(nil)
expect(raw_data_array).should_not be(nil)
end

it 'should query the QuizQuestionnaire with team_ids' do
team_ids = 1
quiz_questionnnaire_ids = []
allow( QuizQuestionnaire).to where(:team_id).and_return(quiz_questionnnaires)
expect(quiz_questionnnaires).should_not be(nil)
end

it 'should get the quiz score' do
allow(Participant).to find(:reviewer_id).and_return(participant)
expect(participant).should_not be(nil)
allow(Participant).to find(:response_map).and_return(quiz_score)
expect(quiz_score).should_not be(nil)
end

end
</pre>
The above codes achieve the test part of this controller——'''''Reputation_Web_Service_Controller''''', it this controller, there are some methods and we choose two methods do the test: '''''db_query''''' and '''''db_query_with_quiz_score'''''.

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-11-03T01:42:22Z

Hren3: /* RSpec Test */

==Introduction==
==='''Introduction to Expertiza'''===
[http://expertiza.ncsu.edu/ Expertiza] is a peer review based system which provides incremental learning from the class. This project has been developed together by faculty and students using [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create, edit and delete assignments, create new assignment topics, assign them to a particular class or selected students, have students work on teams and then review each other's assignments at the end. For the students, they can signup for topics, form teams, and submit their projects and assignments.
Students then review the work done by other students and give suggestions to improve. Teams after reviews are allotted scores and they can refer to the peer comments to further improve their work. It also supports submission of different file types for assignments, including the URLs and wiki pages.
==='''Problem Statement'''===
Online peer-review systems are now in common use in higher education. They free the instructor and course staff from having to provide personally all the feedback that students receive on their work. However, if we want to assure that all students receive competent feedback, or even use peer assigned grades, we need a way to judge which peer reviewers are most credible. The solution is the reputation system. Reputation systems have been deployed as web services, peer-review researchers will be able to use them to calculate scores on assignments, both past and present (past data can be used to tune the algorithms).

For this project, our team's job is to refactor the file: reputation_web_service_controller.rb. This file is the controller to calculate the reputation scores. A “reputation” measures how close a reviewer’s scores are to other reviewers scores. This controller is the sub-class of Application_Controller, in this controller, it implements the calculation and query of reputation scores.
----

=='''Issues to be fixed'''==

Reputation_web_service_controller.rb is a fairly complex file. It contains a lot of methods that are long and hard to understand. These methods need to be broken down into simpler and more specific methods that are easier to read/understand. Also, the few instances of code duplication that exist should be removed.

#Replace class variable with a class instance var, and change all other places using these variables.
#Refactor method db_query
#Use zero? method instead of == 0
#Use find_by instead of dynamic method
#Delete lines which is wrong or useless.
#Comment lines

----

=='''Modified Files'''==

reputation_web_service_controller.rb

----
=='''Approach taken to resolve the issues'''==
==='''Replace class variable with a class instance var, and change all other places using these variables.'''===
In ruby, class variables are started with '@@'and this class variable are shared in the whole inherit chain, and class variables are shared by all objects of a class. For instance variables, in Ruby, instance variables are started with '@' and instance variables belong to one object, can not be used by sub-classes.

In the project, we are asked to replace the class variable with a class instance variables.
So, as the figure 1 shown, in variables declaration part, we changed all class variables to instance variables.

[[File:Reputation code1.png]]

Next, we need to change all other places using these variables.

In action '''send_post_request'''

[[File:Reputaion 2.png]]

[[File:Reputaion3.png]]

[[File:Reputation4.png]]

[[File:Reputation5.png]]

[[File:Reputation6.png]]

----

==='''Refactor method db_query'''===
In the Reputation_web_service controller, the action db_query is important, because it implements the query of peer review grade.To refactor the db_query method, we need to firstly focus on the input variables of this method, there are 4 variables need inputs.
The db_query method is shown as follow:
<pre>
def db_query(assignment_id, another_assignment_id = 0, round_num, hasTopic)
...
end
</pre>
There is a problem: Optional arguments should appear at the end of the argument list, in this method, '''''another_assignment_id''''' is a optional arguments, but in the original version, the optional arguments -- '''''another_assignment_id''''' just appear right behind the argument--'''''assignment_id''''', so we need to refactor it.

[[File:Reputation8.png]]

And the same problems occurs when we assigns value to variable '''''@results'''''. We also need to change it.

[[File:Reputation10.png]]

There is another problem in the db_query method, we can see there is a input variable named '''''hasTopic''''', but actually that's not follow the '''"good Ruby and Rails coding practices"''', so we need to refactor it which shown as follow:

[[File:Reputation9.png]]

Finally, The project asked us'' 'Move Line 26-50 before this method''', it's important for code refactor, because it's very inelegant if you put the comments inside the method. If others want to read your codes, it will cost their much more time so that it needs to be changed. Simply, we can just move the whole comments before the method, and that will make the codes easily for reading.

[[File:Reputation11.png]]

==='''Use zero? method instead of == 0.'''===

In the original version of project, every time there is a zero judgement occurs, it use ''''''==0'''''' or ''''''!=0'''''', of course it can achieve the judgement of zero, but according to the security consideration in ruby on rails development, we need to avoid the use of ''''''==0'''''' or ''''''!=0''''''. So the best way to solve it is using the '''''zero?''''' instead of ''''''==0'''''' or ''''''!=0'''''', which shown as follow:
[[File:Reputation12.png]]

[[File:Reputation13.png]]

==='''Use find_by instead of dynamic method.'''===
This issue is replacing the find_by_id to find_by(id: id). The only difference of this change is syntax and their function worked in the same way. But if the customer want to change the way to find assignment, find_by(id: id) would be easier to change because programmer only need to change the parameter, while find_by_id need to change the whole method and its parameter. Therefore, in this project, using find_by_id is more efficient.
[[File:refactor15.png]]
[[File:refactor16.png]]

==='''Delete lines which is wrong or useless.'''===
In Line 111, the variable inner_msg has never been used, so it can be deleted
[[File:refactor17.png]]
From line 164 to 201 and line 204 to 241, cases for reputation value and do not need to handle the case for assign id. req body would change only in the case for 'Add expert grades'. If these two part failed to be deleted, the body would append three times and output wrong result.
[[File:refactor21.png]]
[[File:refactor19.png]]

==='''Comment lines.'''===
The function for controller is not for output but just control and update the change of view. So puts should not appear in controller

[[File:refactor22.png]]
[[File:refactor23.png]]

==='''RSpec Test'''===
To finish the Test part, we write some test to ensure the functionality works and also to make sure the refactoring does not break any of the existing functionality. These tests check for the basic functionality of the reputation_web_service_controller.
<pre>
describe ReputationWebServiceController do

before(:each) do
user = build(:instructor)
stub_current_user(user, user.role.name, user.role)
request.env['HTTP_REFERER'] = 'www.google.com'
end

it 'should do execute query' do
#controller.json_generator(41,0,2,'peer review grades').should != nil
has_topic = !SignUpTopic.where(41).empty?
raw_data_array = []
raw_data_array = controller.db_query(41,2,has_topic,0)
expect(controller.db_query(41,2,has_topic,0)).to be_an_instance_of(Array)
expect(controller.db_query(41,2,has_topic,0)).should_not be(nil)
expect(raw_data_array).should_not be(nil)
end

it 'should do special query with quiz score' do
#controller.json_generator(41,0,2,'peer review grades').should != nil
raw_data_array = []
raw_data_array = controller.db_query_with_quiz_score(41,0)
expect(controller.db_query_with_quiz_score(41,0)).to be_an_instance_of(Array)
expect(controller.db_query_with_quiz_score(41,0)).should_not be(nil)
expect(raw_data_array).should_not be(nil)
end

it 'should query the QuizQuestionnaire with team_ids' do
team_ids = 1
quiz_questionnnaire_ids = []
allow( QuizQuestionnaire).to where(:team_id).and_return(quiz_questionnnaires)
expect(quiz_questionnnaires).should_not be(nil)
end

it 'should get the quiz score' do
allow(Participant).to find(:reviewer_id).and_return(participant)
expect(participant).should_not be(nil)
allow(Participant).to find(:response_map).and_return(quiz_score)
expect(quiz_score).should_not be(nil)
end

end
</pre>
The above codes achieve the test part of this controller——'''''Reputation_Web_Service_Controller''''', it this controller, there are some methods and we choose two methods do the test: '''''db_query''''' and '''''db_query_with_quiz_score'''''.

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-11-03T01:32:50Z

Hren3: /* RSpec Test */

==Introduction==
==='''Introduction to Expertiza'''===
[http://expertiza.ncsu.edu/ Expertiza] is a peer review based system which provides incremental learning from the class. This project has been developed together by faculty and students using [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create, edit and delete assignments, create new assignment topics, assign them to a particular class or selected students, have students work on teams and then review each other's assignments at the end. For the students, they can signup for topics, form teams, and submit their projects and assignments.
Students then review the work done by other students and give suggestions to improve. Teams after reviews are allotted scores and they can refer to the peer comments to further improve their work. It also supports submission of different file types for assignments, including the URLs and wiki pages.
==='''Problem Statement'''===
Online peer-review systems are now in common use in higher education. They free the instructor and course staff from having to provide personally all the feedback that students receive on their work. However, if we want to assure that all students receive competent feedback, or even use peer assigned grades, we need a way to judge which peer reviewers are most credible. The solution is the reputation system. Reputation systems have been deployed as web services, peer-review researchers will be able to use them to calculate scores on assignments, both past and present (past data can be used to tune the algorithms).

For this project, our team's job is to refactor the file: reputation_web_service_controller.rb. This file is the controller to calculate the reputation scores. A “reputation” measures how close a reviewer’s scores are to other reviewers scores. This controller is the sub-class of Application_Controller, in this controller, it implements the calculation and query of reputation scores.
----

=='''Issues to be fixed'''==

Reputation_web_service_controller.rb is a fairly complex file. It contains a lot of methods that are long and hard to understand. These methods need to be broken down into simpler and more specific methods that are easier to read/understand. Also, the few instances of code duplication that exist should be removed.

#Replace class variable with a class instance var, and change all other places using these variables.
#Refactor method db_query
#Use zero? method instead of == 0
#Use find_by instead of dynamic method
#Delete lines which is wrong or useless.
#Comment lines

----

=='''Modified Files'''==

reputation_web_service_controller.rb

----
=='''Approach taken to resolve the issues'''==
==='''Replace class variable with a class instance var, and change all other places using these variables.'''===
In ruby, class variables are started with '@@'and this class variable are shared in the whole inherit chain, and class variables are shared by all objects of a class. For instance variables, in Ruby, instance variables are started with '@' and instance variables belong to one object, can not be used by sub-classes.

In the project, we are asked to replace the class variable with a class instance variables.
So, as the figure 1 shown, in variables declaration part, we changed all class variables to instance variables.

[[File:Reputation code1.png]]

Next, we need to change all other places using these variables.

In action '''send_post_request'''

[[File:Reputaion 2.png]]

[[File:Reputaion3.png]]

[[File:Reputation4.png]]

[[File:Reputation5.png]]

[[File:Reputation6.png]]

----

==='''Refactor method db_query'''===
In the Reputation_web_service controller, the action db_query is important, because it implements the query of peer review grade.To refactor the db_query method, we need to firstly focus on the input variables of this method, there are 4 variables need inputs.
The db_query method is shown as follow:
<pre>
def db_query(assignment_id, another_assignment_id = 0, round_num, hasTopic)
...
end
</pre>
There is a problem: Optional arguments should appear at the end of the argument list, in this method, '''''another_assignment_id''''' is a optional arguments, but in the original version, the optional arguments -- '''''another_assignment_id''''' just appear right behind the argument--'''''assignment_id''''', so we need to refactor it.

[[File:Reputation8.png]]

And the same problems occurs when we assigns value to variable '''''@results'''''. We also need to change it.

[[File:Reputation10.png]]

There is another problem in the db_query method, we can see there is a input variable named '''''hasTopic''''', but actually that's not follow the '''"good Ruby and Rails coding practices"''', so we need to refactor it which shown as follow:

[[File:Reputation9.png]]

Finally, The project asked us'' 'Move Line 26-50 before this method''', it's important for code refactor, because it's very inelegant if you put the comments inside the method. If others want to read your codes, it will cost their much more time so that it needs to be changed. Simply, we can just move the whole comments before the method, and that will make the codes easily for reading.

[[File:Reputation11.png]]

==='''Use zero? method instead of == 0.'''===

In the original version of project, every time there is a zero judgement occurs, it use ''''''==0'''''' or ''''''!=0'''''', of course it can achieve the judgement of zero, but according to the security consideration in ruby on rails development, we need to avoid the use of ''''''==0'''''' or ''''''!=0''''''. So the best way to solve it is using the '''''zero?''''' instead of ''''''==0'''''' or ''''''!=0'''''', which shown as follow:
[[File:Reputation12.png]]

[[File:Reputation13.png]]

==='''Use find_by instead of dynamic method.'''===
This issue is replacing the find_by_id to find_by(id: id). The only difference of this change is syntax and their function worked in the same way. But if the customer want to change the way to find assignment, find_by(id: id) would be easier to change because programmer only need to change the parameter, while find_by_id need to change the whole method and its parameter. Therefore, in this project, using find_by_id is more efficient.
[[File:refactor15.png]]
[[File:refactor16.png]]

==='''Delete lines which is wrong or useless.'''===
In Line 111, the variable inner_msg has never been used, so it can be deleted
[[File:refactor17.png]]
From line 164 to 201 and line 204 to 241, cases for reputation value and do not need to handle the case for assign id. req body would change only in the case for 'Add expert grades'. If these two part failed to be deleted, the body would append three times and output wrong result.
[[File:refactor21.png]]
[[File:refactor19.png]]

==='''Comment lines.'''===
The function for controller is not for output but just control and update the change of view. So puts should not appear in controller

[[File:refactor22.png]]
[[File:refactor23.png]]

==='''RSpec Test'''===
To finish the Test part, we write some test to ensure the functionality works and also to make sure the refactoring does not break any of the existing functionality. These tests check for the basic functionality of the reputation_web_service_controller.
<pre>
describe ReputationWebServiceController do

before(:each) do
user = build(:instructor)
stub_current_user(user, user.role.name, user.role)
request.env['HTTP_REFERER'] = 'www.google.com'
end

it 'should do execute query' do
#controller.json_generator(41,0,2,'peer review grades').should != nil
has_topic = !SignUpTopic.where(41).empty?
raw_data_array = []
raw_data_array = controller.db_query(41,2,has_topic,0)
expect(controller.db_query(41,2,has_topic,0)).to be_an_instance_of(Array)
expect(controller.db_query(41,2,has_topic,0)).should_not be(nil)
expect(raw_data_array).should_not be(nil)
end

it 'should do special query with quiz score' do
#controller.json_generator(41,0,2,'peer review grades').should != nil
raw_data_array = []
raw_data_array = controller.db_query_with_quiz_score(41,0)
expect(controller.db_query_with_quiz_score(41,0)).to be_an_instance_of(Array)
expect(controller.db_query_with_quiz_score(41,0)).should_not be(nil)
expect(raw_data_array).should_not be(nil)
end

it 'should query the QuizQuestionnaire with team_ids' do
team_ids = 1
quiz_questionnnaire_ids = []
allow( QuizQuestionnaire).to where(:team_id).and_return(quiz_questionnnaires)
expect(quiz_questionnnaires).should_not be(nil)
end

it 'should get the quiz score' do
allow(Participant).to find(:reviewer_id).and_return(participant)
expect(participant).should_not be(nil)
allow(Participant).to find(:response_map).and_return(quiz_score)
expect(quiz_score).should_not be(nil)
end

end
</pre>

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T04:05:59Z

Hren3: /* Approach taken to resolve the issues */

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T04:04:09Z

Hren3: /* Use zero? method instead of == 0. */

File:Reputation13.png

2017-10-26T04:03:25Z

Hren3:

File:Reputation12.png

2017-10-26T04:03:16Z

Hren3:

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T03:52:03Z

Hren3: /* Approach taken to resolve the issues */

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T03:43:10Z

Hren3: /* Replace class variable with a class instance var, and change all other places using these variables. */

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T03:42:00Z

Hren3: /* Refactor method db_query */

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T03:40:20Z

Hren3: /* Refactor method db_query */

File:Reputation11.png

2017-10-26T03:40:02Z

Hren3:

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T03:25:00Z

Hren3: /* Refactor method db_query */

File:Reputation9.png

2017-10-26T03:20:58Z

Hren3:

File:Reputation10.png

2017-10-26T03:20:49Z

Hren3:

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T03:11:27Z

Hren3: /* Refactor method db_query */

File:Reputation8.png

2017-10-26T03:11:14Z

Hren3:

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T03:09:16Z

Hren3: /* Refactor method db_query */

File:Reputation7.png

2017-10-26T03:06:29Z

Hren3:

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T02:54:12Z

Hren3: /* Refactor method db_query */

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T02:30:09Z

Hren3: /* Approach taken to resolve the issues */

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T02:29:08Z

Hren3: /* Replace class variable with a class instance var, and change all other places using these variables. */

File:Reputation6.png

2017-10-26T02:23:24Z

Hren3:

File:Reputation5.png

2017-10-26T02:21:08Z

Hren3:

File:Reputation4.png

2017-10-26T02:14:58Z

Hren3:

File:Reputaion3.png

2017-10-26T02:12:57Z

Hren3: uploaded a new version of "File:Reputaion3.png"

File:Reputaion3.png

2017-10-26T02:11:59Z

Hren3:

File:Reputaion 2.png

2017-10-26T02:05:07Z

Hren3:

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T01:40:43Z

Hren3: /* Replace class variable with a class instance var, and change all other places using these variables. */

File:Reputation code1.png

2017-10-26T01:39:46Z

Hren3: asdadadsasdasd

asdadadsasdasd

File:1.png

2017-10-26T01:25:46Z

Hren3: uploaded a new version of "File:1.png"

Distributed Version Control System

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T01:19:25Z

Hren3:

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T01:03:35Z

Hren3: /* Modified Files */

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T01:00:33Z

Hren3: /* Modified Files */

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T00:59:40Z

Hren3: /* Issues to be fixed */

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-26T00:27:58Z

Hren3: /* Problem Statement */

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-25T23:55:51Z

Hren3: /* Introduction */

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-25T23:54:25Z

Hren3: /* Problem Statement */

==Introduction==
==='''Problem Statement'''===
Online peer-review systems are now in common use in higher education. They free the instructor and course staff from having to provide personally all the feedback that students receive on their work. However, if we want to assure that all students receive competent feedback, or even use peer assigned grades, we need a way to judge which peer reviewers are most credible.
----

=='''Issues to be fixed'''==

Fix Issue #311: when one respond to an advertisement, (s)he should only be able to respond once. When a team advertises for new members, someone who responds to an ad should only be allowed to respond once. Currently, after someone responds to an ad, they can respond again an unlimited number of times.

Fix Issue #227 : if user A got a topic and user B got no topic, then A join B’s team, A’s topic be dropped and A and B end up with a new team with no topic. This issue should be handle carefully because we cannot simply add B to A’s team (imagine, if A has teammate X and B has teammate Y...). One of a potential fix is that, for assignment w/ topics, one cannot not post an ad unless (s)he holds a topic, similarly, one cannot sent invitations unless (s)he holds a topic.

----

=='''Modified Files'''==

1) sign_up_sheet.rb

2) student_teams_helper.rb

3) _table_line.html.erb

4) view.html.erb

----

CSC/ECE 517 Fall 2017/E1772 Refactor reputation web service controller.rb

2017-10-25T23:46:59Z

Hren3: Created page with "==Introduction== ==='''Problem Statement'''=== In Expertiza, there are teammate advertisement, invitation and join team request features which facilitate students to form teams. ..."

==Introduction==
==='''Problem Statement'''===
In Expertiza, there are teammate advertisement, invitation and join team request features which facilitate students to form teams. Teammate advertisement works only for assignments with topics. The topic holders can create advertisement and a trumpet icon will show on the signup sheet page. If one respond to the advertisement, a join team request will be created and the topic holder can see it on “Your team” page. After a student gets a team and the team is not full, (s)he should also be able to send invitations to others.
----
=='''Issues to be fixed'''==

Fix Issue #311: when one respond to an advertisement, (s)he should only be able to respond once. When a team advertises for new members, someone who responds to an ad should only be allowed to respond once. Currently, after someone responds to an ad, they can respond again an unlimited number of times.

Fix Issue #227 : if user A got a topic and user B got no topic, then A join B’s team, A’s topic be dropped and A and B end up with a new team with no topic. This issue should be handle carefully because we cannot simply add B to A’s team (imagine, if A has teammate X and B has teammate Y...). One of a potential fix is that, for assignment w/ topics, one cannot not post an ad unless (s)he holds a topic, similarly, one cannot sent invitations unless (s)he holds a topic.

----

=='''Modified Files'''==

1) sign_up_sheet.rb

2) student_teams_helper.rb

3) _table_line.html.erb

4) view.html.erb

----