Merge pull request #1159 from nishanthvijayan/changelog-improvements

Misc improvements to Revision History

app/helpers/application_helper.rb

@@ -392,12 +392,23 @@ module ApplicationHelper
     end
   end
 
-  # Recieves a PaperTrail::Version object
+  # Recieves a model_name and id
+  # Returns nil if model_name is invalid
   # Returns object in its current state if its alive
-  # Returns object as it was before version's change(unless its a create event's version)
-  # Else Returns object as it was after version's change
-  def get_version_object(version)
-    version.item || version.reify || version.next.reify
+  # Otherwise Returns object state just before deletion
+  def current_or_last_object_state(model_name, id)
+    return nil unless id.present? && model_name.present?
+    begin
+      object = model_name.constantize.find_by(id: id)
+    rescue NameError
+      return nil
+    end
+
+    if object.nil?
+      object_last_version = PaperTrail::Version.where(item_type: model_name, item_id: id).last
+      object = object_last_version.reify if object_last_version
+    end
+    object
   end
 
   def event_change_description(version)
@@ -412,12 +423,6 @@ module ApplicationHelper
       when 'new' then 'resubmitted'
       end
 
-    when version.changeset['start_time'] && version.changeset['start_time'][0].nil?
-      'scheduled'
-
-    when version.changeset['start_time'] && version.changeset['start_time'][1].nil?
-      'unscheduled'
-
     else
       "updated #{updated_attributes(version)} of"
     end
@@ -428,18 +433,17 @@ module ApplicationHelper
   end
 
   def subscription_change_description(version)
-    user = get_version_object(version).user
+    user = current_or_last_object_state(version.item_type, version.item_id).user
     user_name = user.name 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 = get_version_object(version).user
-    else
-      registration_id = get_version_object(version).registration_id
-      registration_last_version = PaperTrail::Version.where(item_type: 'Registration', item_id: registration_id).last
-      user = get_version_object(registration_last_version).user
+      user = current_or_last_object_state(version.item_type, version.item_id).user
+    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
     end
 
     if user.id.to_s == version.whodunnit
@@ -458,7 +462,7 @@ module ApplicationHelper
   end
 
   def comment_change_description(version)
-    user = get_version_object(version).user
+    user = current_or_last_object_state(version.item_type, version.item_id).user
     if version.event == 'create'
       version.previous.nil? ? 'commented on' : "re-added #{user.name}'s comment on"
     else
@@ -467,7 +471,7 @@ module ApplicationHelper
   end
 
   def vote_change_description(version)
-    user = get_version_object(version).user
+    user = current_or_last_object_state(version.item_type, version.item_id).user
     if version.event == 'create'
       version.previous.nil? ? 'voted on' : "re-added #{user.name}'s vote on"
     elsif version.event == 'update'
@@ -493,6 +497,14 @@ module ApplicationHelper
     end
   end
 
+  def event_schedule_change_description(version)
+    case version.event
+    when 'create' then 'scheduled'
+    when 'update' then 'rescheduled'
+    when 'destroy' then 'unscheduled'
+    end
+  end
+
   def general_change_description(version)
     if version.event == 'create'
       'created new'
@@ -502,4 +514,8 @@ module ApplicationHelper
       'deleted'
     end
   end
+
+  def link_if_alive(version, link_text, link_url)
+    version.item ? link_to(link_text, link_url) : link_text
+  end
 end

app/models/event_schedule.rb

@@ -3,6 +3,8 @@ class EventSchedule < ActiveRecord::Base
   belongs_to :event
   belongs_to :room
 
+  has_paper_trail ignore: [:updated_at], meta: { conference_id: :conference_id }
+
   validates :schedule, presence: true
   validates :event, presence: true
   validates :room, presence: true
@@ -28,4 +30,10 @@ class EventSchedule < ActiveRecord::Base
   def intersecting_events
     room.event_schedules.where(start_time: start_time, schedule: schedule).where.not(id: id)
   end
