CSC/ECE 517 Fall 2022 - E2279. Further refactoring and improvement of signup sheet controller

Team

Mentor

• Ed Gehringer (efg)

Team Members

• Juan Benavides (jdbenavi)
• Wei-Chi Chen (wchen37)
• Swetha Sairamakrishnan (ssairam)

Problem Statement

• Provide extensive comments for all the code you view/refactor/modify.
• Ensure that any code changes do not break any functionality. Refactoring method names might cause cascaded updates in other files.

• Address and test effects of add_signup_topics_staggered

• Add_signup_topics_staggered was supposed to make sure that the deadlines are set. [Assignment 1042 has staggered deadlines]. However, it was removed in the last version since it did not do anything different from add_signup_topics.
• Currently in RSpec, it is only implicitly tested in the method create.

• Refactor participants variable in load_add_signup_topics

• [In retrospect, the meaning of this is not clear. The @participants variable is used in a way that is very obscure, with code spread across several views, and no comments saying what it is doing. It used to be that participants (individual students) signed up for topics. Now, only teams can sign up for topics. So @participants does not make sense.] [Has this been fixed?]

• There are nine checks for @assignment.is_intelligent. This method is atrociously named. It simply checks for whether bidding is being used to assign topics.

• Rename the method.
• Figure out some way to remove all or almost all of those checks, which are a gross violation of the Single-Responsibility Principle (as well as cohesion).

• Also see Code Climate issues. To reduce the size of the file, you could move some methods to sign_up_sheet_helper.rb [Has this been done?.]

Prior Work section with last years and last projects background

• CSC/ECE 517 Fall 2022 - E2259. Refactor signup_sheet_controller

We ( @sswetha08 @weichic-ncsu @juanbenavid ) have performed the following refactoring to fix the issues specified for this refactoring project:

• Renamed "sign_up" to be "signup" when it is used as adjective or noun for variable/method/controller name.

• Update method refactored by removing explicit update of every attribute of topic and instead performing this task through update_attributes function provided by rails. Further, update_max_choosers method changed to update_waitlist to support this.

• Removed add_signup_topics_staggered() and replaced it with add_signup_topics() since it does nothing but call add_signup_topics().

• Removed load_add_signup_topics() since it does nothing for add_signup_topics().

• Removed the feature to sign up for a topic as an instructor as there seems to be no way to access this feature from the views.

• List method was too long and has been refactored.

We have also added two new test cases for the controller:

• Destroy signup_sheet when there are other topics in the assignment
• General test case added for rendering list view

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

• DRYed out code used to check for certain errors; however, that was done using an array of messages and then selecting one of them using a constant subscript, whereas it would have been better to pass the messages as parameters.
• The compute_signup_topics method returns results by side-effects (in instance variables); probably this could have been done more cleanly.
• Most of the changes involve regularizing use of "sign_up" vs. "signup"; this should ideally be done in a separate commit so it would be mergeable without merging the whole project.

About Controller

The sign up sheet controller performs the following functions:

• Allows an instructor to add/remove topics to an assignment.
• Allows an instructor to assign/remove students to topics (This has been removed as it is unused/impersonation of student is used instead)
• Allows a student to see the list of available topics which can be bid on for given OSS assignment.

General Design Goals

• Magic Tricks of Testing

• Test incoming command messages by making assertions about direct public side effects.

Plan and List of Work done (specific tasks planned)

• assignment is intelligent? (Swetha)

• participants variable

References to @participants is scattered throughout the project, including some references in signup sheet views. This variable was removed in signup sheet controller in the previous pull request for the Fall 2022 OSS project, but references in views have not been removed. In the OSS project we noticed that test coverage for some of the methods/views in this controller were incomplete, so must make sure the removal of this variable does not cause problems. The naming could also use improvement, as only teams are allowed to signup for projects.

• Address and test effects of add_signup_topics_staggered (Wei-Chi)

• Work done

• RSpec

• When creating a topic which could be found, test the availability in different values of assignment.staggered_deadline.

• Plan

• RSpec

• Test add_signup_topics explicitly with different assignments as the input argument.

• Mandatory

• Test the route from app/views/assignments/edit/_topics.html.erb with different assignment_form.

Test Plan

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.
5. Create signup_sheet when sign_up_topic cannot be found.
6. Destroy signup_sheet when other topics with the same assignment exists.

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

We also ran the rspecs for signup_sheet model and helper just to make sure no functionality was broken.

• rspec spec/helpers/signup_sheet_helper_spec.rb
• rspec spec/models/signup_sheet_spec.rb

New test cases

• Destroy signup_sheet when there are other topics in the assignment
• New, more general test case added for rendering list view, as we noticed that some functionalities we broke while refactoring did not result in failed test cases.

Mandatory

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. Add New Topic or Delete topic using links in the form below

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.

• New Tests

Relevant Links

• https://github.com/weichic-ncsu/expertiza
• Link to Pull Request
• We have deployed the project at http://152.7.99.122:8080/
• Instructor login: username -> instructor6, password -> password
• Student login: username -> student5899, password -> password