From a9b5598c97fc4d3302b61b260097ef41c2ebe377 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 30 Mar 2023 14:44:00 +0200 Subject: Change user settings to be stored in a more optimal way (#23630) Co-authored-by: Claire --- spec/models/user_settings/namespace_spec.rb | 25 +++++++ spec/models/user_settings/setting_spec.rb | 74 +++++++++++++++++++ spec/models/user_settings_spec.rb | 110 ++++++++++++++++++++++++++++ spec/models/user_spec.rb | 14 +--- 4 files changed, 211 insertions(+), 12 deletions(-) create mode 100644 spec/models/user_settings/namespace_spec.rb create mode 100644 spec/models/user_settings/setting_spec.rb create mode 100644 spec/models/user_settings_spec.rb (limited to 'spec/models') diff --git a/spec/models/user_settings/namespace_spec.rb b/spec/models/user_settings/namespace_spec.rb new file mode 100644 index 000000000..ae2fa7b48 --- /dev/null +++ b/spec/models/user_settings/namespace_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe UserSettings::Namespace do + subject { described_class.new(name) } + + let(:name) { :foo } + + describe '#setting' do + before do + subject.setting :bar, default: 'baz' + end + + it 'adds setting to definitions' do + expect(subject.definitions[:'foo.bar']).to have_attributes(name: :bar, namespace: :foo, default_value: 'baz') + end + end + + describe '#definitions' do + it 'returns a hash' do + expect(subject.definitions).to be_a Hash + end + end +end diff --git a/spec/models/user_settings/setting_spec.rb b/spec/models/user_settings/setting_spec.rb new file mode 100644 index 000000000..6e4ec6789 --- /dev/null +++ b/spec/models/user_settings/setting_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe UserSettings::Setting do + subject { described_class.new(name, options) } + + let(:name) { :foo } + let(:options) { { default: default, namespace: namespace } } + let(:default) { false } + let(:namespace) { nil } + + describe '#default_value' do + context 'when default value is a primitive value' do + it 'returns default value' do + expect(subject.default_value).to eq default + end + end + + context 'when default value is a proc' do + let(:default) { -> { 'bar' } } + + it 'returns value from proc' do + expect(subject.default_value).to eq 'bar' + end + end + end + + describe '#type' do + it 'returns a type' do + expect(subject.type).to be_a ActiveModel::Type::Value + end + end + + describe '#type_cast' do + context 'when default value is a boolean' do + let(:default) { false } + + it 'returns boolean' do + expect(subject.type_cast('1')).to be true + end + end + + context 'when default value is a string' do + let(:default) { '' } + + it 'returns string' do + expect(subject.type_cast(1)).to eq '1' + end + end + end + + describe '#to_a' do + it 'returns an array' do + expect(subject.to_a).to eq [name, default] + end + end + + describe '#key' do + context 'when there is no namespace' do + it 'returnsn a symbol' do + expect(subject.key).to eq :foo + end + end + + context 'when there is a namespace' do + let(:namespace) { :bar } + + it 'returns a symbol' do + expect(subject.key).to eq :'bar.foo' + end + end + end +end diff --git a/spec/models/user_settings_spec.rb b/spec/models/user_settings_spec.rb new file mode 100644 index 000000000..f0e4272fd --- /dev/null +++ b/spec/models/user_settings_spec.rb @@ -0,0 +1,110 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe UserSettings do + subject { described_class.new(json) } + + let(:json) { {} } + + describe '#[]' do + context 'when setting is not set' do + it 'returns default value' do + expect(subject[:always_send_emails]).to be false + end + end + + context 'when setting is set' do + let(:json) { { default_language: 'fr' } } + + it 'returns value' do + expect(subject[:default_language]).to eq 'fr' + end + end + + context 'when setting was not defined' do + it 'raises error' do + expect { subject[:foo] }.to raise_error UserSettings::KeyError + end + end + end + + describe '#[]=' do + context 'when value matches type' do + before do + subject[:always_send_emails] = true + end + + it 'updates value' do + expect(subject[:always_send_emails]).to be true + end + end + + context 'when value needs to be type-cast' do + before do + subject[:always_send_emails] = '1' + end + + it 'updates value with a type-cast' do + expect(subject[:always_send_emails]).to be true + end + end + end + + describe '#update' do + before do + subject.update(always_send_emails: true, default_language: 'fr', default_privacy: nil) + end + + it 'updates values' do + expect(subject[:always_send_emails]).to be true + expect(subject[:default_language]).to eq 'fr' + end + + it 'does not set values that are nil' do + expect(subject.as_json).to_not include(default_privacy: nil) + end + end + + describe '#as_json' do + let(:json) { { default_language: 'fr' } } + + it 'returns hash' do + expect(subject.as_json).to eq json + end + end + + describe '.keys' do + it 'returns an array' do + expect(described_class.keys).to be_a Array + end + end + + describe '.definition_for' do + context 'when key is defined' do + it 'returns a setting' do + expect(described_class.definition_for(:always_send_emails)).to be_a UserSettings::Setting + end + end + + context 'when key is not defined' do + it 'returns nil' do + expect(described_class.definition_for(:foo)).to be_nil + end + end + end + + describe '.definition_for?' do + context 'when key is defined' do + it 'returns true' do + expect(described_class.definition_for?(:always_send_emails)).to be true + end + end + + context 'when key is not defined' do + it 'returns false' do + expect(described_class.definition_for?(:foo)).to be false + end + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3e7b59f17..ab883927a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -313,9 +313,9 @@ RSpec.describe User, type: :model do end describe 'settings' do - it 'is instance of Settings::ScopedSettings' do + it 'is instance of UserSettings' do user = Fabricate(:user) - expect(user.settings).to be_a Settings::ScopedSettings + expect(user.settings).to be_a UserSettings end end @@ -379,16 +379,6 @@ RSpec.describe User, type: :model do end end - it_behaves_like 'Settings-extended' do - def create! - User.create!(account: Fabricate(:account, user: nil), email: 'foo@mastodon.space', password: 'abcd1234', agreement: true) - end - - def fabricate - Fabricate(:user) - end - end - describe 'token_for_app' do let(:user) { Fabricate(:user) } let(:app) { Fabricate(:application, owner: user) } -- cgit From b4f38edf74e376f6006def9bf9fc22db40586264 Mon Sep 17 00:00:00 2001 From: Christian Schmidt Date: Fri, 31 Mar 2023 07:33:17 +0200 Subject: Wrong type for user setting when default is defined by lambda (#24321) --- app/models/user_settings/setting.rb | 3 ++- spec/models/user_settings/setting_spec.rb | 32 +++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) (limited to 'spec/models') diff --git a/app/models/user_settings/setting.rb b/app/models/user_settings/setting.rb index c359c593b..5f5504254 100644 --- a/app/models/user_settings/setting.rb +++ b/app/models/user_settings/setting.rb @@ -19,7 +19,8 @@ class UserSettings::Setting end def type - if @default_value.is_a?(TrueClass) || @default_value.is_a?(FalseClass) + case default_value + when TrueClass, FalseClass ActiveModel::Type::Boolean.new else ActiveModel::Type::String.new diff --git a/spec/models/user_settings/setting_spec.rb b/spec/models/user_settings/setting_spec.rb index 6e4ec6789..9884ae4f8 100644 --- a/spec/models/user_settings/setting_spec.rb +++ b/spec/models/user_settings/setting_spec.rb @@ -30,6 +30,38 @@ RSpec.describe UserSettings::Setting do it 'returns a type' do expect(subject.type).to be_a ActiveModel::Type::Value end + + context 'when default value is a boolean' do + let(:default) { false } + + it 'returns boolean' do + expect(subject.type).to be_a ActiveModel::Type::Boolean + end + end + + context 'when default value is a string' do + let(:default) { '' } + + it 'returns string' do + expect(subject.type).to be_a ActiveModel::Type::String + end + end + + context 'when default value is a lambda returning a boolean' do + let(:default) { -> { false } } + + it 'returns boolean' do + expect(subject.type).to be_a ActiveModel::Type::Boolean + end + end + + context 'when default value is a lambda returning a string' do + let(:default) { -> { '' } } + + it 'returns boolean' do + expect(subject.type).to be_a ActiveModel::Type::String + end + end end describe '#type_cast' do -- cgit