Skip to content

Remove owner validation in AdditionalPack transfer

Vijay Hawoldar requested to merge vij-modify-minutes-validation into master

What does this MR do and why?

Originally when changing the namespace linked to a GitLab.com subscription, I thought it was only possible to do it between two namespaces where the user was an owner of both, so when updating Ci::Minutes::AdditionalPack records, we validate that both the source and target namespaces share an owner.

This is not the case - a user can be removed as an owner (or downgraded) for a group they purchased a subscription for, and still wish to change the namespace to another they own. In this scenario, the Ci::Minutes::AdditionalPack would fail and leave minutes associated to the old namespace, even though the subscription will have moved.

This MR removes the shared owner validation.

The service is only used by an internal API with an authorized admin and only used by our own CustomersDot application, so we should not need to re-validate this transfer in any case.

It's worth noting that the namespace update itself also does not perform this validation, as seen here.

How to set up and validate locally

  1. Create some Ci::Minutes::AdditionalPack records for a namespace:
    Ci::Minutes::AdditionalPack.create(namespace: namespace, number_of_minutes: 10000, expires_at: Date.current + 1.year, purchase_xid: 'abc123')
  2. Testing via the the API endpoint to transfer the minutes to a new namespace:
    curl --request PATCH \
      --url http://127.0.0.1:3000/api/v4/namespaces/:id/minutes/move/:target_id \
      --header 'PRIVATE-TOKEN: <your token>'
  3. Testing using the service directly:
    Ci::Minutes::AdditionalPacks::ChangeNamespaceService.new(admin_user, namespace, other_namespace).execute

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Vijay Hawoldar

Merge request reports