Add notes to blocks (#193)

In this commit, we also push note creation into the AccountInteractions
concern, where we can:

1. create and persist model objects inside transactions without having
   to include job queuing inside the transaction
2. commonize the don't-add-a-blank-note check

Finally, this commit resolves a invocation-on-nil in
REST::RelationshipSerializer.

app/controllers/api/v1/accounts_controller.rb

@@ -21,7 +21,7 @@ class Api::V1::AccountsController < Api::BaseController
   end
 
   def block
-    BlockService.new.call(current_user.account, @account)
+    BlockService.new.call(current_user.account, @account, note: params[:note])
     render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships
   end
 

app/models/concerns/account_interactions.rb

@@ -82,21 +82,38 @@ module AccountInteractions
     rel
   end
 
-  def block!(other_account)
+  def block!(other_account, note: nil)
-    block_relationships.find_or_create_by!(target_account: other_account)
+    transaction do
-  end
+      block = block_relationships.find_or_create_by!(target_account: other_account)
-
+      attach_note(block, note)
-  def mute!(other_account, notifications: nil)
+      block
-    notifications = true if notifications.nil?
-    mute = mute_relationships.create_with(hide_notifications: notifications).find_or_create_by!(target_account: other_account)
-    # When toggling a mute between hiding and allowing notifications, the mute will already exist, so the find_or_create_by! call will return the existing Mute without updating the hide_notifications attribute. Therefore, we check that hide_notifications? is what we want and set it if it isn't.
-    if mute.hide_notifications? != notifications
-      mute.update!(hide_notifications: notifications)
     end
-
-    mute
   end
 
+  def mute!(other_account, notifications: nil, note: nil)
+    notifications = true if notifications.nil?
+
+    transaction do
+      mute = mute_relationships.create_with(hide_notifications: notifications).find_or_create_by!(target_account: other_account)
+      attach_note(mute, note)
+
+      # When toggling a mute between hiding and allowing notifications, the mute will already exist, so the find_or_create_by! call will return the existing Mute without updating the hide_notifications attribute. Therefore, we check that hide_notifications? is what we want and set it if it isn't.
+      if mute.hide_notifications? != notifications
+        mute.update!(hide_notifications: notifications)
+      end
+
+      mute
+    end
+  end
+
+  def attach_note(object, note)
+    return if note.blank?
+
+    object.create_note!(note: note)
+  end
+
+  private :attach_note
+
   def mute_conversation!(conversation)
     conversation_mutes.find_or_create_by!(conversation: conversation)
   end

app/serializers/rest/relationship_serializer.rb

@@ -10,7 +10,7 @@ class REST::RelationshipSerializer < ActiveModel::Serializer
   end
 
   def note
-    object.respond_to?(:note) ? object.note.text : nil
+    object.respond_to?(:note) ? object.note&.text : nil
   end
 
   def following

app/services/block_service.rb

@@ -3,13 +3,13 @@
 class BlockService < BaseService
   include StreamEntryRenderer
 
-  def call(account, target_account)
+  def call(account, target_account, note: nil)
     return if account.id == target_account.id
 
     UnfollowService.new.call(account, target_account) if account.following?(target_account)
     UnfollowService.new.call(target_account, account) if target_account.following?(account)
 
-    block = account.block!(target_account)
+    block = account.block!(target_account, note: note)
 
     BlockWorker.perform_async(account.id, target_account.id)
     create_notification(block) unless target_account.local?

app/services/mute_service.rb

@@ -5,8 +5,7 @@ class MuteService < BaseService
     return if account.id == target_account.id
 
     ActiveRecord::Base.transaction do
-      mute = account.mute!(target_account, notifications: notifications)
+      mute = account.mute!(target_account, notifications: notifications, note: note)
-      mute.create_note!(note: note) if !note.blank?
       BlockWorker.perform_async(account.id, target_account.id)
       mute
     end

spec/services/block_service_spec.rb

@@ -15,6 +15,23 @@ RSpec.describe BlockService do
     it 'creates a blocking relation' do
       expect(sender.blocking?(bob)).to be true
     end
+
+    describe 'if a note is present' do
+      let(:note) { 'Too many oats' }
+
+      it 'adds a note to the block' do
+        block = subject.call(sender, bob, note: note)
+        note = block.note
+
+        expect(note.text).to eq('Too many oats')
+      end
+
+      it 'does not add blank notes' do
+        block = subject.call(sender, bob, note: ' ')
+
+        expect(block.note).to be nil
+      end
+    end
   end
 
   describe 'remote OStatus' do