+
+  private
+
+  def conference_id
+    schedule.program.conference_id
+  end
 end

app/models/role.rb

@@ -3,7 +3,7 @@ class Role < ActiveRecord::Base
   has_many :users_roles
   has_many :users, through: :users_roles
 
-  has_paper_trail on: [:update], only: [:name, :description], meta: { conference_id: :resource_id }
+  has_paper_trail on: [:create, :update], only: [:name, :description], meta: { conference_id: :resource_id }
 
   before_destroy :cancel
   scopify

app/models/schedule.rb

@@ -2,4 +2,12 @@ class Schedule < ActiveRecord::Base
   belongs_to :program
   has_many :event_schedules, dependent: :destroy
   has_many :events, through: :event_schedules
+
+  has_paper_trail ignore: [:updated_at], meta: { conference_id: :conference_id }
+
+  private
+
+  def conference_id
+    program.conference_id
+  end
 end

app/views/admin/versions/_object_desc_and_link.html.haml

@@ -1,156 +1,171 @@
-- if version.item_type == 'UsersRole'
-  - users_role = get_version_object(version)
+- case version.item_type
+- when 'UsersRole'
+  - users_role = current_or_last_object_state(version.item_type, version.item_id)
+  - user = current_or_last_object_state('User', users_role.user_id)
   = 'role'
   = link_to users_role.role.name, admin_conference_role_path(conference_id: Conference.find(version.conference_id).short_title, id: users_role.role.name)
   = version.event == 'create' ? 'to' : 'from'
   = 'user'
-  = link_to users_role.user.name, admin_user_path(id: users_role.user.id)
 
-- elsif version.item_type == 'Subscription' || version.item_type == 'Registration'
+  = link_to (user.try(:name) || 'deleted user'), admin_user_path(id: users_role.user.id)
+
+- when 'Subscription', 'Registration'
   = 'conference'
   = link_to Conference.find(version.conference_id).title,
           admin_conference_registrations_path(conference_id: Conference.find(version.conference_id).short_title)
 
-- elsif version.item_type == 'Commercial'
-  - commercial_last_version = get_version_object(PaperTrail::Version.where(item_type: version.item_type, item_id: version.item_id).last)
-  - commercialable_last_version = get_version_object(PaperTrail::Version.where(item_type: commercial_last_version.commercialable_type,
-                                                          item_id: commercial_last_version.commercialable_id).last)
+- when 'Commercial'
+  - commercial = current_or_last_object_state(version.item_type, version.item_id)
+  - commercialable = current_or_last_object_state(commercial.commercialable_type, commercial.commercialable_id)
+  - commercialable_last_version = PaperTrail::Version.where(item_type: commercial.commercialable_type, item_id: commercial.commercialable_id).last
 
-  - case commercial_last_version.commercialable_type
+  - case commercial.commercialable_type
   - when 'Event'
-    =  link_to 'commercial',
-            edit_admin_conference_program_event_path(conference_id: Conference.find(version.conference_id).short_title,
-                id: commercialable_last_version.id, anchor: 'commercials-content')
-    = "in event #{commercialable_last_version.title}"
+    commercial in
+    - if commercialable_last_version.item
+      event
+      =  link_to commercialable.title,
+             admin_conference_program_event_path(conference_id: Conference.find(version.conference_id).short_title, id: commercialable.id)
+    - else
+      = commercialable.title
 
   - when 'Venue'
-    - if Venue.find_by(id: commercialable_last_version.id)
-      =  link_to 'commercial',
-              edit_admin_conference_program_event_path(conference_id: Conference.find(version.conference_id).short_title,
-                  id: commercialable_last_version.id, anchor: 'commercials-content')
+    commercial in venue
+    - if commercialable_last_version.item
+      = link_to commercialable.name,
+            edit_admin_conference_venue_path(conference_id: Conference.find(version.conference_id).short_title,
+                                                id: commercialable.id, anchor: 'commercials-content')
     - else
-      = 'commercial'
-    = "in venue #{commercialable_last_version.name}"
+      = commercialable.name
 
   - when 'Conference'
     = link_to 'commercial',
             admin_conference_commercials_path(conference_id: Conference.find(version.conference_id).short_title)
 
-- elsif %w(EventsRegistration Comment Vote).include?(version.item_type)
+- when 'EventsRegistration', 'Comment', 'Vote', 'Event'
   = 'event'
-  - event_id = get_version_object(version).try(:event_id) || get_version_object(version).try(:commentable_id)
-  = link_to Event.find(event_id).title,
+  - object = current_or_last_object_state(version.item_type, version.item_id)
+  - event_id = object.try(:id) || object.try(:event_id) || object.commentable_id
+  = link_to (current_or_last_object_state('Event', event_id).try(:title) || 'deleted event'),
           admin_conference_program_event_path(conference_id: Conference.find(version.conference_id).short_title, id: event_id)
 
-- elsif version.item_type =='Target'
+- when 'Target'
   = 'target'
-  - if version.item
-    = link_to Target.find(version.item_id).to_s,
-            admin_conference_targets_path(conference_id: Conference.find(version.conference_id).short_title)
-  - else
-    = PaperTrail::Version.where(item_type: 'Target', item_id: version.item_id).last.reify.to_s
+  - 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.find(version.conference_id).short_title)
 
-- elsif  version.item
-  - case version.item_type
-  - when 'Conference'
-    = 'conference'
-    = link_to Conference.find(version.conference_id).title,
-            edit_admin_conference_path(id: Conference.find(version.conference_id).short_title)
+- 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 event'),
+           admin_conference_program_event_path(conference_id: Conference.find(version.conference_id).short_title, id: event_schedule.event_id)
+  in
+  =  link_to "Schedule #{event_schedule.schedule_id}",
+            admin_conference_schedule_path(conference_id: Conference.find(version.conference_id).short_title, id: event_schedule.schedule_id)
 
-  - when 'RegistrationPeriod'
-    = link_to 'registration period',
-            admin_conference_registration_period_path(conference_id: Conference.find(version.conference_id).short_title)
+- when 'Schedule'
+  = link_if_alive version, "Schedule #{version.item_id}",
+            admin_conference_schedule_path(conference_id: Conference.find(version.conference_id).short_title, id: version.item_id)
 
-  - when 'Contact'
-    = link_to 'contact details',
-            edit_admin_conference_contact_path(conference_id: Conference.find(version.conference_id).short_title)
+- when 'Conference'
+  = 'conference'
+  = link_to Conference.find(version.conference_id).title,
+          edit_admin_conference_path(id: Conference.find(version.conference_id).short_title)
 
-  - when 'Program'
-    = link_to 'program',
-            admin_conference_program_path(conference_id: Conference.find(version.conference_id).short_title)
+- when 'RegistrationPeriod'
+  = link_if_alive version, 'registration period',
+          admin_conference_registration_period_path(conference_id: Conference.find(version.conference_id).short_title)
 
-  - when 'Cfp'
-    = link_to 'cfp',
-            admin_conference_program_cfp_path(conference_id: Conference.find(version.conference_id).short_title)
+- when 'Contact'
+  = link_if_alive version, 'contact details',
+          edit_admin_conference_contact_path(conference_id: Conference.find(version.conference_id).short_title)
 
-  - when 'Track'
-    = 'track'
-    = link_to Track.find(version.item_id).name,
-            admin_conference_program_track_path(conference_id: Conference.find(version.conference_id).short_title, id: version.item_id)
+- when 'Program'
+  = link_if_alive version, 'program',
+          admin_conference_program_path(conference_id: Conference.find(version.conference_id).short_title)
 
