-
Notifications
You must be signed in to change notification settings - Fork 1.3k
update campaigns docs to reflect new flow #11972
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving to unblock, but given our discussion today, it's likely we won't have that flow ready in 3.18, and the docs would guide you through a process that doesn't yet exist. Might that confuse existing/beta customers?
Also, do you plan to own the todo comments in the documents or should we do that?
"type": {"type":"string", "enum": ["docker", "command"]}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just making it valid json
"type": {"type":"string", "enum": ["docker", "command"]}, | |
"type": {"type":"string", "enum": ["docker", "command"]} |
This PR is based on the new branch not master. Does that address your concern? I’ll own the TODOs since I have time today and tomorrow. |
Oh sorry, I missed that 🤦♂️ yes, resolves it :) |
This incorporates the docs changes from #10921. These changes are what we're implementing now, so merging them in now will make it easier to review any changes from here.
12ab984
to
dc0ad5c
Compare
* 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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> Co-authored-by: Erik Seliger <[email protected]>
This incorporates the docs changes from #10921. These changes are what we're implementing now, so merging them in now will make it easier to review any changes from here.