Commit c2b724b5 authored by Stan Hu's avatar Stan Hu

Fix inability to set snippet visibility via API

Consider the scenario:

1. The snippet visibility level is set to internal
2. A user attempts to create a private snippet

Previously this would always fail because `default_value_for` would
overwrite the private visibility setting, no matter what
visibility_level was. This was happening because `default_value_for` was
confused by the default value of 0 specified by the database schema.

`default_value_for` attempts to assign the default value in the block by
checking whether the attribute has changed. The problem is that since
the default value by the database was 0, and the user requested 0, this
appeared as though no changes were made. As a result,
`default_value_for` would always overwrite the user's preference.

To fix this, we remove the use of `default_value_for` and only set the
visibility level to the default application setting when no preference
has been given at creation time.

This is similar to the fix in
https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/29578.

Closes https://gitlab.com/gitlab-org/gitlab/issues/34016
parent b370adbe
...@@ -33,8 +33,6 @@ class Snippet < ApplicationRecord ...@@ -33,8 +33,6 @@ class Snippet < ApplicationRecord
default_content_html_invalidator || file_name_changed? default_content_html_invalidator || file_name_changed?
end end
default_value_for(:visibility_level) { Gitlab::CurrentSettings.default_snippet_visibility }
belongs_to :author, class_name: 'User' belongs_to :author, class_name: 'User'
belongs_to :project belongs_to :project
...@@ -139,6 +137,24 @@ class Snippet < ApplicationRecord ...@@ -139,6 +137,24 @@ class Snippet < ApplicationRecord
@link_reference_pattern ||= super("snippets", /(?<snippet>\d+)/) @link_reference_pattern ||= super("snippets", /(?<snippet>\d+)/)
end end
def initialize(attributes = {})
# We can't use default_value_for because the database has a default
# value of 0 for visibility_level. If someone attempts to create a
# private snippet, default_value_for will assume that the
# visibility_level hasn't changed and will use the application
# setting default, which could be internal or public.
#
# To fix the problem, we assign the actual snippet default if no
# explicit visibility has been initialized.
attributes ||= {}
unless visibility_attribute_present?(attributes)
attributes[:visibility_level] = Gitlab::CurrentSettings.default_snippet_visibility
end
super
end
def to_reference(from = nil, full: false) def to_reference(from = nil, full: false)
reference = "#{self.class.reference_prefix}#{id}" reference = "#{self.class.reference_prefix}#{id}"
......
---
title: Fix inability to set snippet visibility via API
merge_request: 18612
author:
type: fixed
...@@ -133,6 +133,32 @@ describe Snippet do ...@@ -133,6 +133,32 @@ describe Snippet do
end end
end end
describe 'when default snippet visibility set to internal' do
using RSpec::Parameterized::TableSyntax
before do
stub_application_setting(default_snippet_visibility: Gitlab::VisibilityLevel::INTERNAL)
end
where(:attribute_name, :value) do
:visibility | 'private'
:visibility_level | Gitlab::VisibilityLevel::PRIVATE
'visibility' | 'private'
'visibility_level' | Gitlab::VisibilityLevel::PRIVATE
end
with_them do
it 'sets the visibility level' do
snippet = described_class.new(attribute_name => value, title: 'test', file_name: 'test.rb', content: 'test data')
expect(snippet.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
expect(snippet.title).to eq('test')
expect(snippet.file_name).to eq('test.rb')
expect(snippet.content).to eq('test data')
end
end
end
describe '.with_optional_visibility' do describe '.with_optional_visibility' do
context 'when a visibility level is provided' do context 'when a visibility level is provided' do
it 'returns snippets with the given visibility' do it 'returns snippets with the given visibility' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment