[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:

  1. Update the documentation to clearly state that any requests that update Snippets with multiple files must use the new files field
  2. If a request is for a multi-file Snippet, we will return an error if the files field is not used
  3. 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
Edited by Vijay Hawoldar