[BE] Add files field to the snippet update API endpoint
Unlike the snippet create API endpoint changes (#217739 (closed)), we need to break a little bit the backward compatibility in this endpoint.
Because now we can have several files in the snippet repository, if the user just passes the file_name
param, we don't know which file the user wants to update the file name from. We need to add new param previous_path
that indicates which was the former file name and, in file_name
, we will store the new name the user wants to use for that file. In the same line of thought, if the user provides only content, we won't know which file he is referring to.
The final definition would be something like:
params do
requires :title, type: String, desc: 'The title of a snippet'
optional :previous_path, type: String, desc: 'The previous path of a snippet file'
given :previous_path do
at_least_one_of :content, :file_name
end
optional :file_name, type: String, desc: 'The new path of a snippet file'
given :file_name do
requires :previous_path, type: String
end
optional :content, type: String, desc: 'The content of a snippet'
given :content do
requires :previous_path, type: String
end
optional :description, type: String, desc: 'The description of a snippet'
optional :visibility, type: String,
values: Gitlab::VisibilityLevel.string_values,
default: 'internal',
desc: 'The visibility of the snippet'
optional :files, type: Array, as: :snippet_files do
requires :previous_path, type: String, desc: 'The previous path of a snippet file'
requires :file_path, type: String, desc: 'The new path of a snippet file'
requires :content, type: String, desc: 'The content of a snippet'
end
mutually_exclusive :files, :content # To avoid having both params
mutually_exclusive :files, :file_name # To avoid having both params
end
Disclaimer: I haven't tested this and there might be (for sure) some easier definition, this was a quick guess.
Nevertheless, we should avoid as far as possible avoid breaking things until we finally release multiple files feature. I'm not sure whether we can statically change the params definition based on a feature flag status. I think if we want to have different arguments dynamically we would have to rely on lambda and Procs and maybe leverage the if
argument that the give
method allows. I mean something like:
given :whatever, ->(_) { Feature.enabled?(:snippet_multiple_files, current_user) } do
...
end
But, if possible it would be better to have different params, the old ones when the flag is disabled and the newer ones if they're enabled. I guess this way is cleaner.
If we can use here feature flags, the flag name must be snippet_multiple_files
and it has to be scoped for users. Therefore, instead of checking if the feature flag is enabled or not globally (Feature.enabled?(:snippet_multiple_files)), we will check if the feature flag is enabled for specific users (Feature.enabled?(:snippet_multiple_files, user)).
Once we have implemented the params definition we will need to adapt those to the call made to the Snippets::UpdateService
.
We need to add also a deprecation notice in the desc
attribute for the file_name
, previous_path
, and content
arguments. The intention is to remove them in the future (14.0) and use files
only.
Proposal
This issue is going to introduce some form of breaking API change due to the nature of multi-file Snippet updates, after the feature flag has been turned on or removed.
To minimize the impact on our users that consume the Snippets REST API, we decided upon the following:
- Updates to Snippets with multiple files must use the new
files
field - Updates to Snippets with a single file may continue to use the existing API fields
To facilitate this, the following changes will need to be made as part of this issue:
- Update the documentation to clearly state that any requests that update Snippets with multiple files must use the new
files
field - If a request is for a multi-file Snippet, we will return an error if the
files
field is not used - If a request is for a single-file Snippet, the request will be treated as it currently is, and perform the action against the first file found for the Snippet