-  - when 'Event'
-    = 'event'
-    = link_to Event.find(version.item_id).title,
-            admin_conference_program_event_path(conference_id: Conference.find(version.conference_id).short_title, id: version.item_id)
+- when 'Cfp'
+  = link_if_alive version, 'cfp',
+          admin_conference_program_cfp_path(conference_id: Conference.find(version.conference_id).short_title)
 
-  - when 'EventType'
-    = 'event type'
-    = link_to EventType.find(version.item_id).title,
-            admin_conference_program_event_types_path(conference_id: Conference.find(version.conference_id).short_title)
+- when '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.find(version.conference_id).short_title, id: version.item_id)
 
-  - when 'Role'
-    = 'role'
-    = link_to Role.find(version.item_id).name,
-            admin_conference_role_path(conference_id: Conference.find(version.conference_id).short_title, id: version.item.name)
+- when 'EventType'
+  = '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.find(version.conference_id).short_title)
 
-  - when 'Venue'
-    = 'venue'
-    = link_to Venue.find(version.item_id).name,
-            admin_conference_venue_path(conference_id: Conference.find(version.conference_id).short_title)
+- when '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.find(version.conference_id).short_title, id: role.name)
 
-  - when 'Lodging'
-    = 'lodging'
-    = link_to Lodging.find(version.item_id).name,
-            admin_conference_lodgings_path(conference_id: Conference.find(version.conference_id).short_title)
+- when '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.find(version.conference_id).short_title)
 
-  - when 'Room'
-    = 'room'
-    = link_to Room.find(version.item_id).name,
-            admin_conference_venue_rooms_path(conference_id: Conference.find(version.conference_id).short_title)
+- when '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.find(version.conference_id).short_title)
 
-  - when 'Sponsor'
-    = 'sponsor'
-    = link_to Sponsor.find(version.item_id).name,
-            admin_conference_sponsors_path(conference_id: Conference.find(version.conference_id).short_title)
+- when '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.find(version.conference_id).short_title)
 
-  - when 'SponsorshipLevel'
-    = 'sponsorship level'
-    = link_to SponsorshipLevel.find(version.item_id).title,
-            admin_conference_sponsorship_levels_path(conference_id: Conference.find(version.conference_id).short_title)
+- when '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.find(version.conference_id).short_title)
 
-  - when 'Ticket'
-    = 'ticket'
-    = link_to Ticket.find(version.item_id).title,
-            admin_conference_ticket_path(conference_id: Conference.find(version.conference_id).short_title, id: version.item_id)
+- when 'SponsorshipLevel'
+  = '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.find(version.conference_id).short_title)
 
-  - when 'Campaign'
-    = 'campaign'
-    = link_to Campaign.find(version.item_id).name,
-            admin_conference_campaigns_path(conference_id: Conference.find(version.conference_id).short_title)
+- when '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.find(version.conference_id).short_title, id: version.item_id)
 
-  - when 'DifficultyLevel'
-    = 'difficulty level'
-    = link_to DifficultyLevel.find(version.item_id).title,
-            admin_conference_program_difficulty_level_path(conference_id: Conference.find(version.conference_id).short_title, id: version.item_id)
+- when '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.find(version.conference_id).short_title)
 
-  - when 'Splashpage'
-    = link_to 'splashpage',
-            admin_conference_splashpage_path(conference_id: Conference.find(version.conference_id).short_title)
+- when 'DifficultyLevel'
+  = '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.find(version.conference_id).short_title, id: version.item_id)
 
-  - when 'EmailSettings'
-    = link_to 'email settings',
-            admin_conference_emails_path(conference_id: Conference.find(version.conference_id).short_title)
+- when 'Splashpage'
+  = link_if_alive version, 'splashpage',
+          admin_conference_splashpage_path(conference_id: Conference.find(version.conference_id).short_title)
 
