CSC/ECE 517 Fall 2022 - E2277. Further Refactoring questionnaires controller.rb

From Expertiza_Wiki
Jump to navigation Jump to search

Introduction

This page gives a description of the changes made related to the CURD operations related to the questionnaire of the Expertiza-based OSS project.

Expertiza Background

Expertiza is a web application where students can submit and peer-review learning objects (articles, codes, websites, etc). Instructors add and grade the assignments submitted by students to Expertiza. Students can be assigned in teams based on their selection of the topics. It has functionalities such as peer reviews in which students can provide feedback on others' work which helps peer in better developing the project. The National Science Foundation supports it.

Problems Encountered

This project builds on E2257. That project focused only on simplifying the methods in questionnaires_controller, while this project's objective is wider. It involves looking at any code related to the questionnaire and performing bug fixes, refactoring and cleanup. This code also introduces the changes made in E2257, as PR related to that required some changes to be made before merging. This was done as some of the current changes build upon changes made in the previous round.

Suggestion provided in the documentation of the problem

  • Use of temporary variables instead of instance variables wherever possible to narrow their scope.
  • question.rb contains a large number of constants. Make explicit comments on their usage, and remove them if not used.
  • Since a controller should only create objects of one type the questions_controller could be introduced to create and edit questions moving the logic of dealing with individual questions into a separate controller and improving the separation of responsibility.

What we have considered

  • Checked the flow of the code, by adding comments to figure out unused functions.
  • Check for unused constants.
  • Checked code climate issues found.
  • Found bugs where expected behaviour wasn't happening.

Since this is an important piece of code with usage at a lot of places we had to be very careful with making changes. We were able to fix most of the bugs and issues we found, but there still exist a few that we didn't have enough time to fix. These have been added to future work so that it can be easier for anyone who picks up the project.

The issues we have handled are as follows:

  • Presence of many unused functions in the questionnaire questionnaire_controller.rb
  • Presence of many unused constants in the model questions.rb
  • Unable to update questionnaire parameters.
  • Error in the logic used to assign sequence numbers to questions in a questionnaire.
  • High severity issue in the way questionnaire and question objects are created (Code Climate)
  • Updates made to issues fixed in E2257

Files Changed

  • app/controllers/questionnaires_controller.rb
  • app/models/question.rb
  • app/helpers/questionnaire_helper.rb
  • app/views/questionnaires/_questionnaire.html.erb
  • spec/controllers/questionnaires_controller_spec.rb

Files Added

  • app/helpers/question_helper.rb

Implementation

  • Creating an object of the required questionnaire type is currently performed using Object.const_get, based on user input values. This is a very bad approach (Severity: Critical) according to CodeClimate reports, thus we aim to look at alternative approaches to create an object of the required questionnaire type.
  • As discussed before try to move the save functionality out of questionnaire_controller, and into questions_controller. Call it from there where ever needed.
  • The questionnaire apparently has a lot of items and not just questions, thus try to rename the usages of questions to items.
  • The code has been cleaned many times and functionalities have been moved around a lot, which has made the code messy with a lot of dead code. Have a closer look at each functionality and flow, add comments to each of them (eg. create_node) and remove unnecessary code (eg. many constants).

Flowchart

The following will be the process carried out to move the save functionality from questionnaires_controller into questions_controller. But we will also be be using it on every other functionality in both the controllers just to make sure they are at the right places.

Testing

Testing Plan

  1. Since we are removing the use of 'Object.const_get' we ran the tests again to see that it is functioning properly.

RSPEC Test Cases

The current version of expertiza included tests for the questionnaire_controller and qeustions_controller. we plan to modify a few of these tests so that it fits the changes that we have proposed.

To run the tests, run the following commands from the expertiza directory

rspec ./spec/controllers/questionnaires_controller_spec.rb
rspec ./spec/controllers/questions_controller_spec.rb

Manual Testing

Testing as Instructor

  1. Login as Instructor.
  2. Click on Questionnaires.
  3. Choose a topic and create a new questionnaire.
  4. Check the save functionality.
  5. Add a new question to the questionnaire and check the save question functionality.
  6. delete a question and check if it's reflected properly.

SOLID Principle

SOLID is a set of rules that guide us on how to achieve that in object-oriented programming. Using SOLID guidelines you can avoid bad design in your code and give a well-structured architecture to your design. Bad design leads to an inflexible and brittle codebase, a simple and small change can result in bugs with bad design.

We implemented the single responsibility principle to divide the questionnaires_controller into questionnaires and questions controller thus ensuring that each controller does only one job.


References

Team Members