From 62c67ea635f1cc467c3b3189b5c9aaff33d15480 Mon Sep 17 00:00:00 2001 From: Rafael Reggiani Manzo Date: Tue, 21 Jan 2014 10:18:11 -0200 Subject: [PATCH] Refactored ReadingsGroupsController --- app/controllers/concerns/ownership_authentication.rb | 10 +++++++++- app/controllers/reading_groups_controller.rb | 52 +++++++++++++++++++++++----------------------------- app/controllers/readings_controller.rb | 3 ++- app/views/reading_groups/show.html.erb | 5 +++-- spec/controllers/concerns/ownership_authentication_spec.rb | 18 ++++++++++++++++++ spec/controllers/reading_groups_controller_spec.rb | 1 - spec/controllers/readings_controller_spec.rb | 4 ++-- 7 files changed, 57 insertions(+), 36 deletions(-) create mode 100644 spec/controllers/concerns/ownership_authentication_spec.rb diff --git a/app/controllers/concerns/ownership_authentication.rb b/app/controllers/concerns/ownership_authentication.rb index 8d291d4..c1459ea 100644 --- a/app/controllers/concerns/ownership_authentication.rb +++ b/app/controllers/concerns/ownership_authentication.rb @@ -10,7 +10,15 @@ module OwnershipAuthentication end def reading_group_owner? - check_reading_group_ownership(params[:id]) + if self.kind_of?(ReadingGroupsController) + id = params[:id] + elsif self.kind_of?(ReadingsController) + id = params[:reading_group_id] + else + raise "Not supported" + end + + check_reading_group_ownership(id) end def reading_owner? diff --git a/app/controllers/reading_groups_controller.rb b/app/controllers/reading_groups_controller.rb index 2e13cea..62afa85 100644 --- a/app/controllers/reading_groups_controller.rb +++ b/app/controllers/reading_groups_controller.rb @@ -1,9 +1,9 @@ include OwnershipAuthentication class ReadingGroupsController < ApplicationController - before_action :authenticate_user!, - except: [:index, :show] + before_action :authenticate_user!, except: [:index, :show] before_action :reading_group_owner?, only: [:edit, :update, :destroy] + before_action :set_reading_group, only: [:show, :edit, :update, :destroy] # GET /reading_groups/new def new @@ -27,19 +27,13 @@ class ReadingGroupsController < ApplicationController # GET /reading_group/1 # GET /reading_group/1.json - def show - set_reading_group - @reading_group_readings = @reading_group.readings - end + def show; end # GET /reading_groups/1/edit # GET /reading_groups/1/edit.json - def edit - set_reading_group - end + def edit; end def update - set_reading_group if @reading_group.update(reading_group_params) redirect_to(reading_group_path(@reading_group.id)) else @@ -50,7 +44,6 @@ class ReadingGroupsController < ApplicationController # DELETE /reading_group/1 # DELETE /reading_group/1.json def destroy - set_reading_group current_user.reading_group_ownerships.find_by_reading_group_id(@reading_group.id).destroy @reading_group.destroy respond_to do |format| @@ -60,26 +53,27 @@ class ReadingGroupsController < ApplicationController end private - # Use callbacks to share common setup or constraints between actions. - def set_reading_group - @reading_group = ReadingGroup.find(params[:id]) - end + + # Use callbacks to share common setup or constraints between actions. + def set_reading_group + @reading_group = ReadingGroup.find(params[:id]) + end - # Never trust parameters from the scary internet, only allow the white list through. - def reading_group_params - params[:reading_group] - end + # Never trust parameters from the scary internet, only allow the white list through. + def reading_group_params + params[:reading_group] + end - # Extracted code from create action - def create_and_redir(format) - if @reading_group.save - current_user.reading_group_ownerships.create reading_group_id: @reading_group.id + # Extracted code from create action + def create_and_redir(format) + if @reading_group.save + current_user.reading_group_ownerships.create reading_group_id: @reading_group.id - format.html { redirect_to reading_group_path(@reading_group.id), notice: 'Reading Group was successfully created.' } - format.json { render action: 'show', status: :created, location: @reading_group } - else - format.html { render action: 'new' } - format.json { render json: @reading_group.errors, status: :unprocessable_entity } - end + format.html { redirect_to reading_group_path(@reading_group.id), notice: 'Reading Group was successfully created.' } + format.json { render action: 'show', status: :created, location: @reading_group } + else + format.html { render action: 'new' } + format.json { render json: @reading_group.errors, status: :unprocessable_entity } end + end end diff --git a/app/controllers/readings_controller.rb b/app/controllers/readings_controller.rb index b2550ce..e34b213 100644 --- a/app/controllers/readings_controller.rb +++ b/app/controllers/readings_controller.rb @@ -3,7 +3,8 @@ include OwnershipAuthentication class ReadingsController < ApplicationController before_action :authenticate_user!, except: [:index] before_action :set_reading, only: [:edit, :update, :destroy] - before_action :reading_owner?, except: [:new] + before_action :reading_owner?, except: [:new, :create] + before_action :reading_group_owner?, only: [:new, :create] def new @reading_group_id = params[:reading_group_id] diff --git a/app/views/reading_groups/show.html.erb b/app/views/reading_groups/show.html.erb index e9975b4..c3a895d 100644 --- a/app/views/reading_groups/show.html.erb +++ b/app/views/reading_groups/show.html.erb @@ -23,13 +23,14 @@ - <% if @reading_group_readings.size == 0 %> + <% reading_group_readings = @reading_group.readings %> + <% if reading_group_readings.size == 0 %> <% col_number = reading_groups_owner?(@reading_group.id) ? 5 : 3 %> There are no readings yet! <% end %> - <% @reading_group_readings.each do |reading| %> + <% reading_group_readings.each do |reading| %> <%= reading.label %> <%= reading.grade %> diff --git a/spec/controllers/concerns/ownership_authentication_spec.rb b/spec/controllers/concerns/ownership_authentication_spec.rb new file mode 100644 index 0000000..e1d5fdb --- /dev/null +++ b/spec/controllers/concerns/ownership_authentication_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe OwnershipAuthentication do + #TODO: test other methods + + describe 'reading_group_owner?' do + context 'Not ReadingGroupsController nor ReadingsController' do + before do + @projects_controller = ProjectsController.new # let doesn't work in here + @projects_controller.extend(OwnershipAuthentication) + end + + it 'should raise an exception' do + expect { @projects_controller.reading_group_owner? }.to raise_error("Not supported") + end + end + end +end \ No newline at end of file diff --git a/spec/controllers/reading_groups_controller_spec.rb b/spec/controllers/reading_groups_controller_spec.rb index 7a37e58..0a6d758 100644 --- a/spec/controllers/reading_groups_controller_spec.rb +++ b/spec/controllers/reading_groups_controller_spec.rb @@ -65,7 +65,6 @@ describe ReadingGroupsController do let(:reading) { FactoryGirl.build(:reading) } before :each do ReadingGroup.expects(:find).with(subject.id.to_s).returns(subject) - subject.expects(:readings).returns(reading) get :show, :id => subject.id end diff --git a/spec/controllers/readings_controller_spec.rb b/spec/controllers/readings_controller_spec.rb index 9b5712f..488cdec 100644 --- a/spec/controllers/readings_controller_spec.rb +++ b/spec/controllers/readings_controller_spec.rb @@ -10,7 +10,7 @@ describe ReadingsController do context 'when the current user owns the reading group' do before :each do - subject.expects(:reading_owner?).returns true + subject.expects(:reading_group_owner?).returns true get :new, reading_group_id: reading_group.id end @@ -38,7 +38,7 @@ describe ReadingsController do context 'when the current user owns the reading group' do before :each do - subject.expects(:reading_owner?).returns true + subject.expects(:reading_group_owner?).returns true end context 'with valid fields' do -- libgit2 0.21.2