Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Fall 2015 E1577 MayYellowRoverJump

2015-11-14T04:21:36Z

Jchen45:

== Introduction ==
[https://expertiza.ncsu.edu/ Expertiza] is an open-source education and classroom web-tool founded by the [http://www.nsf.gov/ National Science Foundation]. Built with Ruby on Rails, it is designed to manage complete courses and students’ work within those courses. Each course can have a collection of instructors and students, though the interaction between instructors and students is minimal. The real emphasis of Expertiza is placed on peer-to-peer interactions, fostering a student-driven learning environment. Courses are comprised of assignments which users complete individually or with a team. Assignments usually encourage or require a team to enforce practicing peer-to-peer interaction.

One of the main tenets of Expertiza is its implicit peer-review system. Assignments inherently have a review stage where, rather than having instructors review a team’s work, other students review a team’s submission for that assignment. When completing a review, a student is presented with essentially a rubric for the assignment, and they fill in each category with the score they deem commensurate with the work of the team. Of course, each category has a comments box for the student to qualify the score they doled out. Each member of the submitting team is notified of the review, and the team can then decide as a whole how to rework their submission based on the feedback in the peer reviews.

There do exist issues, however, with respect to viewing one’s team’s reviews, particularly in the realm of usability. Our team has been tasked with revamping and enhancing the review UI to produce a more focused and uniform user experience for students and instructors alike.

== Assignment ==

=== Description ===

The tasks of the assignment are as follows:

# Compact the review display. Eliminate the blank lines between items within a single review. Instead vary the background color from line to line to improve readability.
# Add the following to the top of each review: who submitted the review. The instructor should see the user’s name and user-ID. A student should see “Reviewer #k”, where k is an integer between 1 and n, the number of reviews that have been submitted for this project. the version number of the review, and the time the review was submitted.
# Add functionality to allow the instructor to view different review rounds. Also, provide instructor with a review report page of all reviewers’ reviews to every project.
# Allow different alternate view: group by question.
# Reduce the duplicate code between instructor and student grade reports.

=== Purpose ===
==== Motivations ====
* Lack of uniformity between student and instructor views
* No defined separation between reviews
* All reviews and review data (comments, question text, etc.) are displayed at once

==== Discussion ====
There is no denying that the usability of viewing peer reviews leaves much to be desired. It lacks uniformity across the student and instructor roles, and the view itself has no semblance of order or organization. Viewing a single student’s review is a chore for both instructors and students, as there is no clear separation between reviews. In addition, all reviews are displayed at once, meaning viewing a single review requires scrolling through the page until the desired review is found. Our goal is to take the same data in the current display and present it in a more focused manner that allows a user, in either the instructor or student role, to absorb and process the content in the peer review more efficiently. Accessing, viewing, and understanding a review should be a far more simple task than what it currently is. In addition to the overhaul of the presentation layer, we also strive to drastically increase code reuse in the controller and model layers of the review module, which will in turn create a more uniform experience for both the instructor and student roles.

=== Scope ===
The scope of this task is limited to enhancing the usability of viewing peer reviews for both students and instructors. It is within our scope to modify the corresponding views for this functionality, as well as the underlying controllers and models as needed. The modifications to the Ruby classes will either be to accommodate changes to the view or to provide a uniform experience for both the instructor and student. As this is more of a user experience task, e.g, changing the way data is displayed to the user, there will be limited modifications to the base functionality of this module. It is not within our scope to change any element of the actual peer review process, such as selecting a topic to review or completing a review of a topic. As a result, we will not be modifying the results of peer reviews; the same peer review data will be present both before and after our task is completed.

== Design ==

=== Discussion of Resolution ===
The goal of this project is to optimize code and UI of review module, to make it more readable and user-friendly. To be more specific, our work focuses on the following specific areas:
* [https://en.wikipedia.org/wiki/Code_refactoring Refactoring] grades_controller.rb and review_mapping_controller.rb to optimize code organization, making code easier to read.
* Modifying UI to be more friendly. Instructor can see users' names and user-IDs, student should see review numbers, like “Reviewer #k” where k is the number of reviews submitted to the project/assignment. Besides, the round of reviews (version number) and submitted time of reviews could also be saw by both students and instructor.
* Modifying UI to make it easier for students/instructor to see reviews of different rounds. Tabs is a good choice, besides drop-down menus are a good alternate. Maybe we also need to modify models to make it adaptive to different rounds of reviews.(Currently, review models will only record the latest version of reviews.)
* Providing a new page to display all reviews of one project/assignment as review report. In this page, reviews will be displayed as a format like “Question 1, Reviews 1, Reviews 2 … Reviews n” (reviewer’s name should also be included here). Besides, here we also need to provide different version/round of reviews of different questions. At the top of this page, there should be a matrix to show the summary of questions(as a row) and reviews(as a column).(How can different version be displayed in the matrix? Using different matrix works?)
* Providing a way to hide or gray the questions, making students/instructor more focus on reviews.
* Providing a search reviews through a specific keyword. And when searching, providing a ‘next’ button to navigate to next keyword place.
* Providing a two-dimensional table to show the scores of each question and reviewer’s name who gives the score to the question.

=== Mock-Ups ===
[[File:Current Student.PNG|border|center|alt=The current student view.|The current review display for students.]]
'''Image 1. The current review display for students.'''

Image 1, above, illustrates the current design of the Student Review Report. We propose entirely removing the summary statistics at the top of the page. The data in these statistics can be displayed in a more efficient manner.

[[File:Heatgrid_studen_condensed.png|border|center|alt=The proposed condensed student display.|The proposed condensed student display.]]
'''Image 2. The proposed student review report, in default collapsed view.'''

The main UI overhaul will occur in displaying the actual reviews. Image 2 illustrates the design for the student review report. To combat the length of the current review display, we have chosen to collapse all data into a single table that can be viewed without the need to endlessly scroll through the page. Each row of the table corresponds to a specific criterion of the review and the scores each reviewer gave with whatever comments they may have offered. The columns dictate which review the score came from. By default, the full criterion text is truncated, while the comments from a review are hidden. Because all of the comments are not available for all reviews in a single screen display, the rightmost field displays the number of comment fields for each criterion which have 10 or more characters. The purpose of this field is to provide the user a quick and effective manner to spark interest into whether or not the the criterion contains meaningful comments.

The colored background of the cells are based on a color scale relative to the score. These colors are added to the design into order to quickly spark interest to the users, allowing them to pick out the essential information without having to iterate through all the data.

[[File:Heatgrid_student_expanded.png|border|center|alt=The proposed expanded student display.|The proposed expanded student display.]]
'''Image 3. The proposed student review report, in expanded view.'''

Clicking the row number will display all comments for that particular criterion, i.e., comments from each review. This way a student can compare comments and scores for a single question across all reviews. Clicking the column header for a specific review will display all the comments left by that reviewer. A student can use this functionality to view the entirety of a single review, including scores and comments.

It is important to note a important data organization difference between the existing design and the proposed design; The existing review report design groups reponses by the reviewer who created them, while the proposed review report design groups responses by the criteria. This new way of grouping responses groups like with like, allow the viewing users to see all scores and comments pertaining to one criterion at once.

[[File:Current Instructor.PNG|border|center|alt=The current instructor view.|The current review display for instructors.]]
'''Image 4. Current design of the Instructor's Review Report.'''

The instructor role will also receive interaction updates using the same underling display structures, but the content differ slightly from that of the student. Instead of reviews in the column headers, student names will take their place. We will be able to reuse the structure in the student's view for this purpose. Additionally, the Instructor view will have scores for all teams, and thus many tables of scores stacked vertically on the view. See Image 5, and notice that there is a partial table towards the bottom.

[[File:Heatgrid_instructor_condensed.png|border|center|alt=The proposed condensed instructor display.|The proposed condensed instructor display.]]
'''Image 5. Proposed design of the Instructor Review Report, in default collapsed view.'''

=== Design Patterns ===
The implementation team expects that the following design patterns be used in the solution:

* '''[https://en.wikipedia.org/wiki/Flyweight_pattern FlyWeight Design Pattern]''': using flyweight design pattern, we can use sharing data/gui to support large numbers of fine-grained objects efficiently. For instance, in expertiza system, students’ review pages and instructor review pages. We can use one page to display in both students’ ‘version’ of review page and instructor’s ‘version’ of review page.

* '''[https://en.wikipedia.org/wiki/Strategy_pattern Strategy Design Pattern]''': using strategy design pattern, we can display one page into different ‘version’ of page according to different role. For instance, in students’ review page, he will see reviewer’s id, and in instructor’s review page, he will see reviewer’s fullname. In here, student and instructor are different strategy.

* '''[https://en.wikipedia.org/wiki/Front_Controller_pattern Front Controller Design Pattern]''': with front controller design pattern, we will display different views based on the URL and currently user’s role.

* '''[https://en.wikipedia.org/wiki/Module_pattern Module Design Pattern]''': by module design pattern, source code can be organized into components that accomplish a particular function or contain everything necessary to accomplish a particular task. In JavaScript files, we use anonymous functions for responsiveness on client side via javascript and jquery.

* '''[https://en.wikipedia.org/wiki/Iterator_pattern Iterator Design Pattern]''': iterator design pattern is used to iterate through data structures in Ruby actions and when rendering the view. For instance, we need to display all reviewer’s reviews to one round, so we need the collection of all reviewer’s reviews and then iterator to display.

* '''[https://en.wikipedia.org/wiki/Facade_pattern Facade Design Pattern]''': facade design pattern provides a unified and higher-level interface to a set of interfaces in a subsystem, which makes the subsystem easier to use. For instance, in students’ review page, grades, commands and feedbacks are from three controller, and they are combined in review controller and display in one review page.

=== Use Cases ===

* '''View All Team Scores and Reviews for Specified Assignment, as Instructor''': Accessed from the Assignment list page. Allows instructor to see all peer reviews for a team for a specified assignment.
* '''View Received Scores and Reviews for Specified Assignment, Grouped by Criteria, as Student (self)''': Accessed from the Student's landing page for a particular assignment. Allows a student to see scores, grouped by criteria, for self's assignment, as well as meta reviews and author reviews.
* '''View Received Scores and Reviews for Specified Assignment, Grouped by Reviewer, as Student (self)''': Accessed from the Student's landing page for a particular assignment, Allows a student to see scores, grouped by submitted reviewer, for self's assignment.

=== Controller Testing ===

Controller testing will be centered around the proposed views which will replace the "view" and "view_my_scores" view in the grades controller. The Functional testing section covers much of the modular functionality that make up the proposed views. It is not anticipated that the testing team will perform significant controller tests.

=== Functional Tests ===

* ColorMapping module: The generation of the heatgrid requires mapping review scores to color groups. Testing of the method's functionality requires various fixture collections which vary is size, range, max, min, and number of duplicates.

* Role Security Testing: In order to DRY the code, and make it maximally orthogonal, the intention is to reuse code where possible. To re-use code across students and participants will require security and role checks to confirm the session user has access to view the requested information. To test this functionality sufficiently will require testing as admin, instructor with access, instructor without access, student, non-participant, participant, teammember, non-teammeber, etc.

* Char Comment Count Module: The generation of the "# of comments with chars > x" result will require the execution of a module. this module will be tested with various collections of comments which vary in size, range, length, and content.

* TeamReview(assignment,team): This is a controller helpful method, which will be called to generate the peer review scores for a particular team (note: no author nor meta reviews.) A collection of review scores and comments will be generated. This will be tested for empty, null, many round, single round, many reviewer, single reviewer scores.

* ParticpantReview(participant): This is a controller helper method, which will be called to generate the review scores for a particular participant. Collections generated will include reviews, author-reviews, meta-reviews for a participant. this will be tested for empty, null, many round, single round, many reviewer, single reviewer scores.

* RubricQuestion(questionnaire): This is a controller helper method, which will be called by many views in the solution. It will return a list of criteria for each found of a questionnaire or rubric. this will be tested for empty, null, many-round, single round rubrics.

=== UI Tests ===
UI will be tested manually, but also automatedly with [https://en.wikipedia.org/wiki/Selenium_(software) Selenium]. Compliance and validation will be checked via an online tool.

== Implementation ==


Target files: app/models/response.rb, app/views/grades/views.html.erb, app/views/grades/_participant.html.erb, app/views/grades/_reviews.html.erb and app/views/popup/team_users_popup.html.erb


Key tables: questionnaires, questions, responses, response_maps, answers.



The UML diagram:
[[File:FinalProjUml2.png|border|center|alt=uml.|UML]]


'''Task 1''': Compact the review display and colorize the background.


The code for displaying review is implemented in a action call 'display_as_html' in app/models/response.rb. The action basically links html code then return it as a string.
To eliminate the blank line between items, some 'BR' tags in the display_as_html need to be delete.
To use different color in the background for different review, we plan to insert id for reviews in _reviews.html.erb, then write css for colorization in the app/assets/stylesheets/ folder.

As show in the mock-up. Different cells with different scores will be colored differently. To keep the code straightforward, coloring will be done in a modular way: 1) get a list of scores, 2) remove repeats, 3) put scores in buckets, 4) associate each bucket with a color.
 

'''Task 2''': Show basic information about each review for both instructor and student.


To extract the data that need to be displayed, the models involve and their relationships to each other need to be analyzed. For students, _reviews.html.erb should be edited. The code to display those data should be insert after 'Review #' and before 'display_as_html' as following:

<pre>
<% if controller.action_name != "view_my_scores" %>
<a name="<%=prefix+"_"+review.map.reviewer.name%>"></a><%= review.display_as_html(prefix) %>
<% else %>

<h2>Review <%= count %></h2>
<!--insert basic information--!> 
<%= review.display_as_html(nil,0, nil) %> 
</pre>
 For instructors, team_users_popup.html.erb should be edited. The information such as reviewers's id, name, review's round number and updated time can be added after the 'else' division:

<pre>
<%if @scores == nil%>

<tr> <td align = "center">No review done yet.</td> </tr>
 
<tr>
<th align = "left"> Reviewer score </th>
</tr>
<tr>
<td align = "center">--</td>
</tr>
 
<%else%>
</pre>
In last column of the mock-up, the computation for the "# of comments with > 10 characters" field will be done in a modular manner as well. This could avoid complete re-tooling when this function need to be replaced. .

'''Task 3 and 4''': Use tab to change reviews according to round, version and questions.


Using the 'round' attribute in the 'response' table to differentiate reviews in distinct rounds, for students the code inside following for-loop located in the _reviews.html.erb wil be rewritten and tab UI in JQuery will be used.

<pre>
<% for review in rscore[:assessments] %>
.
.
.
<% end %>
</pre>
For instructors, 1) in the team_users_popup.html.erb file, the code inside 'else' block should be rewritten to adapt tab UI.
<pre>
<%if @scores == nil%>
.
.
.
<%else%>
.
.
.
<%end%>
</pre>

2) The app/views/grades/_participant.html.erb file will be rewrite to adapt to the new static page.
We should note that there will be no changes or impact on the database or underlying model classes.

==Future Enhancements==
* The project team may attempt to implement sorting of the lists and tables in in both the instructor and student review pages.
* Open Question: how to meet functionality of the various action links in the instructor view, such as: email student, delete score, change score, etc.

==References==
<references/>
https://expertiza.ncsu.edu/
http://www.nsf.gov/
https://en.wikipedia.org/wiki/Code_refactoring
https://en.wikipedia.org/wiki/Flyweight_pattern
https://en.wikipedia.org/wiki/Strategy_pattern
https://en.wikipedia.org/wiki/Front_Controller_pattern
https://en.wikipedia.org/wiki/Module_pattern
https://en.wikipedia.org/wiki/Iterator_pattern
https://en.wikipedia.org/wiki/Selenium_(software)

CSC/ECE 517 Fall 2015 E1577 MayYellowRoverJump

2015-11-14T04:20:31Z

Jchen45:

