Re-attach changesets to campaign that were owned by campaign

Created by: mrnugget

This fixes https://github.com/sourcegraph/sourcegraph/issues/13849 by re-opening changesets that were previously detached from the campaign.

See this Slack thread for a demo video that I recorded to show off this bug fix and what it fixes.

Thoughts on design

I'm not 100% happy with the code in the reconciler yet. I have the feeling that there's some design that makes the distinction between applyCampaign-directed state transitions (e.g. closing a changeset when detaching it from a campaign, or, like here, re-opening a detached one) and user-directed updates (e.g. update title and commit of a PR) clearer and easier to handle.

At its heart this problem revolves around the fact that a subset of the desired changeset state is included in the changeset spec and another set is not (since we don't allow closing in the spec). If we include our internal-state-directives in the changeset specs, they'd need to be in the schema and we'd need to create new changeset specs every time we want to make a change. That seems odd.

What we could do is to make this idea of internal-state-directives clearer by bundling them in a single column, for example, or treating all of them the same way. Not sure yet.

However, I think the code is fine to merge as it is. It's just something I keep thinking about.

Caveat

As it stands, this implementation could lead to changesets being re-opened after the user closed them on the code host. In practice, though, this won't currently happen, since the reconciler is only kicked off on user action, in which case we do want to reopen the changesets.

In order to avoid that, we could add another boolean on the changesets, reopening bool, that, just like closing, would cause the reconciler to reopen the changeset only on applyCampaign.

Created by: sourcegraph-bot

Notifying subscribers in CODENOTIFY files for diff 894e9c90bcce44b79d215faf18d02b351c6d483a...d45bdee30e50f96b76a5bb08ab8954ac11b62771.

Notify	File(s)
@eseliger	enterprise/internal/campaigns/reconciler.go enterprise/internal/campaigns/reconciler_test.go enterprise/internal/campaigns/service_apply_campaign.go enterprise/internal/campaigns/service_apply_campaign_test.go enterprise/internal/campaigns/store_changesets.go enterprise/internal/campaigns/store_changesets_test.go enterprise/internal/campaigns/testing/changeset_source.go

Created by: codecov[bot]

Codecov Report

Merging #14099 into main will decrease coverage by 22.44%. The diff coverage is n/a.

@@             Coverage Diff             @@
##             main   #14099       +/-   ##
===========================================
- Coverage   51.79%   29.34%   -22.45%     
===========================================
  Files        1524      713      -811     
  Lines       77703    20901    -56802     
  Branches     6991     6366      -625     
===========================================
- Hits        40244     6134    -34110     
+ Misses      33833    14722    -19111     
+ Partials     3626       45     -3581     

Flag	Coverage Δ
#go	?
#integration	30.44% <ø> (+0.09%)
#storybook	?
#typescript	29.34% <ø> (-22.00%)
#unit	19.47% <ø> (-14.45%)

Impacted Files	Coverage Δ
shared/src/util/jsonc.ts	0.00% <0.00%> (-100.00%)
shared/src/backend/errors.ts	0.00% <0.00%> (-100.00%)
shared/src/util/wordHelpers.ts	0.00% <0.00%> (-100.00%)
shared/src/settings/settings.ts	0.00% <0.00%> (-100.00%)
shared/src/util/createRecord.ts	0.00% <0.00%> (-100.00%)
shared/src/search/parser/tokens.ts	0.00% <0.00%> (-100.00%)
shared/src/util/rxjs/repeatUntil.ts	0.00% <0.00%> (-100.00%)
shared/src/api/client/api/content.ts	0.00% <0.00%> (-100.00%)
shared/src/api/client/types/hover.ts	0.00% <0.00%> (-100.00%)
shared/src/util/memoizeObservable.ts	0.00% <0.00%> (-100.00%)
... and 998 more