CSC/ECE 517 Spring 2022 - E2231: Allow reviewers to bid on what to review: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
(Created page with "This wiki page is for the information regarding the changes made for the E2223 OSS assignment for Spring 2022, CSC/ECE 517. == Team == === Mentor === * Ed Gehringer (efg) ===...")
 
 
(67 intermediate revisions by 4 users not shown)
Line 1: Line 1:
This wiki page is for the information regarding the changes made for the E2223 OSS assignment for Spring 2022, CSC/ECE 517.
This wiki page is for the information regarding the changes made for the E2231 OSS assignment for Spring 2022, CSC/ECE 517.
== Team ==
= Team =
=== Mentor ===
=== Mentor ===
* Ed Gehringer (efg)
* Divyang Doshi (ddoshi2)


=== Team Members ===
=== Team Members ===
* Palvit Garg (pgarg5)
* Vijaya Durga Kona (vkona)
* Anand Morlaw (asmorlaw)
* Varun Garg (vgarg4)
* Varun Garg (vgarg4)
* Jagdish Kini (jkini)
* Jagdish Kini (jkini)


== Introduction ==
= Introduction =
Expertiza is a open source project developed on Ruby on Rails.  This web application is maintained by the student and faculty at NC State.  This application gives complete control to the instructor to maintain the assignments in their class.  With multiple functionalities such as adding topics, creating groups, and peer reviews, Expertiza is a well developed application that can handle all types of assignments.  To learn more about the full functionality Expertiza has to offer, visit the Expertiza wiki.
Expertiza is a open source project developed on Ruby on Rails.  This web application is maintained by the student and faculty at NC State.  This application gives complete control to the instructor to maintain the assignments in their class.  With multiple functionalities such as adding topics, creating groups, and peer reviews, Expertiza is a well developed application that can handle all types of assignments.  To learn more about the full functionality Expertiza has to offer, visit the Expertiza wiki.


== Peer Review Information ==
= Problem Statement =
Assigning reviews to users is a complicated process. Currently, as per previous year’s implementation on Expertiza’s beta branch, reviews are assigned using the bidding algorithm. Although, the functionality works, but has some bugs which needs to be fixed before we can take it to production.


For users intending to view the deployed Expertiza associated with this assignment, the details are below:
=== What needs to be done ===
* We have deployed the project http://152.7.98.81:8080/
* Review_bids_others_work is a DRY  violation
* Instructor login: username -> instructor6, password -> password
* Run_bidding algorithm should be assign_reviewers
* Student login: username -> student5899,  password -> password
* Currently it doesn't work if some student does not bid. In this case, algorithm needs to be fixed in order to assign arbitrary reviews to anyone who didn’t bid
* Figure out why running the bidding algorithm only works on assignment with teams. What  if the assignment does not have a team? Can we tweak the code to make it work in this case?
* Include a flag to say how many students have submitted their bids, when the students should submit the bid by to make the functionality more intuitive
* Ability to allow the algorithm to run again after running once.
* Why are the methods in review_bid.rb class methods? Can we change them to instance methods or move it to helpers?
* In the previous implementation wiki, there are edge cases which are not exhaustively tested. Should test those edge cases thoroughly and add more edge case testing
* Test whether the topic changes color depending on the number of outstanding bids


== About Controller ==  
= Design =
The sign up sheet controller
== Problems and planned changes (need to change the content) ==
* Allows an instructor to add/remove topics
* Allows an instructor to assign/remove students to topics
* Allows an student to see the list of available topics which can be bid on for given OSS assignment.


== Issues ==
=== Problem 1: Review_bids_others_work is a DRY violation ===
1. Naming is inconsistent.  When used as a verb, “sign up” is two words.  When an adjective or a noun, it should be one word, e.g., “signup_sheet_controller.”  Cf. “sign_up_as_instructor”. Please make the naming consistent.  Of course, this will result in changes in calling methods as well.
In <code>review_bids_others_work.html.erb</code> in sign_up_sheet folder in views it can be seen that some code is DRY.


2. Add_signup_topics_staggered does not do anything different from add_signup_topics. Separate functions are needed, because add_signup_topics_staggered needs to make sure that the deadlines are set. [Assignment 1042 has staggered deadlines]
*''' Proposed Solution ''': <code>review_bids_others_work.html.erb</code> will be refactored. We will try to find the the repetitive code in it and remove it as required without effective the current implementation.


