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

Remove repositoryDiffs #12205

Merged
merged 1 commit into from
Jul 15, 2020
Merged

Remove repositoryDiffs #12205

merged 1 commit into from
Jul 15, 2020

Conversation

eseliger
Copy link
Member

Not needed for new UI, so removing to reduce boilerplate code.

@eseliger eseliger requested review from slimsag and a team as code owners July 15, 2020 14:00
@eseliger eseliger requested review from a team July 15, 2020 14:00
Copy link
Contributor

@mrnugget mrnugget left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No tests need to be updated/removed?

@eseliger
Copy link
Member Author

We had no tests 😢

@eseliger eseliger merged commit c8564ac into campaign-spec-api-v1 Jul 15, 2020
@eseliger eseliger deleted the es/remove-repodiffs branch July 15, 2020 15:06
@LawnGnome
Copy link
Contributor

We had no tests 😢

Task failed successfully.

eseliger added a commit that referenced this pull request Jul 15, 2020
eseliger added a commit that referenced this pull request Jul 16, 2020
mrnugget pushed a commit that referenced this pull request Jul 20, 2020
mrnugget pushed 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 <[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]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants