This repository has been archived by the owner on Sep 30, 2024. It is now read-only.
Prepare codebase for new campaigns workflow #11675
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mrnugget
force-pushed
the
campaign-spec-api-v1
branch
2 times, most recently
from
9b5a3f8 to
227b147
Compare
mrnugget
changed the title
mrnugget
added
campaigns
RFC-157
This was referenced Jun 25, 2020
eseliger
reviewed
Jun 25, 2020
mrnugget
force-pushed
the
campaign-spec-api-v1
branch
from
3171287 to
e0bee27
Compare
This was referenced Jun 30, 2020
|
(Sorry for the noise everybody, I hit "ready for review" around 1-2 weeks too early) |
mrnugget
force-pushed
the
campaign-spec-api-v1
branch
4 times, most recently
from
e22a8cb to
7b9522f
Compare
Codecov Report
@@ Coverage Diff @@
## master #11675 +/- ##
==========================================
- Coverage 50.93% 50.58% -0.36%
==========================================
Files 1428 1422 -6
Lines 81192 79978 -1214
Branches 6847 6660 -187
==========================================
- Hits 41357 40458 -899
+ Misses 36219 36006 -213
+ Partials 3616 3514 -102
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…ign 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 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]>
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.
We don't need it for the new UI anymore, so reducing the headache of maintaining it.
* 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]>
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.
…ate (#12330) * Add a ChangesetState.Valid() method * Rename test method to reflect name of method under test * Update naming and un-export methods
* 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
(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.
Environment variables can only be strings, so we shouldn't accept int, bool etc.
* 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
mrnugget
force-pushed
the
campaign-spec-api-v1
branch
from
26477cb to
3bdde88
Compare
eseliger
approved these changes
Jul 22, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR/branch contains the implementation of https://github.com/sourcegraph/sourcegraph/pull/10921 and serves as a merge-target for other PRs.
PRs targeting (or already merged into) this branch
Backend implementation ticket
We will only merge this into
masterwhen we're confident enough that it won't break the existing campaigns feature in a way that's unfixable before release.Other PRs will target this branch and thus make the review process easier.