From 1c12200003bc16d35325dbf75f743bd94c2f4ddb Mon Sep 17 00:00:00 2001 From: Siddhant Bajaj Date: Mon, 24 Apr 2017 15:39:39 +0530 Subject: [PATCH] Fixed paper trail inconsistent results for numeric values There is a known issue in paper_trail that whenever we Query the 'versions.object' column it evaluates inconsistent results for numeric values due to limitations of SQL wildcard matchers against the serialized objects. So to fix this issue I have manually formed the where query instead of using where_object and where_object_changes. I have also added test for the same. Fixes #1307 --- app/controllers/admin/events_controller.rb | 4 +-- .../admin/events_controller_spec.rb | 26 +++++++++++++++++++ spec/features/versions_spec.rb | 25 ++++++++++++++---- 3 files changed, 48 insertions(+), 7 deletions(-) create mode 100644 spec/controllers/admin/events_controller_spec.rb diff --git a/app/controllers/admin/events_controller.rb b/app/controllers/admin/events_controller.rb index 6cf31ddb..a3b6a3cb 100644 --- a/app/controllers/admin/events_controller.rb +++ b/app/controllers/admin/events_controller.rb @@ -50,8 +50,8 @@ module Admin @ratings = @event.votes.includes(:user) @difficulty_levels = @program.difficulty_levels @versions = @event.versions | - PaperTrail::Version.where(item_type: 'Commercial').where_object(commercialable_id: @event.id, commercialable_type: 'Event') | - PaperTrail::Version.where(item_type: 'Commercial').where_object_changes(commercialable_id: @event.id, commercialable_type: 'Event') | + PaperTrail::Version.where(item_type: 'Commercial').where('object LIKE ?', "%commercialable_id: #{@event.id}\ncommercialable_type: Event%") | + PaperTrail::Version.where(item_type: 'Commercial').where('object_changes LIKE ?', "%commercialable_id:\n- \n- #{@event.id}\ncommercialable_type:\n- \n- Event%") | PaperTrail::Version.where(item_type: 'Vote').where('object_changes LIKE ?', "%\nevent_id:\n- \n- #{@event.id}\n%") | PaperTrail::Version.where(item_type: 'Vote').where('object LIKE ?', "%\nevent_id: #{@event.id}\n%") end diff --git a/spec/controllers/admin/events_controller_spec.rb b/spec/controllers/admin/events_controller_spec.rb new file mode 100644 index 00000000..a413ba7a --- /dev/null +++ b/spec/controllers/admin/events_controller_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe Admin::EventsController do + let(:conference) { create(:conference) } + let(:organizer_role) { Role.find_by(name: 'organizer', resource: conference) } + let(:organizer) { create(:user, role_ids: organizer_role.id) } + let!(:event_without_commercial) { create(:event, program: conference.program) } + let!(:event_with_commercial) { create(:event, program: conference.program) } + let!(:event_commercial) { create(:event_commercial, commercialable: event_with_commercial, url: 'https://www.youtube.com/watch?v=M9bq_alk-sw') } + + with_versioning do + describe 'GET #show' do + before :each do + sign_in(organizer) + get :show, id: event_without_commercial.id, conference_id: conference.short_title + end + + it 'assigns versions' do + versions = event_without_commercial.versions + expect(event_without_commercial.id).to eq event_commercial.id + expect(event_commercial.id).not_to eq event_commercial.commercialable_id + expect(assigns(:versions)).to eq versions + end + end + end +end diff --git a/spec/features/versions_spec.rb b/spec/features/versions_spec.rb index ba43bc27..ff73eee0 100644 --- a/spec/features/versions_spec.rb +++ b/spec/features/versions_spec.rb @@ -4,6 +4,8 @@ feature 'Version' do let!(:conference) { create(:conference) } let!(:organizer_role) { Role.find_by(name: 'organizer', resource: conference) } let!(:organizer) { create(:user, role_ids: [organizer_role.id]) } + let(:event_with_commercial) { create(:event, program: conference.program) } + let(:event_commercial) { create(:event_commercial, commercialable: event_with_commercial, url: 'https://www.youtube.com/watch?v=M9bq_alk-sw') } before(:each) do sign_in organizer @@ -265,15 +267,28 @@ feature 'Version' do end scenario 'display changes in event commercials', feature: true, versioning: true, js: true do - event = create(:event, program: conference.program) - event_commercial = create(:event_commercial, commercialable: event, url: 'https://www.youtube.com/watch?v=M9bq_alk-sw') + event_commercial event_commercial.update_attributes(url: 'https://www.youtube.com/watch?v=VNkDJk5_9eU') event_commercial.destroy visit admin_revision_history_path - expect(page).to have_text("Someone (probably via the console) created new commercial in event #{event.title} in conference #{conference.short_title}") - expect(page).to have_text("Someone (probably via the console) updated url of commercial in event #{event.title} in conference #{conference.short_title}") - expect(page).to have_text("Someone (probably via the console) deleted commercial in event #{event.title} in conference #{conference.short_title}") + expect(page).to have_text("Someone (probably via the console) created new commercial in event #{event_with_commercial.title} in conference #{conference.short_title}") + expect(page).to have_text("Someone (probably via the console) updated url of commercial in event #{event_with_commercial.title} in conference #{conference.short_title}") + expect(page).to have_text("Someone (probably via the console) deleted commercial in event #{event_with_commercial.title} in conference #{conference.short_title}") + end + + scenario 'display changes in event commercials in event history', feature: true, versioning: true, js: true do + event_without_commercial = create(:event, program: conference.program) + event_commercial + + visit admin_conference_program_event_path(conference.short_title, event_with_commercial) + click_link 'History' + expect(page).to have_text('Someone (probably via the console) created new commercial') + visit admin_conference_program_event_path(conference.short_title, event_without_commercial) + click_link 'History' + expect(event_commercial.id).not_to eq event_commercial.commercialable_id + expect(event_without_commercial.id).to eq event_commercial.id + expect(page).to have_no_text('Someone (probably via the console) created new commercial') end scenario 'display changes in users_role', feature: true, versioning: true, js: true do