== Introduction ==
[https://expertiza.ncsu.edu/ Expertiza] is an open-source education and classroom web-tool founded by the [http://www.nsf.gov/ National Science Foundation]. Built with Ruby on Rails, it is designed to manage complete courses and students’ work within those courses. Each course can have a collection of instructors and students, though the interaction between instructors and students is minimal. The real emphasis of Expertiza is placed on peer-to-peer interactions, fostering a student-driven learning environment. Courses are comprised of assignments which users complete individually or with a team. Assignments usually encourage or require a team to enforce practicing peer-to-peer interaction.

One of the main tenets of Expertiza is its implicit peer-review system. Assignments inherently have a review stage where, rather than having instructors review a team’s work, other students review a team’s submission for that assignment. When completing a review, a student is presented with essentially a rubric for the assignment, and they fill in each category with the score they deem commensurate with the work of the team. Of course, each category has a comments box for the student to qualify the score they doled out. Each member of the submitting team is notified of the review, and the team can then decide as a whole how to rework their submission based on the feedback in the peer reviews.

There do exist issues, however, with respect to viewing one’s team’s reviews, particularly in the realm of usability. Our team has been tasked with revamping and enhancing the review UI to produce a more focused and uniform user experience for students and instructors alike.

== Assignment ==

=== Description ===

The tasks of the assignment are as follows:

# Compact the review display. Eliminate the blank lines between items within a single review. Instead vary the background color from line to line to improve readability.
# Add the following to the top of each review: who submitted the review. The instructor should see the user’s name and user-ID. A student should see “Reviewer #k”, where k is an integer between 1 and n, the number of reviews that have been submitted for this project. the version number of the review, and the time the review was submitted.
# Add functionality to allow the instructor to view different review rounds. Also, provide instructor with a review report page of all reviewers’ reviews to every project.
# Allow different alternate view: group by question.
# Reduce the duplicate code between instructor and student grade reports.

=== Purpose ===
==== Motivations ====
* Lack of uniformity between student and instructor views
* No defined separation between reviews
* All reviews and review data (comments, question text, etc.) are displayed at once

==== Discussion ====
There is no denying that the usability of viewing peer reviews leaves much to be desired. It lacks uniformity across the student and instructor roles, and the view itself has no semblance of order or organization. Viewing a single student’s review is a chore for both instructors and students, as there is no clear separation between reviews. In addition, all reviews are displayed at once, meaning viewing a single review requires scrolling through the page until the desired review is found. Our goal is to take the same data in the current display and present it in a more focused manner that allows a user, in either the instructor or student role, to absorb and process the content in the peer review more efficiently. Accessing, viewing, and understanding a review should be a far more simple task than what it currently is. In addition to the overhaul of the presentation layer, we also strive to drastically increase code reuse in the controller and model layers of the review module, which will in turn create a more uniform experience for both the instructor and student roles.

=== Scope ===
The scope of this task is limited to enhancing the usability of viewing peer reviews for both students and instructors. It is within our scope to modify the corresponding views for this functionality, as well as the underlying controllers and models as needed. The modifications to the Ruby classes will either be to accommodate changes to the view or to provide a uniform experience for both the instructor and student. As this is more of a user experience task, e.g, changing the way data is displayed to the user, there will be limited modifications to the base functionality of this module. It is not within our scope to change any element of the actual peer review process, such as selecting a topic to review or completing a review of a topic. As a result, we will not be modifying the results of peer reviews; the same peer review data will be present both before and after our task is completed.

== Design ==

=== Discussion of Resolution ===
The goal of this project is to optimize code and UI of review module, to make it more readable and user-friendly. To be more specific, our work focuses on the following specific areas:
* [https://en.wikipedia.org/wiki/Code_refactoring Refactoring] grades_controller.rb and review_mapping_controller.rb to optimize code organization, making code easier to read.
* Modifying UI to be more friendly. Instructor can see users' names and user-IDs, student should see review numbers, like “Reviewer #k” where k is the number of reviews submitted to the project/assignment. Besides, the round of reviews (version number) and submitted time of reviews could also be saw by both students and instructor.
* Modifying UI to make it easier for students/instructor to see reviews of different rounds. Tabs is a good choice, besides drop-down menus are a good alternate. Maybe we also need to modify models to make it adaptive to different rounds of reviews.(Currently, review models will only record the latest version of reviews.)
* Providing a new page to display all reviews of one project/assignment as review report. In this page, reviews will be displayed as a format like “Question 1, Reviews 1, Reviews 2 … Reviews n” (reviewer’s name should also be included here). Besides, here we also need to provide different version/round of reviews of different questions. At the top of this page, there should be a matrix to show the summary of questions(as a row) and reviews(as a column).(How can different version be displayed in the matrix? Using different matrix works?)
* Providing a way to hide or gray the questions, making students/instructor more focus on reviews.
* Providing a search reviews through a specific keyword. And when searching, providing a ‘next’ button to navigate to next keyword place.
* Providing a two-dimensional table to show the scores of each question and reviewer’s name who gives the score to the question.

=== Mock-Ups ===
[[File:Current Student.PNG|border|center|alt=The current student view.|The current review display for students.]]
'''Image 1. The current review display for students.'''

Image 1, above, illustrates the current design of the Student Review Report. We propose entirely removing the summary statistics at the top of the page. The data in these statistics can be displayed in a more efficient manner.

[[File:Heatgrid_studen_condensed.png|border|center|alt=The proposed condensed student display.|The proposed condensed student display.]]
'''Image 2. The proposed student review report, in default collapsed view.'''

The main UI overhaul will occur in displaying the actual reviews. Image 2 illustrates the design for the student review report. To combat the length of the current review display, we have chosen to collapse all data into a single table that can be viewed without the need to endlessly scroll through the page. Each row of the table corresponds to a specific criterion of the review and the scores each reviewer gave with whatever comments they may have offered. The columns dictate which review the score came from. By default, the full criterion text is truncated, while the comments from a review are hidden. Because all of the comments are not available for all reviews in a single screen display, the rightmost field displays the number of comment fields for each criterion which have 10 or more characters. The purpose of this field is to provide the user a quick and effective manner to spark interest into whether or not the the criterion contains meaningful comments.

The colored background of the cells are based on a color scale relative to the score. These colors are added to the design into order to quickly spark interest to the users, allowing them to pick out the essential information without having to iterate through all the data.

[[File:Heatgrid_student_expanded.png|border|center|alt=The proposed expanded student display.|The proposed expanded student display.]]
'''Image 3. The proposed student review report, in expanded view.'''

Clicking the row number will display all comments for that particular criterion, i.e., comments from each review. This way a student can compare comments and scores for a single question across all reviews. Clicking the column header for a specific review will display all the comments left by that reviewer. A student can use this functionality to view the entirety of a single review, including scores and comments.

It is important to note a important data organization difference between the existing design and the proposed design; The existing review report design groups reponses by the reviewer who created them, while the proposed review report design groups responses by the criteria. This new way of grouping responses groups like with like, allow the viewing users to see all scores and comments pertaining to one criterion at once.

[[File:Current Instructor.PNG|border|center|alt=The current instructor view.|The current review display for instructors.]]
'''Image 4. Current design of the Instructor's Review Report.'''

The instructor role will also receive interaction updates using the same underling display structures, but the content differ slightly from that of the student. Instead of reviews in the column headers, student names will take their place. We will be able to reuse the structure in the student's view for this purpose. Additionally, the Instructor view will have scores for all teams, and thus many tables of scores stacked vertically on the view. See Image 5, and notice that there is a partial table towards the bottom.

[[File:Heatgrid_instructor_condensed.png|border|center|alt=The proposed condensed instructor display.|The proposed condensed instructor display.]]
'''Image 5. Proposed design of the Instructor Review Report, in default collapsed view.'''

=== Design Patterns ===
The implementation team expects that the following design patterns be used in the solution:

* '''[https://en.wikipedia.org/wiki/Flyweight_pattern FlyWeight Design Pattern]''': using flyweight design pattern, we can use sharing data/gui to support large numbers of fine-grained objects efficiently. For instance, in expertiza system, students’ review pages and instructor review pages. We can use one page to display in both students’ ‘version’ of review page and instructor’s ‘version’ of review page.

* '''[https://en.wikipedia.org/wiki/Strategy_pattern Strategy Design Pattern]''': using strategy design pattern, we can display one page into different ‘version’ of page according to different role. For instance, in students’ review page, he will see reviewer’s id, and in instructor’s review page, he will see reviewer’s fullname. In here, student and instructor are different strategy.

* '''[https://en.wikipedia.org/wiki/Front_Controller_pattern Front Controller Design Pattern]''': with front controller design pattern, we will display different views based on the URL and currently user’s role.

* '''[https://en.wikipedia.org/wiki/Module_pattern Module Design Pattern]''': by module design pattern, source code can be organized into components that accomplish a particular function or contain everything necessary to accomplish a particular task. In JavaScript files, we use anonymous functions for responsiveness on client side via javascript and jquery.

* '''[https://en.wikipedia.org/wiki/Iterator_pattern Iterator Design Pattern]''': iterator design pattern is used to iterate through data structures in Ruby actions and when rendering the view. For instance, we need to display all reviewer’s reviews to one round, so we need the collection of all reviewer’s reviews and then iterator to display.

* '''[https://en.wikipedia.org/wiki/Facade_pattern Facade Design Pattern]''': facade design pattern provides a unified and higher-level interface to a set of interfaces in a subsystem, which makes the subsystem easier to use. For instance, in students’ review page, grades, commands and feedbacks are from three controller, and they are combined in review controller and display in one review page.

=== Use Cases ===

* '''View All Team Scores and Reviews for Specified Assignment, as Instructor''': Accessed from the Assignment list page. Allows instructor to see all peer reviews for a team for a specified assignment.
* '''View Received Scores and Reviews for Specified Assignment, Grouped by Criteria, as Student (self)''': Accessed from the Student's landing page for a particular assignment. Allows a student to see scores, grouped by criteria, for self's assignment, as well as meta reviews and author reviews.
* '''View Received Scores and Reviews for Specified Assignment, Grouped by Reviewer, as Student (self)''': Accessed from the Student's landing page for a particular assignment, Allows a student to see scores, grouped by submitted reviewer, for self's assignment.

=== Controller Testing ===

Controller testing will be centered around the proposed views which will replace the "view" and "view_my_scores" view in the grades controller. The Functional testing section covers much of the modular functionality that make up the proposed views. It is not anticipated that the testing team will perform significant controller tests.

=== Functional Tests ===

* ColorMapping module: The generation of the heatgrid requires mapping review scores to color groups. Testing of the method's functionality requires various fixture collections which vary is size, range, max, min, and number of duplicates.

* Role Security Testing: In order to DRY the code, and make it maximally orthogonal, the intention is to reuse code where possible. To re-use code across students and participants will require security and role checks to confirm the session user has access to view the requested information. To test this functionality sufficiently will require testing as admin, instructor with access, instructor without access, student, non-participant, participant, teammember, non-teammeber, etc.

* Char Comment Count Module: The generation of the "# of comments with chars > x" result will require the execution of a module. this module will be tested with various collections of comments which vary in size, range, length, and content.

* TeamReview(assignment,team): This is a controller helpful method, which will be called to generate the peer review scores for a particular team (note: no author nor meta reviews.) A collection of review scores and comments will be generated. This will be tested for empty, null, many round, single round, many reviewer, single reviewer scores.

* ParticpantReview(participant): This is a controller helper method, which will be called to generate the review scores for a particular participant. Collections generated will include reviews, author-reviews, meta-reviews for a participant. this will be tested for empty, null, many round, single round, many reviewer, single reviewer scores.

* RubricQuestion(questionnaire): This is a controller helper method, which will be called by many views in the solution. It will return a list of criteria for each found of a questionnaire or rubric. this will be tested for empty, null, many-round, single round rubrics.

=== UI Tests ===
UI will be tested manually, but also automatedly with [https://en.wikipedia.org/wiki/Selenium_(software) Selenium]. Compliance and validation will be checked via an online tool.

== Implementation ==


Target files: app/models/response.rb, app/views/grades/views.html.erb, app/views/grades/_participant.html.erb, app/views/grades/_reviews.html.erb and app/views/popup/team_users_popup.html.erb


Key tables: questionnaires, questions, responses, response_maps, answers.



The UML diagram:
[[File:FinalProjUml2.png|border|center|alt=uml.|UML]]


'''Task 1''': Compact the review display and colorize the background.


The code for displaying review is implemented in a action call 'display_as_html' in app/models/response.rb. The action basically links html code then return it as a string.
To eliminate the blank line between items, some 'BR' tags in the display_as_html need to be delete.
To use different color in the background for different review, we plan to insert id for reviews in _reviews.html.erb, then write css for colorization in the app/assets/stylesheets/ folder.

As show in the mock-up. Different cells with different scores will be colored differently. To keep the code straightforward, coloring will be done in a modular way: 1) get a list of scores, 2) remove repeats, 3) put scores in buckets, 4) associate each bucket with a color.
 

'''Task 2''': Show basic information about each review for both instructor and student.


To extract the data that need to be displayed, the models involve and their relationships to each other need to be analyzed. For students, _reviews.html.erb should be edited. The code to display those data should be insert after 'Review #' and before 'display_as_html' as following:

<pre>
<% if controller.action_name != "view_my_scores" %>
<a name="<%=prefix+"_"+review.map.reviewer.name%>"></a><%= review.display_as_html(prefix) %>
<% else %>

<h2>Review <%= count %></h2>
<!--insert basic information--!> 
<%= review.display_as_html(nil,0, nil) %> 
</pre>
 For instructors, team_users_popup.html.erb should be edited. The information such as reviewers's id, name, review's round number and updated time can be added after the 'else' division:

<pre>
<%if @scores == nil%>

<tr> <td align = "center">No review done yet.</td> </tr>
 
<tr>
<th align = "left"> Reviewer score </th>
</tr>
<tr>
<td align = "center">--</td>
</tr>
 
<%else%>
</pre>
In last column of the mock-up, the computation for the "# of comments with > 10 characters" field will be done in a modular manner as well. This could avoid complete re-tooling when this function need to be replaced. .

'''Task 3 and 4''': Use tab to change reviews according to round, version and questions.


Using the 'round' attribute in the 'response' table to differentiate reviews in distinct rounds, for students the code inside following for-loop located in the _reviews.html.erb wil be rewritten and tab UI in JQuery will be used.

<pre>
<% for review in rscore[:assessments] %>
.
.
.
<% end %>
</pre>
For instructors, 1) in the team_users_popup.html.erb file, the code inside 'else' block should be rewritten to adapt tab UI.
<pre>
<%if @scores == nil%>
.
.
.
<%else%>
.
.
.
<%end%>
</pre>

2) The app/views/grades/_participant.html.erb file will be rewrite to adapt to the new static page.
We should note that there will be no changes or impact on the database or underlying model classes.

==Future Enhancements==
* The project team may attempt to implement sorting of the lists and tables in in both the instructor and student review pages.
* Open Question: how to meet functionality of the various action links in the instructor view, such as: email student, delete score, change score, etc.

==References==
<references/>
{{reflist}}
https://expertiza.ncsu.edu/
http://www.nsf.gov/
https://en.wikipedia.org/wiki/Code_refactoring
https://en.wikipedia.org/wiki/Flyweight_pattern
https://en.wikipedia.org/wiki/Strategy_pattern
https://en.wikipedia.org/wiki/Front_Controller_pattern
https://en.wikipedia.org/wiki/Module_pattern
https://en.wikipedia.org/wiki/Iterator_pattern
https://en.wikipedia.org/wiki/Selenium_(software)

CSC/ECE 517 Fall 2015 E1577 MayYellowRoverJump

2015-11-14T04:15:02Z

Jchen45:

== Introduction ==
[https://expertiza.ncsu.edu/ Expertiza] is an open-source education and classroom web-tool founded by the [http://www.nsf.gov/ National Science Foundation]. Built with Ruby on Rails, it is designed to manage complete courses and students’ work within those courses. Each course can have a collection of instructors and students, though the interaction between instructors and students is minimal. The real emphasis of Expertiza is placed on peer-to-peer interactions, fostering a student-driven learning environment. Courses are comprised of assignments which users complete individually or with a team. Assignments usually encourage or require a team to enforce practicing peer-to-peer interaction.

One of the main tenets of Expertiza is its implicit peer-review system. Assignments inherently have a review stage where, rather than having instructors review a team’s work, other students review a team’s submission for that assignment. When completing a review, a student is presented with essentially a rubric for the assignment, and they fill in each category with the score they deem commensurate with the work of the team. Of course, each category has a comments box for the student to qualify the score they doled out. Each member of the submitting team is notified of the review, and the team can then decide as a whole how to rework their submission based on the feedback in the peer reviews.

There do exist issues, however, with respect to viewing one’s team’s reviews, particularly in the realm of usability. Our team has been tasked with revamping and enhancing the review UI to produce a more focused and uniform user experience for students and instructors alike.

== Assignment ==

=== Description ===

The tasks of the assignment are as follows:

# Compact the review display. Eliminate the blank lines between items within a single review. Instead vary the background color from line to line to improve readability.
# Add the following to the top of each review: who submitted the review. The instructor should see the user’s name and user-ID. A student should see “Reviewer #k”, where k is an integer between 1 and n, the number of reviews that have been submitted for this project. the version number of the review, and the time the review was submitted.
# Add functionality to allow the instructor to view different review rounds. Also, provide instructor with a review report page of all reviewers’ reviews to every project.
# Allow different alternate view: group by question.
# Reduce the duplicate code between instructor and student grade reports.

=== Purpose ===
==== Motivations ====
* Lack of uniformity between student and instructor views
* No defined separation between reviews
* All reviews and review data (comments, question text, etc.) are displayed at once

==== Discussion ====
There is no denying that the usability of viewing peer reviews leaves much to be desired. It lacks uniformity across the student and instructor roles, and the view itself has no semblance of order or organization. Viewing a single student’s review is a chore for both instructors and students, as there is no clear separation between reviews. In addition, all reviews are displayed at once, meaning viewing a single review requires scrolling through the page until the desired review is found. Our goal is to take the same data in the current display and present it in a more focused manner that allows a user, in either the instructor or student role, to absorb and process the content in the peer review more efficiently. Accessing, viewing, and understanding a review should be a far more simple task than what it currently is. In addition to the overhaul of the presentation layer, we also strive to drastically increase code reuse in the controller and model layers of the review module, which will in turn create a more uniform experience for both the instructor and student roles.

=== Scope ===
The scope of this task is limited to enhancing the usability of viewing peer reviews for both students and instructors. It is within our scope to modify the corresponding views for this functionality, as well as the underlying controllers and models as needed. The modifications to the Ruby classes will either be to accommodate changes to the view or to provide a uniform experience for both the instructor and student. As this is more of a user experience task, e.g, changing the way data is displayed to the user, there will be limited modifications to the base functionality of this module. It is not within our scope to change any element of the actual peer review process, such as selecting a topic to review or completing a review of a topic. As a result, we will not be modifying the results of peer reviews; the same peer review data will be present both before and after our task is completed.

== Design ==

=== Discussion of Resolution ===
The goal of this project is to optimize code and UI of review module, to make it more readable and user-friendly. To be more specific, our work focuses on the following specific areas:
* [https://en.wikipedia.org/wiki/Code_refactoring Refactoring] grades_controller.rb and review_mapping_controller.rb to optimize code organization, making code easier to read.
* Modifying UI to be more friendly. Instructor can see users' names and user-IDs, student should see review numbers, like “Reviewer #k” where k is the number of reviews submitted to the project/assignment. Besides, the round of reviews (version number) and submitted time of reviews could also be saw by both students and instructor.
* Modifying UI to make it easier for students/instructor to see reviews of different rounds. Tabs is a good choice, besides drop-down menus are a good alternate. Maybe we also need to modify models to make it adaptive to different rounds of reviews.(Currently, review models will only record the latest version of reviews.)
* Providing a new page to display all reviews of one project/assignment as review report. In this page, reviews will be displayed as a format like “Question 1, Reviews 1, Reviews 2 … Reviews n” (reviewer’s name should also be included here). Besides, here we also need to provide different version/round of reviews of different questions. At the top of this page, there should be a matrix to show the summary of questions(as a row) and reviews(as a column).(How can different version be displayed in the matrix? Using different matrix works?)
* Providing a way to hide or gray the questions, making students/instructor more focus on reviews.
* Providing a search reviews through a specific keyword. And when searching, providing a ‘next’ button to navigate to next keyword place.
* Providing a two-dimensional table to show the scores of each question and reviewer’s name who gives the score to the question.

=== Mock-Ups ===
[[File:Current Student.PNG|border|center|alt=The current student view.|The current review display for students.]]
'''Image 1. The current review display for students.'''

Image 1, above, illustrates the current design of the Student Review Report. We propose entirely removing the summary statistics at the top of the page. The data in these statistics can be displayed in a more efficient manner.

[[File:Heatgrid_studen_condensed.png|border|center|alt=The proposed condensed student display.|The proposed condensed student display.]]
'''Image 2. The proposed student review report, in default collapsed view.'''

The main UI overhaul will occur in displaying the actual reviews. Image 2 illustrates the design for the student review report. To combat the length of the current review display, we have chosen to collapse all data into a single table that can be viewed without the need to endlessly scroll through the page. Each row of the table corresponds to a specific criterion of the review and the scores each reviewer gave with whatever comments they may have offered. The columns dictate which review the score came from. By default, the full criterion text is truncated, while the comments from a review are hidden. Because all of the comments are not available for all reviews in a single screen display, the rightmost field displays the number of comment fields for each criterion which have 10 or more characters. The purpose of this field is to provide the user a quick and effective manner to spark interest into whether or not the the criterion contains meaningful comments.

The colored background of the cells are based on a color scale relative to the score. These colors are added to the design into order to quickly spark interest to the users, allowing them to pick out the essential information without having to iterate through all the data.

[[File:Heatgrid_student_expanded.png|border|center|alt=The proposed expanded student display.|The proposed expanded student display.]]
'''Image 3. The proposed student review report, in expanded view.'''

Clicking the row number will display all comments for that particular criterion, i.e., comments from each review. This way a student can compare comments and scores for a single question across all reviews. Clicking the column header for a specific review will display all the comments left by that reviewer. A student can use this functionality to view the entirety of a single review, including scores and comments.

It is important to note a important data organization difference between the existing design and the proposed design; The existing review report design groups reponses by the reviewer who created them, while the proposed review report design groups responses by the criteria. This new way of grouping responses groups like with like, allow the viewing users to see all scores and comments pertaining to one criterion at once.

[[File:Current Instructor.PNG|border|center|alt=The current instructor view.|The current review display for instructors.]]
'''Image 4. Current design of the Instructor's Review Report.'''

The instructor role will also receive interaction updates using the same underling display structures, but the content differ slightly from that of the student. Instead of reviews in the column headers, student names will take their place. We will be able to reuse the structure in the student's view for this purpose. Additionally, the Instructor view will have scores for all teams, and thus many tables of scores stacked vertically on the view. See Image 5, and notice that there is a partial table towards the bottom.

[[File:Heatgrid_instructor_condensed.png|border|center|alt=The proposed condensed instructor display.|The proposed condensed instructor display.]]
'''Image 5. Proposed design of the Instructor Review Report, in default collapsed view.'''

=== Design Patterns ===
The implementation team expects that the following design patterns be used in the solution:

* '''[https://en.wikipedia.org/wiki/Flyweight_pattern FlyWeight Design Pattern]''': using flyweight design pattern, we can use sharing data/gui to support large numbers of fine-grained objects efficiently. For instance, in expertiza system, students’ review pages and instructor review pages. We can use one page to display in both students’ ‘version’ of review page and instructor’s ‘version’ of review page.

* '''Strategy Design Pattern''': using strategy design pattern, we can display one page into different ‘version’ of page according to different role. For instance, in students’ review page, he will see reviewer’s id, and in instructor’s review page, he will see reviewer’s fullname. In here, student and instructor are different strategy.

* '''Front Controller Design Pattern''': with front controller design pattern, we will display different views based on the URL and currently user’s role.

* '''Module Design Pattern''': by module design pattern, source code can be organized into components that accomplish a particular function or contain everything necessary to accomplish a particular task. In JavaScript files, we use anonymous functions for responsiveness on client side via javascript and jquery.

* '''Iterator Design Pattern''': iterator design pattern is used to iterate through data structures in Ruby actions and when rendering the view. For instance, we need to display all reviewer’s reviews to one round, so we need the collection of all reviewer’s reviews and then iterator to display.

* '''Facade Design Pattern''': facade design pattern provides a unified and higher-level interface to a set of interfaces in a subsystem, which makes the subsystem easier to use. For instance, in students’ review page, grades, commands and feedbacks are from three controller, and they are combined in review controller and display in one review page.

=== Use Cases ===

* '''View All Team Scores and Reviews for Specified Assignment, as Instructor''': Accessed from the Assignment list page. Allows instructor to see all peer reviews for a team for a specified assignment.
* '''View Received Scores and Reviews for Specified Assignment, Grouped by Criteria, as Student (self)''': Accessed from the Student's landing page for a particular assignment. Allows a student to see scores, grouped by criteria, for self's assignment, as well as meta reviews and author reviews.
* '''View Received Scores and Reviews for Specified Assignment, Grouped by Reviewer, as Student (self)''': Accessed from the Student's landing page for a particular assignment, Allows a student to see scores, grouped by submitted reviewer, for self's assignment.

=== Controller Testing ===

Controller testing will be centered around the proposed views which will replace the "view" and "view_my_scores" view in the grades controller. The Functional testing section covers much of the modular functionality that make up the proposed views. It is not anticipated that the testing team will perform significant controller tests.

=== Functional Tests ===

* ColorMapping module: The generation of the heatgrid requires mapping review scores to color groups. Testing of the method's functionality requires various fixture collections which vary is size, range, max, min, and number of duplicates.

* Role Security Testing: In order to DRY the code, and make it maximally orthogonal, the intention is to reuse code where possible. To re-use code across students and participants will require security and role checks to confirm the session user has access to view the requested information. To test this functionality sufficiently will require testing as admin, instructor with access, instructor without access, student, non-participant, participant, teammember, non-teammeber, etc.

* Char Comment Count Module: The generation of the "# of comments with chars > x" result will require the execution of a module. this module will be tested with various collections of comments which vary in size, range, length, and content.

* TeamReview(assignment,team): This is a controller helpful method, which will be called to generate the peer review scores for a particular team (note: no author nor meta reviews.) A collection of review scores and comments will be generated. This will be tested for empty, null, many round, single round, many reviewer, single reviewer scores.

* ParticpantReview(participant): This is a controller helper method, which will be called to generate the review scores for a particular participant. Collections generated will include reviews, author-reviews, meta-reviews for a participant. this will be tested for empty, null, many round, single round, many reviewer, single reviewer scores.

* RubricQuestion(questionnaire): This is a controller helper method, which will be called by many views in the solution. It will return a list of criteria for each found of a questionnaire or rubric. this will be tested for empty, null, many-round, single round rubrics.

=== UI Tests ===
UI will be tested manually, but also automatedly with Selenium<ref>https://en.wikipedia.org/wiki/Selenium_(software)</ref>. Compliance and validation will be checked via an online tool.

== Implementation ==


Target files: app/models/response.rb, app/views/grades/views.html.erb, app/views/grades/_participant.html.erb, app/views/grades/_reviews.html.erb and app/views/popup/team_users_popup.html.erb


Key tables: questionnaires, questions, responses, response_maps, answers.



The UML diagram:
[[File:FinalProjUml2.png|border|center|alt=uml.|UML]]


'''Task 1''': Compact the review display and colorize the background.


The code for displaying review is implemented in a action call 'display_as_html' in app/models/response.rb. The action basically links html code then return it as a string.
To eliminate the blank line between items, some 'BR' tags in the display_as_html need to be delete.
To use different color in the background for different review, we plan to insert id for reviews in _reviews.html.erb, then write css for colorization in the app/assets/stylesheets/ folder.

As show in the mock-up. Different cells with different scores will be colored differently. To keep the code straightforward, coloring will be done in a modular way: 1) get a list of scores, 2) remove repeats, 3) put scores in buckets, 4) associate each bucket with a color.
 

