CSC/ECE 517 Spring 2024 - E2447. Reimplement Advice Controller
About Expertiza
Expertiza is a software which benefits both instructors and students by providing platform for various types of submissions and providing reusable objects for peer review. Expertiza is an open-source project developed on Ruby on Rails framework. In Expertiza an instructors can not only create and customize new or existing assignments, but he/she can also create a list of topics and subject in which the students can sign up. Along with that, Students can form their teams and groups to work with on various projects and assignments. Students can also peer-review other students' submissions. This enables students to work together to improve each other’s learning experiences by providing feedbacks.
Project Description
This project involves implementing the AdviceController into the reimplementation repository. The controller has limited functionality where it performs validation checks on advice. We will also be making a few refactoring changes and ensuring full test coverage of the methods on the controller.
Advice_controller first checks whether current user has TA privileges or not by implementing action_allowed? method. Secondly it sets the number of advices based on score and sort it in descending order. Then it checks four conditions for the advices.
Methods
- action_allowed?
- invalid_advice?
- edit_advice
- save_advice
Methods
save_advice
Here is the current implementation of save_advice
# save the advice for a questionnaire def save_advice # Stores the questionnaire with given id in URL @questionnaire = Questionnaire.find(params[:id]) begin # checks if advice is present or not unless params[:advice].nil? params[:advice].keys.each do |advice_key| # Updates the advice corresponding to the key QuestionAdvice.update(advice_key, advice: params[:advice][advice_key.to_sym][:advice]) end flash[:notice] = 'The advice was successfully saved!' end rescue ActiveRecord::RecordNotFound # If record not found, redirects to edit_advice render action: 'edit_advice', id: params[:id] end redirect_to action: 'edit_advice', id: params[:id] end
Plans to refactor
- Simplify the flow since all paths lead to redirect_to 'edit_advice' we can remove the rescue call and replace that with a failed flash message
- Refactor the .each to iterate over params and more clearly handle the update of advice
Refactored code
..
Testing (rspec)
save_advice
Here are the current tests for save_advice. They do have complete coverage for the testing save_advice which I will test before and after the refactoring. I will be modifying the does not save piece to ensure the new flash message on failure shows.
describe '#save_advice' do context "when save_advice is called" do # When ad advice is saved let(:questionAdvice1) {build(:question_advice, id:1, score: 1, question_id: 1, advice: "Advice1")} let(:questionAdvice2) {build(:question_advice, id:2, score: 2, question_id: 1, advice: "Advice2")} let(:questionnaire) do build(:questionnaire, id: 1, min_question_score: 1, questions: [build(:question, id: 1, weight: 2, question_advices: [questionAdvice1,questionAdvice2])], max_question_score: 2) end it "saves advice successfully" do # When an advice is saved successfully allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire) allow(QuestionAdvice).to receive(:update).with('1',{:advice => "Hello"}).and_return("Ok") params = {advice: {"1" => {:advice => "Hello"}}, id: 1} session = {user: instructor1} result = get :save_advice, params: params, session: session expect(flash[:notice]).to eq('The advice was successfully saved!') expect(result.status).to eq 302 expect(result).to redirect_to('/advice/edit_advice?id=1') end it "does not save the advice" do # When an advice is not saved allow(Questionnaire).to receive(:find).with('1').and_return(questionnaire) allow(QuestionAdvice).to receive(:update).with(any_args).and_return("Ok") params = {id: 1} session = {user: instructor1} result = get :save_advice, params: params, session: session expect(flash[:notice]).not_to be_present expect(result.status).to eq 302 expect(result).to redirect_to('/advice/edit_advice?id=1') end end end