Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

New campaigns flow: implement moveCampaign mutation #12049

Merged
merged 1 commit into from
Jul 9, 2020

Conversation

mrnugget
Copy link
Contributor

@mrnugget mrnugget commented Jul 9, 2020

This implements the next missing mutation as part of #11675: moveCampaign.

It also adds "null ID" tests and fixes one that was failing.

@mrnugget mrnugget added campaigns batch-changes Issues related to Batch Changes labels Jul 9, 2020
@mrnugget mrnugget added this to the 3.18 milestone Jul 9, 2020
@mrnugget mrnugget requested a review from a team July 9, 2020 13:05
@mrnugget mrnugget requested a review from slimsag as a code owner July 9, 2020 13:05
@mrnugget mrnugget requested a review from a team July 9, 2020 13:05
@mrnugget mrnugget mentioned this pull request Jul 9, 2020
28 tasks
@mrnugget mrnugget removed request for a team and slimsag July 9, 2020 13:06
@mrnugget mrnugget merged this pull request into campaign-spec-api-v1 Jul 9, 2020
@mrnugget mrnugget deleted the new-campaigns-flow/move-campaign branch July 9, 2020 13:39
mrnugget added a commit that referenced this pull request Jul 9, 2020
eseliger pushed a commit that referenced this pull request Jul 15, 2020
eseliger pushed a commit that referenced this pull request Jul 16, 2020
mrnugget added a commit that referenced this pull request Jul 20, 2020
mrnugget added a commit that referenced this pull request Jul 22, 2020
mrnugget added a commit that referenced this pull request Jul 22, 2020
* WIP: New campaigns workflow