'''Task 2''': Show basic information about each review for both instructor and student.


To extract the data that need to be displayed, the models involve and their relationships to each other need to be analyzed. For students, _reviews.html.erb should be edited. The code to display those data should be insert after 'Review #' and before 'display_as_html' as following:

<pre>
<% if controller.action_name != "view_my_scores" %>
<a name="<%=prefix+"_"+review.map.reviewer.name%>"></a><%= review.display_as_html(prefix) %>
<% else %>

<h2>Review <%= count %></h2>
<!--insert basic information--!> 
<%= review.display_as_html(nil,0, nil) %> 
</pre>
 For instructors, team_users_popup.html.erb should be edited. The information such as reviewers's id, name, review's round number and updated time can be added after the 'else' division:

<pre>
<%if @scores == nil%>

<tr> <td align = "center">No review done yet.</td> </tr>
 
<tr>
<th align = "left"> Reviewer score </th>
</tr>
<tr>
<td align = "center">--</td>
</tr>
 
<%else%>
</pre>
In last column of the mock-up, the computation for the "# of comments with > 10 characters" field will be done in a modular manner as well. This could avoid complete re-tooling when this function need to be replaced. .

'''Task 3 and 4''': Use tab to change reviews according to round, version and questions.


Using the 'round' attribute in the 'response' table to differentiate reviews in distinct rounds, for students the code inside following for-loop located in the _reviews.html.erb wil be rewritten and tab UI in JQuery will be used.

<pre>
<% for review in rscore[:assessments] %>
.
.
.
<% end %>
</pre>
For instructors, 1) in the team_users_popup.html.erb file, the code inside 'else' block should be rewritten to adapt tab UI.
<pre>
<%if @scores == nil%>
.
.
.
<%else%>
.
.
.
<%end%>
</pre>

2) The app/views/grades/_participant.html.erb file will be rewrite to adapt to the new static page.
We should note that there will be no changes or impact on the database or underlying model classes.

==Future Enhancements==
* The project team may attempt to implement sorting of the lists and tables in in both the instructor and student review pages.
* Open Question: how to meet functionality of the various action links in the instructor view, such as: email student, delete score, change score, etc.

==References==
<references/>
{{reflist}}
https://expertiza.ncsu.edu/
http://www.nsf.gov/
https://en.wikipedia.org/wiki/Code_refactoring
https://en.wikipedia.org/wiki/Flyweight_pattern

CSC/ECE 517 Fall 2015 E1577 MayYellowRoverJump

2015-11-14T04:10:31Z

Jchen45:

== Introduction ==
[https://expertiza.ncsu.edu/ Expertiza] is an open-source education and classroom web-tool founded by the National Science Foundation. Built with Ruby on Rails, it is designed to manage complete courses and students’ work within those courses. Each course can have a collection of instructors and students, though the interaction between instructors and students is minimal. The real emphasis of Expertiza is placed on peer-to-peer interactions, fostering a student-driven learning environment. Courses are comprised of assignments which users complete individually or with a team. Assignments usually encourage or require a team to enforce practicing peer-to-peer interaction.

One of the main tenets of Expertiza is its implicit peer-review system. Assignments inherently have a review stage where, rather than having instructors review a team’s work, other students review a team’s submission for that assignment. When completing a review, a student is presented with essentially a rubric for the assignment, and they fill in each category with the score they deem commensurate with the work of the team. Of course, each category has a comments box for the student to qualify the score they doled out. Each member of the submitting team is notified of the review, and the team can then decide as a whole how to rework their submission based on the feedback in the peer reviews.

There do exist issues, however, with respect to viewing one’s team’s reviews, particularly in the realm of usability. Our team has been tasked with revamping and enhancing the review UI to produce a more focused and uniform user experience for students and instructors alike.

== Assignment ==

=== Description ===

The tasks of the assignment are as follows:

# Compact the review display. Eliminate the blank lines between items within a single review. Instead vary the background color from line to line to improve readability.
# Add the following to the top of each review: who submitted the review. The instructor should see the user’s name and user-ID. A student should see “Reviewer #k”, where k is an integer between 1 and n, the number of reviews that have been submitted for this project. the version number of the review, and the time the review was submitted.
# Add functionality to allow the instructor to view different review rounds. Also, provide instructor with a review report page of all reviewers’ reviews to every project.
# Allow different alternate view: group by question.
# Reduce the duplicate code between instructor and student grade reports.

=== Purpose ===
==== Motivations ====
* Lack of uniformity between student and instructor views
* No defined separation between reviews
* All reviews and review data (comments, question text, etc.) are displayed at once

==== Discussion ====
There is no denying that the usability of viewing peer reviews leaves much to be desired. It lacks uniformity across the student and instructor roles, and the view itself has no semblance of order or organization. Viewing a single student’s review is a chore for both instructors and students, as there is no clear separation between reviews. In addition, all reviews are displayed at once, meaning viewing a single review requires scrolling through the page until the desired review is found. Our goal is to take the same data in the current display and present it in a more focused manner that allows a user, in either the instructor or student role, to absorb and process the content in the peer review more efficiently. Accessing, viewing, and understanding a review should be a far more simple task than what it currently is. In addition to the overhaul of the presentation layer, we also strive to drastically increase code reuse in the controller and model layers of the review module, which will in turn create a more uniform experience for both the instructor and student roles.

=== Scope ===
The scope of this task is limited to enhancing the usability of viewing peer reviews for both students and instructors. It is within our scope to modify the corresponding views for this functionality, as well as the underlying controllers and models as needed. The modifications to the Ruby classes will either be to accommodate changes to the view or to provide a uniform experience for both the instructor and student. As this is more of a user experience task, e.g, changing the way data is displayed to the user, there will be limited modifications to the base functionality of this module. It is not within our scope to change any element of the actual peer review process, such as selecting a topic to review or completing a review of a topic. As a result, we will not be modifying the results of peer reviews; the same peer review data will be present both before and after our task is completed.

== Design ==

=== Discussion of Resolution ===
The goal of this project is to optimize code and UI of review module, to make it more readable and user-friendly. To be more specific, our work focuses on the following specific areas:
* Refactoring grades_controller.rb and review_mapping_controller.rb to optimize code organization, making code easier to read.
* Modifying UI to be more friendly. Instructor can see users' names and user-IDs, student should see review numbers, like “Reviewer #k” where k is the number of reviews submitted to the project/assignment. Besides, the round of reviews (version number) and submitted time of reviews could also be saw by both students and instructor.
* Modifying UI to make it easier for students/instructor to see reviews of different rounds. Tabs is a good choice, besides drop-down menus are a good alternate. Maybe we also need to modify models to make it adaptive to different rounds of reviews.(Currently, review models will only record the latest version of reviews.)
* Providing a new page to display all reviews of one project/assignment as review report. In this page, reviews will be displayed as a format like “Question 1, Reviews 1, Reviews 2 … Reviews n” (reviewer’s name should also be included here). Besides, here we also need to provide different version/round of reviews of different questions. At the top of this page, there should be a matrix to show the summary of questions(as a row) and reviews(as a column).(How can different version be displayed in the matrix? Using different matrix works?)
* Providing a way to hide or gray the questions, making students/instructor more focus on reviews.
* Providing a search reviews through a specific keyword. And when searching, providing a ‘next’ button to navigate to next keyword place.
* Providing a two-dimensional table to show the scores of each question and reviewer’s name who gives the score to the question.

=== Mock-Ups ===
[[File:Current Student.PNG|border|center|alt=The current student view.|The current review display for students.]]
'''Image 1. The current review display for students.'''

Image 1, above, illustrates the current design of the Student Review Report. We propose entirely removing the summary statistics at the top of the page. The data in these statistics can be displayed in a more efficient manner.

[[File:Heatgrid_studen_condensed.png|border|center|alt=The proposed condensed student display.|The proposed condensed student display.]]
'''Image 2. The proposed student review report, in default collapsed view.'''

The main UI overhaul will occur in displaying the actual reviews. Image 2 illustrates the design for the student review report. To combat the length of the current review display, we have chosen to collapse all data into a single table that can be viewed without the need to endlessly scroll through the page. Each row of the table corresponds to a specific criterion of the review and the scores each reviewer gave with whatever comments they may have offered. The columns dictate which review the score came from. By default, the full criterion text is truncated, while the comments from a review are hidden. Because all of the comments are not available for all reviews in a single screen display, the rightmost field displays the number of comment fields for each criterion which have 10 or more characters. The purpose of this field is to provide the user a quick and effective manner to spark interest into whether or not the the criterion contains meaningful comments.

The colored background of the cells are based on a color scale relative to the score. These colors are added to the design into order to quickly spark interest to the users, allowing them to pick out the essential information without having to iterate through all the data.

[[File:Heatgrid_student_expanded.png|border|center|alt=The proposed expanded student display.|The proposed expanded student display.]]
'''Image 3. The proposed student review report, in expanded view.'''

Clicking the row number will display all comments for that particular criterion, i.e., comments from each review. This way a student can compare comments and scores for a single question across all reviews. Clicking the column header for a specific review will display all the comments left by that reviewer. A student can use this functionality to view the entirety of a single review, including scores and comments.

It is important to note a important data organization difference between the existing design and the proposed design; The existing review report design groups reponses by the reviewer who created them, while the proposed review report design groups responses by the criteria. This new way of grouping responses groups like with like, allow the viewing users to see all scores and comments pertaining to one criterion at once.

[[File:Current Instructor.PNG|border|center|alt=The current instructor view.|The current review display for instructors.]]
'''Image 4. Current design of the Instructor's Review Report.'''

The instructor role will also receive interaction updates using the same underling display structures, but the content differ slightly from that of the student. Instead of reviews in the column headers, student names will take their place. We will be able to reuse the structure in the student's view for this purpose. Additionally, the Instructor view will have scores for all teams, and thus many tables of scores stacked vertically on the view. See Image 5, and notice that there is a partial table towards the bottom.

[[File:Heatgrid_instructor_condensed.png|border|center|alt=The proposed condensed instructor display.|The proposed condensed instructor display.]]
'''Image 5. Proposed design of the Instructor Review Report, in default collapsed view.'''

=== Design Patterns ===
The implementation team expects that the following design patterns be used in the solution:

* '''FlyWeight Design Pattern''': using flyweight design pattern, we can use sharing data/gui to support large numbers of fine-grained objects efficiently. For instance, in expertiza system, students’ review pages and instructor review pages. We can use one page to display in both students’ ‘version’ of review page and instructor’s ‘version’ of review page.

* '''Strategy Design Pattern''': using strategy design pattern, we can display one page into different ‘version’ of page according to different role. For instance, in students’ review page, he will see reviewer’s id, and in instructor’s review page, he will see reviewer’s fullname. In here, student and instructor are different strategy.

* '''Front Controller Design Pattern''': with front controller design pattern, we will display different views based on the URL and currently user’s role.

* '''Module Design Pattern''': by module design pattern, source code can be organized into components that accomplish a particular function or contain everything necessary to accomplish a particular task. In JavaScript files, we use anonymous functions for responsiveness on client side via javascript and jquery.

* '''Iterator Design Pattern''': iterator design pattern is used to iterate through data structures in Ruby actions and when rendering the view. For instance, we need to display all reviewer’s reviews to one round, so we need the collection of all reviewer’s reviews and then iterator to display.

* '''Facade Design Pattern''': facade design pattern provides a unified and higher-level interface to a set of interfaces in a subsystem, which makes the subsystem easier to use. For instance, in students’ review page, grades, commands and feedbacks are from three controller, and they are combined in review controller and display in one review page.

=== Use Cases ===

* '''View All Team Scores and Reviews for Specified Assignment, as Instructor''': Accessed from the Assignment list page. Allows instructor to see all peer reviews for a team for a specified assignment.
* '''View Received Scores and Reviews for Specified Assignment, Grouped by Criteria, as Student (self)''': Accessed from the Student's landing page for a particular assignment. Allows a student to see scores, grouped by criteria, for self's assignment, as well as meta reviews and author reviews.
* '''View Received Scores and Reviews for Specified Assignment, Grouped by Reviewer, as Student (self)''': Accessed from the Student's landing page for a particular assignment, Allows a student to see scores, grouped by submitted reviewer, for self's assignment.

=== Controller Testing ===

Controller testing will be centered around the proposed views which will replace the "view" and "view_my_scores" view in the grades controller. The Functional testing section covers much of the modular functionality that make up the proposed views. It is not anticipated that the testing team will perform significant controller tests.

=== Functional Tests ===

* ColorMapping module: The generation of the heatgrid requires mapping review scores to color groups. Testing of the method's functionality requires various fixture collections which vary is size, range, max, min, and number of duplicates.

* Role Security Testing: In order to DRY the code, and make it maximally orthogonal, the intention is to reuse code where possible. To re-use code across students and participants will require security and role checks to confirm the session user has access to view the requested information. To test this functionality sufficiently will require testing as admin, instructor with access, instructor without access, student, non-participant, participant, teammember, non-teammeber, etc.

* Char Comment Count Module: The generation of the "# of comments with chars > x" result will require the execution of a module. this module will be tested with various collections of comments which vary in size, range, length, and content.

* TeamReview(assignment,team): This is a controller helpful method, which will be called to generate the peer review scores for a particular team (note: no author nor meta reviews.) A collection of review scores and comments will be generated. This will be tested for empty, null, many round, single round, many reviewer, single reviewer scores.

* ParticpantReview(participant): This is a controller helper method, which will be called to generate the review scores for a particular participant. Collections generated will include reviews, author-reviews, meta-reviews for a participant. this will be tested for empty, null, many round, single round, many reviewer, single reviewer scores.

* RubricQuestion(questionnaire): This is a controller helper method, which will be called by many views in the solution. It will return a list of criteria for each found of a questionnaire or rubric. this will be tested for empty, null, many-round, single round rubrics.

=== UI Tests ===
UI will be tested manually, but also automatedly with Selenium<ref>https://en.wikipedia.org/wiki/Selenium_(software)</ref>. Compliance and validation will be checked via an online tool.

== Implementation ==


Target files: app/models/response.rb, app/views/grades/views.html.erb, app/views/grades/_participant.html.erb, app/views/grades/_reviews.html.erb and app/views/popup/team_users_popup.html.erb


Key tables: questionnaires, questions, responses, response_maps, answers.



The UML diagram:
[[File:FinalProjUml2.png|border|center|alt=uml.|UML]]


'''Task 1''': Compact the review display and colorize the background.


The code for displaying review is implemented in a action call 'display_as_html' in app/models/response.rb. The action basically links html code then return it as a string.
To eliminate the blank line between items, some 'BR' tags in the display_as_html need to be delete.
To use different color in the background for different review, we plan to insert id for reviews in _reviews.html.erb, then write css for colorization in the app/assets/stylesheets/ folder.

As show in the mock-up. Different cells with different scores will be colored differently. To keep the code straightforward, coloring will be done in a modular way: 1) get a list of scores, 2) remove repeats, 3) put scores in buckets, 4) associate each bucket with a color.
 

'''Task 2''': Show basic information about each review for both instructor and student.


To extract the data that need to be displayed, the models involve and their relationships to each other need to be analyzed. For students, _reviews.html.erb should be edited. The code to display those data should be insert after 'Review #' and before 'display_as_html' as following:

<pre>
<% if controller.action_name != "view_my_scores" %>
<a name="<%=prefix+"_"+review.map.reviewer.name%>"></a><%= review.display_as_html(prefix) %>
<% else %>

