Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Spring 2019 - Project E1928. Allow reviewers to bid on what to review

2019-04-27T02:21:49Z

Sbekkem: /* Similarity to previous implementation */

== About Expertiza ==
Expertiza is a web application through which students can submit and peer-review learning objects (articles, code, web sites, etc). The Expertiza project is supported by the National Science Foundation. It is used in select courses at NC State and by professors at several other colleges and universities.

== Similarity to previous implementation ==
A similar review bidding feature was implemented in the previous semester, by making use of the '''Top Trading Cycles'''(TTC) algorithm. <br>
Link: [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review] <br>
However, there were a few issues with that implementation, notably:
*The lack of color-coding feature.
Solution: We implmented the color coding feature in our bidding list and the selection list. Following is the screenshot for that implementation.
* Ordering was done by team IDs, which is not a reasonable ordering.
* The content of some newly-added file are very similar with existing ones.
* The UI could have been cleaner.
Solution: Some of their existing code was not DRY and they had some smelly code in their implementation. We have tried to remove most of these inconsistencies in the code.
* There is only one list while topic bidding interface has two lists, one for the the available list of topics and the other list for topics that are selected for the bidding.
Solution: We have implemented the two lists as seen below:
[[File:sri4.JPG]]
[[File:sri5.JPG]]

* There was no "add link to submission" in the bidding list.
Solution: After much deliberation, this feature is not required.
* This implementation altered too many lines of existing Expertiza code.
Solution: The project required us to implement review_bids and student_review models, controllers and view for student_review. We also applied migrations to the existing database to incorporate this functionality.
* Many irrelevant tests were included.

While we would not be writing this implementation from scratch, we would instead be using their [https://github.com/expertiza/expertiza/pull/1322 Pull Request] and remove the possible defects that they had.

== Problem statement ==
Students currently are able to "bid" for topics that they want to do as an assignment. We plan to implement the same bidding feature for reviewing others' work, which is currently assigned on a "first-come-first-serve" basis.

The current first-come-first-serve approach to assigning reviews to students is not efficient since sometimes the same project is requested to be reviewed by multiple students, while other projects are not in demand. By letting the students bid on topics they're interested in, the reviews are assigned more efficiently based on user interest.

In this project we will make use of the Top Trading Cycles algorithm on Expertiza, to implement the review-bidding feature.

== Current Implementation of the bidding ==
Before talking about the functionality of review mapping, it's necessary to describe some of the related models and controllers in Expertiza and how they are organized to fully implement the functionality. It should be noted here that this is an implementation of the bidding feature for assignment topics.

===Models===

[[File:Current_design_review_bidding.jpg]]

1. When an assignment is released, an instance of Assignment is created and corresponding topic instances of SignUpTopics are also imported, indicating that different topics are available for students to choose from within this assignment.

2. Students are assigned as instances of AssignmentParticipant to this assignment and form up their teams(Model Team).

3. Each team registers for several topics and becomes a candidate instance of SignUpTeam. After topics bidding, some of the candidate teams become an official signed-up team of a particular topic.

4. After assignment submission, participants choose their preferred topics and response mappings between assignment(reviewed object), assignment team(reviewee) and assignment participant(reviewer) are created.

5. After the mapping, participants can submit their response and the afterwards is out of discussion range of this document.

===Use Case Diagram===
[[File:Review_use_case.PNG‎]]
===Workflow===

Currently, Expertiza employs FIFO strategy on review assignment. In the ReviewMappingController, the method "add_reviewer" is invoked when students try to request a peer review. When it's done, a new mapping is created, that is why review assignment is in FIFO style.

To understand the basic workflow of bidding on topics, we can look at the LotteryController:

Firstly, teams have different preferences towards available topics. For example, let's say there are three topics within an assignment, namely A, B and C and there are 4 teams W, X, Y, Z. Let's assume each teams preferred order of topics is as follows:

W: B, A, C | X: B, C | Y: C, A, B | Z: A, B, C

This data structure is passed to the back-end service http://peerlogic.csc.ncsu.edu/intelligent_assignment/merge_teams by method "run_intelligent_assignment" of LotteryController. The peerlogic service runs top_trading_cycles algorithm to let the teams have a fair bidding over the topics.

Now, consider a similar bidding approach to peer review assignment. Why is it workable? The following diagram illustrates the similarity of the two bidding model.
[[File:Contrast_topic_review_bidding.jpg]]

As for topics bidding, a few available slots of a particular topic are open for teams contending. In the left part of the diagram, there are 4 slots so only 4 teams can successfully contend for it. Slot is described as "max_choosers" of a sign_up topic in Expertiza. Consequently, there will be 4 different responses after the submission, and this time it is the participants that contend for the responses.

Some discrepancies need additional attention.

1. Slots can be left unoccupied if no team is willing to respond. However, every response needs to have at least some reviewers.

2. The size of bidding data is different, since the number of teams are usually 1/2, 1/3 or 1/4 of the participants.

3. The policy of bidding may be slightly different. Participants are not allowed to review a response submitted by themselves. However, the topic bidding doesn't have this constraint.

== Top Trading Cycles Mechanism ==

In the paper School Choice: A Mechanism Design Approach, author Atila Abdulkadiroglu, and Tayfun Sonmez have described Top Trading Cycles Mechanism. The intuition of the mechanism is students who have the highest priorities are allowed to trade the schools. Students who are assigned to schools are removed and other students who have the highest priority will compete for schools. The mechanism is as follows:

Step1: Each school has a counter (No. of seats/ capacity). Each student has a priority list. Each school points to the student who has the highest priority to the school. A cycle is formed which is an ordered
list (s1, i1, s2, ...., sk,ik). Every student in the cycle points to the school he/she is admitted and removed from the list. The counter of school is reduced by one and when it becomes zero it is removed from the list.

Step k: Remaining students point to their favorite school among the remaining schools and each remaining school points to the student with the highest priority among the remaining students. There is at least one student. Every student in the cycle points to the school he/she is admitted and removed from the list. The counter of school is reduced by one and when it becomes zero it is removed from the list.

The algorithm terminates when all students are assigned a seat.

1. Top Trading Cycles algorithm: [https://www.jstor.org/stable/pdf/3132114.pdf?casa_token=Bp8Syf8Q0yYAAAAA:f7zDJdWurp5cYlHtSHaUxRbDc1o4sL0Qx8UTnk7C5eeXPhgyiTOaWqgftX-nNJ48s6SGyPBzH3_U3Yv6Pfapo1OJaj-estvNyRl60LNQi5QgGW3LmAW8pQ]

== Bidding policy (Stable marriage problem) ==
As part of this assignment, we have to analyse the implementation of the stable marriage problem which is a stable sorting algorithm. The following summary of how the algorithm works, is taken from the "College Admissions and the Stability of Marriage (1962)" by D. Gale and L. S. Shapley. <br><br>
A certain community consists of n men and n women. Each person ranks those of the opposite sex in accordance with his or her preferences for a marriage partner. We
seek a satisfactory way of marrying off all members of the community. Imitating our earlier deﬁnition, we call a set of marriages unstable (and here the suitability of the
term is quite clear) if under it there are a man and a woman who are not married to each other but prefer each other to their actual mates.

'''Deﬁnition:''' An assignment of couples will be called unstable if there are two men α and β who are married to women A and B, respectively, although β prefers A to B and A prefers β to α.

So, we can ask the question: For any pattern of preferences is it possible to ﬁnd a stable set of marriages?

Before giving the answer let us look at some examples.

Example 1. The following is the “ranking matrix” of three men, α, β, and γ , and three women, A, B, and C.

{| class="wikitable"
|-
! Man\Woman !! A !! B !! C
|-
| α || 1,3 || 2,2 || 3,1
|-
| β || 3,1 || 1,3 || 2,2
|-
| γ || 2,2 || 3,1 || 1,3
|}

The ﬁrst number of each pair in the matrix gives the ranking of women by the men, the second number is the ranking of the men by the women. Thus, α ranks A ﬁrst, B second, C third, while A ranks β ﬁrst, γ second, and α third, etc.

There are six possible sets of marriages; of these, three are stable. One of these is realized by giving each man his ﬁrst choice, thus α marries A, β marries B, and γ marries C. Note that although each woman gets her last choice, the arrangement is nevertheless stable. Alternatively one may let the women have their ﬁrst choices and marry α to C, β to A, and γ to B. The third stable arrangement is to give everyone his or her second choice and have α marry B, β marry C, and γ marry A. The reader will easily verify that all other arrangements are unstable.

Based on this approach, in the existing implementation of Expertiza, we have match_new_teams_to_topics method in the lottery_controller that performs this stable sorting algorithm for teams that have bid for an assignment. Our objective would be to implement a similar strategy, but we would use students instead of teams since there are no "team reviews". Every student selects their own assignment to review.

== Design strategy ==
Almost all of the functionality for our project has already been implemented in the review portion of the Expertiza system - where the teams are allowed to bid on topics they want to work for their project. We plan to reuse the code to keep the implementation DRY but we need to add some additional features to make it work for the review bidding functionality. '''Delegation''' is a way to make composition as powerful for reuse as inheritance. The Delegation pattern will help us reduce the coupling of methods to their original class as we have components that seem to behave identically, but we realize that this situation can change in the future.

[[File:Untitled_Diagram.png]]

====Controller====
Because of this, we plan to approach this project by first using delegation pattern to add biding capability to the [https://github.com/expertiza/expertiza/blob/master/app/controllers/review_mapping_controller.rb review mapping controller] for choosing topics. Apart from this, a new controller,[https://github.com/expertiza/expertiza/pull/1322/files#diff-e064188effa2297543b98ac353bba4e3 review bids controller] is implemented to handle interactions similar to the one done in [https://github.com/expertiza/expertiza/blob/master/app/controllers/lottery_controller.rb lottery controller]. It handles the following implementations.

1. Trigger the bidding by sending a request to PeerLogic backend with proper parameters retrieved from ReviewBid records.

2. Process the response from PeerLogic and transform it into records of ReviewResponseMap.

====View====
The bidding interface for reviews is followed from [https://github.com/expertiza/expertiza/pull/778/commits similar] implementation for topic bidding. We need to modify the ''code'' to implement the heat-map showing us the demand density for each project to be reviewed, the list of all available topics to review and the list of topics selected already. <br>

'''Color Coding Scheme:''' <br>
The color coding scheme would range from Red for the most in-demand topics to green for the least contested topics. <br>
The top 10% of in-demand topics will be coded in Red, the next 30% of the topics will be in orange, the next 30% in yellow and the last 30% in the green. We are using percentages instead of hard-coding the color coding scheme so that it works dynamically with changing number of students. <br>
After we allow the student users to bid for their topic of interest we need to provide access to instructors to start the bidding algorithm.

====Model====
To maintain the original functionality of Expertiza as well as to add review bidding, a new model to handle bidding data called ReviewBid is created. ReviewBid contains bidding assignment information, its biddable topics as well as the participants' preference. ReviewBid is served as the input of the bidding algorithm. We decided to let participants bid on assignment teams rather than topics for simplicity.

Here is the definition of ReviewBid:

{| class="wikitable"
!Field Name !!Type !!Description
|-
!id
|int(11)
|unique identifier for the record
|-
!team_id
|int(11)
|the work of team to be bid on
|-
!participant_id
|int(11)
|participant id
|-
!priority
|text
|participant's preference towards the team
|-
!created_at
|timestamp
|
|-
!updated_at
|timestamp
|
|-
|}

== Implementation ==

Allow Instructor to set review strategy to '''bidding''': <br> <br>
[[File:sri1.JPG]] <br> <br>

After setting the review strategy to '''bidding''', participants are assigned a default bidding list, ordered by topic's name.<br> <br>
[[File:Default_bidding_list.png]]<br> <br>

Participants can drag the items up or down to alter the priority.<br> <br>
[[File:Altered_bidding_list.png]]<br> <br>

Instructors can start bidding by clicking the following icon.<br> <br>
[[File:sri3.JPG]]<br> <br>

After the bidding is done, participants can login and see what they are assigned with.<br> <br>
[[File:Bidding_result.png]]<br> <br>

== Test Plan ==

Automated tests are to be written for model ReviewBid, controller ReviewBidController. We also plan to write basic tests to check the correctness of Gale Shapley algorithm.

UI Tests:

In addition to automated tests we will also perform manual testing of the newly added features which include the following:

1. The student should be able to bid the reviews when Professor enables bid option for students on that particular assignment.

2. The reviews prioritized by the student should show colors i.e. green, orange and red indicating how likely he is going to get the review that he bid.

3. At maximum, a student should be able to four assignments.

== References ==

# College Admissions and the Stability of Marriage (1962), D. Gale and L. S. Shapley [https://www-jstor-org.prox.lib.ncsu.edu/stable/2312726?Search=yes&resultItemClick=true&searchText=College&searchText=Admissions&searchText=and&searchText=the&searchText=Stability&searchText=of&searchText=Marriage&searchUri=%2Faction%2FdoBasicSearch%3Fgroup%3Dnone%26amp%3Bfc%3Doff%26amp%3BQuery%3DCollege%2BAdmissions%2Band%2Bthe%2BStability%2Bof%2BMarriage%26amp%3Bwc%3Don%26amp%3Bacc%3Don&ab_segments=0%2Fdefault-2%2Fcontrol&refreqid=search%3A6c17f9ae92b22bc404fc3b7641ac908a&seq=1#metadata_info_tab_contents ]
# CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review]
# Pull Request #1322 (E1856. Allow reviewers to bid on what to review) [https://github.com/expertiza/expertiza/pull/1322 ]
# Top Trading Cycles Algorithm[https://www.jstor.org/stable/pdf/3132114.pdf?casa_token=Bp8Syf8Q0yYAAAAA:f7zDJdWurp5cYlHtSHaUxRbDc1o4sL0Qx8UTnk7C5eeXPhgyiTOaWqgftX-nNJ48s6SGyPBzH3_U3Yv6Pfapo1OJaj-estvNyRl60LNQi5QgGW3LmAW8pQ]

File:Sri5.JPG

2019-04-27T02:20:58Z

Sbekkem:

File:Sri4.JPG

2019-04-27T02:20:47Z

Sbekkem:

CSC/ECE 517 Spring 2019 - Project E1928. Allow reviewers to bid on what to review

2019-04-27T02:15:45Z

Sbekkem: /* Implementation */

== About Expertiza ==
Expertiza is a web application through which students can submit and peer-review learning objects (articles, code, web sites, etc). The Expertiza project is supported by the National Science Foundation. It is used in select courses at NC State and by professors at several other colleges and universities.

== Similarity to previous implementation ==
A similar review bidding feature was implemented in the previous semester, by making use of the '''Top Trading Cycles'''(TTC) algorithm. <br>
Link: [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review] <br>
However, there were a few issues with that implementation, notably:
*The lack of color-coding feature.
Solution: We implmented the color coding feature in our bidding list and the selection list. Following is the screenshot for that implementation.
* Ordering was done by team IDs, which is not a reasonable ordering.
* The content of some newly-added file are very similar with existing ones.
* The UI could have been cleaner.
Solution: Some of their existing code was not DRY and they had some smelly code in their implementation. We have tried to remove most of these inconsistencies in the code.
* There is only one list while topic bidding interface has two lists, one for the the available list of topics and the other list for topics that are selected for the bidding.
Solution: We have implemented the two lists as seen below:
[[File:Dde1.png|thumb|center|400px]]
[[File:Dde2.png]]{ width: 100% !important; height: auto !important; }

* There was no "add link to submission" in the bidding list.
Solution: After much deliberation, this feature is not required.
* This implementation altered too many lines of existing Expertiza code.
Solution: The project required us to implement review_bids and student_review models, controllers and view for student_review. We also applied migrations to the existing database to incorporate this functionality.
* Many irrelevant tests were included.

While we would not be writing this implementation from scratch, we would instead be using their [https://github.com/expertiza/expertiza/pull/1322 Pull Request] and remove the possible defects that they had.

== Problem statement ==
Students currently are able to "bid" for topics that they want to do as an assignment. We plan to implement the same bidding feature for reviewing others' work, which is currently assigned on a "first-come-first-serve" basis.

The current first-come-first-serve approach to assigning reviews to students is not efficient since sometimes the same project is requested to be reviewed by multiple students, while other projects are not in demand. By letting the students bid on topics they're interested in, the reviews are assigned more efficiently based on user interest.

In this project we will make use of the Top Trading Cycles algorithm on Expertiza, to implement the review-bidding feature.

== Current Implementation of the bidding ==
Before talking about the functionality of review mapping, it's necessary to describe some of the related models and controllers in Expertiza and how they are organized to fully implement the functionality. It should be noted here that this is an implementation of the bidding feature for assignment topics.

===Models===

[[File:Current_design_review_bidding.jpg]]

1. When an assignment is released, an instance of Assignment is created and corresponding topic instances of SignUpTopics are also imported, indicating that different topics are available for students to choose from within this assignment.

2. Students are assigned as instances of AssignmentParticipant to this assignment and form up their teams(Model Team).

3. Each team registers for several topics and becomes a candidate instance of SignUpTeam. After topics bidding, some of the candidate teams become an official signed-up team of a particular topic.

4. After assignment submission, participants choose their preferred topics and response mappings between assignment(reviewed object), assignment team(reviewee) and assignment participant(reviewer) are created.

5. After the mapping, participants can submit their response and the afterwards is out of discussion range of this document.

===Use Case Diagram===
[[File:Review_use_case.PNG‎]]
===Workflow===

Currently, Expertiza employs FIFO strategy on review assignment. In the ReviewMappingController, the method "add_reviewer" is invoked when students try to request a peer review. When it's done, a new mapping is created, that is why review assignment is in FIFO style.

To understand the basic workflow of bidding on topics, we can look at the LotteryController:

Firstly, teams have different preferences towards available topics. For example, let's say there are three topics within an assignment, namely A, B and C and there are 4 teams W, X, Y, Z. Let's assume each teams preferred order of topics is as follows:

W: B, A, C | X: B, C | Y: C, A, B | Z: A, B, C

This data structure is passed to the back-end service http://peerlogic.csc.ncsu.edu/intelligent_assignment/merge_teams by method "run_intelligent_assignment" of LotteryController. The peerlogic service runs top_trading_cycles algorithm to let the teams have a fair bidding over the topics.

Now, consider a similar bidding approach to peer review assignment. Why is it workable? The following diagram illustrates the similarity of the two bidding model.
[[File:Contrast_topic_review_bidding.jpg]]

As for topics bidding, a few available slots of a particular topic are open for teams contending. In the left part of the diagram, there are 4 slots so only 4 teams can successfully contend for it. Slot is described as "max_choosers" of a sign_up topic in Expertiza. Consequently, there will be 4 different responses after the submission, and this time it is the participants that contend for the responses.

Some discrepancies need additional attention.

1. Slots can be left unoccupied if no team is willing to respond. However, every response needs to have at least some reviewers.

2. The size of bidding data is different, since the number of teams are usually 1/2, 1/3 or 1/4 of the participants.

3. The policy of bidding may be slightly different. Participants are not allowed to review a response submitted by themselves. However, the topic bidding doesn't have this constraint.

== Top Trading Cycles Mechanism ==

In the paper School Choice: A Mechanism Design Approach, author Atila Abdulkadiroglu, and Tayfun Sonmez have described Top Trading Cycles Mechanism. The intuition of the mechanism is students who have the highest priorities are allowed to trade the schools. Students who are assigned to schools are removed and other students who have the highest priority will compete for schools. The mechanism is as follows:

Step1: Each school has a counter (No. of seats/ capacity). Each student has a priority list. Each school points to the student who has the highest priority to the school. A cycle is formed which is an ordered
list (s1, i1, s2, ...., sk,ik). Every student in the cycle points to the school he/she is admitted and removed from the list. The counter of school is reduced by one and when it becomes zero it is removed from the list.

Step k: Remaining students point to their favorite school among the remaining schools and each remaining school points to the student with the highest priority among the remaining students. There is at least one student. Every student in the cycle points to the school he/she is admitted and removed from the list. The counter of school is reduced by one and when it becomes zero it is removed from the list.

The algorithm terminates when all students are assigned a seat.

1. Top Trading Cycles algorithm: [https://www.jstor.org/stable/pdf/3132114.pdf?casa_token=Bp8Syf8Q0yYAAAAA:f7zDJdWurp5cYlHtSHaUxRbDc1o4sL0Qx8UTnk7C5eeXPhgyiTOaWqgftX-nNJ48s6SGyPBzH3_U3Yv6Pfapo1OJaj-estvNyRl60LNQi5QgGW3LmAW8pQ]

== Bidding policy (Stable marriage problem) ==
As part of this assignment, we have to analyse the implementation of the stable marriage problem which is a stable sorting algorithm. The following summary of how the algorithm works, is taken from the "College Admissions and the Stability of Marriage (1962)" by D. Gale and L. S. Shapley. <br><br>
A certain community consists of n men and n women. Each person ranks those of the opposite sex in accordance with his or her preferences for a marriage partner. We
seek a satisfactory way of marrying off all members of the community. Imitating our earlier deﬁnition, we call a set of marriages unstable (and here the suitability of the
term is quite clear) if under it there are a man and a woman who are not married to each other but prefer each other to their actual mates.

'''Deﬁnition:''' An assignment of couples will be called unstable if there are two men α and β who are married to women A and B, respectively, although β prefers A to B and A prefers β to α.

So, we can ask the question: For any pattern of preferences is it possible to ﬁnd a stable set of marriages?

Before giving the answer let us look at some examples.

Example 1. The following is the “ranking matrix” of three men, α, β, and γ , and three women, A, B, and C.

{| class="wikitable"
|-
! Man\Woman !! A !! B !! C
|-
| α || 1,3 || 2,2 || 3,1
|-
| β || 3,1 || 1,3 || 2,2
|-
| γ || 2,2 || 3,1 || 1,3
|}

The ﬁrst number of each pair in the matrix gives the ranking of women by the men, the second number is the ranking of the men by the women. Thus, α ranks A ﬁrst, B second, C third, while A ranks β ﬁrst, γ second, and α third, etc.

There are six possible sets of marriages; of these, three are stable. One of these is realized by giving each man his ﬁrst choice, thus α marries A, β marries B, and γ marries C. Note that although each woman gets her last choice, the arrangement is nevertheless stable. Alternatively one may let the women have their ﬁrst choices and marry α to C, β to A, and γ to B. The third stable arrangement is to give everyone his or her second choice and have α marry B, β marry C, and γ marry A. The reader will easily verify that all other arrangements are unstable.

Based on this approach, in the existing implementation of Expertiza, we have match_new_teams_to_topics method in the lottery_controller that performs this stable sorting algorithm for teams that have bid for an assignment. Our objective would be to implement a similar strategy, but we would use students instead of teams since there are no "team reviews". Every student selects their own assignment to review.

== Design strategy ==
Almost all of the functionality for our project has already been implemented in the review portion of the Expertiza system - where the teams are allowed to bid on topics they want to work for their project. We plan to reuse the code to keep the implementation DRY but we need to add some additional features to make it work for the review bidding functionality. '''Delegation''' is a way to make composition as powerful for reuse as inheritance. The Delegation pattern will help us reduce the coupling of methods to their original class as we have components that seem to behave identically, but we realize that this situation can change in the future.

[[File:Untitled_Diagram.png]]

====Controller====
Because of this, we plan to approach this project by first using delegation pattern to add biding capability to the [https://github.com/expertiza/expertiza/blob/master/app/controllers/review_mapping_controller.rb review mapping controller] for choosing topics. Apart from this, a new controller,[https://github.com/expertiza/expertiza/pull/1322/files#diff-e064188effa2297543b98ac353bba4e3 review bids controller] is implemented to handle interactions similar to the one done in [https://github.com/expertiza/expertiza/blob/master/app/controllers/lottery_controller.rb lottery controller]. It handles the following implementations.

1. Trigger the bidding by sending a request to PeerLogic backend with proper parameters retrieved from ReviewBid records.

2. Process the response from PeerLogic and transform it into records of ReviewResponseMap.

====View====
The bidding interface for reviews is followed from [https://github.com/expertiza/expertiza/pull/778/commits similar] implementation for topic bidding. We need to modify the ''code'' to implement the heat-map showing us the demand density for each project to be reviewed, the list of all available topics to review and the list of topics selected already. <br>

'''Color Coding Scheme:''' <br>
The color coding scheme would range from Red for the most in-demand topics to green for the least contested topics. <br>
The top 10% of in-demand topics will be coded in Red, the next 30% of the topics will be in orange, the next 30% in yellow and the last 30% in the green. We are using percentages instead of hard-coding the color coding scheme so that it works dynamically with changing number of students. <br>
After we allow the student users to bid for their topic of interest we need to provide access to instructors to start the bidding algorithm.

====Model====
To maintain the original functionality of Expertiza as well as to add review bidding, a new model to handle bidding data called ReviewBid is created. ReviewBid contains bidding assignment information, its biddable topics as well as the participants' preference. ReviewBid is served as the input of the bidding algorithm. We decided to let participants bid on assignment teams rather than topics for simplicity.

Here is the definition of ReviewBid:

{| class="wikitable"
!Field Name !!Type !!Description
|-
!id
|int(11)
|unique identifier for the record
|-
!team_id
|int(11)
|the work of team to be bid on
|-
!participant_id
|int(11)
|participant id
|-
!priority
|text
|participant's preference towards the team
|-
!created_at
|timestamp
|
|-
!updated_at
|timestamp
|
|-
|}

== Implementation ==

Allow Instructor to set review strategy to '''bidding''': <br> <br>
[[File:sri1.JPG]] <br> <br>

After setting the review strategy to '''bidding''', participants are assigned a default bidding list, ordered by topic's name.<br> <br>
[[File:Default_bidding_list.png]]<br> <br>

Participants can drag the items up or down to alter the priority.<br> <br>
[[File:Altered_bidding_list.png]]<br> <br>

Instructors can start bidding by clicking the following icon.<br> <br>
[[File:sri3.JPG]]<br> <br>

After the bidding is done, participants can login and see what they are assigned with.<br> <br>
[[File:Bidding_result.png]]<br> <br>

== Test Plan ==

Automated tests are to be written for model ReviewBid, controller ReviewBidController. We also plan to write basic tests to check the correctness of Gale Shapley algorithm.

UI Tests:

In addition to automated tests we will also perform manual testing of the newly added features which include the following:

1. The student should be able to bid the reviews when Professor enables bid option for students on that particular assignment.

2. The reviews prioritized by the student should show colors i.e. green, orange and red indicating how likely he is going to get the review that he bid.

3. At maximum, a student should be able to four assignments.

== References ==

# College Admissions and the Stability of Marriage (1962), D. Gale and L. S. Shapley [https://www-jstor-org.prox.lib.ncsu.edu/stable/2312726?Search=yes&resultItemClick=true&searchText=College&searchText=Admissions&searchText=and&searchText=the&searchText=Stability&searchText=of&searchText=Marriage&searchUri=%2Faction%2FdoBasicSearch%3Fgroup%3Dnone%26amp%3Bfc%3Doff%26amp%3BQuery%3DCollege%2BAdmissions%2Band%2Bthe%2BStability%2Bof%2BMarriage%26amp%3Bwc%3Don%26amp%3Bacc%3Don&ab_segments=0%2Fdefault-2%2Fcontrol&refreqid=search%3A6c17f9ae92b22bc404fc3b7641ac908a&seq=1#metadata_info_tab_contents ]
# CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review]
# Pull Request #1322 (E1856. Allow reviewers to bid on what to review) [https://github.com/expertiza/expertiza/pull/1322 ]
# Top Trading Cycles Algorithm[https://www.jstor.org/stable/pdf/3132114.pdf?casa_token=Bp8Syf8Q0yYAAAAA:f7zDJdWurp5cYlHtSHaUxRbDc1o4sL0Qx8UTnk7C5eeXPhgyiTOaWqgftX-nNJ48s6SGyPBzH3_U3Yv6Pfapo1OJaj-estvNyRl60LNQi5QgGW3LmAW8pQ]

File:Sri3.JPG

2019-04-27T02:15:09Z

Sbekkem:

CSC/ECE 517 Spring 2019 - Project E1928. Allow reviewers to bid on what to review

2019-04-27T02:13:23Z

2019-04-27T01:59:09Z

Sbekkem: /* Implementation */

== About Expertiza ==
Expertiza is a web application through which students can submit and peer-review learning objects (articles, code, web sites, etc). The Expertiza project is supported by the National Science Foundation. It is used in select courses at NC State and by professors at several other colleges and universities.

== Similarity to previous implementation ==
A similar review bidding feature was implemented in the previous semester, by making use of the '''Top Trading Cycles'''(TTC) algorithm. <br>
Link: [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review] <br>
However, there were a few issues with that implementation, notably:
*The lack of color-coding feature.
Solution: We implmented the color coding feature in our bidding list and the selection list. Following is the screenshot for that implementation.
* Ordering was done by team IDs, which is not a reasonable ordering.
* The content of some newly-added file are very similar with existing ones.
* The UI could have been cleaner.
Solution: Some of their existing code was not DRY and they had some smelly code in their implementation. We have tried to remove most of these inconsistencies in the code.
* There is only one list while topic bidding interface has two lists, one for the the available list of topics and the other list for topics that are selected for the bidding.
Solution: We have implemented the two lists as seen below:
[[File:Dde1.png|400px]]
[[File:Dde2.png|400px]]

* There was no "add link to submission" in the bidding list.
Solution: After much deliberation, this feature is not required.
* This implementation altered too many lines of existing Expertiza code.
Solution: The project required us to implement review_bids and student_review models, controllers and view for student_review. We also applied migrations to the existing database to incorporate this functionality.
* Many irrelevant tests were included.

While we would not be writing this implementation from scratch, we would instead be using their [https://github.com/expertiza/expertiza/pull/1322 Pull Request] and remove the possible defects that they had.

== Problem statement ==
Students currently are able to "bid" for topics that they want to do as an assignment. We plan to implement the same bidding feature for reviewing others' work, which is currently assigned on a "first-come-first-serve" basis.

The current first-come-first-serve approach to assigning reviews to students is not efficient since sometimes the same project is requested to be reviewed by multiple students, while other projects are not in demand. By letting the students bid on topics they're interested in, the reviews are assigned more efficiently based on user interest.

In this project we will make use of the Top Trading Cycles algorithm on Expertiza, to implement the review-bidding feature.

== Current Implementation of the bidding ==
Before talking about the functionality of review mapping, it's necessary to describe some of the related models and controllers in Expertiza and how they are organized to fully implement the functionality. It should be noted here that this is an implementation of the bidding feature for assignment topics.

===Models===

[[File:Current_design_review_bidding.jpg]]

1. When an assignment is released, an instance of Assignment is created and corresponding topic instances of SignUpTopics are also imported, indicating that different topics are available for students to choose from within this assignment.

2. Students are assigned as instances of AssignmentParticipant to this assignment and form up their teams(Model Team).

3. Each team registers for several topics and becomes a candidate instance of SignUpTeam. After topics bidding, some of the candidate teams become an official signed-up team of a particular topic.

4. After assignment submission, participants choose their preferred topics and response mappings between assignment(reviewed object), assignment team(reviewee) and assignment participant(reviewer) are created.

5. After the mapping, participants can submit their response and the afterwards is out of discussion range of this document.

===Use Case Diagram===
[[File:Review_use_case.PNG‎]]
===Workflow===

Currently, Expertiza employs FIFO strategy on review assignment. In the ReviewMappingController, the method "add_reviewer" is invoked when students try to request a peer review. When it's done, a new mapping is created, that is why review assignment is in FIFO style.

To understand the basic workflow of bidding on topics, we can look at the LotteryController:

Firstly, teams have different preferences towards available topics. For example, let's say there are three topics within an assignment, namely A, B and C and there are 4 teams W, X, Y, Z. Let's assume each teams preferred order of topics is as follows:

W: B, A, C | X: B, C | Y: C, A, B | Z: A, B, C

This data structure is passed to the back-end service http://peerlogic.csc.ncsu.edu/intelligent_assignment/merge_teams by method "run_intelligent_assignment" of LotteryController. The peerlogic service runs top_trading_cycles algorithm to let the teams have a fair bidding over the topics.

Now, consider a similar bidding approach to peer review assignment. Why is it workable? The following diagram illustrates the similarity of the two bidding model.
[[File:Contrast_topic_review_bidding.jpg]]

As for topics bidding, a few available slots of a particular topic are open for teams contending. In the left part of the diagram, there are 4 slots so only 4 teams can successfully contend for it. Slot is described as "max_choosers" of a sign_up topic in Expertiza. Consequently, there will be 4 different responses after the submission, and this time it is the participants that contend for the responses.

Some discrepancies need additional attention.

1. Slots can be left unoccupied if no team is willing to respond. However, every response needs to have at least some reviewers.

2. The size of bidding data is different, since the number of teams are usually 1/2, 1/3 or 1/4 of the participants.

3. The policy of bidding may be slightly different. Participants are not allowed to review a response submitted by themselves. However, the topic bidding doesn't have this constraint.

== Top Trading Cycles Mechanism ==

In the paper School Choice: A Mechanism Design Approach, author Atila Abdulkadiroglu, and Tayfun Sonmez have described Top Trading Cycles Mechanism. The intuition of the mechanism is students who have the highest priorities are allowed to trade the schools. Students who are assigned to schools are removed and other students who have the highest priority will compete for schools. The mechanism is as follows:

Step1: Each school has a counter (No. of seats/ capacity). Each student has a priority list. Each school points to the student who has the highest priority to the school. A cycle is formed which is an ordered
list (s1, i1, s2, ...., sk,ik). Every student in the cycle points to the school he/she is admitted and removed from the list. The counter of school is reduced by one and when it becomes zero it is removed from the list.

Step k: Remaining students point to their favorite school among the remaining schools and each remaining school points to the student with the highest priority among the remaining students. There is at least one student. Every student in the cycle points to the school he/she is admitted and removed from the list. The counter of school is reduced by one and when it becomes zero it is removed from the list.

The algorithm terminates when all students are assigned a seat.

1. Top Trading Cycles algorithm: [https://www.jstor.org/stable/pdf/3132114.pdf?casa_token=Bp8Syf8Q0yYAAAAA:f7zDJdWurp5cYlHtSHaUxRbDc1o4sL0Qx8UTnk7C5eeXPhgyiTOaWqgftX-nNJ48s6SGyPBzH3_U3Yv6Pfapo1OJaj-estvNyRl60LNQi5QgGW3LmAW8pQ]

== Bidding policy (Stable marriage problem) ==
As part of this assignment, we have to analyse the implementation of the stable marriage problem which is a stable sorting algorithm. The following summary of how the algorithm works, is taken from the "College Admissions and the Stability of Marriage (1962)" by D. Gale and L. S. Shapley. <br><br>
A certain community consists of n men and n women. Each person ranks those of the opposite sex in accordance with his or her preferences for a marriage partner. We
seek a satisfactory way of marrying off all members of the community. Imitating our earlier deﬁnition, we call a set of marriages unstable (and here the suitability of the
term is quite clear) if under it there are a man and a woman who are not married to each other but prefer each other to their actual mates.

'''Deﬁnition:''' An assignment of couples will be called unstable if there are two men α and β who are married to women A and B, respectively, although β prefers A to B and A prefers β to α.

So, we can ask the question: For any pattern of preferences is it possible to ﬁnd a stable set of marriages?

Before giving the answer let us look at some examples.

Example 1. The following is the “ranking matrix” of three men, α, β, and γ , and three women, A, B, and C.

{| class="wikitable"
|-
! Man\Woman !! A !! B !! C
|-
| α || 1,3 || 2,2 || 3,1
|-
| β || 3,1 || 1,3 || 2,2
|-
| γ || 2,2 || 3,1 || 1,3
|}

The ﬁrst number of each pair in the matrix gives the ranking of women by the men, the second number is the ranking of the men by the women. Thus, α ranks A ﬁrst, B second, C third, while A ranks β ﬁrst, γ second, and α third, etc.

There are six possible sets of marriages; of these, three are stable. One of these is realized by giving each man his ﬁrst choice, thus α marries A, β marries B, and γ marries C. Note that although each woman gets her last choice, the arrangement is nevertheless stable. Alternatively one may let the women have their ﬁrst choices and marry α to C, β to A, and γ to B. The third stable arrangement is to give everyone his or her second choice and have α marry B, β marry C, and γ marry A. The reader will easily verify that all other arrangements are unstable.

Based on this approach, in the existing implementation of Expertiza, we have match_new_teams_to_topics method in the lottery_controller that performs this stable sorting algorithm for teams that have bid for an assignment. Our objective would be to implement a similar strategy, but we would use students instead of teams since there are no "team reviews". Every student selects their own assignment to review.

== Design strategy ==
Almost all of the functionality for our project has already been implemented in the review portion of the Expertiza system - where the teams are allowed to bid on topics they want to work for their project. We plan to reuse the code to keep the implementation DRY but we need to add some additional features to make it work for the review bidding functionality. '''Delegation''' is a way to make composition as powerful for reuse as inheritance. The Delegation pattern will help us reduce the coupling of methods to their original class as we have components that seem to behave identically, but we realize that this situation can change in the future.

[[File:Untitled_Diagram.png]]

====Controller====
Because of this, we plan to approach this project by first using delegation pattern to add biding capability to the [https://github.com/expertiza/expertiza/blob/master/app/controllers/review_mapping_controller.rb review mapping controller] for choosing topics. Apart from this, a new controller,[https://github.com/expertiza/expertiza/pull/1322/files#diff-e064188effa2297543b98ac353bba4e3 review bids controller] is implemented to handle interactions similar to the one done in [https://github.com/expertiza/expertiza/blob/master/app/controllers/lottery_controller.rb lottery controller]. It handles the following implementations.

1. Trigger the bidding by sending a request to PeerLogic backend with proper parameters retrieved from ReviewBid records.

2. Process the response from PeerLogic and transform it into records of ReviewResponseMap.

====View====
The bidding interface for reviews is followed from [https://github.com/expertiza/expertiza/pull/778/commits similar] implementation for topic bidding. We need to modify the ''code'' to implement the heat-map showing us the demand density for each project to be reviewed, the list of all available topics to review and the list of topics selected already. <br>

'''Color Coding Scheme:''' <br>
The color coding scheme would range from Red for the most in-demand topics to green for the least contested topics. <br>
The top 10% of in-demand topics will be coded in Red, the next 30% of the topics will be in orange, the next 30% in yellow and the last 30% in the green. We are using percentages instead of hard-coding the color coding scheme so that it works dynamically with changing number of students. <br>
After we allow the student users to bid for their topic of interest we need to provide access to instructors to start the bidding algorithm.

====Model====
To maintain the original functionality of Expertiza as well as to add review bidding, a new model to handle bidding data called ReviewBid is created. ReviewBid contains bidding assignment information, its biddable topics as well as the participants' preference. ReviewBid is served as the input of the bidding algorithm. We decided to let participants bid on assignment teams rather than topics for simplicity.

Here is the definition of ReviewBid:

{| class="wikitable"
!Field Name !!Type !!Description
|-
!id
|int(11)
|unique identifier for the record
|-
!team_id
|int(11)
|the work of team to be bid on
|-
!participant_id
|int(11)
|participant id
|-
!priority
|text
|participant's preference towards the team
|-
!created_at
|timestamp
|
|-
!updated_at
|timestamp
|
|-
|}

== Implementation ==

Allow Instructor to set review strategy to '''bidding''': <br> <br>
[[File:1.jpg]] <br> <br>

After setting the review strategy to '''bidding''', participants are assigned a default bidding list, ordered by topic's name.<br> <br>
[[File:Default_bidding_list.png]]<br> <br>

Participants can drag the items up or down to alter the priority.<br> <br>
[[File:Altered_bidding_list.png]]<br> <br>

Instructors can start bidding by clicking the following icon.<br> <br>
[[File:Bidding_icon.png]]<br> <br>

After the bidding is done, participants can login and see what they are assigned with.<br> <br>
[[File:Bidding_result.png]]<br> <br>

== Test Plan ==

Automated tests are to be written for model ReviewBid, controller ReviewBidController. We also plan to write basic tests to check the correctness of Gale Shapley algorithm.

UI Tests:

In addition to automated tests we will also perform manual testing of the newly added features which include the following:

1. The student should be able to bid the reviews when Professor enables bid option for students on that particular assignment.

2. The reviews prioritized by the student should show colors i.e. green, orange and red indicating how likely he is going to get the review that he bid.

3. At maximum, a student should be able to four assignments.

== References ==

# College Admissions and the Stability of Marriage (1962), D. Gale and L. S. Shapley [https://www-jstor-org.prox.lib.ncsu.edu/stable/2312726?Search=yes&resultItemClick=true&searchText=College&searchText=Admissions&searchText=and&searchText=the&searchText=Stability&searchText=of&searchText=Marriage&searchUri=%2Faction%2FdoBasicSearch%3Fgroup%3Dnone%26amp%3Bfc%3Doff%26amp%3BQuery%3DCollege%2BAdmissions%2Band%2Bthe%2BStability%2Bof%2BMarriage%26amp%3Bwc%3Don%26amp%3Bacc%3Don&ab_segments=0%2Fdefault-2%2Fcontrol&refreqid=search%3A6c17f9ae92b22bc404fc3b7641ac908a&seq=1#metadata_info_tab_contents ]
# CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review [http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2018/E1856_Allow_reviewers_to_bid_on_what_to_review]
# Pull Request #1322 (E1856. Allow reviewers to bid on what to review) [https://github.com/expertiza/expertiza/pull/1322 ]
# Top Trading Cycles Algorithm[https://www.jstor.org/stable/pdf/3132114.pdf?casa_token=Bp8Syf8Q0yYAAAAA:f7zDJdWurp5cYlHtSHaUxRbDc1o4sL0Qx8UTnk7C5eeXPhgyiTOaWqgftX-nNJ48s6SGyPBzH3_U3Yv6Pfapo1OJaj-estvNyRl60LNQi5QgGW3LmAW8pQ]

File:1.JPG

2019-04-27T01:57:42Z

Sbekkem: uploaded a new version of "File:1.JPG"

2019-04-01T22:53:19Z

Sbekkem:

=E1911 Refactoring criterion.rb=

This page provides a description of the Expertiza based OSS project.

__TOC__

==About Expertiza==

[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

==Problem Statement==
The bulk of the code in criterion is HTML. It is the violation of MVC architecture - Model should not concern itself with how the data is displayed. This code needs to be moved to a partial file, and the partial file needs to be called in all appropriate places which call the criterion’s model methods. Once the logic for the view is moved out of the model, the model should only be left with business logic. This business logic code can also be refactored. Plus there are virtually no comments, properly comment the code.

==About Criterion.rb==
Criterion is one of the types of questions that can be added to a questionnaire. There are many other types of question objects like dropdown that have the implementation of similar methods. As of now, the criterion model holds 4 methods that are called from different places in the application. Each of these methods is storing a string value which contains the necessary HTML lines to display the required functionalities to the user. This string value is then returned to the calling methods from the respective views. This HTML string is then entered wherever necessary. This is ideally the exact function of a partial, and this page needs to be heavily reformatted to use partials instead of returning a string containing html code.

===The pull request===
https://github.com/expertiza/expertiza/pull/1407

===The edit method===
This method is one of the several other methods in other files like criterion.rb which gets called when the type of the question being created in the respective model. In the case of this problem, which deals with criterion type questions, the edit method defined in criterion.rb is called when the question.type is equal to "Criterion". This method allows for a user to create the criterion question and enter the necessary details. This method is called only from two view files - one for creating (_questionnaire.html.erb partial file) and another for editing (edit.html.erb). Both these files are in questionnaires view. Verified this using both RubyMine and the grep command. Another interesting thing to note is the use of "self." to get the attributes associated with question object. When we move this code to a partial, we need to change this in the erb file. The outcome of this method's refactoring will completely remove this method from the model file criterion.rb as there is no business logic involved here.

<pre>
def edit(_count)
html = '<td align="center"><a rel="nofollow" data-method="delete" href="/questions/' + self.id.to_s + '">Remove</a></td>'

html += '<td><input size="6" value="' + self.seq.to_s + '" name="question[' + self.id.to_s + '][seq]"'
html += ' id="question_' + self.id.to_s + '_seq" type="text"></td>'

html += '<td><textarea cols="50" rows="1" name="question[' + self.id.to_s + '][txt]"'
html += ' id="question_' + self.id.to_s + '_txt" placeholder="Edit question content here">' + self.txt + '</textarea></td>'

html += '<td><input size="10" disabled="disabled" value="' + self.type + '" name="question[' + self.id.to_s + '][type]"'
html += ' id="question_' + self.id.to_s + '_type" type="text"></td>'

html += '<td><input size="2" value="' + self.weight.to_s
html += '" name="question[' + self.id.to_s + '][weight]" id="question_' + self.id.to_s + '_weight" type="text"></td>'

html += '<td>text area size <input size="3" value="' + self.size.to_s
html += '" name="question[' + self.id.to_s + '][size]" id="question_' + self.id.to_s + '_size" type="text"></td>'

html += '<td> max_label <input size="10" value="' + self.max_label.to_s + '" name="question[' + self.id.to_s
html += '][max_label]" id="question_' + self.id.to_s + '_max_label" type="text"> min_label <input size="12" value="' + self.min_label.to_s
html += '" name="question[' + self.id.to_s + '][min_label]" id="question_' + self.id.to_s + '_min_label" type="text"></td>'

safe_join(["<tr>".html_safe, "</tr>".html_safe], html.html_safe)
end
</pre>

The safe_join is later used to return the HTML string to the calling view.

===The view_question_text method===
This method has the same explanation as the edit method except the fact that this method is called only during viewing of questionnaires by an instructor. We observed that this method is also called by in student_quiz view. On further analysis, we found that the student_quiz does not use criterion object and has only three options: TrueFalse, MultipleChoiceRadio and MultipleChoiceCheckbox. So no refactoring is required here. Verified this method isn't called anywhere else using RubyMine and grep command. The outcome of this method's refactoring will completely remove this method from the model file criterion.rb as there is no business logic in this method and is completely html code.

<pre>
def view_question_text
html = '<TD align="left"> ' + self.txt + ' </TD>'
html += '<TD align="left">' + self.type + '</TD>'
html += '<td align="center">' + self.weight.to_s + '</TD>'
questionnaire = self.questionnaire
if !self.max_label.nil? && !self.min_label.nil?
html += '<TD align="center"> (' + self.min_label + ') ' + questionnaire.min_question_score.to_s
html += ' to ' + questionnaire.max_question_score.to_s + ' (' + self.max_label + ')</TD>'
else
html += '<TD align="center">' + questionnaire.min_question_score.to_s + ' to ' + questionnaire.max_question_score.to_s + '</TD>'
end

safe_join(["<TR>".html_safe, "</TR>".html_safe], html.html_safe)
end
</pre>

===The complete method===

This method is one of the several other methods in other files like criterion.rb which gets called when the user responds to the question being created in the respective model. In the case of this problem, which deals with criterion type question responses, the complete method defined in criterion.rb is called when the question.type is equal to "Criterion". This method allows for a user to respond to the criterion question and enter the necessary details. This method is called only from one view file i.e. response.html.erb. This file is in response view. Verified this using both RubyMine and the grep command. Another interesting thing to note is the use of "self." to get the attributes associated with question object. When we move this code to a partial, we need to change this in the erb file. The outcome of this method's refactoring will completely remove this method from the model file criterion.rb as there is no business logic involved here.

<pre>
def complete(count, answer = nil, questionnaire_min, questionnaire_max, dropdown_or_scale)
if self.size.nil?
cols = '70'
rows = '1'
else
cols = self.size.split(',')[0]
rows = self.size.split(',')[1]
end

html = '<div><label for="responses_' + count.to_s + '">' + self.txt + '</label></div>'
# show advice for each criterion question
question_advices = QuestionAdvice.where(question_id: self.id).sort_by(&:id)
advice_total_length = 0

question_advices.each do |question_advice|
advice_total_length += question_advice.advice.length if question_advice.advice && question_advice.advice != ""
end

if !question_advices.empty? and advice_total_length > 0
html += '<a id="showAdivce_' + self.id.to_s + '" onclick="showAdvice(' + self.id.to_s + ')">Show advice</a>'
html += '<script>'
html += 'function showAdvice(i){'
html += 'var element = document.getElementById("showAdivce_" + i.toString());'
html += 'var show = element.innerHTML == "Hide advice";'
html += 'if (show){'
html += 'element.innerHTML="Show advice";'
html += '}else{'
html += 'element.innerHTML="Hide advice";}'
html += 'toggleAdvice(i);}'

html += 'function toggleAdvice(i) {'
html += 'var elem = document.getElementById(i.toString() + "_myDiv");'
html += 'if (elem.style.display == "none") {'
html += 'elem.style.display = "";'
html += '} else {'
html += 'elem.style.display = "none";}}'
html += '</script>'

html += '<div id="' + self.id.to_s + '_myDiv" style="display: none;">'
# [2015-10-26] Zhewei:
# best to order advices high to low, e.g., 5 to 1
# each level used to be a link;
# clicking on the link caused the dropbox to be filled in with the corresponding number
question_advices.reverse.each_with_index do |question_advice, index|
html += '<a id="changeScore_>' + self.id.to_s + '" onclick="changeScore(' + count.to_s + ',' + index.to_s + ')">'
html += (self.questionnaire.max_question_score - index).to_s + ' - ' + question_advice.advice + '</a><br/>'
html += '<script>'
html += 'function changeScore(i, j) {'
html += 'var elem = jQuery("#responses_" + i.toString() + "_score");'
html += 'var opts = elem.children("option").length;'
html += 'elem.val((' + self.questionnaire.max_question_score.to_s + ' - j).toString());}'
html += '</script>'
end
html += '</div>'
end

if dropdown_or_scale == 'dropdown'
current_value = ""
current_value += 'data-current-rating =' + answer.answer.to_s if !answer.nil?
html += '<div><select id="responses_' + count.to_s + '_score" name="responses[' + count.to_s + '][score]" class="review-rating" ' + current_value + '>'
html += "<option value = ''>--</option>"
questionnaire_min.upto(questionnaire_max).each do |j|
html += if !answer.nil? and j == answer.answer
'<option value=' + j.to_s + ' selected="selected">'
else
'<option value=' + j.to_s + '>'
end

html += j.to_s
if j == questionnaire_min
html += "-" + self.min_label if self.min_label.present?
elsif j == questionnaire_max
html += "-" + self.max_label if self.max_label.present?
end
html += "</option>"
end
html += "</select></div><br><br>"
html += '<textarea' + ' id="responses_' + count.to_s + '_comments"' \
' name="responses[' + count.to_s + '][comment]" class="tinymce">'
html += answer.comments unless answer.nil?
html += '</textarea></td>'
elsif dropdown_or_scale == 'scale'
html += '<input id="responses_' + count.to_s + '_score" name="responses[' + count.to_s + '][score]" type="hidden"'
html += 'value="' + answer.answer.to_s + '"' unless answer.nil?
html += '>'

html += '<table>'
html += '<tr><td width="10%"></td>'
(questionnaire_min..questionnaire_max).each do |j|
html += '<td width="10%"><label>' + j.to_s + '</label></td>'
end
html += '<td width="10%"></td></tr><tr>'

html += if !self.min_label.nil?
'<td width="10%">' + self.min_label + '</td>'
else
'<td width="10%"></td>'
end
(questionnaire_min..questionnaire_max).each do |j|
html += '<td width="10%"><input type="radio" id="' + j.to_s + '" value="' + j.to_s + '" name="Radio_' + self.id.to_s + '"'
html += 'checked="checked"' if (!answer.nil? and answer.answer == j) or (answer.nil? and questionnaire_min == j)
html += '></td>'
end
html += '<script>jQuery("input[name=Radio_' + self.id.to_s + ']:radio").change(function() {'
html += 'var response_score = jQuery("#responses_' + count.to_s + '_score");'
html += 'var checked_value = jQuery("input[name=Radio_' + self.id.to_s + ']:checked").val();'
html += 'response_score.val(checked_value);});</script>'

html += if !self.max_label.nil?
'<td width="10%">' + self.max_label + '</td>'
else
'<td width="10%"></td>'
end

html += '<td width="10%"></td></tr></table>'
html += '<textarea cols=' + cols + ' rows=' + rows + ' id="responses_' + count.to_s + '_comments"' \
' name="responses[' + count.to_s + '][comment]" class="tinymce">'
html += answer.comments unless answer.nil?
html += '</textarea>'

end
safe_join(["".html_safe, "".html_safe], html.html_safe)
end
</pre>

===The view_completed_question method===

==Proposed Solution==
All these methods contain HTML text. What we propose to do is to pick these lines of code and move them into an appropriate partial, in the required format. The next task is to find out where in the entire application do these methods get called. The multiple overriding of the method calls in this poorly structured application makes it a challenging task. These method-calls then have to be replaced with the appropriate rendering of a partial in the views. This would make the structure more MVC oriented, and help keep it clean and understandable for the next developer who accesses this file.

==Implementation==
The changes proposed above were implemented as described below.

===The Edit Method===
All the code in the model was just html and thus was completely removed from there. It was moved to the partial file (views/questionnaires/_criterion_edit.html.rb) and the necessary changes were made. Since it was all HTML code, no comments were actually required. However, we still added comments describing the partial file itself. Other comments were added whenever changes were made.
====Affected View Files====
'''1. views/questionnaires/edit.html.erb -''' This file calls the edit method for all types of question objects and the object type takes care of invoking the right overridden method. We modified this file to call the partial instead of edit method only for Criterion type of question objects. This view is invoked during edit on the questionnaire. Here are the changes:

<pre>
<% for @question in @questionnaire.questions %>
- <%-# The following line call certain method of the object, which returns html string-%>
+ <%# E1911: Call the criterion_edit partial if question is of type Criterion -%>
+ <% if @question.is_a? Criterion %>
+ <%= render :partial => 'criterion_edit' %>
+ <% else %>
<%=@question.edit%>
+ <% end %>
...
</pre>

'''2. views/questionnaires/_questionnaire.html.erb -''' This partial file is called by new.html.erb when anew question is created. Same logic and changes as above.
'''3. views/questionnaires/_criterion_edit.html.erb -''' This file has all the html code that was earlier in the edit method of the criterion model. We changed the variables from "self.xxx" to "@question.xxx" because self was referring to question object in this context. Apart from this we changed the code to erb format. No more changes were deemed necessary. Please refer to the pull request to refer to the changes in this file as they are pure html and really long to be put in this wiki.

===The view_question_text Method===
This method did not contain any business logic and thus all code was formatted and moved to a new partial file (views/questionnaires/_criterion_view.html.erb). Again, no comments were required due to complete html code and a comment describing the partial was added in addition to other necessary comments in other files.

====Affected View Files====
'''1. views/questionnaires/view.html.erb -''' Modified to invoke the partial if question is of type Criterion only. Also changed the question variable to a instance variable (question => @question) to extend it's scope to the partial. This view is invoked during the view of questions in a questionnaire. Here are the changes:

<pre>
- <%questions.each do |question| %>
- <%if question.is_a? Question%>
- <%=question.view_question_text.html_safe%>
+ <% for @question in questions %>
+ <%# E1911: Call the criterion_view partial if question is of type Criterion %>
+ <%if @question.is_a? Criterion %>
+ <%= render :partial => 'criterion_view' %>
+ <%elsif @question.is_a? Question%>
+ <%= @question.view_question_text.html_safe %>
<%end%>
</pre>

'''2. views/questionnaires/_criterion_view.html.erb -''' This partial file has the moved html code from view_question_text. Note that the "self.xxx" have been updated to "@question.xxx" as self referred to question object discussed in 1. Comment added to describe the partial file. Refer the pull request to look at the code.

===The complete Method===
All the code in the model included html thus was completely removed from there. It was moved to the partial file (app/views/questionnaires/_criterion_edit.html.erb) and the necessary changes were made. Since it was all HTML code, no comments were actually required. However, we still added comments describing the partial file itself. Other comments were added whenever changes were made.
====Affected View Files====
'''1. views/questionnaires/edit.html.erb -''' This file calls the edit method for all types of question objects and the object type takes care of invoking the right overridden method. We modified this file to call the partial instead of edit method only for Criterion type of question objects. This view is invoked during edit on the questionnaire. Here are the changes:

<pre>
<% for @question in @questionnaire.questions %>
- <%-# The following line call certain method of the object, which returns html string-%>
+ <%# E1911: Call the criterion_edit partial if question is of type Criterion -%>
+ <% if @question.is_a? Criterion %>
+ <%= render :partial => 'criterion_edit' %>
+ <% else %>
<%=@question.edit%>
+ <% end %>
...
</pre>

'''2. views/questionnaires/_questionnaire.html.erb -''' This partial file is called by new.html.erb when anew question is created. Same logic and changes as above.
'''3. views/questionnaires/_criterion_edit.html.erb -''' This file has all the html code that was earlier in the edit method of the criterion model. We changed the variables from "self.xxx" to "@question.xxx" because self was referring to question object in this context. Apart from this we changed the code to erb format. No more changes were deemed necessary. Please refer to the pull request to refer to the changes in this file as they are pure html and really long to be put in this wiki.

===The view_completed_question Method===

====Affected View Files====
'''1.

E1911 Refactor Criterion

2019-04-01T21:42:47Z

Sbekkem:

E1911 Refactor Criterion

2019-04-01T21:28:51Z

Sbekkem:

E1911 Refactor Criterion

2019-04-01T21:26:38Z

E1911 Refactor Criterion

2019-03-26T02:12:40Z

Sbekkem:

E1911 Refactor Criterion

2019-03-25T19:54:58Z

Sbekkem:

==E1553. Refactoring the Versions Controller==

This page provides a description of the Expertiza based OSS project.

__TOC__

===About Expertiza===

[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

===Problem Statement===
The following tasks were accomplished in this project:

* Improved the clarity of code by improving the variable and parameter names.
* Long character strings were taken and given appropriate names.
* Handled pagination by a separate helper module, which can be used by multiple controllers.
* Implemented action_allowed for access_control to prevent unauthorized access of methods.
* Prevented displaying of all versions for all users and tables when a user views the index page.
* Added missing CRUD methods to Versions Controller
* Added RSPEC testcases for testing changes done in Versions Controller

===About Versions Controller===
This class manages different versions of reviews. If a reviewer reviews a submission, and after that, the author revises the submission, the next time the reviewer does a review (s)he will create a new version. Sometimes it’s necessary to find the current version of a review; sometimes it’s necessary to find all versions. Similarly, a user may want to delete the current version of a review, or all versions of a review. Pagination of versions helps the user to view a subset of versions at a time. Considering the huge number of versions in the system, it is very useful to have a pagination mechanism and a filtering mechanism which can be applied on the whole set of versions. The idea is to display the versions in an ordered, comprehensible and logical manner. In Expertiza the gem ‘will_paginate’ is used to achieve pagination.

===Current Implementation===

=====Functionality=====
* Any user irrespective of his/ her privileges can view all the versions.
::The versions which a particular user can view should be restricted based on the privileges of the user. For instance, only a user with Administrator privileges should be able to view all the versions in the system. However, that is not the case now. Every user can view all the versions irrespective of whether the user is a student or an administrator.
* Any user can delete any version
::The versions which a particular user can delete should be restricted based on the privileges of the user. For instance, a student should not be allowed to delete any version. According to the current implementation any user can delete any version in the system.
* Filtering of versions were restricted to the current user
::The filtering options on versions were restricted to the current user. Sometimes a user might want to view versions associated with other users. For instance, an instructor might want to view the list of versions created by a particular student. This is not possible with the current implementation.

=====Drawbacks and Solutions=====
* '''Problem 1''': The method paginate_list is doing more than one thing.
::The method paginate_list was building a complex search criteria based on the input params, getting the list of versions from the Database matching this search criteria and then calling the Page API. All these tasks in a single method made it difficult to understand.
<pre>
# For filtering the versions list with proper search and pagination.
def paginate_list(id, user_id, item_type, event, datetime)
# Set up the search criteria
criteria = ''
criteria = criteria + "id = #{id} AND " if id && id.to_i > 0
if current_user_role? == 'Super-Administrator'
criteria = criteria + "whodunnit = #{user_id} AND " if user_id && user_id.to_i > 0
end
criteria = criteria + "whodunnit = #{current_user.try(:id)} AND " if current_user.try(:id) && current_user.try(:id).to_i > 0
criteria = criteria + "item_type = '#{item_type}' AND " if item_type && !(item_type.eql? 'Any')
criteria = criteria + "event = '#{event}' AND " if event && !(event.eql? 'Any')
criteria = criteria + "created_at >= '#{time_to_string(params[:start_time])}' AND "
criteria = criteria + "created_at <= '#{time_to_string(params[:end_time])}' AND "

if current_role == 'Instructor' || current_role == 'Administrator'

end

# Remove the last ' AND '
criteria = criteria[0..-5]

versions = Version.page(params[:page]).order('id').per_page(25).where(criteria)
versions
end
</pre>
* '''Solution''': The implementation has been changed in such a way that the versions which a user is allowed to see depends on the privileges of the user. The approach we have taken is as follows:
**An administrator can see all the versions
**An instructor can see all the versions created by him and other users who are in his course or are participants in the assignments he creates.
**A TA can see all the versions created by him and other users who are in the course for which he/ she assists.
**A Student can see all the versions created by him/ her.
* '''Problem 2''': The search criteria created in the method paginate_list was difficult to comprehend.
::The code which builds the search criteria in the method paginate_list uses many string literals and conditions and is hardly intuitive. The programmer will have to spend some time to understand what the code is really doing.
* '''Solution''': The implementation has been changed. A student is not allowed to delete any versions now. Other types of users, for instance administrators, instructors and TAs are allowed to delete only the versions they are authorized to view.
* '''Problem 3''': The paginate method can be moved to a helper class.
::VersionsController is not the only component which require to paginate items. There are other components too. For instance, the UsersController has to paginate the list of users. Hence the Paginate method can be moved to a helper class which can be accessed by other components as well.
* '''Solution''': The filtering options has also been enhanced. The current user can now choose as part of the version search filter any user from a list of users if the current user is authorized to see the versions created by that user.

===New Implementation===
*The method paginate_list has been split into 2 methods now.
** BuildSearchCriteria – as the name suggests the sole purpose of this method is to build a search criteria based on the input search filters when the current user initiates a search in versions.
** paginate_list – this method will call the paginate API.
:First the search criteria is built, then the criteria is applied to versions in the database to get all versions which matches the criteria and then the retrieved versions are paginated.
<pre>
# pagination.
def paginate_list(versions)
paginate(versions, VERSIONS_PER_PAGE);
end

def BuildSearchCriteria(id, user_id, item_type, event)
# Set up the search criteria
search_criteria = ''
search_criteria = search_criteria + add_id_filter_if_valid(id).to_s
if current_user_role? == 'Super-Administrator'
search_criteria = search_criteria + add_user_filter_for_super_admin(user_id).to_s
end
search_criteria = search_criteria + add_user_filter
search_criteria = search_criteria + add_version_type_filter(item_type).to_s
search_criteria = search_criteria + add_event_filter(event).to_s
search_criteria = search_criteria + add_date_time_filter
search_criteria
end
</pre>
* The string literals and conditions in the method paginate_list were replaced with methods with intuitive names so that the programmer can understand the code more easily. We also removed an empty if clause and a redundant statement.
<pre>
def add_id_filter_if_valid (id)
"id = #{id} AND " if id && id.to_i > 0
end

def add_user_filter_for_super_admin (user_id)
"whodunnit = #{user_id} AND " if user_id && user_id.to_i > 0
end

def add_user_filter
"whodunnit = #{current_user.try(:id)} AND " if current_user.try(:id) && current_user.try(:id).to_i > 0
end

def add_event_filter (event)
"event = '#{event}' AND " if event && !(event.eql? 'Any')
end

def add_date_time_filter
"created_at >= '#{time_to_string(params[:start_time])}' AND " +
"created_at <= '#{time_to_string(params[:end_time])}'"
end

def add_version_type_filter (version_type)
"item_type = '#{version_type}' AND " if version_type && !(version_type.eql? 'Any')
end
</pre>
* The paginate method has been moved to the helper class Pagination_Helper. This new method can be now reused by the different components like UsersController etc. The method receives two parameters, first the list to paginate and second the number of items to be displayed in a page.

<pre>
module PaginationHelper

def paginate (items, number_of_items_per_page)
items.page(params[:page]).per_page(number_of_items_per_page)
end

end
</pre>

===Code improvements===
* Introduced a constant VERSIONS_PER_PAGE and assigned the value 25 to it. The pagination algorithm for VersionsController displays at most 25 versions in a page. The existing implementation uses the value 25 straight in the code and there are few problems associated with such an approach.
** It is not easy to understand what 25 is unless the programmer takes a close look at the code.
** In case if the value 25 is used at more than one places and in future a new requirement comes to show at most 30 versions in a page, all the values will have to be modified. It is not very DRY.
* The VersionsController was overriding AccessHelper - action_allowed? method to return true in all the cases. This was violating the whole purpose of the method action_allowed?. The purpose of this method is to determine whether the user who is triggering a CRUD operation is allowed to do so. So when the current user invokes a CRUD operation, the action_allowed? method is invoked first and if the method returns true the CRUD operation is triggered or else the user is intimated with a message and gracefully exited. Hence, when the action_allowed? method is overridden to return true always, it results in providing unauthorized access to certain users.

<pre>
def action_allowed?
true
end
</pre>

:With the new implementation the AccessHelper - action_allowed? method has been modified in such a way that unauthorized access is prevented. As per the new algorithm, 'new', 'create', 'edit', 'update' cannot be invoked by any user. These operations can be accessed only by ‘papertrail’ gem. Only an ‘Administrator’ or ‘Super-Administrator’ can call 'destroy_all' method. All the other methods are accessible to ‘Administrator’, ‘Super-Administrator’, ‘Instructor’, ‘Teaching Assistant’ and ‘Student’.

<pre>
def action_allowed?
case params[:action]
when 'new', 'create', 'edit', 'update'
#Modifications can only be done by papertrail
return false
when 'destroy_all'
['Super-Administrator',
'Administrator'].include? current_role_name
else
#Allow all others
['Super-Administrator',
'Administrator',
'Instructor',
'Teaching Assistant',
'Student'].include? current_role_name
end
end
</pre>

===Automated Testing using RSPEC===
The current version of expertiza did not have any test for VersionsController. Using the test driven development(TDD) approach, we have added an exhaustive set of RSPEC tests for VersionsController, to test all the modifications we have done to the code of the controller class. The tests use double and stub features of rspec-rails gem, to fake the log in by different users - Administrator, Instructor, Student etc. The tests can be executed "rpec spec" command as shown below.
<pre>
user-expertiza $rspec spec
.
.
.
Finished in 5.39 seconds (files took 25.33 seconds to load)
66 examples, 0 failures

Randomized with seed 19254
.
.
</pre>

===Testing from UI===
Following are a few testcases with respectto our code changes that can be tried from UI:
1. To go to versions index page, type in the following url after logging in:
http://152.46.16.81:3000/versions

2. After logging in as student/instructor or admin : Try accessing the new, create, edit, update actions. These actions are not allowed to any of the users.
http://152.46.16.81:3000/versions/new
This calls the new action. In the current production version of expertiza, it is unhandled and application gives a default 404 page.

3. Another feature that can be tested from UI is Pagination. Try searching for a user's versions and see if the results are paginated or not. Search here:
http://152.46.16.81:3000/versions/search

4. Visit the same URL as step 3, you should see only the students under that instructor in the users dropdown.

===References===

#[https://github.com/expertiza/expertiza Expertiza on GitHub]
#[https://github.com/WintersLt/expertiza GitHub Project Repository Fork]
#[http://expertiza.ncsu.edu/ The live Expertiza website]
#[http://bit.ly/myexpertiza Demo link]
#[http://wikis.lib.ncsu.edu/index.php/Expertiza Expertiza project documentation wiki]
#[https://relishapp.com/rspec Rspec Documentation]
#Clean Code: A handbook of agile software craftsmanship. Author: Robert C Martin

E1911 Refactor Criterion

2019-03-25T19:54:11Z

Sbekkem: Created page with "efewkflewk"

efewkflewk