Require license and public creator when making models public

app/models/collection.rb

@@ -27,6 +27,8 @@ class Collection < ApplicationRecord
   validates :name, uniqueness: {case_sensitive: false}
   validates :public_id, multimodel_uniqueness: {case_sensitive: false, check: FederailsCommon::FEDIVERSE_USERNAMES}
 
+  validate :validate_publishable
+
   after_create_commit :after_create
   after_update_commit :after_update
 
@@ -110,4 +112,13 @@ def after_create
   def after_update
     Activity::CollectionPublishedJob.set(wait: 5.seconds).perform_later(id) if just_became_public?
   end
+
+  def validate_publishable
+    # If the model will be public
+    if caber_relations.find { |it| it.subject.nil? }
+      # Check required fields
+      errors.add :creator, :private if creator && !creator.public?
+      errors.add :collection, :private if collection && !collection.public?
+    end
+  end
 end

app/models/model.rb

@@ -53,6 +53,8 @@ class Model < ApplicationRecord
   validates :license, spdx: true, allow_nil: true
   validates :public_id, multimodel_uniqueness: {case_sensitive: false, check: FederailsCommon::FEDIVERSE_USERNAMES}
 
+  validate :validate_publishable
+
   scoped_search on: [:name, :caption]
   scoped_search on: :notes, aliases: [:description], only_explicit: true
   scoped_search relation: :library, on: :name, rename: :library, only_explicit: true, default_operator: :eq
@@ -164,26 +166,21 @@ def organize!
 
   def split!(files: [])
     new_model = dup
-    new_model.name = "Copy of #{name}"
-    new_model.public_id = nil
-    new_model.tags = tags
+    new_model.update(
+      name: "Copy of #{name}",
+      public_id: nil,
+      tags: tags,
+      preview_file: files.include?(preview_file) ? preview_file : nil,
+      caber_relations_attributes: caber_relations.all.map { |it| {permission: it.permission, subject: it.subject} }
+    )
     new_model.organize!
+    # Clear preview file if it was moved
+    update!(preview_file: nil) if files.include?(preview_file)
     # Move files
     files.each do |file|
       file.update!(model: new_model)
       file.reattach!
     end
-    # Clear preview file appropriately
-    if files.include?(preview_file)
-      update!(preview_file: nil)
-    else
-      new_model.update!(preview_file: nil)
-    end
-    # Copy permissions
-    new_model.caber_relations.destroy_all
-    caber_relations.each do |relation|
-      new_model.grant_permission_to(relation.permission, relation.subject)
-    end
     # Done!
     new_model
   end
@@ -336,4 +333,14 @@ def noteworthy_change?
       "name_lower"
     ]).empty?
   end
+
+  def validate_publishable
+    # If the model will be public
+    if caber_relations.find { |it| it.subject.nil? }
+      # Check required fields
+      errors.add :license, :blank if license.nil?
+      errors.add :creator, :blank if creator.nil?
+      errors.add :creator, :private if creator && !creator.public?
+    end
+  end
 end

config/locales/en.yml

@@ -106,6 +106,12 @@ en:
         username: Account name
     errors:
       models:
+        collection:
+          attributes:
+            collection:
+              private: must be public
+            creator:
+              private: must be public
         doorkeeper/application:
           attributes:
             redirect_uri:
@@ -127,6 +133,8 @@ en:
               unsafe: cannot be a privileged system path
         model:
           attributes:
+            creator:
+              private: must be public
             library:
               nested: can't be changed, model contains other models
             license:

spec/factories/model.rb

@@ -45,6 +45,7 @@
 
     trait :public do
       caber_relations_attributes { [{subject: nil, permission: "view"}] }
+      creator factory: [:creator, :public]
     end
   end
 end

spec/models/collection_spec.rb

@@ -26,4 +26,29 @@
       }.to have_enqueued_job(Activity::CollectionPublishedJob).once
     end
   end
+
+  context "when making a collection public" do
+    let!(:collection) { create(:collection) }
+
+    before do
+      collection.update(
+        caber_relations_attributes: [{subject: nil, permission: "view"}],
+        creator: create(:creator),
+        collection: create(:collection)
+      )
+      collection.validate
+    end
+
+    it "requires creator to be public if set" do
+      expect(collection.errors[:creator]).to include "must be public"
+    end
+
+    it "requires collection to be public if set" do
+      expect(collection.errors[:collection]).to include "must be public"
+    end
+
+    it "doesn't make collection public if validation failed" do
+      expect(collection.reload.public?).to be false
+    end
+  end
 end

spec/models/comment_spec.rb

@@ -79,7 +79,7 @@
 
   context "with non-public commenter" do
     let(:commenter) { create(:creator) }
-    let(:commentable) { create(:model, :public, creator: commenter) }
+    let(:commentable) { create(:model, :public) }
 
     it "Does not post a Federails Activity on creation" do
       expect { create(:comment, commenter: commenter, commentable: commentable) }.not_to change(Federails::Activity, :count)

spec/models/model_spec.rb

@@ -503,8 +503,35 @@
     end
   end
 
+  context "when making a model public" do
+    let!(:model) { create(:model, license: nil) }
+
+    before do
+      model.clear_changes_information
+      model.update(caber_relations_attributes: [{subject: nil, permission: "view"}])
+      model.validate
+    end
+
+    it "requires a creator" do
+      expect(model.errors[:creator]).to include "can't be blank"
+    end
+
+    it "requires a public creator" do
+      model.update(creator: create(:creator))
+      expect(model.errors[:creator]).to include "must be public"
+    end
+
+    it "requires a license" do
+      expect(model.errors[:license]).to include "can't be blank"
+    end
+
+    it "doesn't make model public if validation failed" do
+      expect(model.reload.public?).to be false
+    end
+  end
+
   context "when updating a private model" do
-    let!(:model) { create(:model) }
+    let!(:model) { create(:model, creator: create(:creator, :public)) }
 
     before do
       model.clear_changes_information
@@ -548,12 +575,6 @@
       }.to have_enqueued_job(Activity::ModelPublishedJob).once
     end
 
-    it "queues normal update activity job if the creator was changed to a private one" do
-      expect {
-        model.update!(creator: create(:creator))
-      }.to have_enqueued_job(Activity::ModelUpdatedJob).once
-    end
-
     it "queues collected activity job if the collection was changed to a public one" do
       expect {
         model.update!(collection: create(:collection, :public))