<h2>Review <%= count %></h2>
<!--insert basic information--!> 
<%= review.display_as_html(nil,0, nil) %> 
</pre>
 For instructors, team_users_popup.html.erb should be edited. The information such as reviewers's id, name, review's round number and updated time can be added after the 'else' division:

<pre>
<%if @scores == nil%>

<tr> <td align = "center">No review done yet.</td> </tr>
 
<tr>
<th align = "left"> Reviewer score </th>
</tr>
<tr>
<td align = "center">--</td>
</tr>
 
<%else%>
</pre>
In last column of the mock-up, the computation for the "# of comments with > 10 characters" field will be done in a modular manner as well. This could avoid complete re-tooling when this function need to be replaced. .

'''Task 3 and 4''': Use tab to change reviews according to round, version and questions.


Using the 'round' attribute in the 'response' table to differentiate reviews in distinct rounds, for students the code inside following for-loop located in the _reviews.html.erb wil be rewritten and tab UI in JQuery will be used.

<pre>
<% for review in rscore[:assessments] %>
.
.
.
<% end %>
</pre>
For instructors, 1) in the team_users_popup.html.erb file, the code inside 'else' block should be rewritten to adapt tab UI.
<pre>
<%if @scores == nil%>
.
.
.
<%else%>
.
.
.
<%end%>
</pre>

2) The app/views/grades/_participant.html.erb file will be rewrite to adapt to the new static page.
We should note that there will be no changes or impact on the database or underlying model classes.

==Future Enhancements==
* The project team may attempt to implement sorting of the lists and tables in in both the instructor and student review pages.
* Open Question: how to meet functionality of the various action links in the instructor view, such as: email student, delete score, change score, etc.

==References==
<references/>
{{reflist}}

CSC/ECE 517 Fall 2015/oss E1556 CHM

2015-11-09T21:13:59Z

Jchen45:

E1556. Refactoring SuggestionController.rb
== Project description==
This project is about testing and optimizing of SuggestionController of Expertiza system. Suggestion Controller is a module for students to suggest a new topic for their writing assignments, and instructor can approve the suggestion.

Typically, there are three cases when instructor approves the suggestion. First, if the student already has a topic and when suggesting a new topic, he chooses 'Yes' in the signup_preference, he will enroll the new suggested topic automatically after the instructor approves the suggested topic. Second, if the student is in the waitlist of a topic, and when suggesting a new topic, he chooses 'Yes' in the signup_preference, he will enroll the new suggested topic and be removed from the former waitlist. Third, if the student is in the waitlist of a topic, and when suggesting a new topic, he chooses 'No' in the signup_preference, after the instructor approves the new topic, he will remain in the waitlist of former topic, and new topic is left as 'no chooser'.

Besides, Suggestion Controller also needs to be optimized from two aspects. First, the syntax need to be upgraded from rails 3.x to rails 4.x. Second, refactoring the mailer part is necessary.
== Expertiza==
The Expertiza project is software to create reusable learning objects through peer review. It also supports team projects, and the submission of almost any document type, including URLs and wiki pages.

== Optimization==
=== Syntax ===
For the code to be coordinated with Rails 4 syntax, there is one major difference between Ruby 1.9 and 1.8 need to be change in the suggestion_controller.rb.
The hash operator using the "hash rocket":
<pre>
{ :key => 'value' }
</pre>
Need to be changed into:
<pre>
{ key : 'value' }
</pre>

=== Refactor ===
The approve_suggestion() method is quite long in the original code, and the send email function in the method is achieved twice, which doesn’t conform with DRY principle.

The logic of the approve_suggestion() method is described as follows. First, approve the suggestion by create a new record in the SignUpTopic model, set relative parameters, and save the new record. Then send notification to the team. In the notification part, if the student doesn’t have a team, a new team should be created and assigned to the suggested topic.
Based on the logic, the approve_suggestion() method can be clearly divided into two parts: approve suggestion part, and notification part. In the notification part, send email function can be written as a single method in order not to repeat. Besides, creating a new team can also be written as a new method.

After refactor, there are four new methods: approve, notification, create_new_team, and send_email. Approve and notification are called within approve_suggestion, while create_new_team and send_email are called within notification.
<pre>
def create_new_team
new_team = AssignmentTeam.create(name: 'Team' + @user_id.to_s + '_' + rand(1000).to_s, parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
t_user = TeamsUser.create(team_id: new_team.id, user_id: @user_id)
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: new_team.id, is_waitlisted: 0)
parent = TeamNode.create(parent_id: @signuptopic.assignment_id, node_object_id: new_team.id)
TeamUserNode.create(parent_id: parent.id, node_object_id: t_user.id)
end

def send_email
proposer = User.find(@user_id)
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = Array.new
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
{ to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has already been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
}
).deliver
end

def approve
@suggestion = Suggestion.find(params[:id])
@user_id = User.where(name: @suggestion.unityID).first.id
@team_id = TeamsUser.team_id(@suggestion.assignment_id, @user_id)
@topic_id = SignedUpTeam.topic_id(@suggestion.assignment_id, @user_id)
@signuptopic = SignUpTopic.new
@signuptopic.topic_identifier = 'S' + Suggestion.where("assignment_id = ? and id <= ?", @suggestion.assignment_id, @suggestion.id).size.to_s
@signuptopic.topic_name = @suggestion.title
@signuptopic.assignment_id = @suggestion.assignment_id
@signuptopic.max_choosers = 1;
if @signuptopic.save && @suggestion.update_attribute('status', 'Approved')
flash[:notice] = 'Successfully approved the suggestion.'
else
flash[:error] = 'Error when approving the suggestion.'
end
end

def notification
if @suggestion.signup_preference == 'Y'
#if this user do not have team in this assignment, create one for him/her and assign this topic to this team.
if @team_id.nil?
create_new_team
else #this user has a team in this assignment, check whether this team has topic or not
if @topic_id.nil?
#clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end

def approve_suggestion
approve
notification
redirect_to action: 'show', id: @suggestion
end
</pre>

== Test ==
=== Rspec===
[[Rspec]] was used to conduct all the tests.
RSpec is a Behaviour-Driven Development tool for Ruby programmers. BDD is an approach to software development that combines Test-Driven Development, Domain Driven Design, and Acceptance Test-Driven Planning. RSpec helps you do the TDD part of that equation, focusing on the documentation and design aspects of TDD.

=== UI test ===
The following steps can be used for testing suggestion controller in the UI side.

For instructor, after log in, please click 'assignment' and click the pencil shape button to edit the assignment:

[[File:6.png]]

The open the suggestions function by check the second checkbox under 'topic' tab:


[[File:5.png]]


Click save button at the end of the page:


[[File:7.png]]


After a student suggestion as topic (see description below), click the archive box shape to view the suggestion:


[[File:8.png]]


You should be able to see the list of suggested topics.


[[File:9.png]]




For student users, first you need to login your account, find a certain course, and make suggestion


[[File:1.png]]


Then, based on your need, choose if you want to work on the suggestion you suggested.


[[File:2.png]]


After saving, there will be a flash message on your webpage, and the suggested topic would be shown.


[[File:3.png]]


=== Test1 ===
In the first test, we are going to test the result of approving a student's suggestion topic if the student is in a waitlist. He will be removed from the waitlist and added to the new list, if he selected the signup_preference to be 'Yes'.
We choose 'Writing Assignment 1a' of 'CSC/ECE 517, Spring 2015' as test assignment. I simulate creating a new suggestion with student5717, in team 'Writing Assignment 1a Team14'.

First, log in as Student5717 and suggest a new topic, and choose 'Yes' in signup_preference.
<pre>
# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# suggest a new suggestion
visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'RSpect'
fill_in 'suggestion_description', with: 'RSpect is a ROR test framework. It focus on function test'
# select 'suggestion_signup_preference', with: 'Y'
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)
</pre>
Then, log out Student5717, and log in with 'instructor6' account, who is the manager of this course. Then approve the suggest.
<pre>
# Logout current account student5717
visit '/suggestion/new?id=711'
click_link "Logout"
expect(current_path).to eq("/")

# Login with account instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

# approve the suggestion
expect(page).to have_content('Manage content')
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
expect(page).to have_content('RSpect')

num = Suggestion.last.id
visit "/suggestion/"+num.to_s
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: RSpect')
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')
</pre>
Finally, check if suggestion approved successfully. I need to check topic list with 'instructor6' account logged in and check the selected topic in student5717 account.
<pre>
# check if is not in waitlist
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team14 student5717 (waitlisted)")

# Logout current account instructor6
visit '/suggestion/new?id=711'
click_link "Logout"
current_path.should == "/"

# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# Check if you select the topic successfully
visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): RSpect')
</pre>
=== Test2 ===
For the second test, Writing Assignment 1a team1, whose team id is 23781, was chosen to perform a serial of action. Team no.23781 is holding a topic: Amazon S3 and Rails. And Writing Assignment 1a team5, whose team id is 23800, is in the waiting list of this topic.
First, sign in as Student 5404 from team1, send a suggestion for new topic and indicate they want to choose their suggested topic.
<pre>
@newtopic = 'Violet and Zoe'

#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#suggest a topic:
# signup_preference default to be Y
visit "/student_task/view?id=28634"
expect(page).to have_content('Submit or Review work')
visit "/suggestion/new?id=711"
expect(page).to have_content('New suggestion')
fill_in 'Title',with: @newtopic
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)

#logout
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')
</pre>

Then sign in as instructor6 and approve the suggested topic.
<pre>
#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

#approve the suggestion
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
num = Suggestion.last.id.to_s
visit "/suggestion/"+num
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: '+@newtopic)
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')

#logout as instructor6
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
</pre>
Finally, to check the results. On the one hand, sign in as student5404 again and see if her/his team is holding the new topic. On the other hand sign in as instructor6 and check if team no.23800 is holding the old topic: Amazon S3 and Rails.
<pre>
#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#check the approved suggestion in topics list
visit "/sign_up_sheet/list?assignment_id=711"
expect(page).to have_content("Your approved suggested topic")

# switch to the new topic
num2 = SignUpTopic.last.id.to_s
visit "/sign_up_sheet/switch_original_topic_to_approved_suggested_topic/"+num2+"?assignment_id=711"
expect(page).to have_content("Your topic(s): "+@newtopic)

#logout student5404
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')

#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

# check if team1 is has not enrolled
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team1 student5404 student5731 ")
expect(page).to have_content("Writing assignment 1a_Team5 student5740 student5704")
expect(page).to have_content("S1 Violet and Zoe Writing assignment 1a_Team1 student5404 student5731")
</pre>
=== Test3 ===
The third test is similar to the second one. Team no.23781 and assignment no.711 are chosen for this test again.
First, student no.5404 login to the system, visit the assignment page, and make a topic suggestion. In the suggestion, instead of choosing “yes” in signup preference, the student chooses “no” in order not to use the suggested topic.

<pre>
visit 'content_pages/view'
fill_in "User Name", with: "student5404"
fill_in "Password", with: "password"
click_button "SIGN IN"

visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'test title'
fill_in 'suggestion_description', with: 'test description'
select 'No', from: "suggestion_signup_preference"
click_button "Submit"

click_link "Logout"
</pre>
After the suggestion is made, login as an instructor, find the assignment, and approve the suggestion.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

num = Suggestion.last.id
path = "/suggestion/" + num.to_s
visit path
click_button "Approve suggestion"

click_link "Logout"
</pre>
In the final step, we check if the new topic is shown in the topic list, then login as student no.5401 again, check if they still hold their old topic.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): Amazon S3 and Rails')
click_link "Logout"

visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit "/assignments/711/edit#tabs-2"
expect(page).to have_content("Amazon S3 and Rails")
expect(page).to have_content("test title")
</pre>
=== Result ===
Here is the result of our tests. All test cases passed.
[[File:suggest_controller_test_result.png]]

CSC/ECE 517 Fall 2015/oss E1556 CHM

2015-11-09T21:13:35Z

Jchen45:

File:Suggest controller test result.png

2015-11-09T21:11:55Z

Jchen45:

CSC/ECE 517 Fall 2015 E1577 MayYellowRoverJump

2015-11-08T18:38:25Z

Jchen45: Add description, resolution and design patterns

== Intro ==
Expertiza is an open-source education and classroom web-tool founded by the National Science Foundation. Built with Ruby on Rails, it is designed to manage complete courses and students’ work within those courses. Each course can have a collection of instructors and students, though the interaction between instructors and students is minimal. The real emphasis of Expertiza is placed on peer-to-peer interactions, fostering a student-driven learning environment. Courses are comprised of assignments which users complete individually or with a team. Assignments usually encourage or require a team to enforce practicing peer-to-peer interaction.

One of the main tenets of Expertiza is its implicit peer-review system. Assignments inherently have a review stage where, rather than having instructors review a team’s work, other students review a team’s submission for that assignment. When completing a review, a student is presented with essentially a rubric for the assignment, and they fill in each category with the score they deem commensurate with the work of the team. Of course, each category has a comments box for the student to qualify the score they doled out. Each member of the submitting team is notified of the review, and the team can then decide as a whole how to rework their submission based on the feedback in the peer reviews.

There do exist issues, however, with respect to viewing one’s team’s reviews, particularly in the realm of usability. Our team has been tasked with revamping and enhancing the review UI to produce a more focused and uniform user experience for students and instructors alike.

== Assignment ==

=== Description ===
Currently, in expertiza system, the review pages of students and instructor are different pages, which violates Don’t Repeat Yourself(DRY) principle and makes page UI non-orthogonal. The instructor should be able to see all reviewers’ work. Besides, instructor should be able to access the review report of all reviews, which displays all reviewer’s review to every project. The goal of our project is to (1)develop a uniform way to display both students’ review page and instructor’s review page, (2)provide instructor with a review report page of all reviewers’ reviews to every project.

=== Purpose ===
There is no denying that the usability of viewing peer reviews leaves much to be desired. It lacks uniformity across varying roles (student and instructor), and the view itself has no semblance of order or organization. Viewing a single student’s review is a chore for both instructor and student, as there is no clear separation between reviews. Our goal is to take the same data in the current display and present it in a more focused manner that allows a user, in either the instructor or student role, to absorb and process the content in the peer review more efficiently. Accessing, viewing, and understanding a review should be a far more simple task than what it currently is. In addition to the overhaul of the presentation layer, we also strive to drastically increase code reuse in the controller and model layers of the review module, which will in turn create a more uniform experience for both the instructor and student roles.

=== Scope ===
The scope of this task is limited to enhancing the usability of viewing peer reviews for both students and instructors. It is within our scope to modify the corresponding views for this functionality, as well as the underlying controllers and models as needed. The modifications to the Ruby classes will either be to accommodate changes to the view or to provide a uniform experience for both the instructor and student. As this is more of a user experience task, e.g, changing the way data is displayed to the user, there will be limited modifications to the base functionality of this module. It is not within our scope to change any element of the actual peer review process, such as selecting a topic to review or completing a review of a topic. As a result, we will not be modifying the results of peer reviews; the same peer review data will be present both before and after our task is completed.

== Design ==

=== Discussion of Resolution ===
The goal of this project is to optimize code and UI of review module, to make it more readable and user-friendly. To be more specific, our work focus on the following specific areas:
1. Refactoring grades_controller.rb and review_mapping_controller.rb, to optimize code organize, making code easier to read.
2. Modifying UI to be more friendly. Instructor could see user’s name and user-ID, student should see reviews number, like “Reviewer #k” where k is the number of reviews submitted to the project/assignment. Besides, the round of reviews (version number) and submitted time of reviews could also be saw by both students and instructor.
3. Modifying UI to make it easier for students/instructor to see reviews of different rounds. Tabs is a good choice, besides drop-down menus are a good alternate. Maybe we also need to modify models to make it adaptive to different rounds of reviews.(Currently, review models will only record the latest version of reviews.)
4. Providing a new page to display all reviews of one project/assignment as review report. In this page, reviews will be displayed as a format like “Question 1, Reviews 1, Reviews 2 … Reviews n” (reviewer’s name should also be included here). Besides, here we also need to provide different version/round of reviews of different questions. At the top of this page, there should be a matrix to show the summary of questions(as a row) and reviews(as a column).(How can different version be displayed in the matrix? Using different matrix works?)
5. Providing a way to hide or gray the questions, making students/instructor more focus on reviews.
6. Providing a search reviews through a specific keyword. And when searching, providing a ‘next’ button to navigate to next keyword place.
7. Providing a two-dimensional table to show the scores of each question and reviewer’s name who gives the score to the question.

=== Mock-Ups ===
We’ll outline our proposed changes starting from the top of the review page and working our way down.

[[File:Mock Up 1.png|border]]

We propose minimal, if any, changes to the stats portion at the top of the view. This section already looks refined and provides a nice summary of the reviews submitted for a particular assignment. The only changes that might be incurred would be to the links to “hide reviews” and “show author feedbacks.” It may not make sense for these actions to remain once the presentation of the review content changes.

The first instance of major change will occur in the table under the stats section. The table will become interactive, dictating which individual review to show at any given point. To increase readability of the table headers, they will either be spaced out more along the top or rotated to display at an angle. The "Average" in "Average score" will be removed, as it is not indicative of the value held in the table. The percentages will contain links to display that particular review in a table below. Clicking that link will automatically scroll the page down to take the user to the displayed review.

[[File:Mock Up 2.png|border]]

The main UI overhaul will occur in displaying the actual reviews. We intend to pull all of that content and place it in a tabbed table, similar to an Excel spreadsheet. Rather than have the confusing options to show or hide a review from a different round, the table will have multiple tabs, one for each version of the review, which will be labeled with the the particular review phase they belong to. Clicking these tabs, the user can toggle between the review they would like to look at. Each review question will have it's own row in the table, with the following columns: Question Number, Question, Score, Reviewer Comments. These columns will be interactive, i.e., clicking a column header will sort the table based on that column. Under each question number, there will be an option to "View Aggregate Scores," which will compile a table consisting of the selected question from every review. In this manner, a user can swiftly compare the scores they received on a single question from every review.

Note that the specific interaction schemes outlined thus far, including the screenshots, have applied to the student role. The instructor role will also receive interaction updates using the same underling display structures, but the content differ slightly from that of the student. Instead of a 2 x N table like the student role has holding the scores from each review, the instructor will have a M x N table, where M is the number of students in the course and N is the number of assignments for that course. The column headers will become the assignment names, and the rows will have the names of students. The i, j entry will contain the score that Student i gave for Assignment j. This value will be interactive, controlling which review is displayed below the table. The columns will be sortable just as they are in the student view. Clicking a score will display that particular review below the master table in a tabbed tabular view, just as it is for the student's view. We will be able to reuse the structure in the student's view for this purpose.

=== Design Patterns ===
* '''FlyWeight Design Pattern''': using flyweight design pattern, we can use sharing data/gui to support large numbers of fine-grained objects efficiently. For instance, in expertiza system, students’ review pages and instructor review pages. We can use one page to display in both students’ ‘version’ of review page and instructor’s ‘version’ of review page.

* '''Strategy Design Pattern''': using strategy design pattern, we can display one page into different ‘version’ of page according to different role. For instance, in students’ review page, he will see reviewer’s id, and in instructor’s review page, he will see reviewer’s fullname. In here, student and instructor are different strategy.

* '''Front Controller Design Pattern''': with front controller design pattern, we will display different views based on the URL and currently user’s role.

* '''Module Design Pattern''': by module design pattern, source code can be organized into components that accomplish a particular function or contain everything necessary to accomplish a particular task. In JavaScript files, we use anonymous functions for responsiveness on client side via javascript and jquery.

* '''Iterator Design Pattern''': iterator design pattern is used to iterate through data structures in Ruby actions and when rendering the view. For instance, we need to display all reviewer’s reviews to one round, so we need the collection of all reviewer’s reviews and then iterator to display.

* '''Facade Design Pattern''': facade design pattern provides a unified and higher-level interface to a set of interfaces in a subsystem, which makes the subsystem easier to use. For instance, in students’ review page, grades, commands and feedbacks are from three controller, and they are combined in review controller and display in one review page.

=== Use Cases ===

* '''View Review Page as Student''' : General use case, capturing a student viewing his or her own reviews with corresponding user account.
* '''View Review Page as Instructor''' : General use case, an instructor associated with the course in which to review's assignment is included, viewing reviews for a student participating in that assignment.
* '''Expand All Reviews''' : Reviews may be initially condensed in the UI by default. This use case covers the ability for a viewing user to expand expand all reviews in the UI at once, with one click of the mouse.
* '''Condense each Review''' : The ability to expand and condense each review of the scores page.
* '''View Review Detail: (As Student)''' : The ability to see review detail (questions, scores answers), of each review, as student.
* '''View Review Detail: (As Instructor)''' : The ability to see review detail (questions, scores answers), of each review, as instructor.
* '''View Reviews, with Versions in a comparable manner''' : The ability to see different versions or rounds of each review, either side by side, or or tab from one to the other.
* '''View Reviews, Grouped by Review''' :
* '''View Reviews, Grouped by Question''' :

=== Proposed Tests ===
* UI Testing:
* Controller Testing: view person with no reviews, view person with reviews but no scores nor comments, view reviews with scores --,0,1,2,3,4,5. view reviews with comments with length 10,20,30,40,50,60,70,80,90,100,110,120,130,140,150,160,170 characters.

== Implementation ==


Target files: app/models/response.rb, app/views/grades/_reviews.html.erb and app/views/popup/team_users_popup.html.erb


Key tables: questionnaires, questions, responses, response_maps, answers.


'''Task 1''': Compact the review display and colorize the background.


The code for displaying review is implemented in a action call 'display_as_html' in app/models/response.rb. The action basically links html code then return it as a string.
To eliminate the blank line between items, some 'BR' tags in the display_as_html need to be delete.
To use different color in the background for different review, we plan to insert id for reviews in _reviews.html.erb, then write css for colorization in the app/assets/stylesheets/ folder.

 

'''Task 2''': Show basic information about each review for both instructor and student.


To extract the data that need to be displayed, the models involve and their relationships to each other need to be analyzed. For students, _reviews.html.erb should be edited. The code to display those data should be insert after 'Review #' and before 'display_as_html' as following:

<pre>
<% if controller.action_name != "view_my_scores" %>
<a name="<%=prefix+"_"+review.map.reviewer.name%>"></a><%= review.display_as_html(prefix) %>
<% else %>

<h2>Review <%= count %></h2>
<!--insert basic information--!> 
<%= review.display_as_html(nil,0, nil) %> 
</pre>
 For instructors, team_users_popup.html.erb should be edited. The information such as reviewers's id, name, review's round number and updated time can be added after the 'else' division:

<pre>
<%if @scores == nil%>

<tr> <td align = "center">No review done yet.</td> </tr>
 
<tr>
<th align = "left"> Reviewer score </th>
</tr>
<tr>
<td align = "center">--</td>
</tr>
 
<%else%>
</pre>


'''Task 3''': Use tab to change reviews according to round number.



Tab UI in JQuery can be used.


== Suggestions for Future Improvements ==

-make the instructor and student view for reviewing scores and grades orthogonal (e.g. use the same code.)

==References==
{{reflist}}

CSC/ECE 517 Fall 2015/oss E1556 CHM

2015-10-31T01:10:44Z

Jchen45:

E1556. Refactoring SuggestionController.rb
== Project description==
This project is about testing and optimizing of SuggestionController of Expertiza system. Suggestion Controller is a module for students to suggest a new topic for their writing assignments, and instructor can approve the suggestion.

Typically, there are three cases when instructor approves the suggestion. First, if the student already has a topic and when suggesting a new topic, he chooses 'Yes' in the signup_preference, he will enroll the new suggested topic automatically after the instructor approves the suggested topic. Second, if the student is in the waitlist of a topic, and when suggesting a new topic, he chooses 'Yes' in the signup_preference, he will enroll the new suggested topic and be removed from the former waitlist. Third, if the student is in the waitlist of a topic, and when suggesting a new topic, he chooses 'No' in the signup_preference, after the instructor approves the new topic, he will remain in the waitlist of former topic, and new topic is left as 'no chooser'.

Besides, Suggestion Controller also needs to be optimized from two aspects. First, the syntax need to be upgraded from rails 3.x to rails 4.x. Second, refactoring the mailer part is necessary.
== Expertiza==
== Optimization==
=== Syntax ===
For the code to be coordinated with Rails 4 syntax, there is one major difference between Ruby 1.9 and 1.8 need to be change in the suggestion_controller.rb.
The hash operator using the "hash rocket":
<pre>
{ :key => 'value' }
</pre>
Need to be changed into:
<pre>
{ key : 'value' }
</pre>

=== Refator ===
The approve_suggestion() method is quite long in the original code, and the send email function in the method is achieved twice, which doesn’t conform with DRY principle.

The logic of the approve_suggestion() method is described as follows. First, approve the suggestion by create a new record in the SignUpTopic model, set relative parameters, and save the new record. Then send notification to the team. In the notification part, if the student doesn’t have a team, a new team should be created and assigned to the suggested topic.
Based on the logic, the approve_suggestion() method can be clearly divided into two parts: approve suggestion part, and notification part. In the notification part, send email function can be written as a single method in order not to repeat. Besides, creating a new team can also be written as a new method.

After refactor, there are four new methods: approve, notification, create_new_team, and send_email. Approve and notification are called within approve_suggestion, while create_new_team and send_email are called within notification.
<pre>
def create_new_team
new_team = AssignmentTeam.create(name: 'Team' + @user_id.to_s + '_' + rand(1000).to_s, parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
t_user = TeamsUser.create(team_id: new_team.id, user_id: @user_id)
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: new_team.id, is_waitlisted: 0)
parent = TeamNode.create(parent_id: @signuptopic.assignment_id, node_object_id: new_team.id)
TeamUserNode.create(parent_id: parent.id, node_object_id: t_user.id)
end

def send_email
proposer = User.find(@user_id)
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = Array.new
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
{ to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has already been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
}
).deliver
end

def approve
@suggestion = Suggestion.find(params[:id])
@user_id = User.where(name: @suggestion.unityID).first.id
@team_id = TeamsUser.team_id(@suggestion.assignment_id, @user_id)
@topic_id = SignedUpTeam.topic_id(@suggestion.assignment_id, @user_id)
@signuptopic = SignUpTopic.new
@signuptopic.topic_identifier = 'S' + Suggestion.where("assignment_id = ? and id <= ?", @suggestion.assignment_id, @suggestion.id).size.to_s
@signuptopic.topic_name = @suggestion.title
@signuptopic.assignment_id = @suggestion.assignment_id
@signuptopic.max_choosers = 1;
if @signuptopic.save && @suggestion.update_attribute('status', 'Approved')
flash[:notice] = 'Successfully approved the suggestion.'
else
flash[:error] = 'Error when approving the suggestion.'
end
end

def notification
if @suggestion.signup_preference == 'Y'
#if this user do not have team in this assignment, create one for him/her and assign this topic to this team.
if @team_id.nil?
create_new_team
else #this user has a team in this assignment, check whether this team has topic or not
if @topic_id.nil?
#clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end

def approve_suggestion
approve
notification
redirect_to action: 'show', id: @suggestion
end
</pre>

== Test ==
=== Rspec===
[[Rspec]] was used to conduct all the tests.
RSpec is a Behaviour-Driven Development tool for Ruby programmers. BDD is an approach to software development that combines Test-Driven Development, Domain Driven Design, and Acceptance Test-Driven Planning. RSpec helps you do the TDD part of that equation, focusing on the documentation and design aspects of TDD.

=== Test1 ===
In the first test, we are going to test the result of approving a student's suggestion topic if the student is in a waitlist. He will be removed from the waitlist and added to the new list, if he selected the signup_preference to be 'Yes'.
We choose 'Writing Assignment 1a' of 'CSC/ECE 517, Spring 2015' as test assignment. I simulate creating a new suggestion with student5717, in team 'Writing Assignment 1a Team14'.

First, log in as Student5717 and suggest a new topic, and choose 'Yes' in signup_preference.
<pre>
# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# suggest a new suggestion
visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'RSpect'
fill_in 'suggestion_description', with: 'RSpect is a ROR test framework. It focus on function test'
# select 'suggestion_signup_preference', with: 'Y'
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)
</pre>
Then, log out Student5717, and log in with 'instructor6' account, who is the manager of this course. Then approve the suggest.
<pre>
# Logout current account student5717
visit '/suggestion/new?id=711'
click_link "Logout"
expect(current_path).to eq("/")

# Login with account instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

# approve the suggestion
expect(page).to have_content('Manage content')
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
expect(page).to have_content('RSpect')

num = Suggestion.last.id
visit "/suggestion/"+num.to_s
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: RSpect')
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')
</pre>
Finally, check if suggestion approved successfully. I need to check topic list with 'instructor6' account logged in and check the selected topic in student5717 account.
<pre>
# check if is not in waitlist
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team14 student5717 (waitlisted)")

# Logout current account instructor6
visit '/suggestion/new?id=711'
click_link "Logout"
current_path.should == "/"

# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# Check if you select the topic successfully
visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): RSpect')
</pre>
=== Test2 ===
For the second test, Writing Assignment 1a team1, whose team id is 23781, was chosen to perform a serial of action. Team no.23781 is holding a topic: Amazon S3 and Rails. And Writing Assignment 1a team5, whose team id is 23800, is in the waiting list of this topic.
First, sign in as Student 5404 from team1, send a suggestion for new topic and indicate they want to choose their suggested topic.
<pre>
@newtopic = 'Violet and Zoe'

#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#suggest a topic:
# signup_preference default to be Y
visit "/student_task/view?id=28634"
expect(page).to have_content('Submit or Review work')
visit "/suggestion/new?id=711"
expect(page).to have_content('New suggestion')
fill_in 'Title',with: @newtopic
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)

#logout
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')
</pre>

Then sign in as instructor6 and approve the suggested topic.
<pre>
#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

#approve the suggestion
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
num = Suggestion.last.id.to_s
visit "/suggestion/"+num
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: '+@newtopic)
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')

#logout as instructor6
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
</pre>
Finally, to check the results. On the one hand, sign in as student5404 again and see if her/his team is holding the new topic. On the other hand sign in as instructor6 and check if team no.23800 is holding the old topic: Amazon S3 and Rails.
<pre>
#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#check the approved suggestion in topics list
visit "/sign_up_sheet/list?assignment_id=711"
expect(page).to have_content("Your approved suggested topic")

# switch to the new topic
num2 = SignUpTopic.last.id.to_s
visit "/sign_up_sheet/switch_original_topic_to_approved_suggested_topic/"+num2+"?assignment_id=711"
expect(page).to have_content("Your topic(s): "+@newtopic)

#logout student5404
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')

#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

# check if team1 is has not enrolled
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team1 student5404 student5731 ")
expect(page).to have_content("Writing assignment 1a_Team5 student5740 student5704")
expect(page).to have_content("S1 Violet and Zoe Writing assignment 1a_Team1 student5404 student5731")
</pre>
=== Test3 ===
The third test is similar to the second one. Team no.23781 and assignment no.711 are chosen for this test again.
First, student no.5404 login to the system, visit the assignment page, and make a topic suggestion. In the suggestion, instead of choosing “yes” in signup preference, the student chooses “no” in order not to use the suggested topic.

<pre>
visit 'content_pages/view'
fill_in "User Name", with: "student5404"
fill_in "Password", with: "password"
click_button "SIGN IN"

visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'test title'
fill_in 'suggestion_description', with: 'test description'
select 'No', from: "suggestion_signup_preference"
click_button "Submit"

click_link "Logout"
</pre>
After the suggestion is made, login as an instructor, find the assignment, and approve the suggestion.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

num = Suggestion.last.id
path = "/suggestion/" + num.to_s
visit path
click_button "Approve suggestion"

click_link "Logout"
</pre>
In the final step, we check if the new topic is shown in the topic list, then login as student no.5401 again, check if they still hold their old topic.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): Amazon S3 and Rails')
click_link "Logout"

visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit "/assignments/711/edit#tabs-2"
expect(page).to have_content("Amazon S3 and Rails")
expect(page).to have_content("test title")
</pre>
=== Result ===
Here is the result of our tests. All test cases passed.
<pre>
ChendeMacBook-Pro:expertiza chen$ rspec spec/controllers/suggestion_controller_spec.rb
[Coveralls] Set up the SimpleCov formatter.
[Coveralls] Using SimpleCov's 'rails' settings.

Randomized with seed 8804
/Users/chen/RubymineProjects/expertiza/app/models/suggestion.rb:5: warning: circular argument reference - assignment_id
......

Deprecation Warnings:

Using `should` from rspec-expectations' old `:should` syntax without explicitly enabling the syntax is deprecated. Use the new `:expect` syntax or explicitly enable `:should` with `config.expect_with(:rspec) { |c| c.syntax = :should }` instead. Called from /Users/chen/RubymineProjects/expertiza/spec/controllers/suggestion_controller_spec.rb:67:in `block (3 levels) in <top (required)>'.

If you need more of the backtrace for any of these deprecations to
identify where to make the necessary changes, you can configure
`config.raise_errors_for_deprecations!`, and it will turn the
deprecation warnings into errors, giving you the full backtrace.

1 deprecation warning total

Finished in 1 minute 6.74 seconds (files took 5.3 seconds to load)
6 examples, 0 failures

Randomized with seed 8804

Coverage report generated for RSpec to /Users/chen/RubymineProjects/expertiza/coverage. 1638 / 5233 LOC (31.3%) covered.
[Coveralls] Outside the CI environment, not sending data.
</pre>

CSC/ECE 517 Fall 2015/oss E1556 CHM

2015-10-31T01:10:20Z

Jchen45:

E1556. Refactoring SuggestionController.rb
== Project description==
This project is about testing and optimizing of SuggestionController of Expertiza system. Suggestion Controller is a module for students to suggest a new topic for their writing assignments, and instructor can approve the suggestion. Typically, there are three cases when instructor approves the suggestion. First, if the student already has a topic and when suggesting a new topic, he chooses 'Yes' in the signup_preference, he will enroll the new suggested topic automatically after the instructor approves the suggested topic. Second, if the student is in the waitlist of a topic, and when suggesting a new topic, he chooses 'Yes' in the signup_preference, he will enroll the new suggested topic and be removed from the former waitlist. Third, if the student is in the waitlist of a topic, and when suggesting a new topic, he chooses 'No' in the signup_preference, after the instructor approves the new topic, he will remain in the waitlist of former topic, and new topic is left as 'no chooser'.

Besides, Suggestion Controller also needs to be optimized from two aspects. First, the syntax need to be upgraded from rails 3.x to rails 4.x. Second, refactoring the mailer part is necessary.
== Expertiza==
== Optimization==
=== Syntax ===
For the code to be coordinated with Rails 4 syntax, there is one major difference between Ruby 1.9 and 1.8 need to be change in the suggestion_controller.rb.
The hash operator using the "hash rocket":
<pre>
{ :key => 'value' }
</pre>
Need to be changed into:
<pre>
{ key : 'value' }
</pre>

=== Refator ===
The approve_suggestion() method is quite long in the original code, and the send email function in the method is achieved twice, which doesn’t conform with DRY principle.

The logic of the approve_suggestion() method is described as follows. First, approve the suggestion by create a new record in the SignUpTopic model, set relative parameters, and save the new record. Then send notification to the team. In the notification part, if the student doesn’t have a team, a new team should be created and assigned to the suggested topic.
Based on the logic, the approve_suggestion() method can be clearly divided into two parts: approve suggestion part, and notification part. In the notification part, send email function can be written as a single method in order not to repeat. Besides, creating a new team can also be written as a new method.

After refactor, there are four new methods: approve, notification, create_new_team, and send_email. Approve and notification are called within approve_suggestion, while create_new_team and send_email are called within notification.
<pre>
def create_new_team
new_team = AssignmentTeam.create(name: 'Team' + @user_id.to_s + '_' + rand(1000).to_s, parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
t_user = TeamsUser.create(team_id: new_team.id, user_id: @user_id)
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: new_team.id, is_waitlisted: 0)
parent = TeamNode.create(parent_id: @signuptopic.assignment_id, node_object_id: new_team.id)
TeamUserNode.create(parent_id: parent.id, node_object_id: t_user.id)
end

def send_email
proposer = User.find(@user_id)
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = Array.new
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
{ to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has already been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
}
).deliver
end

def approve
@suggestion = Suggestion.find(params[:id])
@user_id = User.where(name: @suggestion.unityID).first.id
@team_id = TeamsUser.team_id(@suggestion.assignment_id, @user_id)
@topic_id = SignedUpTeam.topic_id(@suggestion.assignment_id, @user_id)
@signuptopic = SignUpTopic.new
@signuptopic.topic_identifier = 'S' + Suggestion.where("assignment_id = ? and id <= ?", @suggestion.assignment_id, @suggestion.id).size.to_s
@signuptopic.topic_name = @suggestion.title
@signuptopic.assignment_id = @suggestion.assignment_id
@signuptopic.max_choosers = 1;
if @signuptopic.save && @suggestion.update_attribute('status', 'Approved')
flash[:notice] = 'Successfully approved the suggestion.'
else
flash[:error] = 'Error when approving the suggestion.'
end
end

def notification
if @suggestion.signup_preference == 'Y'
#if this user do not have team in this assignment, create one for him/her and assign this topic to this team.
if @team_id.nil?
create_new_team
else #this user has a team in this assignment, check whether this team has topic or not
if @topic_id.nil?
#clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end

def approve_suggestion
approve
notification
redirect_to action: 'show', id: @suggestion
end
</pre>

== Test ==
=== Rspec===
[[Rspec]] was used to conduct all the tests.
RSpec is a Behaviour-Driven Development tool for Ruby programmers. BDD is an approach to software development that combines Test-Driven Development, Domain Driven Design, and Acceptance Test-Driven Planning. RSpec helps you do the TDD part of that equation, focusing on the documentation and design aspects of TDD.

=== Test1 ===
In the first test, we are going to test the result of approving a student's suggestion topic if the student is in a waitlist. He will be removed from the waitlist and added to the new list, if he selected the signup_preference to be 'Yes'.
We choose 'Writing Assignment 1a' of 'CSC/ECE 517, Spring 2015' as test assignment. I simulate creating a new suggestion with student5717, in team 'Writing Assignment 1a Team14'.

First, log in as Student5717 and suggest a new topic, and choose 'Yes' in signup_preference.
<pre>
# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# suggest a new suggestion
visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'RSpect'
fill_in 'suggestion_description', with: 'RSpect is a ROR test framework. It focus on function test'
# select 'suggestion_signup_preference', with: 'Y'
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)
</pre>
Then, log out Student5717, and log in with 'instructor6' account, who is the manager of this course. Then approve the suggest.
<pre>
# Logout current account student5717
visit '/suggestion/new?id=711'
click_link "Logout"
expect(current_path).to eq("/")

# Login with account instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

# approve the suggestion
expect(page).to have_content('Manage content')
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
expect(page).to have_content('RSpect')

num = Suggestion.last.id
visit "/suggestion/"+num.to_s
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: RSpect')
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')
</pre>
Finally, check if suggestion approved successfully. I need to check topic list with 'instructor6' account logged in and check the selected topic in student5717 account.
<pre>
# check if is not in waitlist
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team14 student5717 (waitlisted)")

# Logout current account instructor6
visit '/suggestion/new?id=711'
click_link "Logout"
current_path.should == "/"

# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# Check if you select the topic successfully
visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): RSpect')
</pre>
=== Test2 ===
For the second test, Writing Assignment 1a team1, whose team id is 23781, was chosen to perform a serial of action. Team no.23781 is holding a topic: Amazon S3 and Rails. And Writing Assignment 1a team5, whose team id is 23800, is in the waiting list of this topic.
First, sign in as Student 5404 from team1, send a suggestion for new topic and indicate they want to choose their suggested topic.
<pre>
@newtopic = 'Violet and Zoe'