3. Several method names can be improved.
=== Problem 2: Run_bidding algorithm should be assign_reviewers ===
In this issue, <code>Run_bidding_algorithm</code> algorithm should be assigned a new name which is <code>assign_reviewers</code>.  


4. What are differences between signup_as_instructor and signup_as_instructor_action methods? Investigate if they are needed and improve the method names if both are needed. Provide comments as to what each method does.
*''' Proposed Solution ''': <code>Run_bidding_algorithm</code> will be refactored to a new name which is <code>assign_reviewers</code>. Accordingly, we will have to refractor this on the calling side too.


5. The list method is too long and is sparsely commented. Provide comments and identify if the method can be split or made cleaner by moving code to models or helper methods.
=== Problem 3: Currently it doesn't work if some student does not bid. In this case, the algorithm needs to be fixed in order to assign arbitrary reviews to anyone who didn’t bid ===
This issue is quite an important one. The algorithm currently needs valid bids from all the people and would not work if there is a user who does not bid for any of the projects.
*''' Proposed Solution ''': For the above case, the best solution that should work out is assigning the bids to random projects where a user does not bid for any of the projects to review. The code needs to be refactored and enhanced accordingly to take care of the modification.


6. Signup_as_instructor_action has if-else ladder. It can be made more elegant.
=== Problem 4: Figure out why running the bidding algorithm only works on assignments with teams. What if the assignment does not have a team? Can we tweak the code to make it work in this case? ===
So this is an open-ended issue. Currently, the bidding algorithm only works on assignments with the team. Currently, if the assignment does not have a team then no project gets assigned to the user until a team is formed. Yes, we can tweak the code so that an individual user gets assigned a project, but it won't make much sense as this is supposed to be a group project and there will be limited topics for the same. If the algorithm will go on assigning projects to individual users also then the pool will get exhausted.


7.Delete_signup and delete_signup_as_instructor have much in common and violate the DRY principle. Refactor.
=== Problem 5: Include a flag to say how many students have submitted their bids, when the students should submit the bid by to make the functionality more intuitive ===
For this issue we need to include a count which contains the count of the number of students/teams who have made their bids.


8.Giving elegant and more understandable names to flash in flash_delete_signup_message function
*''' Proposed Solution ''': For this we would include a count in the method <code>assign_bidding</code> of controller <code>review_bids_controller.rb</code>. This attribute will help us to keep track of the number of students bidding for reviews. We will increment this count whenever a team or a student bids for a particular review and decrement it if all the bids are removed by any team. This count will be stored in DB and shown to the users in the view page.


== Solutions ==
=== Problem 6: Ability to allow the algorithm to run again after running once. ===
1. Changed the name of the controller from sign_up_sheet_controller.rb --> signup_sheet_controller.rb. Also changed several method names that had 'sign_up' being used as an adjective or a noun to 'signup'.
Bidding algorithm is allowed to run only once in the TOPICS webpage in the process of managing Assignments. The button <code>Run Review Algorithm</code> is enabled only when the parameter of the assignment <code>can_choose_topic_to_review</code> is true.


https://github.com/palvitgarg99/expertiza/commit/9e99f56c9f8051fa5ee6b6a4bdf3659ec50852dc
*''' Proposed Solution ''': Dependency on <code>can_choose_topic_to_review</code> attribute will be removed for the visibility of the button <code>Run Review Algorithm</code>
https://github.com/palvitgarg99/expertiza/commit/66fc8bc5fc3850639306ad2be79afaa3de1fad70


2. The requirement that deadlines be submitted is no longer implemented and therefore a separate function for "staggered" is no longer needed.
=== Problem 7: Why are the methods in review_bid.rb class methods? Can we change them to instance methods or move it to helpers? ===
The methods in rebiew_bid.rb are to get bidding data, to assign review topics, assign topic to reviewer and for getting individual reviewer_ids bidding data. These all are class methods. If we convert these to the instance methods, then we will have to create an object and then call these methods. So we will have to go through the flow to check whether it fits perfectly. If we are not able to convert these to instance methods, then a better approach would be to move these functions to a helper function.


https://github.com/palvitgarg99/expertiza/commit/db12bf9c05831576183363b6bfe68bbad64e9c5a
=== Problem 8: In the previous implementation wiki, there are edge cases that are not exhaustively tested. Should test those edge cases thoroughly and add more edge case testing ===
This issue is quite critical and pivotal to ensuring the sound functioning of the complete code base. The testing should encapsulate all the edge cases where the code behaves differently than the expectation.


3. Renaming function ad_info
*''' Proposed Solution ''': We need to work on capturing all the scenarios and increasing the number of test cases to encapsulate all the edge cases which will help in the smooth functioning of the application. The code needs to be modified and refactored according to the testing results obtained to handle the additional conditions that are supposedly tricky.


https://github.com/palvitgarg99/expertiza/commit/6507f15f6d23f44900465536a91cc6c80f90ad75
=== Problem 9: Test whether the topic changes color depending on the number of outstanding bids ===
<code>get_topic_bg_color_review_bids</code> in <code>review_bids_helper.rb</code> and <code>review_bids_show.html.erb</code> are responsible for the color changes based on the number of participants and bid size.
*''' Proposed Solution ''': A new test case will be added in <code>review_bids_helper_spec.rb</code> that test the changes in background color based on the number of participants and outstanding size.


4. Remove signup_as_instructor, since its not being used anywhere as such.
= Diagram =
'''Author's View''' : The flow for authors to email reviewers regarding the work they have been reviewed for:


https://github.com/palvitgarg99/expertiza/commit/e92e704f5cb6c549d440784339bcce6eba408a6b
[[File:E2231-1.png|500px]] <br/>


5. The list method is broken into 2 functions. The small part of computing signup topics is created. Also comments has been added to understand the function better.
= Testing =
 
; RSpec
https://github.com/palvitgarg99/expertiza/commit/4b7bb6113dfb8984d7aa4dca467b55f67de3c93a
The main task here of testing is to make sure no functionality is breaking because of the refactoring.
https://github.com/palvitgarg99/expertiza/commit/bd86f5a4ec8aa02c27a0cd34c85bd21776feca2c
https://github.com/palvitgarg99/expertiza/commit/56be9eed8ac89632041608947108f87dfbcb28ab
 
6. Signup_as_instructor_action's if-else ladder has been made more elegant.
 
https://github.com/palvitgarg99/expertiza/commit/50aae43edb7fcec3804deb93df28471db1b31c15
 
7. delete_signup and delete_signup_as_instructor violated the DRY principle due to both functions using identical if/else statements.  To solve this issue, flash_delete_signup_message function was created.
 
https://github.com/palvitgarg99/expertiza/commit/ddf35e88a2486d5ca748d3d93b15d9f6f4984af9
https://github.com/palvitgarg99/expertiza/commit/df1342b5239bb4e49214149b877a5ccf9c6117ae
 
8. Flash messages names have been changed and given for meaningful names.


https://github.com/palvitgarg99/expertiza/commit/47b4b11e4b4b2e6793dbd54259ae36749ed113a7
* If we decide to move the code of review_bid.rb to a helper as discussed as a possible approach for problem 7, then we will have to add some additional test cases to make sure the functionality is still working as necessary.
 
* For Problem 5 if we add a new attribute to count the number of students that have made bids we need to add a test case to check if the count is incremented or decremented correctly
== Testing ==
* For Problem 6, we need to add a test case to check if the <code>Run Review Algorithm</code> button is enabled to run the algorithm again
; RSpec
* For Problem 9, A new test case will be added in <code>review_bids_helper_spec.rb</code> that tests the changes in background color based on the number of participants and outstanding size.
: As such no functionality changed in any function, only refactoring done. All the refactoring was carefully verified by running rspec tests. To run the spec file, run:
* Unit test cases will be added to verify the logic behind the methods to calculate the values for their metrics
: rspec spec/controllers/signup_sheet_controller_spec.rb




Line 88: Line 88:
; Physical Testing
; Physical Testing


We found that add_signup_topics_staggered was simply calling add_signup_topics function. So we eliminated the add_signup_topics_staggered function and manually checked and found that it is working perfectly fine.
* We will have to run the project and check for all possible functionality that might get affected because of the refactoring done.
 
* Also for problem 5 we need to check if the count of bidding is displayed.
Delete signup and delete signup as instructor method successfully broken down to remove the dry code by creating a new helper function. This functionality has not been broken due to any changes and has been checked by running rspec and testing manually.
:  
:  
=== Edge Cases and Pre-Conditions ===
=== Edge Cases and Pre-Conditions ===
# When dropping topic if submission already done.
 
# When deadline from dropping topic has passed.
# Deleting topic when topic cannot be found.
# Signup in case when user cannot be found.
:
:


=== Login as instructor ===
= Reference =
# Hover on Manage Tab
#[https://github.com/expertiza/expertiza Expertiza on GitHub]
# Click on Assignments Tab
#[https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2151._Allow_reviewers_to_bid_on_what_to_review Previous Documentation]
# Click on Edit Button for "OSS project & documentation" assignment
#[https://www.youtube.com/watch?v=wxXALQNWFcc Previous Video]
# Select the Topic Tab
#[https://github.com/expertiza/expertiza/pull/2150 Previous Pull Request]
# Click the Green Check for any topic
# Enter a student UnityID and click submit
 
=== Login as student ===
# Click on OSS project/Writing assignment 2
# Click on Signup sheet
# The list must be visible, which indicates functionality is as before after refactoring list function.

Latest revision as of 02:43, 7 April 2022

This wiki page is for the information regarding the changes made for the E2231 OSS assignment for Spring 2022, CSC/ECE 517.

Team

Mentor

  • Divyang Doshi (ddoshi2)

Team Members

  • Vijaya Durga Kona (vkona)
  • Anand Morlaw (asmorlaw)
  • Varun Garg (vgarg4)
  • Jagdish Kini (jkini)

Introduction

Expertiza is a open source project developed on Ruby on Rails. This web application is maintained by the student and faculty at NC State. This application gives complete control to the instructor to maintain the assignments in their class. With multiple functionalities such as adding topics, creating groups, and peer reviews, Expertiza is a well developed application that can handle all types of assignments. To learn more about the full functionality Expertiza has to offer, visit the Expertiza wiki.

Problem Statement

Assigning reviews to users is a complicated process. Currently, as per previous year’s implementation on Expertiza’s beta branch, reviews are assigned using the bidding algorithm. Although, the functionality works, but has some bugs which needs to be fixed before we can take it to production.

What needs to be done

  • Review_bids_others_work is a DRY violation
  • Run_bidding algorithm should be assign_reviewers
  • Currently it doesn't work if some student does not bid. In this case, algorithm needs to be fixed in order to assign arbitrary reviews to anyone who didn’t bid
  • Figure out why running the bidding algorithm only works on assignment with teams. What if the assignment does not have a team? Can we tweak the code to make it work in this case?
  • Include a flag to say how many students have submitted their bids, when the students should submit the bid by to make the functionality more intuitive
  • Ability to allow the algorithm to run again after running once.
  • Why are the methods in review_bid.rb class methods? Can we change them to instance methods or move it to helpers?
  • In the previous implementation wiki, there are edge cases which are not exhaustively tested. Should test those edge cases thoroughly and add more edge case testing
  • Test whether the topic changes color depending on the number of outstanding bids

Design

Problems and planned changes (need to change the content)

Problem 1: Review_bids_others_work is a DRY violation

In review_bids_others_work.html.erb in sign_up_sheet folder in views it can be seen that some code is DRY.

  • Proposed Solution : review_bids_others_work.html.erb will be refactored. We will try to find the the repetitive code in it and remove it as required without effective the current implementation.

Problem 2: Run_bidding algorithm should be assign_reviewers

In this issue, Run_bidding_algorithm algorithm should be assigned a new name which is assign_reviewers.

  • Proposed Solution : Run_bidding_algorithm will be refactored to a new name which is assign_reviewers. Accordingly, we will have to refractor this on the calling side too.

Problem 3: Currently it doesn't work if some student does not bid. In this case, the algorithm needs to be fixed in order to assign arbitrary reviews to anyone who didn’t bid