* Remove old UI for switch to new campaigns workflow (#11891)

* Add a basic implementation of the applyCampaign mutation (#11934)

* update campaigns docs to reflect new flow (#11972)

* update GraphQL API to remove unneeded namespace param, add applyCampaign test (#11982)

* add applyCampaign resolver test, temporarily un-implement createCampaign

The createCampaign mutation is strictly less powerful than applyCampaign, and it adds needless complexity to implement createCampaign now, especially since we'll be changing the impl of applyCampaign a lot. So, let's remove the createCampaign impl for now.

Then we also need to change the resolver test from testing createCampaign to testing applyCampaign.

* remove unneeded namespace param from createCampaign and applyCampaign mutations

The namespace is immutably stored in the campaign spec, which is also provided as a param.

* Implement moveCampaign mutation (#12049)

* Fix campaigns web code after rebase

* Implement ChangesetSpec resolver and validation of specs (#12092)

* Implement ChangesetSpec.Description in resolver and data layer

* Add ChangesetSpec/CampaignSpec schema and Validate() method

* Validate ChangesetSpec against schema on create

* Validate CampaignSpec against schema

* Allow YAML as CampaignSpec input (caveat: 'on' needs to be quoted)

* Fix critical whitespace error

* Fix CampaignSpec.ParsedInput response

* Remove debug log statement

* Remove TODO comment

* Fix indentation in raw specs for testing

* Run prettier

Co-authored-by: Erik Seliger <erikseliger@me.com>

* Use fork of ghodss/yaml to use 'on' as key in CampaignSpec (#12153)

As we noticed last week: we can't use `on` as a key in a `CampaignSpec`
because `yaml.YAMLToJSON` would convert that `on` into a `true` and that
in turn would fail when validated.

The solution here is to use a custom fork of `ghodss/yaml` that allows
passing in a custom unmarshal-function which does _not_ do the
conversion.

The function we're passing in comes from the yaml.v3 library which
changed its behavior when parsing boolean values (see
ghodss/yaml#65).

We lose the ability to use `YAMLToJSONStrict` since that only works with
yaml.v2, but yaml.v3 already warns about duplicated keys and the JSON
schema validation gives us enough of a safety net for the rest.

* Remove duplication in ChangesetSpec/CampaignSpec.UnmarshalValidate (#12156)

* Remove repositoryDiffs (#12205)

* Remove openChangesets resolver (#12209)

We don't need it for the new UI anymore, so reducing the headache of maintaining it.

* Implement minimal resolvers to incorporate schema changes (#12215)

* Use same changeset resolver for hidden and visible changesets (#12221)

* Fix compilation after rebase

* Delete expired CampaignSpecs and ChangesetSpecs (#12210)

* Fix compilation after rebase

* Delete expired Campaign/ChangesetSpecs

* Return correct ExpiresAt in Campaign/ChangesetSpec resolvers

* Clean up tests for spec expiry

* Remove unneeded Truncate

Co-authored-by: Erik Seliger <erikseliger@me.com>

* Remove unused dependency (#12245)

* Implement placeholders for status and preview API (#12226)

* Remove store.GetRepoIDsForFailedChangesetJobs (#12323)

This was a left-over from the last PR that removed some functionality.

I know that there's a lot more code that's technically unused now, but
this one stood out since it had the really specific usecase of loading
error messages for a campaign.

* Another set of small follow-ups after introducing ExternalChangesetState (#12330)

* Add a ChangesetState.Valid() method

* Rename test method to reflect name of method under test

* Update naming and un-export methods

* Implement missing bits in CampaignSpec/ChangesetSpec resolvers (#12247)

* Fix intendation of query in test

* Implement CampaignSpec.ViewerCanAdminister

* Add safety-check to ChangesetSpecDescription.Diff method

* Clean up repository permissions test

* Implement rest of ChangesetSpec and CampaignSpec resolvers

* Add options to CountChangesetSpecsOpts

* Implement changesetSpecConnectionResolver

* Reduce test data after reducing test cases

* Change type checks on ChangesetSpecDescription

* Query __typename in test to make sure we get the right response

* Use same repo query pattern in ChangesetResolver as in changesetSpecResolver

* Implement the after param for changesetSpecs connection

* Check for permissions in applyCampaign and moveCampaign

* Add a test for ChangesetSpec.Description.Diff

* Remove isHidden filter

* Do not yield ChangesetSpec if repo is (soft-)deleted

* Fix rebase gone wrong

* Check namespace perms in CreateCampaignSpec/MoveCampaign (#12362)

(This is part #11675 and the new workflow)

Before this, every user could create a campaign spec in any org or user
namespace. And the same was true for moveCampaign: a user could move a
campaign into namespace they wanted.

This changes both implementations in the service layer to check for the
permissions of the current user (saved in the ctx):

1. If it's a site admin, they can create/move things in any namespace.
2. If not and the target namespace is an org, we check for org
   membership.
3. If not and the target namespace is a user, we check that it's the
   same as the current user.

* Make schema for env more strict to only accept strings (#12335)

Environment variables can only be strings, so we shouldn't accept int, bool etc.

* Drop old campaigns tables and add missing columns (#12388)

* Drop old campaigns tables and add missing columns

This drops the following tables:

- changeset_jobs
- patches
- patch_sets

Why drop them? Since we decided we're going to migrate existing
campaigns into read-only versions and that we don't want to keep working
state (changeset_jobs) around, it doesn't make sense to keep the tables.
Why keep empty tables?

So, this removes the tables and all the code that depends on them.

It also does two other things:

- Add a `changeset_spec_id` column to `changesets` (that's not yet read/written to in the `campaigns.Store`)
- Add three `diff_stat_*` columns to `changeset_specs`, populate them when creating a new `ChangesetSpec` and reading/writing them in the `campaigns.Store`.

Why? Because I don't want to write another migration that adds diff
stats to changeset specs in case those were created between the
introduction of changeset specs and us adding the fields.

* Do not show changeset count in frontend

* Add notice to WIP campaigns docs that they're WIP (#12393)

* Campaign frontend cleanup work (#12256)

Co-authored-by: Quinn Slack <sqs@sourcegraph.com>
Co-authored-by: Erik Seliger <erikseliger@me.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
batch-changes Issues related to Batch Changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants