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

From Expertiza_Wiki
Revision as of 01:00, 6 December 2022 by Aumasan (talk | contribs)
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.

Acting on Feedback from previous Pull Request (PR#2472).

  1. Adjusted method names as suggested
  2. Revised the comments section
  3. Removed unnecessary constants used in questions.rb

Codeclimate high severity issue because of usage of the following method "const_get"

This problem was solved by introducing factory methods for both Questionnaire and Questions object. These methods were then added to the corresponding helper functions.

After the introduction of the factory method.

remove unused methods in questionnaires_controller.rb

There were a few methods that were never called throughout the workflow. We checked their use case and removed it accordingly.

Removing unused constants

There were many constants that were initialized in 'questions.rb' which were not used anywhere in the whole system. hence checked its use cases and removed it accordingly

bugFixes

  1. Previously changing the questionnaire's name and then saving it wouldn't save the changes. This was caused due to the original logic that was written in the questionnaires view. Thus we introduced a button for updating the questionnaire parameters.

Add an image here.

  1. The initial logic for assigning the sequence number for a question was to just assign it with (no_of_existing_questions)+1. This won't function properly in cases where the instructor deletes multiple questions and then adds a single question. We solved this issue by deciding the sequence number with max(sequence_number)+1.

Removing the below section 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.

removing the below section 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.


Should we add this? 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 issue we found, but there still exsist a few that we didn't have time to fix. These have been added to future work so that it can be easier for anyone who picks up the project.


Removing below section

Questionnaire_controller

This is the main controller used to create questionnaires in expertiza, there are multiple types of questionnaires in use at the moment as follows:

  • Review Questionnaire
  • Metareview Questionnaire
  • Author Feedback Questionnaire
  • Teammate Review Questionnaire
  • Survey Questionnaire
  • Assignment Survey Questionnaire
  • Global Survey Questionnaire
  • Course Survey Questionnaire
  • Bookmark Rating Questionnaire
  • Quiz Questionnaire

We figure out what type of questionnaire the user wants to create and return an object of that type. These questionnaires have sub-items, which are saved when the questionnaire is saved. The whole questionnaire has to be editted and saved to save an individual question. Currently the individual questions save is also part of the questionnaire_controller. This controller is also responsible for creating a node for the questionnaire and storing it in file structure.

Files Changed

  • app/controllers/questionnaires_controller.rb
  • spec/controllers/questionnaires_controller_spec.rb
  • spec/controllers/questions_controller_spec.rb
  • app/models/question.rb

files added

  • app/helpers/question_helper.rb

Removing this section

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 need to test if the changes do the same function.
  2. Since the questionnaire controller is split into questionnaire and questions controller corresponding tests should be changed accordingly.
  3. Once all the questions are renamed to items. we need to make sure that the questions_controller_spec.rb is changed accordingly

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

Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Fall_2022_-_E2277._Further_Refactoring_questionnaires_controller.rb&oldid=147168"