-  - when 'User'
-    - if version.event == 'update'
-      = 'user'
-      = link_to User.find(version.item_id).name, admin_user_path(id: version.item_id)
+- when 'EmailSettings'
+  = link_if_alive version, 'email settings',
+          admin_conference_emails_path(conference_id: Conference.find(version.conference_id).short_title)
 
-- else
-  = version.item_type.underscore.tr('_', ' ')
-  / The last deleted version's name/title is used to describe all the changes in object
-  - last_deleted_version = PaperTrail::Version.where(item_type: version.item_type, item_id: version.item_id).last.reify
-  = last_deleted_version.try(:title) || last_deleted_version.try(:name)
+- when 'User'
+  - if version.event == 'update'
+    = 'user'
+    = link_to (current_or_last_object_state('User', version.item_id).try(:name) || 'deleted user'), admin_user_path(id: version.item_id)
 
 - unless %w(Conference Subscription Registration User).include?(version.item_type)
   = "in conference"

app/views/admin/versions/index.html.haml

@@ -41,6 +41,9 @@
             - when 'User'
               = user_change_description(version)
 
+            - when 'EventSchedule'
+              = event_schedule_change_description(version)
+
             - else
               = general_change_description(version)
 

app/views/shared/_object_changes.html.haml

@@ -12,17 +12,43 @@
         - if can? :revert_attribute, version
           %th Action
     %tbody
-      - if version.event!= 'destroy'
+      - if version.event != 'destroy'
         - version.changeset.reject{ |_, values| values[0].blank? && values[1].blank? }.each do |attribute, values|
           %tr
             %td= attribute
-            - if version.event != 'create'
-              %td= values[0].blank? ?  '-' : values[0]
-            %td= values[1].blank? ?  '-' : values[1]
+
+            / If the attribute is an associated model, show the value of the record it corresponds to, not just the ID.
+            / Eg. when the version is of an Event, if the attribute is event_type_id it shows
+            / Workshop (ID: 116) instead of just 116
+            - if attribute.include?('_id')
+              - model_name = attribute.chomp('_id').camelize
+              - if version.event != 'create'
+                %td
+                  - associated_object = current_or_last_object_state(model_name, values[0])
+                  = associated_object.try(:title) || associated_object.try(:name)
+                  = values[0].blank? ? '-' : "(ID: #{values[0]})"
+              %td
+                - associated_object = current_or_last_object_state(model_name, values[1])
+                = associated_object.try(:title) || associated_object.try(:name)
+                = (values[1].blank? ? '-' : "(ID: #{values[1]})")
+
+            - else
+              - if version.event != 'create'
+                %td= values[0].blank? ?  '-' : values[0]
+              %td= values[1].blank? ?  '-' : values[1]
+
             - if can? :revert_attribute, version
               %td= link_to 'Revert', admin_revision_history_revert_attribute_path(id: version.id, attribute: attribute), class: 'btn btn-sm btn-primary', data: { confirm: "Are you sure you want to revert #{attribute}?" }
       - else
         - version.reify.attributes.each do |attribute, value|
           %tr
             %td= attribute
-            %td= value.blank? ?  '-' : value
+
+            - if attribute.include?('_id')
+              - model_name = attribute.chomp('_id').camelize
+              %td
+                - associated_object = current_or_last_object_state(model_name, value)
+                = associated_object.try(:title) || associated_object.try(:name)
+                = value.blank? ? '-' : "(ID: #{value})"
+            - else
+              %td= value.blank? ?  '-' : value

spec/features/versions_spec.rb

@@ -58,6 +58,7 @@ feature 'Version' do
     new_conference.update_attributes(title: 'New Con', short_title: 'NewCon')
 
     visit admin_revision_history_path
+    select '100', from: 'versionstable_length'
     expect(page).to have_text('Someone (probably via the console) created new conference New Con')
     expect(page).to have_text('Someone (probably via the console) created new event type Talk in conference NewCon')
     expect(page).to have_text('Someone (probably via the console) created new event type Workshop in conference NewCon')