From df883c9b52d444b7f075616eadd90f915ecb60af Mon Sep 17 00:00:00 2001
From: Andrew Kvalheim <Andrew@Kvalhe.im>
Date: Fri, 3 Apr 2020 09:51:35 -0700
Subject: [PATCH] Deconflate organization/conference IDs in role versions

Resolves failing test spec/features/versions_spec.rb:321

Partially reverts 81853d1ef91c2328e28ea676079428adbba33765
---
 app/models/role.rb                            | 12 +++++-
 app/models/users_role.rb                      |  9 ++--
 .../versions/_object_desc_and_link.html.haml  | 42 +++++++------------
 ...0331214534_add_organization_to_versions.rb | 37 ++++++++++++++++
 db/schema.rb                                  |  4 +-
 spec/features/versions_spec.rb                |  2 -
 6 files changed, 69 insertions(+), 37 deletions(-)
 create mode 100644 db/migrate/20200331214534_add_organization_to_versions.rb

diff --git a/app/models/role.rb b/app/models/role.rb
index 2be87d7b3b..7b67a158eb 100644
--- a/app/models/role.rb
+++ b/app/models/role.rb
@@ -5,7 +5,9 @@ class Role < ApplicationRecord
   has_many :users_roles
   has_many :users, through: :users_roles
 
-  has_paper_trail on: [:create, :update], only: [:name, :description], meta: { conference_id: :resource_id }
+  has_paper_trail on:   [:create, :update],
+                  only: [:name, :description],
+                  meta: { conference_id: :conference_id, organization_id: :organization_id }
 
   before_destroy :cancel
   scopify
@@ -14,6 +16,14 @@ class Role < ApplicationRecord
 
   validates :name, uniqueness: { scope: :resource }
 
+  def conference_id
+    resource_type == 'Conference' ? resource_id : nil
+  end
+
+  def organization_id
+    resource_type == 'Organization' ? resource_id : nil
+  end
+
   private
 
   # Needed to ensure that removing all user from role doesn't remove role.
diff --git a/app/models/users_role.rb b/app/models/users_role.rb
index b5dbe9fbdf..eb046f3d5d 100644
--- a/app/models/users_role.rb
+++ b/app/models/users_role.rb
@@ -4,11 +4,8 @@ class UsersRole < ApplicationRecord
   belongs_to :role
   belongs_to :user
 
-  has_paper_trail on: [:create, :destroy], meta: { conference_id: :conference_id }
+  delegate :conference_id, :organization_id, to: :role
 
-  private
-
-  def conference_id
-    role.resource_id
-  end
+  has_paper_trail on:   [:create, :destroy],
+                  meta: { conference_id: :conference_id, organization_id: :organization_id }
 end
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 1728b999e7..be9f474194 100644
--- a/app/views/admin/versions/_object_desc_and_link.html.haml
+++ b/app/views/admin/versions/_object_desc_and_link.html.haml
@@ -15,18 +15,14 @@
   - role = current_or_last_object_state('Role', object.role_id)
   - role_name = role.try(:name) || PaperTrail::Version.where(item_type: 'Role', item_id: object.role_id).last.changeset[:name].second
   role
-  - if role_name == 'organization_admin'
-    - if Organization.find_by(id: version.conference_id)
-      -# organization_admin belongs to organization and not conferences
-      - organization = Organization.find_by(id: version.conference_id)
-      = link_if_alive version, role_name,
-            admins_admin_organization_path(organization), organization
-    - else
-      (Deleted Organization)
-  - else
+  - if version.conference_id
     - 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(conference_short_title,role.try(:name) || ' '), conference
+  - elsif version.organization_id
+    - organization = Organization.find(version.organization_id)
+    = link_if_alive version, role_name,
+          admins_admin_organization_path(organization), organization
 
   = version.event == 'create' ? 'to' : 'from'
   user
@@ -133,19 +129,15 @@
 - when 'Role'
   role
   - role_name = object.try(:name) || PaperTrail::Version.where(item_type: 'Role', item_id: version.item_id).last.changeset[:name].second
