CSC/ECE 517 Spring 2022 - E2223. Refactor sign up sheet controller.rb

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)

Team Members

• Palvit Garg (pgarg5)
• 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.

Peer Review Information

For users intending to view the deployed Expertiza associated with this assignment, the details are below:

• We have deployed the project http://152.7.98.81:8080/
• Instructor login: username -> instructor6, password -> password
• Student login: username -> student5899, password -> password

About Controller

The sign up sheet controller

• 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

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.

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]

3. Several method names can be improved.

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.

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.

6. Signup_as_instructor_action has if-else ladder. It can be made more elegant.

7.Delete_signup and delete_signup_as_instructor have much in common and violate the DRY principle. Refactor.

8.Giving elegant and more understandable names to flash in flash_delete_signup_message function

Solutions

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'.

https://github.com/palvitgarg99/expertiza/commit/9e99f56c9f8051fa5ee6b6a4bdf3659ec50852dc 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.

https://github.com/palvitgarg99/expertiza/commit/db12bf9c05831576183363b6bfe68bbad64e9c5a

3. Renaming function ad_info

https://github.com/palvitgarg99/expertiza/commit/6507f15f6d23f44900465536a91cc6c80f90ad75

4. Remove signup_as_instructor, since its not being used anywhere as such.

https://github.com/palvitgarg99/expertiza/commit/e92e704f5cb6c549d440784339bcce6eba408a6b

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.

https://github.com/palvitgarg99/expertiza/commit/4b7bb6113dfb8984d7aa4dca467b55f67de3c93a 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

Testing

RSpec

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:

rspec spec/controllers/signup_sheet_controller_spec.rb

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.

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

1. When dropping topic if submission already done.
2. When deadline from dropping topic has passed.
3. Deleting topic when topic cannot be found.
4. Signup in case when user cannot be found.

Login as instructor

1. Hover on Manage Tab
2. Click on Assignments Tab
3. Click on Edit Button for "OSS project & documentation" assignment
4. Select the Topic Tab
5. Click the Green Check for any topic
6. Enter a student UnityID and click submit

Login as student

1. Click on OSS project/Writing assignment 2
2. Click on Signup sheet
3. The list must be visible, which indicates functionality is as before after refactoring list function.