Skip to content

a8n: Add ability to update Campaign's CampaignPlan in UpdateCampaign

Warren Giffordrequested to merge
a8n/update-campaign-plan-id into master

Created by: mrnugget

This PR implements the majority of the implementation proposed in RFC 95 (see rest of ticket for what's missing). It started out as a proof of concept but as I realized that updating a campaign might not be that complicated if we say that updating a Campaign means changing its CampaignPlanID I implemented it "properly" (there's still bits missing, like Bitbucket Server support, see bottom of this description).

That being said: this idea behind updating a Campaign (and the API) still needs to be validated by @eseliger and @felixfbecker. That's what I expect as their review here.

What I want from @tsenart @ryanslade is a "standard" technical review with feedback on APIs and implementation, so I can work on the PR and possibly shift its direction/implementation while @eseliger and @felixfbecker think about the frontend side of this implementation.

What's in here?

It extends the GraphQL API to accept an optional plan: ID in UpdateCampaignArgs. If the ID points to a finished CampaignPlan, the Campaign will be updated to the new CampaignPlan.

Every CampaignJob (changeset in GraphQL) belonging to the new CampaignPlan will then be compared with the existing ChangesetJobs/Changesets pairs (ExternalChangeset in GraphQL) to decide whether to

  1. detach ExternalChangeset from Campaign and close it on codehost
  2. update title/body/diff of ExternalChangeset on codehost
  3. keep ExternalChangeset as it is
  4. create new ExternalChangeset on the codehost

The decision what to do with which changeset happens synchronously in the updateCampaign mutation.

Then, asynchronously, the changesets are updated/created/closed on the codehost.

What's missing?

The update process as described above is implemented in this PR, except:

  • Close detached changesets on the codehosts
  • Updating changesets on BitbucketServer. That only works for GitHub now.

But I don't think these need to necessarily be in this PR and can be addressed in follow-ups.

Merge request reports