-  - if role_name == 'organization_admin'
-    - if Organization.find_by(id: version.conference_id)
-      -# organization_admin belongs to organization and not conferences
-      - organization = Organization.find_by(id: version.conference_id)
-      = link_if_alive version, role_name,
-            admins_admin_organization_path(organization), organization
-    - else
-      (Role Deleted)
-  - else
+  - if version.conference_id
     - 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
+  - elsif version.organization_id
+    - organization = Organization.find(version.organization_id)
+    = link_if_alive version, role_name,
+            admins_admin_organization_path(organization), organization
 
 - when 'Venue'
   venue
@@ -203,20 +195,16 @@
     = link_to_user(version.item_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
-    - if Organization.find_by(id: version.conference_id)
-      -# organization_admin belongs to organization and not conferences
-      - organization = Organization.find_by(id: version.conference_id)
-      = link_to_organization(version.conference_id)
-    - else
-      (Organization Deleted)
-  - elsif version.item_type == 'Commercial'
+  - if version.item_type == '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)
     - unless commercial.commercialable_type == 'Conference'
       in conference
       = link_to_conference(version.conference_id)
+  - elsif version.organization_id
+    - organization = Organization.find(version.organization_id)
+    in organization
+    = link_to_organization(version.organization_id)
   - else
     in conference
     = link_to_conference(version.conference_id)
diff --git a/db/migrate/20200331214534_add_organization_to_versions.rb b/db/migrate/20200331214534_add_organization_to_versions.rb
new file mode 100644
index 0000000000..532d8bf51a
--- /dev/null
+++ b/db/migrate/20200331214534_add_organization_to_versions.rb
@@ -0,0 +1,37 @@
+class AddOrganizationToVersions < ActiveRecord::Migration[5.2]
+  def up
+    add_reference :versions, :organization
+    deconflate
+    PaperTrail::Version.reset_column_information
+  end
+
+  def down
+    conflate
+    remove_reference :versions, :organization
+    PaperTrail::Version.reset_column_information
+  end
+
+  private
+
+  def conflate
+    say 'conflate'
+
+    PaperTrail::Version.where.not(organization_id: nil).each do |version|
+      version.update_attributes conference_id: version.organization_id
+    end
+  end
+
+  def deconflate
+    say 'deconflate'
+
+    PaperTrail::Version.where.not(conference_id: nil).where(item_type: %(Role UsersRole)).each do |version|
+      id = version.conference_id
+
+      if Organization.exists?(id)
+        raise "version #{version.id} conflates organization #{id} with conference #{id}" if Conference.exists?(id)
+
+        version.update_attributes conference_id: nil, organization_id: id
+      end
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 2ad25c7ea7..8c08084b3d 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20181229233811) do
+ActiveRecord::Schema.define(version: 2020_03_31_214534) do
 
   create_table "answers", force: :cascade do |t|
     t.string "title"
@@ -637,7 +637,9 @@
     t.text "object_changes"
     t.datetime "created_at"
     t.integer "conference_id"
+    t.integer "organization_id"
     t.index ["item_type", "item_id"], name: "index_versions_on_item_type_and_item_id"
+    t.index ["organization_id"], name: "index_versions_on_organization_id"
   end
 
   create_table "votes", force: :cascade do |t|
diff --git a/spec/features/versions_spec.rb b/spec/features/versions_spec.rb
index cdbdeb7a38..7c62e46060 100644
--- a/spec/features/versions_spec.rb
+++ b/spec/features/versions_spec.rb
@@ -331,12 +331,10 @@
     end
 
     it 'is recorded to history when user is added' do
-      skip('fails since paper_trail 12.2.0')
       expect(page).to have_text(/added role organization_admin with ID \d+ to user #{user.name} in organization #{conference.organization.name}/)
     end
 
     it 'is recorded to history when user is removed' do
-      skip('fails since paper_trail 12.2.0')
       expect(page).to have_text(/removed role organization_admin with ID \d+ from user #{user.name} in organization #{conference.organization.name}/)
     end
   end