Skip to content

Commit

Permalink
Apply suggestions from comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mayorova committed Jun 6, 2024
1 parent 8375bc7 commit 286bf51
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 6 deletions.
8 changes: 6 additions & 2 deletions app/models/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def self.columns
end

def self.non_null_columns_names
columns.select { |column| column.null == false }.map(&:name)
columns.select { |column| !column.null }.map(&:name)
end

def approval_required_editable?
Expand All @@ -39,10 +39,14 @@ def approval_required_disabled?
not_custom_account_plans.size > 1 && account_plans_ui_visible?
end

def assign_attributes(attrs, options = {})
super(sanitize_attributes(attrs), options)
end

def update(attrs)
update_approval_required(attrs) if approval_required_editable?

super(sanitize_attributes(attrs))
super(attrs)
end

def set_forum_enabled
Expand Down
6 changes: 3 additions & 3 deletions test/integration/admin/api/settings_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def setup

test 'update' do
params = { access_token: token, signups_enabled: false, change_account_plan_permission: 'invalid', change_service_plan_permission: 'invalid'}
assert 'request', settings.change_account_plan_permission
assert_equal 'request', settings.change_account_plan_permission
assert settings.signups_enabled

put admin_api_settings_path(format: :json), params: params
Expand All @@ -50,8 +50,8 @@ def setup
assert_response :success

settings.reload
assert 'direct', settings.change_account_plan_permission
assert 'none', settings.change_service_plan_permission
assert_equal 'direct', settings.change_account_plan_permission
assert_equal 'none', settings.change_service_plan_permission
assert_not settings.signups_enabled
end

Expand Down
8 changes: 7 additions & 1 deletion test/unit/settings_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,21 @@ def test_require_cc_on_signup_visible_ui_switch_on_rolling_updates
assert settings.monthly_billing_enabled
end

test 'empty values sanitized for non-null columns' do
test 'empty values are skipped for non-null columns' do
settings.update(public_search: true)
assert settings.reload.public_search

settings.update(public_search: "")
assert_not settings.previous_changes[:public_search]
assert settings.reload.public_search

settings.update(public_search: nil)
assert_not settings.previous_changes[:public_search]
assert settings.reload.public_search

settings.update(public_search: "false")
assert settings.previous_changes[:public_search]
assert_not settings.reload.public_search
end

test "validate change plan permission values" do
Expand Down

0 comments on commit 286bf51

Please sign in to comment.