Skip to content

Migrate snippet service subclasses to BaseProjectServices [RUN ALL RSPEC] [RUN AS-IF-FOSS]

Chad Woolley requested to merge caw-snippets-use-base-container-service into master

What does this MR do?

What

  • Continuation of refactoring discussed in !30979 (comment 337747596), and continued in previous MR !59182 (merged) for issuables.
  • Refactors Snippet-related service classes to subclass BaseContainerService instead of BaseService, via Projects::BaseProjectService.

Not in scope

  • Refactoring PersonalSnippet services to not be a subclass of BaseContainerService, because it does not involve a project. We could potentially split `BaseContainerService into modules in a separate refactor. We could open an issue, but this will likely need to be holistically addressed for many service classes, perhaps as part of https://gitlab.com/gitlab-org/architecture/tasks/-/issues/7

Why

This provides a greater degree of type safety in order to more easily and safely introduce a new named argument spam_params to support !58603 (merged).

The additional type safety will also make future refactors easier and less risky, such as https://gitlab.com/gitlab-org/architecture/tasks/-/issues/7 and !54792 (merged).

Screenshots (strongly suggested)

Refactored class hierarchy

Expand to see class hierarchy changed, or view screenshot below:

Summary of class hierarchy changed
Snippets::BaseService (base_service.rb)
    Snippets::CreateService (create_service.rb)
    Snippets::UpdateService (update_service.rb)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

See sections above for a detailed discussion of the risk.

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by Chad Woolley

Merge request reports

Loading