#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#suggest a topic:
# signup_preference default to be Y
visit "/student_task/view?id=28634"
expect(page).to have_content('Submit or Review work')
visit "/suggestion/new?id=711"
expect(page).to have_content('New suggestion')
fill_in 'Title',with: @newtopic
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)

#logout
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')
</pre>

Then sign in as instructor6 and approve the suggested topic.
<pre>
#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

#approve the suggestion
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
num = Suggestion.last.id.to_s
visit "/suggestion/"+num
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: '+@newtopic)
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')

#logout as instructor6
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
</pre>
Finally, to check the results. On the one hand, sign in as student5404 again and see if her/his team is holding the new topic. On the other hand sign in as instructor6 and check if team no.23800 is holding the old topic: Amazon S3 and Rails.
<pre>
#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#check the approved suggestion in topics list
visit "/sign_up_sheet/list?assignment_id=711"
expect(page).to have_content("Your approved suggested topic")

# switch to the new topic
num2 = SignUpTopic.last.id.to_s
visit "/sign_up_sheet/switch_original_topic_to_approved_suggested_topic/"+num2+"?assignment_id=711"
expect(page).to have_content("Your topic(s): "+@newtopic)

#logout student5404
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')

#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

# check if team1 is has not enrolled
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team1 student5404 student5731 ")
expect(page).to have_content("Writing assignment 1a_Team5 student5740 student5704")
expect(page).to have_content("S1 Violet and Zoe Writing assignment 1a_Team1 student5404 student5731")
</pre>
=== Test3 ===
The third test is similar to the second one. Team no.23781 and assignment no.711 are chosen for this test again.
First, student no.5404 login to the system, visit the assignment page, and make a topic suggestion. In the suggestion, instead of choosing “yes” in signup preference, the student chooses “no” in order not to use the suggested topic.

<pre>
visit 'content_pages/view'
fill_in "User Name", with: "student5404"
fill_in "Password", with: "password"
click_button "SIGN IN"

visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'test title'
fill_in 'suggestion_description', with: 'test description'
select 'No', from: "suggestion_signup_preference"
click_button "Submit"

click_link "Logout"
</pre>
After the suggestion is made, login as an instructor, find the assignment, and approve the suggestion.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

num = Suggestion.last.id
path = "/suggestion/" + num.to_s
visit path
click_button "Approve suggestion"

click_link "Logout"
</pre>
In the final step, we check if the new topic is shown in the topic list, then login as student no.5401 again, check if they still hold their old topic.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): Amazon S3 and Rails')
click_link "Logout"

visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit "/assignments/711/edit#tabs-2"
expect(page).to have_content("Amazon S3 and Rails")
expect(page).to have_content("test title")
</pre>
=== Result ===
Here is the result of our tests. All test cases passed.
<pre>
ChendeMacBook-Pro:expertiza chen$ rspec spec/controllers/suggestion_controller_spec.rb
[Coveralls] Set up the SimpleCov formatter.
[Coveralls] Using SimpleCov's 'rails' settings.

Randomized with seed 8804
/Users/chen/RubymineProjects/expertiza/app/models/suggestion.rb:5: warning: circular argument reference - assignment_id
......

Deprecation Warnings:

Using `should` from rspec-expectations' old `:should` syntax without explicitly enabling the syntax is deprecated. Use the new `:expect` syntax or explicitly enable `:should` with `config.expect_with(:rspec) { |c| c.syntax = :should }` instead. Called from /Users/chen/RubymineProjects/expertiza/spec/controllers/suggestion_controller_spec.rb:67:in `block (3 levels) in <top (required)>'.

If you need more of the backtrace for any of these deprecations to
identify where to make the necessary changes, you can configure
`config.raise_errors_for_deprecations!`, and it will turn the
deprecation warnings into errors, giving you the full backtrace.

1 deprecation warning total

Finished in 1 minute 6.74 seconds (files took 5.3 seconds to load)
6 examples, 0 failures

Randomized with seed 8804

Coverage report generated for RSpec to /Users/chen/RubymineProjects/expertiza/coverage. 1638 / 5233 LOC (31.3%) covered.
[Coveralls] Outside the CI environment, not sending data.
</pre>

CSC/ECE 517 Fall 2015/oss E1556 CHM

2015-10-31T01:10:01Z

Jchen45: add Project Description

E1556. Refactoring SuggestionController.rb
== Project description==
This project is about testing and optimizing of SuggestionController of Expertiza system. Suggestion Controller is a module for students to suggest a new topic for their writing assignments, and instructor can approve the suggestion. Typically, there are three cases when instructor approves the suggestion. First, if the student already has a topic and when suggesting a new topic, he chooses 'Yes' in the signup_preference, he will enroll the new suggested topic automatically after the instructor approves the suggested topic. Second, if the student is in the waitlist of a topic, and when suggesting a new topic, he chooses 'Yes' in the signup_preference, he will enroll the new suggested topic and be removed from the former waitlist. Third, if the student is in the waitlist of a topic, and when suggesting a new topic, he chooses 'No' in the signup_preference, after the instructor approves the new topic, he will remain in the waitlist of former topic, and new topic is left as 'no chooser'.
Besides, Suggestion Controller also needs to be optimized from two aspects. First, the syntax need to be upgraded from rails 3.x to rails 4.x. Second, refactoring the mailer part is necessary.
== Expertiza==
== Optimization==
=== Syntax ===
For the code to be coordinated with Rails 4 syntax, there is one major difference between Ruby 1.9 and 1.8 need to be change in the suggestion_controller.rb.
The hash operator using the "hash rocket":
<pre>
{ :key => 'value' }
</pre>
Need to be changed into:
<pre>
{ key : 'value' }
</pre>

=== Refator ===
The approve_suggestion() method is quite long in the original code, and the send email function in the method is achieved twice, which doesn’t conform with DRY principle.

The logic of the approve_suggestion() method is described as follows. First, approve the suggestion by create a new record in the SignUpTopic model, set relative parameters, and save the new record. Then send notification to the team. In the notification part, if the student doesn’t have a team, a new team should be created and assigned to the suggested topic.
Based on the logic, the approve_suggestion() method can be clearly divided into two parts: approve suggestion part, and notification part. In the notification part, send email function can be written as a single method in order not to repeat. Besides, creating a new team can also be written as a new method.

After refactor, there are four new methods: approve, notification, create_new_team, and send_email. Approve and notification are called within approve_suggestion, while create_new_team and send_email are called within notification.
<pre>
def create_new_team
new_team = AssignmentTeam.create(name: 'Team' + @user_id.to_s + '_' + rand(1000).to_s, parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
t_user = TeamsUser.create(team_id: new_team.id, user_id: @user_id)
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: new_team.id, is_waitlisted: 0)
parent = TeamNode.create(parent_id: @signuptopic.assignment_id, node_object_id: new_team.id)
TeamUserNode.create(parent_id: parent.id, node_object_id: t_user.id)
end

def send_email
proposer = User.find(@user_id)
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = Array.new
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
{ to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has already been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
}
).deliver
end

def approve
@suggestion = Suggestion.find(params[:id])
@user_id = User.where(name: @suggestion.unityID).first.id
@team_id = TeamsUser.team_id(@suggestion.assignment_id, @user_id)
@topic_id = SignedUpTeam.topic_id(@suggestion.assignment_id, @user_id)
@signuptopic = SignUpTopic.new
@signuptopic.topic_identifier = 'S' + Suggestion.where("assignment_id = ? and id <= ?", @suggestion.assignment_id, @suggestion.id).size.to_s
@signuptopic.topic_name = @suggestion.title
@signuptopic.assignment_id = @suggestion.assignment_id
@signuptopic.max_choosers = 1;
if @signuptopic.save && @suggestion.update_attribute('status', 'Approved')
flash[:notice] = 'Successfully approved the suggestion.'
else
flash[:error] = 'Error when approving the suggestion.'
end
end

def notification
if @suggestion.signup_preference == 'Y'
#if this user do not have team in this assignment, create one for him/her and assign this topic to this team.
if @team_id.nil?
create_new_team
else #this user has a team in this assignment, check whether this team has topic or not
if @topic_id.nil?
#clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end

def approve_suggestion
approve
notification
redirect_to action: 'show', id: @suggestion
end
</pre>

== Test ==
=== Rspec===
[[Rspec]] was used to conduct all the tests.
RSpec is a Behaviour-Driven Development tool for Ruby programmers. BDD is an approach to software development that combines Test-Driven Development, Domain Driven Design, and Acceptance Test-Driven Planning. RSpec helps you do the TDD part of that equation, focusing on the documentation and design aspects of TDD.

=== Test1 ===
In the first test, we are going to test the result of approving a student's suggestion topic if the student is in a waitlist. He will be removed from the waitlist and added to the new list, if he selected the signup_preference to be 'Yes'.
We choose 'Writing Assignment 1a' of 'CSC/ECE 517, Spring 2015' as test assignment. I simulate creating a new suggestion with student5717, in team 'Writing Assignment 1a Team14'.

First, log in as Student5717 and suggest a new topic, and choose 'Yes' in signup_preference.
<pre>
# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# suggest a new suggestion
visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'RSpect'
fill_in 'suggestion_description', with: 'RSpect is a ROR test framework. It focus on function test'
# select 'suggestion_signup_preference', with: 'Y'
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)
</pre>
Then, log out Student5717, and log in with 'instructor6' account, who is the manager of this course. Then approve the suggest.
<pre>
# Logout current account student5717
visit '/suggestion/new?id=711'
click_link "Logout"
expect(current_path).to eq("/")

# Login with account instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

# approve the suggestion
expect(page).to have_content('Manage content')
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
expect(page).to have_content('RSpect')

num = Suggestion.last.id
visit "/suggestion/"+num.to_s
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: RSpect')
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')
</pre>
Finally, check if suggestion approved successfully. I need to check topic list with 'instructor6' account logged in and check the selected topic in student5717 account.
<pre>
# check if is not in waitlist
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team14 student5717 (waitlisted)")

# Logout current account instructor6
visit '/suggestion/new?id=711'
click_link "Logout"
current_path.should == "/"

# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# Check if you select the topic successfully
visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): RSpect')
</pre>
=== Test2 ===
For the second test, Writing Assignment 1a team1, whose team id is 23781, was chosen to perform a serial of action. Team no.23781 is holding a topic: Amazon S3 and Rails. And Writing Assignment 1a team5, whose team id is 23800, is in the waiting list of this topic.
First, sign in as Student 5404 from team1, send a suggestion for new topic and indicate they want to choose their suggested topic.
<pre>
@newtopic = 'Violet and Zoe'

#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#suggest a topic:
# signup_preference default to be Y
visit "/student_task/view?id=28634"
expect(page).to have_content('Submit or Review work')
visit "/suggestion/new?id=711"
expect(page).to have_content('New suggestion')
fill_in 'Title',with: @newtopic
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)

#logout
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')
</pre>

Then sign in as instructor6 and approve the suggested topic.
<pre>
#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

#approve the suggestion
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
num = Suggestion.last.id.to_s
visit "/suggestion/"+num
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: '+@newtopic)
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')

#logout as instructor6
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
</pre>
Finally, to check the results. On the one hand, sign in as student5404 again and see if her/his team is holding the new topic. On the other hand sign in as instructor6 and check if team no.23800 is holding the old topic: Amazon S3 and Rails.
<pre>
#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#check the approved suggestion in topics list
visit "/sign_up_sheet/list?assignment_id=711"
expect(page).to have_content("Your approved suggested topic")

# switch to the new topic
num2 = SignUpTopic.last.id.to_s
visit "/sign_up_sheet/switch_original_topic_to_approved_suggested_topic/"+num2+"?assignment_id=711"
expect(page).to have_content("Your topic(s): "+@newtopic)

#logout student5404
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')

#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

# check if team1 is has not enrolled
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team1 student5404 student5731 ")
expect(page).to have_content("Writing assignment 1a_Team5 student5740 student5704")
expect(page).to have_content("S1 Violet and Zoe Writing assignment 1a_Team1 student5404 student5731")
</pre>
=== Test3 ===
The third test is similar to the second one. Team no.23781 and assignment no.711 are chosen for this test again.
First, student no.5404 login to the system, visit the assignment page, and make a topic suggestion. In the suggestion, instead of choosing “yes” in signup preference, the student chooses “no” in order not to use the suggested topic.

<pre>
visit 'content_pages/view'
fill_in "User Name", with: "student5404"
fill_in "Password", with: "password"
click_button "SIGN IN"

visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'test title'
fill_in 'suggestion_description', with: 'test description'
select 'No', from: "suggestion_signup_preference"
click_button "Submit"

click_link "Logout"
</pre>
After the suggestion is made, login as an instructor, find the assignment, and approve the suggestion.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

num = Suggestion.last.id
path = "/suggestion/" + num.to_s
visit path
click_button "Approve suggestion"

click_link "Logout"
</pre>
In the final step, we check if the new topic is shown in the topic list, then login as student no.5401 again, check if they still hold their old topic.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): Amazon S3 and Rails')
click_link "Logout"

visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit "/assignments/711/edit#tabs-2"
expect(page).to have_content("Amazon S3 and Rails")
expect(page).to have_content("test title")
</pre>
=== Result ===
Here is the result of our tests. All test cases passed.
<pre>
ChendeMacBook-Pro:expertiza chen$ rspec spec/controllers/suggestion_controller_spec.rb
[Coveralls] Set up the SimpleCov formatter.
[Coveralls] Using SimpleCov's 'rails' settings.

Randomized with seed 8804
/Users/chen/RubymineProjects/expertiza/app/models/suggestion.rb:5: warning: circular argument reference - assignment_id
......

Deprecation Warnings:

Using `should` from rspec-expectations' old `:should` syntax without explicitly enabling the syntax is deprecated. Use the new `:expect` syntax or explicitly enable `:should` with `config.expect_with(:rspec) { |c| c.syntax = :should }` instead. Called from /Users/chen/RubymineProjects/expertiza/spec/controllers/suggestion_controller_spec.rb:67:in `block (3 levels) in <top (required)>'.

If you need more of the backtrace for any of these deprecations to
identify where to make the necessary changes, you can configure
`config.raise_errors_for_deprecations!`, and it will turn the
deprecation warnings into errors, giving you the full backtrace.

1 deprecation warning total

Finished in 1 minute 6.74 seconds (files took 5.3 seconds to load)
6 examples, 0 failures

Randomized with seed 8804

Coverage report generated for RSpec to /Users/chen/RubymineProjects/expertiza/coverage. 1638 / 5233 LOC (31.3%) covered.
[Coveralls] Outside the CI environment, not sending data.
</pre>

CSC/ECE 517 Fall 2015/oss E1556 CHM

2015-10-31T00:59:25Z

Jchen45:

E1556. Refactoring SuggestionController.rb
== Project description==
== Expertiza==
== Optimization==
=== Syntax ===
For the code to be coordinated with Rails 4 syntax, there is one major difference between Ruby 1.9 and 1.8 need to be change in the suggestion_controller.rb.
The hash operator using the "hash rocket":
<pre>
{ :key => 'value' }
</pre>
Need to be changed into:
<pre>
{ key : 'value' }
</pre>

=== Refator ===
The approve_suggestion() method is quite long in the original code, and the send email function in the method is achieved twice, which doesn’t conform with DRY principle.

The logic of the approve_suggestion() method is described as follows. First, approve the suggestion by create a new record in the SignUpTopic model, set relative parameters, and save the new record. Then send notification to the team. In the notification part, if the student doesn’t have a team, a new team should be created and assigned to the suggested topic.
Based on the logic, the approve_suggestion() method can be clearly divided into two parts: approve suggestion part, and notification part. In the notification part, send email function can be written as a single method in order not to repeat. Besides, creating a new team can also be written as a new method.

After refactor, there are four new methods: approve, notification, create_new_team, and send_email. Approve and notification are called within approve_suggestion, while create_new_team and send_email are called within notification.
<pre>
def create_new_team
new_team = AssignmentTeam.create(name: 'Team' + @user_id.to_s + '_' + rand(1000).to_s, parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
t_user = TeamsUser.create(team_id: new_team.id, user_id: @user_id)
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: new_team.id, is_waitlisted: 0)
parent = TeamNode.create(parent_id: @signuptopic.assignment_id, node_object_id: new_team.id)
TeamUserNode.create(parent_id: parent.id, node_object_id: t_user.id)
end

def send_email
proposer = User.find(@user_id)
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = Array.new
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
{ to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has already been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
}
).deliver
end

def approve
@suggestion = Suggestion.find(params[:id])
@user_id = User.where(name: @suggestion.unityID).first.id
@team_id = TeamsUser.team_id(@suggestion.assignment_id, @user_id)
@topic_id = SignedUpTeam.topic_id(@suggestion.assignment_id, @user_id)
@signuptopic = SignUpTopic.new
@signuptopic.topic_identifier = 'S' + Suggestion.where("assignment_id = ? and id <= ?", @suggestion.assignment_id, @suggestion.id).size.to_s
@signuptopic.topic_name = @suggestion.title
@signuptopic.assignment_id = @suggestion.assignment_id
@signuptopic.max_choosers = 1;
if @signuptopic.save && @suggestion.update_attribute('status', 'Approved')
flash[:notice] = 'Successfully approved the suggestion.'
else
flash[:error] = 'Error when approving the suggestion.'
end
end

def notification
if @suggestion.signup_preference == 'Y'
#if this user do not have team in this assignment, create one for him/her and assign this topic to this team.
if @team_id.nil?
create_new_team
else #this user has a team in this assignment, check whether this team has topic or not
if @topic_id.nil?
#clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end

def approve_suggestion
approve
notification
redirect_to action: 'show', id: @suggestion
end
</pre>

== Test ==
=== Rspec===
[[Rspec]] was used to conduct all the tests.
RSpec is a Behaviour-Driven Development tool for Ruby programmers. BDD is an approach to software development that combines Test-Driven Development, Domain Driven Design, and Acceptance Test-Driven Planning. RSpec helps you do the TDD part of that equation, focusing on the documentation and design aspects of TDD.

=== Test1 ===
In the first test, we are going to test the result of approving a student's suggestion topic if the student is in a waitlist. He will be removed from the waitlist and added to the new list, if he selected the signup_preference to be 'Yes'.
We choose 'Writing Assignment 1a' of 'CSC/ECE 517, Spring 2015' as test assignment. I simulate creating a new suggestion with student5717, in team 'Writing Assignment 1a Team14'.

First, log in as Student5717 and suggest a new topic, and choose 'Yes' in signup_preference.
<pre>
# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# suggest a new suggestion
visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'RSpect'
fill_in 'suggestion_description', with: 'RSpect is a ROR test framework. It focus on function test'
# select 'suggestion_signup_preference', with: 'Y'
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)
</pre>
Then, log out Student5717, and log in with 'instructor6' account, who is the manager of this course. Then approve the suggest.
<pre>
# Logout current account student5717
visit '/suggestion/new?id=711'
click_link "Logout"
expect(current_path).to eq("/")

# Login with account instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

# approve the suggestion
expect(page).to have_content('Manage content')
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
expect(page).to have_content('RSpect')

num = Suggestion.last.id
visit "/suggestion/"+num.to_s
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: RSpect')
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')
</pre>
Finally, check if suggestion approved successfully. I need to check topic list with 'instructor6' account logged in and check the selected topic in student5717 account.
<pre>
# check if is not in waitlist
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team14 student5717 (waitlisted)")

# Logout current account instructor6
visit '/suggestion/new?id=711'
click_link "Logout"
current_path.should == "/"

# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# Check if you select the topic successfully
visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): RSpect')
</pre>
=== Test2 ===
For the second test, Writing Assignment 1a team1, whose team id is 23781, was chosen to perform a serial of action. Team no.23781 is holding a topic: Amazon S3 and Rails. And Writing Assignment 1a team5, whose team id is 23800, is in the waiting list of this topic.
First, sign in as Student 5404 from team1, send a suggestion for new topic and indicate they want to choose their suggested topic.
<pre>
@newtopic = 'Violet and Zoe'

#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#suggest a topic:
# signup_preference default to be Y
visit "/student_task/view?id=28634"
expect(page).to have_content('Submit or Review work')
visit "/suggestion/new?id=711"
expect(page).to have_content('New suggestion')
fill_in 'Title',with: @newtopic
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)

#logout
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')
</pre>

Then sign in as instructor6 and approve the suggested topic.
<pre>
#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

#approve the suggestion
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
num = Suggestion.last.id.to_s
visit "/suggestion/"+num
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: '+@newtopic)
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')

#logout as instructor6
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
</pre>
Finally, to check the results. On the one hand, sign in as student5404 again and see if her/his team is holding the new topic. On the other hand sign in as instructor6 and check if team no.23800 is holding the old topic: Amazon S3 and Rails.
<pre>
#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#check the approved suggestion in topics list
visit "/sign_up_sheet/list?assignment_id=711"
expect(page).to have_content("Your approved suggested topic")

# switch to the new topic
num2 = SignUpTopic.last.id.to_s
visit "/sign_up_sheet/switch_original_topic_to_approved_suggested_topic/"+num2+"?assignment_id=711"
expect(page).to have_content("Your topic(s): "+@newtopic)

#logout student5404
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')

#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

# check if team1 is has not enrolled
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team1 student5404 student5731 ")
expect(page).to have_content("Writing assignment 1a_Team5 student5740 student5704")
expect(page).to have_content("S1 Violet and Zoe Writing assignment 1a_Team1 student5404 student5731")
</pre>
=== Test3 ===
The third test is similar to the second one. Team no.23781 and assignment no.711 are chosen for this test again.
First, student no.5404 login to the system, visit the assignment page, and make a topic suggestion. In the suggestion, instead of choosing “yes” in signup preference, the student chooses “no” in order not to use the suggested topic.

<pre>
visit 'content_pages/view'
fill_in "User Name", with: "student5404"
fill_in "Password", with: "password"
click_button "SIGN IN"

visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'test title'
fill_in 'suggestion_description', with: 'test description'
select 'No', from: "suggestion_signup_preference"
click_button "Submit"

click_link "Logout"
</pre>
After the suggestion is made, login as an instructor, find the assignment, and approve the suggestion.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

num = Suggestion.last.id
path = "/suggestion/" + num.to_s
visit path
click_button "Approve suggestion"

click_link "Logout"
</pre>
In the final step, we check if the new topic is shown in the topic list, then login as student no.5401 again, check if they still hold their old topic.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): Amazon S3 and Rails')
click_link "Logout"

visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit "/assignments/711/edit#tabs-2"
expect(page).to have_content("Amazon S3 and Rails")
expect(page).to have_content("test title")
</pre>
=== Result ===
Here is the result of our tests. All test cases passed.
<pre>
ChendeMacBook-Pro:expertiza chen$ rspec spec/controllers/suggestion_controller_spec.rb
[Coveralls] Set up the SimpleCov formatter.
[Coveralls] Using SimpleCov's 'rails' settings.

Randomized with seed 8804
/Users/chen/RubymineProjects/expertiza/app/models/suggestion.rb:5: warning: circular argument reference - assignment_id
......

Deprecation Warnings:

Using `should` from rspec-expectations' old `:should` syntax without explicitly enabling the syntax is deprecated. Use the new `:expect` syntax or explicitly enable `:should` with `config.expect_with(:rspec) { |c| c.syntax = :should }` instead. Called from /Users/chen/RubymineProjects/expertiza/spec/controllers/suggestion_controller_spec.rb:67:in `block (3 levels) in <top (required)>'.

If you need more of the backtrace for any of these deprecations to
identify where to make the necessary changes, you can configure
`config.raise_errors_for_deprecations!`, and it will turn the
deprecation warnings into errors, giving you the full backtrace.

1 deprecation warning total

Finished in 1 minute 6.74 seconds (files took 5.3 seconds to load)
6 examples, 0 failures

Randomized with seed 8804

Coverage report generated for RSpec to /Users/chen/RubymineProjects/expertiza/coverage. 1638 / 5233 LOC (31.3%) covered.
[Coveralls] Outside the CI environment, not sending data.
</pre>

CSC/ECE 517 Fall 2015/oss E1556 CHM

2015-10-31T00:57:47Z

Jchen45: add test result

E1556. Refactoring SuggestionController.rb
== Project description==
== Expertiza==
== Optimization==
=== Syntax ===
For the code to be coordinated with Rails 4 syntax, there is one major difference between Ruby 1.9 and 1.8 need to be change in the suggestion_controller.rb.
The hash operator using the "hash rocket":
<pre>
{ :key => 'value' }
</pre>
Need to be changed into:
<pre>
{ key : 'value' }
</pre>

=== Refator ===
The approve_suggestion() method is quite long in the original code, and the send email function in the method is achieved twice, which doesn’t conform with DRY principle.

The logic of the approve_suggestion() method is described as follows. First, approve the suggestion by create a new record in the SignUpTopic model, set relative parameters, and save the new record. Then send notification to the team. In the notification part, if the student doesn’t have a team, a new team should be created and assigned to the suggested topic.
Based on the logic, the approve_suggestion() method can be clearly divided into two parts: approve suggestion part, and notification part. In the notification part, send email function can be written as a single method in order not to repeat. Besides, creating a new team can also be written as a new method.

After refactor, there are four new methods: approve, notification, create_new_team, and send_email. Approve and notification are called within approve_suggestion, while create_new_team and send_email are called within notification.
<pre>
def create_new_team
new_team = AssignmentTeam.create(name: 'Team' + @user_id.to_s + '_' + rand(1000).to_s, parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
t_user = TeamsUser.create(team_id: new_team.id, user_id: @user_id)
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: new_team.id, is_waitlisted: 0)
parent = TeamNode.create(parent_id: @signuptopic.assignment_id, node_object_id: new_team.id)
TeamUserNode.create(parent_id: parent.id, node_object_id: t_user.id)
end

def send_email
proposer = User.find(@user_id)
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = Array.new
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
{ to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has already been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
}
).deliver
end

def approve
@suggestion = Suggestion.find(params[:id])
@user_id = User.where(name: @suggestion.unityID).first.id
@team_id = TeamsUser.team_id(@suggestion.assignment_id, @user_id)
@topic_id = SignedUpTeam.topic_id(@suggestion.assignment_id, @user_id)
@signuptopic = SignUpTopic.new
@signuptopic.topic_identifier = 'S' + Suggestion.where("assignment_id = ? and id <= ?", @suggestion.assignment_id, @suggestion.id).size.to_s
@signuptopic.topic_name = @suggestion.title
@signuptopic.assignment_id = @suggestion.assignment_id
@signuptopic.max_choosers = 1;
if @signuptopic.save && @suggestion.update_attribute('status', 'Approved')
flash[:notice] = 'Successfully approved the suggestion.'
else
flash[:error] = 'Error when approving the suggestion.'
end
end

def notification
if @suggestion.signup_preference == 'Y'
#if this user do not have team in this assignment, create one for him/her and assign this topic to this team.
if @team_id.nil?
create_new_team
else #this user has a team in this assignment, check whether this team has topic or not
if @topic_id.nil?
#clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end

def approve_suggestion
approve
notification
redirect_to action: 'show', id: @suggestion
end
</pre>

== Test ==
=== Rspec===
[[Rspec]] was used to conduct all the tests.

=== Test1 ===
In the first test, we are going to test the result of approving a student's suggestion topic if the student is in a waitlist. He will be removed from the waitlist and added to the new list, if he selected the signup_preference to be 'Yes'.
We choose 'Writing Assignment 1a' of 'CSC/ECE 517, Spring 2015' as test assignment. I simulate creating a new suggestion with student5717, in team 'Writing Assignment 1a Team14'.

First, log in as Student5717 and suggest a new topic, and choose 'Yes' in signup_preference.
<pre>
# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# suggest a new suggestion
visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'RSpect'
fill_in 'suggestion_description', with: 'RSpect is a ROR test framework. It focus on function test'
# select 'suggestion_signup_preference', with: 'Y'
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)
</pre>
Then, log out Student5717, and log in with 'instructor6' account, who is the manager of this course. Then approve the suggest.
<pre>
# Logout current account student5717
visit '/suggestion/new?id=711'
click_link "Logout"
expect(current_path).to eq("/")

# Login with account instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

# approve the suggestion
expect(page).to have_content('Manage content')
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
expect(page).to have_content('RSpect')

num = Suggestion.last.id
visit "/suggestion/"+num.to_s
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: RSpect')
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')
</pre>
Finally, check if suggestion approved successfully. I need to check topic list with 'instructor6' account logged in and check the selected topic in student5717 account.
<pre>
# check if is not in waitlist
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team14 student5717 (waitlisted)")

# Logout current account instructor6
visit '/suggestion/new?id=711'
click_link "Logout"
current_path.should == "/"

# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# Check if you select the topic successfully
visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): RSpect')
</pre>
=== Test2 ===
For the second test, Writing Assignment 1a team1, whose team id is 23781, was chosen to perform a serial of action. Team no.23781 is holding a topic: Amazon S3 and Rails. And Writing Assignment 1a team5, whose team id is 23800, is in the waiting list of this topic.
First, sign in as Student 5404 from team1, send a suggestion for new topic and indicate they want to choose their suggested topic.
<pre>
@newtopic = 'Violet and Zoe'

#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#suggest a topic:
# signup_preference default to be Y
visit "/student_task/view?id=28634"
expect(page).to have_content('Submit or Review work')
visit "/suggestion/new?id=711"
expect(page).to have_content('New suggestion')
fill_in 'Title',with: @newtopic
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)

#logout
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')
</pre>

Then sign in as instructor6 and approve the suggested topic.
<pre>
#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

#approve the suggestion
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
num = Suggestion.last.id.to_s
visit "/suggestion/"+num
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: '+@newtopic)
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')

#logout as instructor6
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
</pre>
Finally, to check the results. On the one hand, sign in as student5404 again and see if her/his team is holding the new topic. On the other hand sign in as instructor6 and check if team no.23800 is holding the old topic: Amazon S3 and Rails.
<pre>
#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#check the approved suggestion in topics list
visit "/sign_up_sheet/list?assignment_id=711"
expect(page).to have_content("Your approved suggested topic")

# switch to the new topic
num2 = SignUpTopic.last.id.to_s
visit "/sign_up_sheet/switch_original_topic_to_approved_suggested_topic/"+num2+"?assignment_id=711"
expect(page).to have_content("Your topic(s): "+@newtopic)

#logout student5404
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')

#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

# check if team1 is has not enrolled
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team1 student5404 student5731 ")
expect(page).to have_content("Writing assignment 1a_Team5 student5740 student5704")
expect(page).to have_content("S1 Violet and Zoe Writing assignment 1a_Team1 student5404 student5731")
</pre>
=== Test3 ===
The third test is similar to the second one. Team no.23781 and assignment no.711 are chosen for this test again.
First, student no.5404 login to the system, visit the assignment page, and make a topic suggestion. In the suggestion, instead of choosing “yes” in signup preference, the student chooses “no” in order not to use the suggested topic.

<pre>
visit 'content_pages/view'
fill_in "User Name", with: "student5404"
fill_in "Password", with: "password"
click_button "SIGN IN"

visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'test title'
fill_in 'suggestion_description', with: 'test description'
select 'No', from: "suggestion_signup_preference"
click_button "Submit"

click_link "Logout"
</pre>
After the suggestion is made, login as an instructor, find the assignment, and approve the suggestion.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

num = Suggestion.last.id
path = "/suggestion/" + num.to_s
visit path
click_button "Approve suggestion"

click_link "Logout"
</pre>
In the final step, we check if the new topic is shown in the topic list, then login as student no.5401 again, check if they still hold their old topic.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): Amazon S3 and Rails')
click_link "Logout"

visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit "/assignments/711/edit#tabs-2"
expect(page).to have_content("Amazon S3 and Rails")
expect(page).to have_content("test title")
</pre>
=== Result ===
Here is the result of our tests. All test cases passed.
<pre>
ChendeMacBook-Pro:expertiza chen$ rspec spec/controllers/suggestion_controller_spec.rb
[Coveralls] Set up the SimpleCov formatter.
[Coveralls] Using SimpleCov's 'rails' settings.

Randomized with seed 8804
/Users/chen/RubymineProjects/expertiza/app/models/suggestion.rb:5: warning: circular argument reference - assignment_id
......

Deprecation Warnings:

Using `should` from rspec-expectations' old `:should` syntax without explicitly enabling the syntax is deprecated. Use the new `:expect` syntax or explicitly enable `:should` with `config.expect_with(:rspec) { |c| c.syntax = :should }` instead. Called from /Users/chen/RubymineProjects/expertiza/spec/controllers/suggestion_controller_spec.rb:67:in `block (3 levels) in <top (required)>'.

If you need more of the backtrace for any of these deprecations to
identify where to make the necessary changes, you can configure
`config.raise_errors_for_deprecations!`, and it will turn the
deprecation warnings into errors, giving you the full backtrace.

1 deprecation warning total

Finished in 1 minute 6.74 seconds (files took 5.3 seconds to load)
6 examples, 0 failures

Randomized with seed 8804

Coverage report generated for RSpec to /Users/chen/RubymineProjects/expertiza/coverage. 1638 / 5233 LOC (31.3%) covered.
[Coveralls] Outside the CI environment, not sending data.
</pre>

CSC/ECE 517 Fall 2015/oss E1556 CHM

2015-10-31T00:47:35Z

Jchen45:

E1556. Refactoring SuggestionController.rb
== Project description==
== Expertiza==
== Optimization==
=== Syntax ===
For the code to be coordinated with Rails 4 syntax, there is one major difference between Ruby 1.9 and 1.8 need to be change in the suggestion_controller.rb.
The hash operator using the "hash rocket":
<pre>
{ :key => 'value' }
</pre>
Need to be changed into:
<pre>
{ key : 'value' }
</pre>

=== Refator ===
The approve_suggestion() method is quite long in the original code, and the send email function in the method is achieved twice, which doesn’t conform with DRY principle.

The logic of the approve_suggestion() method is described as follows. First, approve the suggestion by create a new record in the SignUpTopic model, set relative parameters, and save the new record. Then send notification to the team. In the notification part, if the student doesn’t have a team, a new team should be created and assigned to the suggested topic.
Based on the logic, the approve_suggestion() method can be clearly divided into two parts: approve suggestion part, and notification part. In the notification part, send email function can be written as a single method in order not to repeat. Besides, creating a new team can also be written as a new method.

After refactor, there are four new methods: approve, notification, create_new_team, and send_email. Approve and notification are called within approve_suggestion, while create_new_team and send_email are called within notification.
<pre>
def create_new_team
new_team = AssignmentTeam.create(name: 'Team' + @user_id.to_s + '_' + rand(1000).to_s, parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
t_user = TeamsUser.create(team_id: new_team.id, user_id: @user_id)
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: new_team.id, is_waitlisted: 0)
parent = TeamNode.create(parent_id: @signuptopic.assignment_id, node_object_id: new_team.id)
TeamUserNode.create(parent_id: parent.id, node_object_id: t_user.id)
end

def send_email
proposer = User.find(@user_id)
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = Array.new
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
{ to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has already been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
}
).deliver
end

def approve
@suggestion = Suggestion.find(params[:id])
@user_id = User.where(name: @suggestion.unityID).first.id
@team_id = TeamsUser.team_id(@suggestion.assignment_id, @user_id)
@topic_id = SignedUpTeam.topic_id(@suggestion.assignment_id, @user_id)
@signuptopic = SignUpTopic.new
@signuptopic.topic_identifier = 'S' + Suggestion.where("assignment_id = ? and id <= ?", @suggestion.assignment_id, @suggestion.id).size.to_s
@signuptopic.topic_name = @suggestion.title
@signuptopic.assignment_id = @suggestion.assignment_id
@signuptopic.max_choosers = 1;
if @signuptopic.save && @suggestion.update_attribute('status', 'Approved')
flash[:notice] = 'Successfully approved the suggestion.'
else
flash[:error] = 'Error when approving the suggestion.'
end
end

def notification
if @suggestion.signup_preference == 'Y'
#if this user do not have team in this assignment, create one for him/her and assign this topic to this team.
if @team_id.nil?
create_new_team
else #this user has a team in this assignment, check whether this team has topic or not
if @topic_id.nil?
#clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end

def approve_suggestion
approve
notification
redirect_to action: 'show', id: @suggestion
end
</pre>

== Test ==
=== Rspec===
[[Rspec]] was used to conduct all the tests.

=== Test1 ===
In the first test, we are going to test the result of approving a student's suggestion topic if the student is in a waitlist. He will be removed from the waitlist and added to the new list, if he selected the signup_preference to be 'Yes'.
We choose 'Writing Assignment 1a' of 'CSC/ECE 517, Spring 2015' as test assignment. I simulate creating a new suggestion with student5717, in team 'Writing Assignment 1a Team14'.

First, log in as Student5717 and suggest a new topic, and choose 'Yes' in signup_preference.
<pre>
# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# suggest a new suggestion
visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'RSpect'
fill_in 'suggestion_description', with: 'RSpect is a ROR test framework. It focus on function test'
# select 'suggestion_signup_preference', with: 'Y'
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)
</pre>
Then, log out Student5717, and log in with 'instructor6' account, who is the manager of this course. Then approve the suggest.
<pre>
# Logout current account student5717
visit '/suggestion/new?id=711'
click_link "Logout"
expect(current_path).to eq("/")

# Login with account instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

# approve the suggestion
expect(page).to have_content('Manage content')
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
expect(page).to have_content('RSpect')

num = Suggestion.last.id
visit "/suggestion/"+num.to_s
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: RSpect')
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')
</pre>
Finally, check if suggestion approved successfully. I need to check topic list with 'instructor6' account logged in and check the selected topic in student5717 account.
<pre>
# check if is not in waitlist
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team14 student5717 (waitlisted)")

# Logout current account instructor6
visit '/suggestion/new?id=711'
click_link "Logout"
current_path.should == "/"

# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# Check if you select the topic successfully
visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): RSpect')
</pre>
=== Test2 ===
For the second test, Writing Assignment 1a team1, whose team id is 23781, was chosen to perform a serial of action. Team no.23781 is holding a topic: Amazon S3 and Rails. And Writing Assignment 1a team5, whose team id is 23800, is in the waiting list of this topic.
First, sign in as Student 5404 from team1, send a suggestion for new topic and indicate they want to choose their suggested topic.
<pre>
@newtopic = 'Violet and Zoe'

#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#suggest a topic:
# signup_preference default to be Y
visit "/student_task/view?id=28634"
expect(page).to have_content('Submit or Review work')
visit "/suggestion/new?id=711"
expect(page).to have_content('New suggestion')
fill_in 'Title',with: @newtopic
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)

#logout
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')
</pre>

Then sign in as instructor6 and approve the suggested topic.
<pre>
#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

#approve the suggestion
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
num = Suggestion.last.id.to_s
visit "/suggestion/"+num
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: '+@newtopic)
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')

#logout as instructor6
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
</pre>
Finally, to check the results. On the one hand, sign in as student5404 again and see if her/his team is holding the new topic. On the other hand sign in as instructor6 and check if team no.23800 is holding the old topic: Amazon S3 and Rails.
<pre>
#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#check the approved suggestion in topics list
visit "/sign_up_sheet/list?assignment_id=711"
expect(page).to have_content("Your approved suggested topic")

