From 16f294f3e83af0b7a6cdda65c816ed2a14836a45 Mon Sep 17 00:00:00 2001 From: shlok007 Date: Wed, 23 Aug 2017 19:53:35 +0530 Subject: [PATCH] minor inprovements and included organizations in changelog --- app/controllers/admin/versions_controller.rb | 12 +- app/helpers/paths_helper.rb | 8 - app/helpers/versions_helper.rb | 31 ++-- app/models/admin_ability.rb | 2 +- app/models/conference.rb | 4 + app/models/organization.rb | 2 + .../versions/_object_desc_and_link.html.haml | 140 +++++++++++------- app/views/admin/versions/index.html.haml | 2 +- .../admin/versions_controller_spec.rb | 50 ++++++- spec/features/cfp_ability_spec.rb | 2 +- spec/features/info_desk_ability_spec.rb | 2 +- spec/features/versions_spec.rb | 34 ++++- 12 files changed, 199 insertions(+), 90 deletions(-) diff --git a/app/controllers/admin/versions_controller.rb b/app/controllers/admin/versions_controller.rb index 8048595f..5056f3ff 100644 --- a/app/controllers/admin/versions_controller.rb +++ b/app/controllers/admin/versions_controller.rb @@ -1,14 +1,18 @@ module Admin class VersionsController < Admin::BaseController - load_resource :conference, find_by: :short_title + load_resource :conference, find_by: :short_title, only: :index load_and_authorize_resource class: PaperTrail::Version def index - @conf_ids_with_role = current_user.is_admin? ? Conference.pluck(:short_title) : Conference.with_role([:organizer, :cfp, :info_desk], current_user).pluck(:short_title) + @conferences_with_role = current_user.is_admin? ? Conference.pluck(:short_title) : Conference.with_role([:organizer, :cfp, :info_desk], current_user).pluck(:short_title) + + if current_user.has_role? :organization_admin, :any + @conferences_with_role = Organization.with_role('organization_admin', current_user).map { |org| org.conferences.pluck :short_title }.flatten + end + @conferences_with_role.uniq! return if @conference.blank? - authorize! :index, PaperTrail::Version.new(conference_id: @conference.id) - @versions = @versions.where(conference_id: @conference.id) + @versions = PaperTrail::Version.where(conference_id: @conference.id).accessible_by(current_ability) end def revert_attribute diff --git a/app/helpers/paths_helper.rb b/app/helpers/paths_helper.rb index 97de8bca..df01ab2f 100644 --- a/app/helpers/paths_helper.rb +++ b/app/helpers/paths_helper.rb @@ -2,14 +2,6 @@ module PathsHelper ## # Includes functions related to links or redirects ## - def link_to_user(user_id) - user = User.find_by(id: user_id) - if user - link_to user.name, admin_user_path(id: user_id) - else - 'Someone (probably via the console)' - end - end def active_nav_li(link) if current_page?(link) diff --git a/app/helpers/versions_helper.rb b/app/helpers/versions_helper.rb index 10ccef74..5788fe9d 100644 --- a/app/helpers/versions_helper.rb +++ b/app/helpers/versions_helper.rb @@ -6,6 +6,14 @@ module VersionsHelper version.item && conference ? link_to(link_text, link_url) : "#{link_text} with ID #{version.item_id}" end + def link_to_organization(organization_id) + return 'deleted organization' unless organization_id + + org = Organization.find_by(id: organization_id) + return current_or_last_object_state('Organization', organization_id).try(:name) unless org + org.name.to_s + end + def link_to_conference(conference_id) return 'deleted conference' if conference_id.nil? @@ -26,12 +34,12 @@ module VersionsHelper if user link_to user.name, admin_user_path(id: user_id) else - name = current_or_last_object_state('User', user_id).try(:name) + name = current_or_last_object_state('User', user_id).try(:name) || PaperTrail::Version.where(item_type: 'User', item_id: user_id).last.changeset['name'].second if PaperTrail::Version.where(item_type: 'User', item_id: user_id).any? "#{name ? name : 'Unknown user'} with ID #{user_id}" end end - # Recieves a model_name and id + # Receives a model_name and id # Returns nil if model_name is invalid # Returns object in its current state if its alive # Otherwise Returns object state just before deletion @@ -51,30 +59,31 @@ module VersionsHelper end def subscription_change_description(version) - user = current_or_last_object_state(version.item_type, version.item_id).user - user_name = user.name unless user.id.to_s == version.whodunnit + user_id = current_or_last_object_state(version.item_type, version.item_id).user_id + user_name = User.find_by(id: user_id).try(:name) || current_or_last_object_state('User', user_id).try(:name) || PaperTrail::Version.where(item_type: 'User', item_id: user_id).last.changeset[:name].second unless user_id.to_s == version.whodunnit version.event == 'create' ? "subscribed #{user_name} to" : "unsubscribed #{user_name} from" end def registration_change_description(version) if version.item_type == 'Registration' - user = current_or_last_object_state(version.item_type, version.item_id).user + user_id = current_or_last_object_state(version.item_type, version.item_id).user_id elsif version.item_type == 'EventsRegistration' registration_id = current_or_last_object_state(version.item_type, version.item_id).registration_id - user = current_or_last_object_state('Registration', registration_id).user + user_id = current_or_last_object_state('Registration', registration_id).user_id end + user_name = User.find_by(id: user_id).try(:name) || current_or_last_object_state('User', user_id).try(:name) || PaperTrail::Version.where(item_type: 'User', item_id: user_id).last.changeset[:name].second - if user.id.to_s == version.whodunnit + if user_id.to_s == version.whodunnit case version.event when 'create' then 'registered to' when 'update' then "updated #{updated_attributes(version)} of the registration for" - when 'destroy' then 'unregistered from' + when 'destroy' then 'unregistered from' end else case version.event - when 'create' then "registered #{user.name} to" - when 'update' then "updated #{updated_attributes(version)} of #{user.name}'s registration for" - when 'destroy' then "unregistered #{user.name} from" + when 'create' then "registered #{user_name} to" + when 'update' then "updated #{updated_attributes(version)} of #{user_name}'s registration for" + when 'destroy' then "unregistered #{user_name} from" end end end diff --git a/app/models/admin_ability.rb b/app/models/admin_ability.rb index 5c08a073..7fd805d1 100644 --- a/app/models/admin_ability.rb +++ b/app/models/admin_ability.rb @@ -208,7 +208,7 @@ class AdminAbility end can [:index, :revert_object, :revert_attribute], PaperTrail::Version, - item_type: %w(Event EventType Track DifficultyLevel EmailSettings Room Cfp Program Comment), conference_id: conf_ids_for_cfp + item_type: %w[Event EventType Track DifficultyLevel EmailSettings Room Cfp Program Comment], conference_id: conf_ids_for_cfp can [:index, :revert_object, :revert_attribute], PaperTrail::Version, ["item_type = 'Commercial' AND conference_id IN (?) AND (object LIKE '%Event%' OR object_changes LIKE '%Event%')", conf_ids_for_cfp] do |version| version.item_type == 'Commercial' && conf_ids_for_cfp.include?(version.conference_id) && diff --git a/app/models/conference.rb b/app/models/conference.rb index 919bf7e5..034e755b 100644 --- a/app/models/conference.rb +++ b/app/models/conference.rb @@ -760,6 +760,10 @@ class Conference < ActiveRecord::Base self end + def to_param + short_title + end + private # Returns a different html colour for every i and consecutive colors are diff --git a/app/models/organization.rb b/app/models/organization.rb index de3d4f94..0354df5b 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -1,6 +1,8 @@ class Organization < ActiveRecord::Base resourcify :roles, dependent: :delete_all + has_paper_trail + has_many :conferences, dependent: :destroy after_create :create_roles diff --git a/app/views/admin/versions/_object_desc_and_link.html.haml b/app/views/admin/versions/_object_desc_and_link.html.haml index c5b5e84e..12f9dbbc 100644 --- a/app/views/admin/versions/_object_desc_and_link.html.haml +++ b/app/views/admin/versions/_object_desc_and_link.html.haml @@ -1,17 +1,32 @@ -- conference = Conference.find_by(id: version.conference_id) -- conference_short_title = conference.try(:short_title) || current_or_last_object_state(version.item_type, version.item_id).try(:conference).try(:short_title) || '' +- unless version.item_type == 'Role' || version.item_type == 'UsersRole' + - conference = Conference.find_by(id: version.conference_id) + - conference_short_title = conference.try(:short_title) || current_or_last_object_state('Conference', version.conference_id).try(:short_title) || ' ' - case version.item_type +- when 'Organization' + organization + = link_to_organization(version.item_id) + - when 'UsersRole' - users_role = current_or_last_object_state(version.item_type, version.item_id) - = 'role' - = link_to users_role.role.name, admin_conference_role_path(conference.short_title, users_role.role.name) + - role = Role.find_by(id: users_role.role_id) if users_role + role + - if role.name == 'organization_admin' + -# organization_admin belongs to organization and not conferences + - organization = Organization.find(version.conference_id) + = link_if_alive version, role.name, + admins_admin_organization_path(organization), organization + - else + - conference = Conference.find_by(id: version.conference_id) + - conference_short_title = conference.try(:short_title) || current_or_last_object_state('Conference', version.conference_id).try(:short_title) || ' ' + = link_if_alive version, role.try(:name), admin_conference_role_path(role.try(:name) || ' ', conference_short_title), conference + = version.event == 'create' ? 'to' : 'from' - = 'user' + user = link_to_user(users_role.user_id) - when 'Subscription', 'Registration' - = 'conference' + conference = link_to_conference(version.conference_id) - when 'Commercial' @@ -20,72 +35,71 @@ - case commercial.commercialable_type - when 'Event' - = 'commercial in event' + commercial in event - if commercialable && conference = link_to commercialable.title, - admin_conference_program_event_path(conference_id: conference.short_title, - id: commercialable.id) + admin_conference_program_event_path(conference, commercialable.id) - else = commercialable.title = "with ID #{commercialable.id}" - when 'Venue' - = 'commercial in venue' + commercial in venue - if commercialable && conference = link_to commercialable.name, - edit_admin_conference_venue_path(conference_id: conference_short_title, - id: commercialable.id, anchor: 'commercials-content') + edit_admin_conference_venue_path(conference_short_title, + commercialable.id, anchor: 'commercials-content') - else = commercialable.name = "with ID #{commercialable.id}" - when 'Conference' - = 'commercial in conference' + commercial in conference - if commercialable = link_to commercialable.short_title, - admin_conference_commercials_path(conference_id: commercialable.short_title) + admin_conference_commercials_path(commercialable.short_title) - else = commercialable.short_title = "with ID #{commercialable.id}" - when 'EventsRegistration', 'Comment', 'Vote', 'Event' - = 'event' + event - object = current_or_last_object_state(version.item_type, version.item_id) - event_id = object.try(:event_id) || object.try(:commentable_id) || object.id = link_to (current_or_last_object_state('Event', event_id).try(:title) || 'deleted event'), - admin_conference_program_event_path(conference_id: conference_short_title, id: event_id) + admin_conference_program_event_path(conference_short_title, event_id) - when 'Target' - = 'target' + target - target = current_or_last_object_state(version.item_type, version.item_id) - = link_if_alive version, target.to_s, admin_conference_targets_path(conference_id: conference_short_title), conference + = link_if_alive version, target.to_s, admin_conference_targets_path(conference_short_title), conference - when 'EventSchedule' - event_schedule = current_or_last_object_state(version.item_type, version.item_id) event = link_to (current_or_last_object_state('Event', event_schedule.event_id).try(:title) || 'deleted'), - admin_conference_program_event_path(conference_id: conference_short_title, id: event_schedule.event_id) + admin_conference_program_event_path(conference_short_title, event_schedule.event_id) in = link_to "Schedule #{event_schedule.schedule_id}", - admin_conference_schedule_path(conference_id: conference_short_title, id: event_schedule.schedule_id) + admin_conference_schedule_path(conference_short_title, event_schedule.schedule_id) - when 'Schedule' = link_if_alive version, "Schedule #{version.item_id}", - admin_conference_schedule_path(conference_id: conference_short_title, id: version.item_id), + admin_conference_schedule_path(conference_short_title, version.item_id), conference - when 'Conference' - = 'conference' + conference = link_to_conference(version.item_id) - when 'RegistrationPeriod' = link_if_alive version, 'registration period', - admin_conference_registration_period_path(conference_id: conference_short_title), + admin_conference_registration_period_path(conference_short_title), conference - when 'Contact' = link_if_alive version, 'contact details', - edit_admin_conference_contact_path(conference_id: conference_short_title), + edit_admin_conference_contact_path(conference_short_title), conference - when 'Booth' @@ -96,108 +110,120 @@ - when 'Program' = link_if_alive version, 'program', - admin_conference_program_path(conference_id: conference_short_title), + admin_conference_program_path(conference_short_title), conference - when 'Cfp' - = 'cfp for' + cfp for - cfp = current_or_last_object_state(version.item_type, version.item_id) = link_if_alive version, cfp.cfp_type, - admin_conference_program_cfp_path(conference_id: conference_short_title, id: version.item_id), + admin_conference_program_cfp_path(conference_short_title, version.item_id), conference - when 'Track' - = 'track' + track - track = current_or_last_object_state(version.item_type, version.item_id) = link_if_alive version, track.name, - admin_conference_program_track_path(conference_id: conference_short_title, id: track.try(:short_name)), + admin_conference_program_track_path(conference_short_title, track.try(:short_name)), conference - when 'EventType' - = 'event type' + event type - event_type = current_or_last_object_state(version.item_type, version.item_id) = link_if_alive version, event_type.title, - admin_conference_program_event_types_path(conference_id: conference_short_title), + admin_conference_program_event_types_path(conference_short_title), conference - when 'Role' - = 'role' + role - role = current_or_last_object_state(version.item_type, version.item_id) - = link_if_alive version, role.name, - admin_conference_role_path(conference_id: conference_short_title, id: role.name), - conference + - role_name = role.try(:name) || PaperTrail::Version.where(item_type: 'Role', item_id: version.item_id).last.changeset[:name].second + - if role_name == 'organization_admin' + -# organization_admin belongs to organization and not conferences + - organization = Organization.find(version.conference_id) + = link_if_alive version, role_name, + admins_admin_organization_path(organization), organization + - else + - conference = Conference.find_by(id: version.conference_id) + - conference_short_title = conference.try(:short_title) || current_or_last_object_state('Conference', version.conference_id).try(:short_title) || ' ' + = link_if_alive version, role_name, + admin_conference_role_path(conference_short_title, role_name), conference - when 'Venue' - = 'venue' + venue - venue = current_or_last_object_state(version.item_type, version.item_id) = link_if_alive version, venue.name, - admin_conference_venue_path(conference_id: conference_short_title), + admin_conference_venue_path(conference_short_title), conference - when 'Lodging' - = 'lodging' + lodging - lodging = current_or_last_object_state(version.item_type, version.item_id) = link_if_alive version, lodging.name, - admin_conference_lodgings_path(conference_id: conference_short_title), + admin_conference_lodgings_path(conference_short_title), conference - when 'Room' - = 'room' + room - room = current_or_last_object_state(version.item_type, version.item_id) = link_if_alive version, room.name, - admin_conference_venue_rooms_path(conference_id: conference_short_title), + admin_conference_venue_rooms_path(conference_short_title), conference - when 'Sponsor' - = 'sponsor' + sponsor - sponsor = current_or_last_object_state(version.item_type, version.item_id) = link_if_alive version, sponsor.name, - admin_conference_sponsors_path(conference_id: conference_short_title), + admin_conference_sponsors_path(conference_short_title), conference - when 'SponsorshipLevel' - = 'sponsorship level' + sponsorship level - sponsorship_level = current_or_last_object_state(version.item_type, version.item_id) = link_if_alive version, sponsorship_level.title, - admin_conference_sponsorship_levels_path(conference_id: conference_short_title), + admin_conference_sponsorship_levels_path(conference_short_title), conference - when 'Ticket' - = 'ticket' + ticket - ticket = current_or_last_object_state(version.item_type, version.item_id) = link_if_alive version, ticket.title, - admin_conference_ticket_path(conference_id: conference_short_title, id: version.item_id), + admin_conference_ticket_path(conference_short_title, version.item_id), conference - when 'Campaign' - = 'campaign' + campaign - campaign = current_or_last_object_state(version.item_type, version.item_id) = link_if_alive version, campaign.name, - admin_conference_campaigns_path(conference_id: conference_short_title), + admin_conference_campaigns_path(conference_short_title), conference - when 'DifficultyLevel' - = 'difficulty level' + difficulty level - difficulty_level = current_or_last_object_state(version.item_type, version.item_id) = link_if_alive version, difficulty_level.title, - admin_conference_program_difficulty_level_path(conference_id: conference_short_title, id: version.item_id), + admin_conference_program_difficulty_level_path(conference_short_title, version.item_id), conference - when 'Splashpage' = link_if_alive version, 'splashpage', - admin_conference_splashpage_path(conference_id: conference_short_title), + admin_conference_splashpage_path(conference_short_title), conference - when 'EmailSettings' = link_if_alive version, 'email settings', - admin_conference_emails_path(conference_id: conference_short_title), + admin_conference_emails_path(conference_short_title), conference - when 'User' - if version.event == 'update' - = 'user' + user = link_to_user(version.item_id) -- unless %w(Conference Subscription Registration User).include?(version.item_type) - = 'in conference' - = link_to_conference(version.conference_id) +- unless %w(Conference Subscription Registration User Organization).include?(version.item_type) + - if (version.item_type == 'Role' && role_name == 'organization_admin') || (version.item_type == 'UsersRole' && role.name == 'organization_admin') + in organization + = link_to_organization(version.conference_id) + - else + in conference + = link_to_conference(version.conference_id) diff --git a/app/views/admin/versions/index.html.haml b/app/views/admin/versions/index.html.haml index 3a5113b1..963e0a57 100644 --- a/app/views/admin/versions/index.html.haml +++ b/app/views/admin/versions/index.html.haml @@ -8,7 +8,7 @@ %span.caret %ul.dropdown-menu %li= link_to 'All Conferences & Users', admin_revision_history_path - - @conf_ids_with_role.each do |conference_short_title| + - @conferences_with_role.each do |conference_short_title| %li= link_to conference_short_title, admin_conference_revision_history_path(conference_id: conference_short_title) %h1 Revision History diff --git a/spec/controllers/admin/versions_controller_spec.rb b/spec/controllers/admin/versions_controller_spec.rb index f1400e3b..f04bc33d 100644 --- a/spec/controllers/admin/versions_controller_spec.rb +++ b/spec/controllers/admin/versions_controller_spec.rb @@ -4,6 +4,9 @@ describe Admin::VersionsController do let!(:conference) { create(:conference, short_title: 'exampletitle', description: 'Example Description') } let(:admin) { create(:admin) } + let(:role_organizer) { conference.roles.find_by(name: 'organizer') } + let(:role_cfp) { conference.roles.find_by(name: 'cfp') } + let(:role_info_desk) { conference.roles.find_by(name: 'info_desk') } with_versioning do describe 'GET #revert' do @@ -99,11 +102,54 @@ describe Admin::VersionsController do end describe 'GET #index' do - it 'raises error if user is not an organizer of specified conference' do + it 'raises error if user is not of any role' do user = create(:user) sign_in user get :index, conference_id: conference.short_title - expect(flash[:alert]).to match('You are not authorized to access this area.') + expect(flash[:alert]).to match('You are not authorized to access this page.') + end + + context 'with conference' do + before :each do + @user = create(:user) + + conference.update_attributes(short_title: 'testtitle', description: 'Some random text') + @version_organizer = PaperTrail::Version.last + create(:cfp, program: conference.program) + @version_cfp = PaperTrail::Version.last + registration = create(:registration, conference: conference) + registration.update_attributes(attended: true) + @version_info_desk = PaperTrail::Version.last + end + + it 'when user has role cfp' do + @user.roles = [role_cfp] + sign_in @user + get :index, conference_id: conference.short_title + + expect(assigns(:versions).include?(@version_cfp)).to eq true + expect(assigns(:versions).include?(@version_organizer)).to eq false + end + + it 'when user has role info_desk' do + @user.roles = [role_info_desk] + sign_in @user + get :index, conference_id: conference.short_title + + expect(assigns(:versions).include?(@version_info_desk)).to eq true + expect(assigns(:versions).include?(@version_organizer)).to eq false + expect(assigns(:versions).include?(@version_cfp)).to eq false + end + + it 'when user has role organizer' do + @user.roles = [role_organizer] + sign_in @user + get :index, conference_id: conference.short_title + + expect(assigns(:versions).include?(@version_organizer)).to eq true + expect(assigns(:versions).include?(@version_cfp)).to eq true + expect(assigns(:versions).include?(@version_info_desk)).to eq true + end end end end diff --git a/spec/features/cfp_ability_spec.rb b/spec/features/cfp_ability_spec.rb index 1607401c..7229ab8d 100644 --- a/spec/features/cfp_ability_spec.rb +++ b/spec/features/cfp_ability_spec.rb @@ -298,7 +298,7 @@ feature 'Has correct abilities' do expect(current_path).to eq(root_path) visit admin_revision_history_path - expect(current_path).to eq(root_path) + expect(current_path).to eq(admin_revision_history_path) end end end diff --git a/spec/features/info_desk_ability_spec.rb b/spec/features/info_desk_ability_spec.rb index c005f9de..f4ef4fce 100644 --- a/spec/features/info_desk_ability_spec.rb +++ b/spec/features/info_desk_ability_spec.rb @@ -160,7 +160,7 @@ feature 'Has correct abilities' do expect(current_path).to eq(edit_admin_conference_resource_path(conference.short_title, conference.resources.first)) visit admin_revision_history_path - expect(current_path).to eq(root_path) + expect(current_path).to eq(admin_revision_history_path) visit admin_conference_path(conference.short_title) expect(current_path).to eq(admin_conference_path(conference.short_title)) diff --git a/spec/features/versions_spec.rb b/spec/features/versions_spec.rb index 2f1f7f8e..0db2d1f9 100644 --- a/spec/features/versions_spec.rb +++ b/spec/features/versions_spec.rb @@ -93,7 +93,7 @@ feature 'Version' do expect(page).to have_text("Someone (probably via the console) deleted lodging Hotel XYZ with ID #{lodging_id} in conference #{conference.short_title}") end - scenario 'display changes in role', feature: true, versioning: true, js: true do + scenario 'display changes in conference role', feature: true, versioning: true, js: true do visit edit_admin_conference_role_path(conference.short_title, 'cfp') fill_in 'role_description', with: 'For the members of the call for papers team' click_button 'Update Role' @@ -301,14 +301,40 @@ feature 'Version' do 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 + scenario 'display changes in organization', feature: true, versioning: true, js: true do + admin = create(:admin) + sign_in admin + + visit new_admin_organization_path + fill_in 'organization_name', with: 'New org' + click_button 'Create Organization' + + visit admin_revision_history_path + expect(page).to have_text('created new organization New org') + end + + scenario 'display changes in users_role for organization role', feature: true, versioning: true, js: true do user = create(:user) + role = Role.find_by(resource_id: conference.organization.id, resource_type: 'Organization') + user.add_role :organization_admin, conference.organization + user_role = UsersRole.find_by(user_id: user.id, role_id: role.id) + user.remove_role :organization_admin, conference.organization + + visit admin_revision_history_path + expect(page).to have_text("added role organization_admin with ID #{user_role.id} to user #{user.name} in organization #{conference.organization.name}") + expect(page).to have_text("removed role organization_admin with ID #{user_role.id} from user #{user.name} in organization #{conference.organization.name}") + end + + scenario 'display changes in users_role for conference role', feature: true, versioning: true, js: true do + user = create(:user) + role = Role.find_by(name: 'cfp', resource_id: conference.id, resource_type: 'Conference') user.add_role :cfp, conference + user_role = UsersRole.find_by(user_id: user.id, role_id: role.id) user.remove_role :cfp, conference visit admin_revision_history_path - expect(page).to have_text("added role cfp to user #{user.name} in conference #{conference.short_title}") - expect(page).to have_text("removed role cfp from user #{user.name} in conference #{conference.short_title}") + expect(page).to have_text("added role cfp with ID #{user_role.id} to user #{user.name} in conference #{conference.short_title}") + expect(page).to have_text("removed role cfp with ID #{user_role.id} from user #{user.name} in conference #{conference.short_title}") end scenario 'display changes in email settings', feature: true, versioning: true, js: true do