CSC/ECE 517 Spring 2023 -E2347. Reimplement duties controller.rb and badges controller.rb

From Expertiza_Wiki
Jump to navigation Jump to search

Problem Statement

The problem at hand involves the Duties and Badge modules of Expertiza. Mainly, the task is to improve the code logic and design by following the Object Oriented Design principles and methods and make the code base more efficient, easy to read and structured. We have two controller classes "duties_controller.rb" and "badge_controller.rb".

The duties_controller in the Duties module defines several actions, including create, edit, update, and delete. The create action is used to save the new duty to the database, while the edit action renders the form for editing an existing duty. The update action updates the duty in the database, and the delete action deletes a duty from the database.

On the other hand, the Badge module has a create action that creates a new Badge instance using user parameters and saves an image file if attached. It also updates the image_name attribute of the badge instance. Additionally, there is a new action that sets up a new Badge instance and a redirect_to_assignment method to redirect to the previous page visited by the user. This method is used with the new action to keep track of the page the user was on when they requested a new Badge instance.

The task is to ensure that the Duties and Badge modules function correctly and that the create, edit, update, delete, and new actions all work as intended as well as to optimize, restructure and reimplement the code by following the Object Oriented way. This will help manage duties and badges in Expertiza effectively.

Aim of this Project

The aim of this Project is:

  1. Re-implementing CRUD operations for each controller
  2. Fix bugs and minor flows in the existing code base and functionality
  3. Removing redundant or unclear code logic
  4. Writing RSpec test scripts for both the controllers

By meeting these objectives, both controllers can perform their fundamental create, read, update, and delete operations, while also improving the code's readability and functionality. Additionally, it will aid in addressing any current bugs, thereby enhancing the accuracy of the code.

Design Implementation

We have given two controllers, duties_controller.rb and badges_controller.rb where we have to reimplement the code using a more object-oriented and efficient way. One potential way to refactor this code in a more object-oriented and efficient manner is to extract responsibilities into smaller objects with clear roles and interfaces. This can help reduce complexity and coupling between different parts of the system, making it easier to maintain and extend over time. Additionally, adopting design patterns and principles like SOLID can help improve the overall quality and modularity of the codebase. Overall, investing time and effort in improving the design and architecture of the code can pay off in the long term by reducing technical debt and enabling faster and more effective development.

After gaining a grasp of the system's flow, we delved into the code within the controllers. Upon initial inspection, it became apparent that the controllers were managing all of the CRUD operations but there were some minor changes that can enhance it and make it more readable and easy to understand. We discerned which functionalities did not pertain to the duties_controler as well as the badges_controller and determined which redundant code should be eliminated. Our reimplementation project has addressed the following issues:


  • duties_controller.rb: To reimplement the DutiesController in a more modular, reusable and efficient, we can do the following changes :-
  1. Check if the user is authorized to perform the actions on the resource.
  2. Removed the action_allowed? method.
  3. As the record is already found, we can remove 'find' from 'update_attributes'.
  4. Changed the delete action to destroy for consistency with Rails conventions.
  5. Add a rescue block in set_duty to handle cases when the duty record is not found.
  6. Display errors in a more user friendly format.
  7. Remove 'AuthorizationHelper' since we are using 'authorize_resource'.
  8. Change the parameter fetching of assignment_id to '@duty.assignment_id' as this increases dependency on user input a lot.
  9. Removing unnecessary semicolons.

Here is the previous code of the 'duties_controller.rb' 

# frozen_string_literal: true

class DutiesController < ApplicationController
  include AuthorizationHelper

  # duties can be created/modified by Teaching Assistants, Instructor, Admin, Super Admin
  def action_allowed?
    current_user_has_ta_privileges?
  end

  before_action :set_duty, only: %i[show edit update destroy]

  # GET /duties
  def index
    @duties = Duty.all
  end

  # GET /duties/1
  def show; end

  # GET /duties/new
  def new
    @duty = Duty.new
    @id = params[:id]
  end

  # GET /duties/1/edit
  def edit; end

  # POST /duties
  def create
    @duty = Duty.new(duty_params)

    if @duty.save
      # When the duty (role) is created successfully we return back to the assignment edit page
      redirect_to edit_assignment_path(params[:duty][:assignment_id]), notice: 'Role was successfully created.'
    else
      redirect_to_create_page_and_show_error
    end
  end

  # PATCH/PUT /duties/1
  def update
    @duty = Duty.find(params[:id])

    if @duty.update_attributes(duty_params)
      redirect_to edit_assignment_path(params[:duty][:assignment_id]), notice: 'Role was successfully updated.'
    else
      redirect_to_create_page_and_show_error
    end
  end

  def delete
    @duty = Duty.find(params[:id])
    @duty.destroy
    redirect_to edit_assignment_path(params[:assignment_id]),
                notice: 'Role was successfully deleted.'
  end

  private

  # Use callbacks to share common setup or constraints between actions.
  def set_duty
    @duty = Duty.find(params[:id])
  end

  def redirect_to_create_page_and_show_error
    error_messages = []
    @duty.errors.each { |_, error| error_messages.append(error) }
    error_message = error_messages.join('. ')
    flash[:error] = error_message
    redirect_to action: :new, id: params[:duty][:assignment_id]
  end

  def duty_params
    params.require(:duty).permit(:assignment_id, :max_members_for_duty, :name)
  end
end

The following is the existing code for 'duty.rb': 