# switch to the new topic
num2 = SignUpTopic.last.id.to_s
visit "/sign_up_sheet/switch_original_topic_to_approved_suggested_topic/"+num2+"?assignment_id=711"
expect(page).to have_content("Your topic(s): "+@newtopic)

#logout student5404
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')

#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

# check if team1 is has not enrolled
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team1 student5404 student5731 ")
expect(page).to have_content("Writing assignment 1a_Team5 student5740 student5704")
expect(page).to have_content("S1 Violet and Zoe Writing assignment 1a_Team1 student5404 student5731")
</pre>
=== Test3 ===
The third test is similar to the second one. Team no.23781 and assignment no.711 are chosen for this test again.
First, student no.5404 login to the system, visit the assignment page, and make a topic suggestion. In the suggestion, instead of choosing “yes” in signup preference, the student chooses “no” in order not to use the suggested topic.

<pre>
visit 'content_pages/view'
fill_in "User Name", with: "student5404"
fill_in "Password", with: "password"
click_button "SIGN IN"

visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'test title'
fill_in 'suggestion_description', with: 'test description'
select 'No', from: "suggestion_signup_preference"
click_button "Submit"

click_link "Logout"
</pre>
After the suggestion is made, login as an instructor, find the assignment, and approve the suggestion.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

num = Suggestion.last.id
path = "/suggestion/" + num.to_s
visit path
click_button "Approve suggestion"

click_link "Logout"
</pre>
In the final step, we check if the new topic is shown in the topic list, then login as student no.5401 again, check if they still hold their old topic.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): Amazon S3 and Rails')
click_link "Logout"

visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit "/assignments/711/edit#tabs-2"
expect(page).to have_content("Amazon S3 and Rails")
expect(page).to have_content("test title")
</pre>
=== Result ===
Here is the result of our tests. All test cases passed.
[[File:https://drive.google.com/file/d/0B6_SjmCmcdT1T3ZUbUV5N3c0NDQ/view?usp=sharing]]

CSC/ECE 517 Fall 2015/oss E1556 CHM

2015-10-31T00:41:59Z

Jchen45:

E1556. Refactoring SuggestionController.rb
== Project description==
== Expertiza==
== Optimization==
=== Syntax ===
For the code to be coordinated with Rails 4 syntax, there is one major difference between Ruby 1.9 and 1.8 need to be change in the suggestion_controller.rb.
The hash operator using the "hash rocket":
<pre>
{ :key => 'value' }
</pre>
Need to be changed into:
<pre>
{ key : 'value' }
</pre>

=== Refator ===
The approve_suggestion() method is quite long in the original code, and the send email function in the method is achieved twice, which doesn’t conform with DRY principle.

The logic of the approve_suggestion() method is described as follows. First, approve the suggestion by create a new record in the SignUpTopic model, set relative parameters, and save the new record. Then send notification to the team. In the notification part, if the student doesn’t have a team, a new team should be created and assigned to the suggested topic.
Based on the logic, the approve_suggestion() method can be clearly divided into two parts: approve suggestion part, and notification part. In the notification part, send email function can be written as a single method in order not to repeat. Besides, creating a new team can also be written as a new method.

After refactor, there are four new methods: approve, notification, create_new_team, and send_email. Approve and notification are called within approve_suggestion, while create_new_team and send_email are called within notification.
<pre>
def create_new_team
new_team = AssignmentTeam.create(name: 'Team' + @user_id.to_s + '_' + rand(1000).to_s, parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
t_user = TeamsUser.create(team_id: new_team.id, user_id: @user_id)
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: new_team.id, is_waitlisted: 0)
parent = TeamNode.create(parent_id: @signuptopic.assignment_id, node_object_id: new_team.id)
TeamUserNode.create(parent_id: parent.id, node_object_id: t_user.id)
end

def send_email
proposer = User.find(@user_id)
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = Array.new
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
{ to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has already been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
}
).deliver
end

def approve
@suggestion = Suggestion.find(params[:id])
@user_id = User.where(name: @suggestion.unityID).first.id
@team_id = TeamsUser.team_id(@suggestion.assignment_id, @user_id)
@topic_id = SignedUpTeam.topic_id(@suggestion.assignment_id, @user_id)
@signuptopic = SignUpTopic.new
@signuptopic.topic_identifier = 'S' + Suggestion.where("assignment_id = ? and id <= ?", @suggestion.assignment_id, @suggestion.id).size.to_s
@signuptopic.topic_name = @suggestion.title
@signuptopic.assignment_id = @suggestion.assignment_id
@signuptopic.max_choosers = 1;
if @signuptopic.save && @suggestion.update_attribute('status', 'Approved')
flash[:notice] = 'Successfully approved the suggestion.'
else
flash[:error] = 'Error when approving the suggestion.'
end
end

def notification
if @suggestion.signup_preference == 'Y'
#if this user do not have team in this assignment, create one for him/her and assign this topic to this team.
if @team_id.nil?
create_new_team
else #this user has a team in this assignment, check whether this team has topic or not
if @topic_id.nil?
#clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end

def approve_suggestion
approve
notification
redirect_to action: 'show', id: @suggestion
end
</pre>

== Test ==
=== Rspec===
[[Rspec]] was used to conduct all the tests.

=== Test1 ===
In the first test, we are going to test the result of approving a student's suggestion topic if the student is in a waitlist. He will be removed from the waitlist and added to the new list, if he selected the signup_preference to be 'Yes'.
We choose 'Writing Assignment 1a' of 'CSC/ECE 517, Spring 2015' as test assignment. I simulate creating a new suggestion with student5717, in team 'Writing Assignment 1a Team14'.

First, log in as Student5717 and suggest a new topic, and choose 'Yes' in signup_preference.
<pre>
# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# suggest a new suggestion
visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'RSpect'
fill_in 'suggestion_description', with: 'RSpect is a ROR test framework. It focus on function test'
# select 'suggestion_signup_preference', with: 'Y'
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)
</pre>
Then, log out Student5717, and log in with 'instructor6' account, who is the manager of this course. Then approve the suggest.
<pre>
# Logout current account student5717
visit '/suggestion/new?id=711'
click_link "Logout"
expect(current_path).to eq("/")

# Login with account instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

# approve the suggestion
expect(page).to have_content('Manage content')
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
expect(page).to have_content('RSpect')

num = Suggestion.last.id
visit "/suggestion/"+num.to_s
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: RSpect')
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')
</pre>
Finally, check if suggestion approved successfully. I need to check topic list with 'instructor6' account logged in and check the selected topic in student5717 account.
<pre>
# check if is not in waitlist
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team14 student5717 (waitlisted)")

# Logout current account instructor6
visit '/suggestion/new?id=711'
click_link "Logout"
current_path.should == "/"

# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# Check if you select the topic successfully
visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): RSpect')
</pre>
=== Test2 ===
For the second test, Writing Assignment 1a team1, whose team id is 23781, was chosen to perform a serial of action. Team no.23781 is holding a topic: Amazon S3 and Rails. And Writing Assignment 1a team5, whose team id is 23800, is in the waiting list of this topic.
First, sign in as Student 5404 from team1, send a suggestion for new topic and indicate they want to choose their suggested topic.
<pre>
@newtopic = 'Violet and Zoe'

#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#suggest a topic:
# signup_preference default to be Y
visit "/student_task/view?id=28634"
expect(page).to have_content('Submit or Review work')
visit "/suggestion/new?id=711"
expect(page).to have_content('New suggestion')
fill_in 'Title',with: @newtopic
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)

#logout
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')
</pre>

Then sign in as instructor6 and approve the suggested topic.
<pre>
#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

#approve the suggestion
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
num = Suggestion.last.id.to_s
visit "/suggestion/"+num
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: '+@newtopic)
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')

#logout as instructor6
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
</pre>
Finally, to check the results. On the one hand, sign in as student5404 again and see if her/his team is holding the new topic. On the other hand sign in as instructor6 and check if team no.23800 is holding the old topic: Amazon S3 and Rails.
<pre>
#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#check the approved suggestion in topics list
visit "/sign_up_sheet/list?assignment_id=711"
expect(page).to have_content("Your approved suggested topic")

# switch to the new topic
num2 = SignUpTopic.last.id.to_s
visit "/sign_up_sheet/switch_original_topic_to_approved_suggested_topic/"+num2+"?assignment_id=711"
expect(page).to have_content("Your topic(s): "+@newtopic)

#logout student5404
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')

#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

# check if team1 is has not enrolled
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team1 student5404 student5731 ")
expect(page).to have_content("Writing assignment 1a_Team5 student5740 student5704")
expect(page).to have_content("S1 Violet and Zoe Writing assignment 1a_Team1 student5404 student5731")
</pre>
=== Test3 ===
The third test is similar to the second one. Team no.23781 and assignment no.711 are chosen for this test again.
First, student no.5404 login to the system, visit the assignment page, and make a topic suggestion. In the suggestion, instead of choosing “yes” in signup preference, the student chooses “no” in order not to use the suggested topic.

<pre>
visit 'content_pages/view'
fill_in "User Name", with: "student5404"
fill_in "Password", with: "password"
click_button "SIGN IN"

visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'test title'
fill_in 'suggestion_description', with: 'test description'
select 'No', from: "suggestion_signup_preference"
click_button "Submit"

click_link "Logout"
</pre>
After the suggestion is made, login as an instructor, find the assignment, and approve the suggestion.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

num = Suggestion.last.id
path = "/suggestion/" + num.to_s
visit path
click_button "Approve suggestion"

click_link "Logout"
</pre>
In the final step, we check if the new topic is shown in the topic list, then login as student no.5401 again, check if they still hold their old topic.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): Amazon S3 and Rails')
click_link "Logout"

visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit "/assignments/711/edit#tabs-2"
expect(page).to have_content("Amazon S3 and Rails")
expect(page).to have_content("test title")
</pre>
=== Result ===

CSC/ECE 517 Fall 2015/oss E1556 CHM

2015-10-31T00:41:20Z

Jchen45: Add test1 wiki

E1556. Refactoring SuggestionController.rb
== Project description==
== Expertiza==
== Optimization==
=== Syntax ===
For the code to be coordinated with Rails 4 syntax, there is one major difference between Ruby 1.9 and 1.8 need to be change in the suggestion_controller.rb.
The hash operator using the "hash rocket":
<pre>
{ :key => 'value' }
</pre>
Need to be changed into:
<pre>
{ key : 'value' }
</pre>

=== Refator ===
The approve_suggestion() method is quite long in the original code, and the send email function in the method is achieved twice, which doesn’t conform with DRY principle.

The logic of the approve_suggestion() method is described as follows. First, approve the suggestion by create a new record in the SignUpTopic model, set relative parameters, and save the new record. Then send notification to the team. In the notification part, if the student doesn’t have a team, a new team should be created and assigned to the suggested topic.
Based on the logic, the approve_suggestion() method can be clearly divided into two parts: approve suggestion part, and notification part. In the notification part, send email function can be written as a single method in order not to repeat. Besides, creating a new team can also be written as a new method.

After refactor, there are four new methods: approve, notification, create_new_team, and send_email. Approve and notification are called within approve_suggestion, while create_new_team and send_email are called within notification.
<pre>
def create_new_team
new_team = AssignmentTeam.create(name: 'Team' + @user_id.to_s + '_' + rand(1000).to_s, parent_id: @signuptopic.assignment_id, type: 'AssignmentTeam')
t_user = TeamsUser.create(team_id: new_team.id, user_id: @user_id)
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: new_team.id, is_waitlisted: 0)
parent = TeamNode.create(parent_id: @signuptopic.assignment_id, node_object_id: new_team.id)
TeamUserNode.create(parent_id: parent.id, node_object_id: t_user.id)
end

def send_email
proposer = User.find(@user_id)
teams_users = TeamsUser.where(team_id: @team_id)
cc_mail_list = Array.new
teams_users.each do |teams_user|
cc_mail_list << User.find(teams_user.user_id).email if teams_user.user_id != proposer.id
end
Mailer.suggested_topic_approved_message(
{ to: proposer.email,
cc: cc_mail_list,
subject: "Suggested topic '#{@suggestion.title}' has already been approved",
body: {
approved_topic_name: @suggestion.title,
proposer: proposer.name
}
}
).deliver
end

def approve
@suggestion = Suggestion.find(params[:id])
@user_id = User.where(name: @suggestion.unityID).first.id
@team_id = TeamsUser.team_id(@suggestion.assignment_id, @user_id)
@topic_id = SignedUpTeam.topic_id(@suggestion.assignment_id, @user_id)
@signuptopic = SignUpTopic.new
@signuptopic.topic_identifier = 'S' + Suggestion.where("assignment_id = ? and id <= ?", @suggestion.assignment_id, @suggestion.id).size.to_s
@signuptopic.topic_name = @suggestion.title
@signuptopic.assignment_id = @suggestion.assignment_id
@signuptopic.max_choosers = 1;
if @signuptopic.save && @suggestion.update_attribute('status', 'Approved')
flash[:notice] = 'Successfully approved the suggestion.'
else
flash[:error] = 'Error when approving the suggestion.'
end
end

def notification
if @suggestion.signup_preference == 'Y'
#if this user do not have team in this assignment, create one for him/her and assign this topic to this team.
if @team_id.nil?
create_new_team
else #this user has a team in this assignment, check whether this team has topic or not
if @topic_id.nil?
#clean waitlists
SignedUpTeam.where(team_id: @team_id, is_waitlisted: 1).destroy_all
SignedUpTeam.create(topic_id: @signuptopic.id, team_id: @team_id, is_waitlisted: 0)
else
@signuptopic.private_to = @user_id
@signuptopic.save
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end
else
#if this team has topic, Expertiza will send an email (suggested_topic_approved_message) to this team
send_email
end
end

def approve_suggestion
approve
notification
redirect_to action: 'show', id: @suggestion
end
</pre>

== Test ==
=== Rspec===
[[Rspec]] was used to conduct all the tests.

=== Test1 ===
In the first test, we are going to test the result of approving a student's suggestion topic if the student is in a waitlist. He will be removed from the waitlist and added to the new list, if he selected the signup_preference to be 'Yes'.
We choose 'Writing Assignment 1a' of 'CSC/ECE 517, Spring 2015' as test assignment. I simulate creating a new suggestion with student5717, in team 'Writing Assignment 1a Team14'.
First, log in as Student5717 and suggest a new topic, and choose 'Yes' in signup_preference.
<pre>
# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# suggest a new suggestion
visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'RSpect'
fill_in 'suggestion_description', with: 'RSpect is a ROR test framework. It focus on function test'
# select 'suggestion_signup_preference', with: 'Y'
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)
</pre>
Then, log out Student5717, and log in with 'instructor6' account, who is the manager of this course. Then approve the suggest.
<pre>
# Logout current account student5717
visit '/suggestion/new?id=711'
click_link "Logout"
expect(current_path).to eq("/")

# Login with account instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

# approve the suggestion
expect(page).to have_content('Manage content')
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
expect(page).to have_content('RSpect')

num = Suggestion.last.id
visit "/suggestion/"+num.to_s
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: RSpect')
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')
</pre>
Finally, check if suggestion approved successfully. I need to check topic list with 'instructor6' account logged in and check the selected topic in student5717 account.
<pre>
# check if is not in waitlist
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team14 student5717 (waitlisted)")

# Logout current account instructor6
visit '/suggestion/new?id=711'
click_link "Logout"
current_path.should == "/"

# Login with student5717 account
visit 'content_pages/view'
expect(page).to have_content('Welcome')
fill_in "User Name", with: "student5717"
fill_in "Password", with: "password"
click_button "SIGN IN"

# Check if you select the topic successfully
visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): RSpect')
</pre>
=== Test2 ===
For the second test, Writing Assignment 1a team1, whose team id is 23781, was chosen to perform a serial of action. Team no.23781 is holding a topic: Amazon S3 and Rails. And Writing Assignment 1a team5, whose team id is 23800, is in the waiting list of this topic.
First, sign in as Student 5404 from team1, send a suggestion for new topic and indicate they want to choose their suggested topic.
<pre>
@newtopic = 'Violet and Zoe'

#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#suggest a topic:
# signup_preference default to be Y
visit "/student_task/view?id=28634"
expect(page).to have_content('Submit or Review work')
visit "/suggestion/new?id=711"
expect(page).to have_content('New suggestion')
fill_in 'Title',with: @newtopic
expect{click_button "Submit"}.to change(Suggestion, :count).by(1)

#logout
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')
</pre>

Then sign in as instructor6 and approve the suggested topic.
<pre>
#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

#approve the suggestion
visit '/suggestion/list?id=711&type=Assignment'
expect(page).to have_content('Suggested topics for Writing assignment 1a')
num = Suggestion.last.id.to_s
visit "/suggestion/"+num
expect(page).to have_content('Suggestion')
expect(page).to have_content('Title: '+@newtopic)
click_button "Approve suggestion"
visit "/suggestion/"+num.to_s
expect(page).to have_content('status: Approved')

#logout as instructor6
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
</pre>
Finally, to check the results. On the one hand, sign in as student5404 again and see if her/his team is holding the new topic. On the other hand sign in as instructor6 and check if team no.23800 is holding the old topic: Amazon S3 and Rails.
<pre>
#sign in as student5404:
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Assignments')

#check the approved suggestion in topics list
visit "/sign_up_sheet/list?assignment_id=711"
expect(page).to have_content("Your approved suggested topic")

# switch to the new topic
num2 = SignUpTopic.last.id.to_s
visit "/sign_up_sheet/switch_original_topic_to_approved_suggested_topic/"+num2+"?assignment_id=711"
expect(page).to have_content("Your topic(s): "+@newtopic)

#logout student5404
click_link "Logout"
expect(current_path).to eq("/")
visit '/suggestion/new?id=711'
expect(page).to have_content('This is not allowed')
expect(page).to have_content('Welcome')
expect(page).to have_no_content('User: student5404')

#sign in as instructor6
visit 'content_pages/view'
expect(page).to have_content('Welcome.')
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"
expect(page).to have_content('Manage content')

# check if team1 is has not enrolled
visit "/assignments/711/edit#tabs-2"
expect(page).to have_no_content(" Writing assignment 1a_Team1 student5404 student5731 ")
expect(page).to have_content("Writing assignment 1a_Team5 student5740 student5704")
expect(page).to have_content("S1 Violet and Zoe Writing assignment 1a_Team1 student5404 student5731")
</pre>
=== Test3 ===
The third test is similar to the second one. Team no.23781 and assignment no.711 are chosen for this test again.
First, student no.5404 login to the system, visit the assignment page, and make a topic suggestion. In the suggestion, instead of choosing “yes” in signup preference, the student chooses “no” in order not to use the suggested topic.

<pre>
visit 'content_pages/view'
fill_in "User Name", with: "student5404"
fill_in "Password", with: "password"
click_button "SIGN IN"

visit '/suggestion/new?id=711'
fill_in 'suggestion_title', with: 'test title'
fill_in 'suggestion_description', with: 'test description'
select 'No', from: "suggestion_signup_preference"
click_button "Submit"

click_link "Logout"
</pre>
After the suggestion is made, login as an instructor, find the assignment, and approve the suggestion.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

num = Suggestion.last.id
path = "/suggestion/" + num.to_s
visit path
click_button "Approve suggestion"

click_link "Logout"
</pre>
In the final step, we check if the new topic is shown in the topic list, then login as student no.5401 again, check if they still hold their old topic.
<pre>
visit 'content_pages/view'
fill_in "User Name", with: 'student5404'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit '/sign_up_sheet/list?assignment_id=711'
expect(page).to have_content('Your topic(s): Amazon S3 and Rails')
click_link "Logout"

visit 'content_pages/view'
fill_in "User Name", with: 'instructor6'
fill_in "Password", with: 'password'
click_button "SIGN IN"

visit "/assignments/711/edit#tabs-2"
expect(page).to have_content("Amazon S3 and Rails")
expect(page).to have_content("test title")
</pre>
=== Result ===