DEV: Improve `array` type in service contracts

This patch improves the custom `array` type available in contracts.
It’s now able to split strings on `|` on top of `,`, and to be more
consistent, it also tries to cast the resulting items to integers.
This commit is contained in:
Loïc Guitaut 2024-10-17 15:26:43 +02:00 committed by Loïc Guitaut
parent 2ee63e4164
commit 23c486799f
3 changed files with 31 additions and 11 deletions

View File

@ -9,7 +9,7 @@ module ActiveSupportTypeExtensions
def cast_value(value)
case value
when String
value.split(",")
cast_value(value.split(/,(?!.*\|)|\|(?!.*,)/))
when ::Array
value.map { |item| Integer(item, exception: false) || item }
else

View File

@ -19,7 +19,7 @@ module Chat
include Service::Base
policy :chat_enabled
step :cast_new_allowed_groups_to_array
contract { attribute :new_allowed_groups, :array }
policy :not_everyone_allowed
model :users
model :memberships_to_remove
@ -32,15 +32,11 @@ module Chat
SiteSetting.chat_enabled
end
def cast_new_allowed_groups_to_array(new_allowed_groups:)
context[:new_allowed_groups] = new_allowed_groups.to_s.split("|").map(&:to_i)
def not_everyone_allowed(contract:)
contract.new_allowed_groups.exclude?(Group::AUTO_GROUPS[:everyone])
end
def not_everyone_allowed(new_allowed_groups:)
!new_allowed_groups.include?(Group::AUTO_GROUPS[:everyone])
end
def fetch_users(new_allowed_groups:)
def fetch_users(contract:)
User
.real
.activated
@ -50,8 +46,8 @@ module Chat
.joins(:user_chat_channel_memberships)
.distinct
.then do |users|
break users if new_allowed_groups.blank?
users.where(<<~SQL, new_allowed_groups)
break users if contract.new_allowed_groups.blank?
users.where(<<~SQL, contract.new_allowed_groups)
users.id NOT IN (
SELECT DISTINCT group_users.user_id
FROM group_users

View File

@ -14,6 +14,30 @@ RSpec.describe ActiveSupportTypeExtensions::Array do
end
end
context "when 'value' is a string of numbers" do
let(:value) { "1,2,3" }
it "splits it with strings casted as integers" do
expect(casted_value).to eq([1, 2, 3])
end
end
context "when 'value' is a string of numbers separated by '|'" do
let(:value) { "1|2|3" }
it "splits it with strings casted as integers" do
expect(casted_value).to eq([1, 2, 3])
end
end
context "when 'value' has mixed separators" do
let(:value) { "1,2,3|4" }
it "splits only on one of the separators" do
expect(casted_value).to eq(["1,2,3", 4])
end
end
context "when 'value' is an array" do
let(:value) { %w[existing array] }