Handle Snippet types in Create::Snippet service

Why

Snippets::CreateService is growing in complexity as it handles both snippets and personal snippets with conditional build logic.

We can improve the readability, test coverage, and ease of changes by extracting a few services out of this.

What

Extract existing functionality out into new services. Rough outline

  • PersonalSnippets::BuildService
  • PersonalSnippets::CreateService
  • ProjectSnippets::BuildService
  • PersonalSnippets::CreateService

Might be worth breaking out Validation service too.

Once those are extracted, the original Snippets::CreateService would serve as a place to isolate conditional logic to a single source of truth and call the appropriate extracted services.

Pseudo code example to show general shape of idea:

module Snippets
  class CreateService < Snippets::BaseService
    # ...
    
    def execute
      if project
        snippet = ProjectSnippets::Build(params)
        ProjectSnippets::Validate(snippet)
        ProjectSnippets::Create(snippet)
      else
        snippet = PersonalSnippets::Build(params)
        PersonalSnippets::Validate(snippet)
        PersonalSnippets::Create(snippet)
      end
    end
  end
end

This refactor uses techniques we already have in the code base (search services/build for example) to keep this class from getting too tangled in conditional logic around the "type" of Snippet and the various operations like build -> validate -> create also needing special treatment.

More files to clean up during refactor:

spec/lib/gitlab/github_gists_import/importer/gist_importer_spec.rb

lib/gitlab/github_gists_import/importer/gist_importer.rb

Propoasl

Keep MRs smaller consolidated by concept

  • 1 MR for PersonalSnippets::BuildService and ProjectSnippets::BuildService
  • 1 MR for PersonalSnippets::ValidateService and ProjectSnippets::ValidateService
  • 1 MR for PersonalSnippets::CreateService and ProjectSnippets::CreateService
  • 1 MR to use services in lib/gitlab/github_gists_import/importer/gist_importer.rb

This should keep it reviewable and conceptually cohesive. The MRs that introduce the services should probably go ahead and invoke them in the create service.

Edited by Hunter Stewart