This issue is quite an important one. The algorithm currently needs valid bids from all the people and would not work if there is a user who does not bid for any of the projects.

  • Proposed Solution : For the above case, the best solution that should work out is assigning the bids to random projects where a user does not bid for any of the projects to review. The code needs to be refactored and enhanced accordingly to take care of the modification.

Problem 4: Figure out why running the bidding algorithm only works on assignments with teams. What if the assignment does not have a team? Can we tweak the code to make it work in this case?

So this is an open-ended issue. Currently, the bidding algorithm only works on assignments with the team. Currently, if the assignment does not have a team then no project gets assigned to the user until a team is formed. Yes, we can tweak the code so that an individual user gets assigned a project, but it won't make much sense as this is supposed to be a group project and there will be limited topics for the same. If the algorithm will go on assigning projects to individual users also then the pool will get exhausted.

Problem 5: Include a flag to say how many students have submitted their bids, when the students should submit the bid by to make the functionality more intuitive

For this issue we need to include a count which contains the count of the number of students/teams who have made their bids.

  • Proposed Solution : For this we would include a count in the method assign_bidding of controller review_bids_controller.rb. This attribute will help us to keep track of the number of students bidding for reviews. We will increment this count whenever a team or a student bids for a particular review and decrement it if all the bids are removed by any team. This count will be stored in DB and shown to the users in the view page.

Problem 6: Ability to allow the algorithm to run again after running once.

Bidding algorithm is allowed to run only once in the TOPICS webpage in the process of managing Assignments. The button Run Review Algorithm is enabled only when the parameter of the assignment can_choose_topic_to_review is true.

  • Proposed Solution : Dependency on can_choose_topic_to_review attribute will be removed for the visibility of the button Run Review Algorithm

Problem 7: Why are the methods in review_bid.rb class methods? Can we change them to instance methods or move it to helpers?

The methods in rebiew_bid.rb are to get bidding data, to assign review topics, assign topic to reviewer and for getting individual reviewer_ids bidding data. These all are class methods. If we convert these to the instance methods, then we will have to create an object and then call these methods. So we will have to go through the flow to check whether it fits perfectly. If we are not able to convert these to instance methods, then a better approach would be to move these functions to a helper function.

Problem 8: In the previous implementation wiki, there are edge cases that are not exhaustively tested. Should test those edge cases thoroughly and add more edge case testing

This issue is quite critical and pivotal to ensuring the sound functioning of the complete code base. The testing should encapsulate all the edge cases where the code behaves differently than the expectation.

  • Proposed Solution : We need to work on capturing all the scenarios and increasing the number of test cases to encapsulate all the edge cases which will help in the smooth functioning of the application. The code needs to be modified and refactored according to the testing results obtained to handle the additional conditions that are supposedly tricky.

Problem 9: Test whether the topic changes color depending on the number of outstanding bids

get_topic_bg_color_review_bids in review_bids_helper.rb and review_bids_show.html.erb are responsible for the color changes based on the number of participants and bid size.

  • Proposed Solution : A new test case will be added in review_bids_helper_spec.rb that test the changes in background color based on the number of participants and outstanding size.

Diagram

Author's View : The flow for authors to email reviewers regarding the work they have been reviewed for:


Testing

RSpec

The main task here of testing is to make sure no functionality is breaking because of the refactoring.

  • If we decide to move the code of review_bid.rb to a helper as discussed as a possible approach for problem 7, then we will have to add some additional test cases to make sure the functionality is still working as necessary.
  • For Problem 5 if we add a new attribute to count the number of students that have made bids we need to add a test case to check if the count is incremented or decremented correctly
  • For Problem 6, we need to add a test case to check if the Run Review Algorithm button is enabled to run the algorithm again
  • For Problem 9, A new test case will be added in review_bids_helper_spec.rb that tests the changes in background color based on the number of participants and outstanding size.
  • Unit test cases will be added to verify the logic behind the methods to calculate the values for their metrics


Physical Testing
  • We will have to run the project and check for all possible functionality that might get affected because of the refactoring done.
  • Also for problem 5 we need to check if the count of bidding is displayed.

Edge Cases and Pre-Conditions

Reference

  1. Expertiza on GitHub
  2. Previous Documentation
  3. Previous Video
  4. Previous Pull Request