helper method to migrate posts between users

app/lib/lite_trail/active_record_class_methods.rb

@@ -42,14 +42,16 @@ module LiteTrail::ActiveRecordClassMethods
                -> { order(created_at: :asc) },
                class_name: lite_trail_class.name,
                as: :item,
-               autosave: false
+               autosave: false,
+               dependent: :destroy
     else
       has_many :versions,
                -> { order(created_at: :asc) },
                class_name: lite_trail_class.name,
                inverse_of: :item,
                foreign_key: :item_id,
-               autosave: false
+               autosave: false,
+               dependent: :destroy
     end
 
     after_update do
@@ -85,21 +87,5 @@ module LiteTrail::ActiveRecordClassMethods
     after_save do
       self.versions.filter(&:new_record?).each(&:save!)
     end
-
-    after_destroy do
-      attributes = self.attributes
-      map_attribute.each do |attr_name, mapper|
-        if attributes[attr_name]
-          attributes[attr_name] = mapper.map_to(attributes[attr_name])
-        end
-      end if map_attribute
-
-      self.versions << lite_trail_class.create!({
-        event: "destroy",
-        item: self,
-        schema_version: schema_version,
-        diff: attributes,
-      })
-    end
   end
 end

app/models/concerns/immutable_model.rb

@@ -2,8 +2,6 @@ module ImmutableModel
   extend ActiveSupport::Concern
 
   included do
-    def readonly?
-      !new_record?
-    end
+    before_update { raise ActiveRecord::ReadOnlyRecord }
   end
 end

app/models/domain/fa/user.rb

@@ -11,33 +11,72 @@ class Domain::Fa::User < ReduxApplicationRecord
   has_one :disco,
           class_name: "::Domain::Fa::UserFactor",
           inverse_of: :user,
-          foreign_key: :user_id
+          foreign_key: :user_id,
+          dependent: :destroy
 
   has_one :avatar,
           class_name: "::Domain::Fa::UserAvatar",
-          inverse_of: :user
+          inverse_of: :user,
+          dependent: :destroy
 
   enum :state, [
          :ok, # so far so good, user may not yet be scanned
          :scan_error, # user has been removed or otherwise, see state_detail
        ]
 
-  has_many :follows,
+  has_many :follower_joins,
            class_name: "::Domain::Fa::Follow",
-           foreign_key: :follower_id
+           foreign_key: :follower_id,
+           dependent: :destroy
+
+  has_many :followed_joins,
+           class_name: "::Domain::Fa::Follow",
+           foreign_key: :followed_id,
+           dependent: :destroy
+
+  # Domain::Fa::User
+  has_many :followers,
+           through: :follower_joins
+
+  # Domain::Fa::User
+  has_many :followeds,
+           through: :followed_joins
 
   validates_presence_of(:name, :url_name)
-  before_validation do
-    self.url_name ||= self.class.name_to_url_name(self.name) if self.name
+  validate do
+    if name && url_name
+      expected = self.class.name_to_url_name(name)
+      if url_name != expected
+        errors.add(
+          :name,
+          "name '#{name}' does not match url_name, expected #{expected} but was #{url_name}"
+        )
+      end
+    end
   end
+
   after_initialize do
     self.state ||= :ok
     self.state_detail ||= {}
     self.log_entry_detail ||= {}
   end
 
+  before_destroy do
+    throw :abort if posts.any?
+  end
+
+  def take_posts_from(other_user)
+    return if other_user == self
+    other_posts = other_user.posts
+    other_posts.update_all(creator_id: self.id)
+    other_user.posts.reload
+    self.posts.reload
+  end
+
   def avatar_or_create
-    avatar || create_avatar!
+    self.class.transaction do
+      avatar || create_avatar!
+    end
   end
 
   def due_for_page_scan?

app/models/lite_trail/abstract_version.rb

@@ -1,6 +1,6 @@
 class LiteTrail::AbstractVersion < ReduxApplicationRecord
   self.abstract_class = true
-  include ImmutableModel
+  before_update { raise ActiveRecord::ReadOnlyRecord }
 
   def reify
     versions_arr = item.versions
@@ -9,11 +9,7 @@ class LiteTrail::AbstractVersion < ReduxApplicationRecord
       raise("item.versions (#{item.item_type}/#{item.item_id}) does not contain self: #{self.id}")
     end
 
-    model = if self.event == "destroy"
-        self.item.class.new
-      else
-        self.item.dup
-      end
+    model = self.item.dup
 
     # unapply versions in reverse order
     (versions_arr.length - 1).downto(self_idx).each do |idx|
@@ -46,13 +42,6 @@ class LiteTrail::AbstractVersion < ReduxApplicationRecord
 
         model.send(:"#{attr_name}=", attr_before)
       end
-    elsif self.event == "destroy"
-      self.diff.each do |attr_name, attr_value|
-        if mapper_config[attr_name]
-          attr_value = mapper_config[attr_name].map_from(attr_value)
-        end
-        item.send(:"#{attr_name}=", attr_value)
-      end
     end
   end
 end