class Duty < ApplicationRecord
  belongs_to :assignment
  # validates name with format matching regex, length to be at least 3 and
  # same name cannot be assigned to multiple duties in particular assignment.
  validates :name,
            format: { with: /\A[^`!@#\$%\^&*+_=]+\z/,
                      message: 'Please enter a valid role name' },
            length: {
              minimum: 3,
              message: 'Role name is too short (minimum is 3 characters)'
            },
            uniqueness: {
              case_sensitive: false, scope: :assignment,
              message: 'The role "%{value}" is already present for this assignment'
            }
  validates_numericality_of :max_members_for_duty,
                            only_integer: true,
                            greater_than_or_equal_to: 1,
                            message: 'Value for max members for role is invalid'

  # E2147 : check if the duty selected is available for selection in that particular team. Checks whether
  # current duty count in the team is less than the max_members_for_duty set for that particular duty
  def can_be_assigned?(team)
    max_members_for_duty > team.participants.select { |team_member| team_member.duty_id == id }.count
  end
end
  • badges_controller.rb: To reimplement the BadgesController in a more object-oriented and efficient way, you could split it into a Badge model and a BadgesController controller, following the Model-View-Controller (MVC) pattern. Earlier, all the CRUD methods as well as the dependent methods were written in the same controller class which is not much of the preferred way to write code logic in an MVC pattern and follow the principles. We planned to move the dependent and validation method to the Badge.rb model class where it would be more easier and coder-friendly to understand and work with the code in the future as well.

Here is the previous code of the "badges_controller.rb" 

class BadgesController < ApplicationController
  include AuthorizationHelper

  def action_allowed?
    current_user_has_ta_privileges?
  end

  def new
    @badge = Badge.new
    session[:return_to] ||= request.referer
  end

  def redirect_to_assignment
    redirect_to session.delete(:return_to)
  end

  def create
    @badge = Badge.new(badge_params)
    image_file = params[:badge][:image_file]
    if image_file
      File.open(Rails.root.join('app', 'assets', 'images', 'badges', image_file.original_filename), 'wb') do |file|
        file.write(image_file.read)
      end
      @badge.image_name = image_file.original_filename
    else
      @badge.image_name = ''
    end

    respond_to do |format|
      if @badge.save
        format.html { redirect_to session.delete(:return_to), notice: 'Badge was successfully created' }
      else
        format.html { render :new }
        format.json { render json: @badge.errors, status: :unprocessable_entity }
      end
    end
  end

  def badge_params
    params.require(:badge).permit(:name, :description, :image_name)
  end
end

Here is how we planned to implement the "Badge.rb" model class:

This model will handle the validations and image upload for the Badge object. The upload_image method can be called from the create action in the BadgesController to save the image to the file system. 

class Badge < ApplicationRecord

  def self.upload_image(image_file)
    return '' unless image_file

    image_name = image_file.original_filename
    File.open(Rails.root.join('app', 'assets', 'images', 'badges', image_name), 'wb') do |file|
      file.write(image_file.read)
    end
    image_name
  end
end

Similarly, we planned to re-implement the BadgesController.rb as follows:

This BadgesController now follows the RESTful conventions, which makes it easier to understand and maintain. It also leverages the Badge model to handle the validations and image upload. 

class BadgesController < ApplicationController
  include AuthorizationHelper

  before_action :set_return_to, only: [:new]
  before_action :set_badge, only: [:show, :edit, :update, :destroy]

  def action_allowed?
    current_user_has_ta_privileges?
  end

  # GET /badges
  def index
    @badges = Badge.all
  end

  # GET /badges/new
  def new
    @badge = Badge.new
  end

  # POST /badges
  def create
    @badge = Badge.new(badge_params)
    @badge.image_name = Badge.upload_image(params[:badge][:image_file])

    respond_to do |format|
      if @badge.save
        format.html { redirect_to redirect_to_url, notice: 'Badge was successfully created' }
      else
        format.html { render :new }
        format.json { render json: @badge.errors, status: :unprocessable_entity }
      end
    end
  end

  # PATCH/PUT /badges/1
  def update
    if @badge.update(badge_params)
      redirect_to @badge, notice: 'Badge was successfully updated.'
    else
      render :edit
    end
  end

  private

  def set_badge
    @badge = Badge.find(params[:id])
  end

  def set_return_to
    session[:return_to] ||= request.referer
  end

  def redirect_to_url
    session.delete(:return_to) || badges_url
  end

  def badge_params
    params.require(:badge).permit(:name, :description, :image_name)
  end
end

Testing Methodology

For testing the DutiesController and BadgesController, as well as the Duty and Badge model classes, we plan to employ Postman to test each route of the DutiesController and BadgesController. We plan to test if the expected response is obtained by making valid requests using the correct parameters for each endpoint.

In addition, we will check if each method behaves as anticipated by making requests with an incorrect number of parameters, invalid parameter types, and values, and checked if the endpoint throws a status 422 Invalid Request as the response. Testing via Postman will help ensure that each API endpoint handled both valid and invalid requests and behave as intended.

Moreover, we will be using RSpec test cases. This will allow us to write tests that are independent of any external dependencies and can be easily run to validate the functionality of our code. With RSpec, we can write unit tests that focus on individual pieces of functionality and integration tests that test the interactions between different components. By using RSpec, we can have greater confidence in the correctness of our code and quickly identify any issues that may arise as changes are made. We look forward to utilizing RSpec to ensure that our DutiesController, BadgesController, Duty, and Badge model classes are thoroughly tested and free of errors.

Contributors

This feature was created as part of Dr. Edward Gehringer's "CSC/ECE 517: Object-Oriented Design and Development" class, Spring 2023.
The contributors:
Sanket Tangade(sstangad@ncsu.edu)
Kunal Patil(kpatil5@ncsu.edu)
Yash Sonar(ysonar@ncsu.edu)

Mentor:
Kartiki Bhandakkar(kbhanda3@ncsu.edu)

Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Spring_2023_-E2347._Reimplement_duties_controller.rb_and_badges_controller.rb&oldid=149442"