config/routes.rb

@@ -15,6 +15,8 @@ Rails.application.routes.draw do
   scope constraints: VpnOnlyRouteConstraint.new do
     mount GoodJob::Engine => "jobs"
     namespace :api do
+      get "search/user/:prefix", to: "search#user"
+
       namespace :fa do
         post :enqueue_objects, to: "/domain/fa/api#enqueue_objects"
         post :object_statuses, to: "/domain/fa/api#object_statuses"

spec/models/domain/fa/user_spec.rb

@@ -0,0 +1,67 @@
+require "rails_helper"
+
+describe Domain::Fa::User do
+  it "can be built from legacy" do
+    old_user = Legacy::Fa::User.find_by(name: "Meesh") || raise
+    new_user = Domain::Fa::User.find_or_build_from_legacy(old_user)
+    assert new_user.new_record?
+    assert_equal "Meesh", new_user.name
+    assert_equal "Meesh", new_user.full_name
+    assert_equal "meesh", new_user.url_name
+    assert_equal "PrOn Artist", new_user.artist_type
+    assert_match /Refunds will be granted/, new_user.profile_html
+
+    [
+      :registered_at,
+      :created_at,
+      :updated_at,
+    ].each do |attr|
+      assert_equal old_user.send(attr), new_user.send(attr)
+    end
+    assert_equal old_user.scanned_gallery, new_user.scanned_gallery_at
+    assert_equal old_user.scanned_page, new_user.scanned_page_at
+
+    assert_equal 1715730, new_user.num_pageviews
+    assert_equal 1225, new_user.num_submissions
+    assert_equal 40123, new_user.num_comments_recieved
+    assert_equal 17386, new_user.num_comments_given
+    assert_equal 13, new_user.num_journals
+    assert_equal 893478, new_user.num_favorites
+  end
+
+  it "validates that url_name and name match using the conversion rules" do
+    user = Domain::Fa::User.create({
+      name: "Foo_User",
+      url_name: "foouser",
+    })
+    assert user.valid?, user.errors.messages
+  end
+
+  it "user can be destroyed" do
+    user = SpecUtil.create_domain_fa_user(name: "Foo", url_name: "foo")
+    user.avatar_or_create
+    user.destroy
+  end
+
+  it "posts can be moved from one user to another" do
+    user1 = SpecUtil.create_domain_fa_user(name: "Foo", url_name: "foo")
+    user2 = SpecUtil.create_domain_fa_user(name: "Bar", url_name: "bar")
+    post1 = SpecUtil.create_domain_fa_post(creator: user1)
+
+    # should not be able to destroy when there are posts associated with the user
+    expect(user1.destroy).to be_falsey
+
+    expect(user1.posts).to eq([post1])
+    expect(user2.posts).to eq([])
+
+    user2.take_posts_from(user1)
+    expect(user1.posts).to eq([])
+    expect(user2.posts).to eq([post1])
+
+    post1.reload
+    expect(post1.creator).to eq(user2)
+
+    # should be able to destroy the user
+    expect(user1.destroy).to be_truthy
+  end
+end

spec/spec_util.rb

@@ -167,9 +167,12 @@ class SpecUtil
     end
   end
 
-  def self.build_domain_fa_user(name: nil)
+  def self.build_domain_fa_user(name: nil, url_name: nil)
+    name ||= random_string
+    url_name ||= Domain::Fa::User.name_to_url_name(name)
     Domain::Fa::User.new(
-      name: name || random_string,
+      name: name,
+      url_name: url_name,
     )
   end
 

test/models/domain/fa/user_test.rb

@@ -1,29 +1 @@
-class Domain::Fa::UserTest < ActiveSupport::TestCase
-  test "can build from legacy" do
-    old_user = Legacy::Fa::User.find_by(name: "Meesh") || raise
-    new_user = Domain::Fa::User.find_or_build_from_legacy(old_user)
-    assert new_user.new_record?
-    assert_equal "Meesh", new_user.name
-    assert_equal "Meesh", new_user.full_name
-    assert_equal "meesh", new_user.url_name
-    assert_equal "PrOn Artist", new_user.artist_type
-    assert_match /Refunds will be granted/, new_user.profile_html
 
-    [
-      :registered_at,
-      :created_at,
-      :updated_at,
-    ].each do |attr|
-      assert_equal old_user.send(attr), new_user.send(attr)
-    end
-    assert_equal old_user.scanned_gallery, new_user.scanned_gallery_at
-    assert_equal old_user.scanned_page, new_user.scanned_page_at
-
-    assert_equal 1715730, new_user.num_pageviews
-    assert_equal 1225, new_user.num_submissions
-    assert_equal 40123, new_user.num_comments_recieved
-    assert_equal 17386, new_user.num_comments_given
-    assert_equal 13, new_user.num_journals
-    assert_equal 893478, new_user.num_favorites
-  end
-end