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

batches: Surface commitSigningConfiguration on BatchChangesCodeHost type#52733

Merged
st0nebreaker merged 30 commits into
batches/commit-signingfrom
becca/new-BatchChangesCodeHost-property
Jun 5, 2023
Merged

batches: Surface commitSigningConfiguration on BatchChangesCodeHost type#52733
st0nebreaker merged 30 commits into
batches/commit-signingfrom
becca/new-BatchChangesCodeHost-property

Conversation

@st0nebreaker

@st0nebreaker st0nebreaker commented May 31, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #52484.

  • Surfaces a GitHubAppConfiguration property on BatchChangesCodeHost type
  • Normalizes github_app.base_url
    • Adds migration to fix past entries
  • Adds a check to not create duplicate batches GH Apps on the same base_url
    • Follow up issue: #52792

image

Test plan

  • Query new field
query {
  batchChangesCodeHosts {
    nodes {
      externalServiceURL
      supportsCommitSigning
      commitSigningConfiguration {
        __typename
        	... on GitHubAppConfiguration {
          	appID
            name
            appURL
            logo
          }
      }
    }
  }
}
  • sg migration up successfully adds a trailing / on any github_apps.base_url missing it
  • Change this line to if domain == types.ReposDomain {, go through the UI to add a new GitHub App on an existing base_url, observe it error

BolajiOlajide and others added 5 commits May 23, 2023 20:23
@BolajiOlajide @st0nebreaker @kopancek
batches: Add domain column to github_apps migration (#52332)
48291ea 
Co-authored-by: Becca Steinbrecher <becca.steinbrecher@sourcegraph.com>
Co-authored-by: Milan Freml <kopancek@users.noreply.github.com>
close #52264
@st0nebreaker @kopancek
@BolajiOlajide @courier-new
Expose new domain property on github_app nodes (#52269)
14b81ad 
Closes #52270
Exposes new `domain` property from #52268

## Test plan
- Make sure there is a GitHub App on your instance
- In the API Console ensure the following query doesn't fail and returns
the `domain` property:
```
query {
  gitHubApps {
    nodes {
      name
      domain
    }
  }
}
````

---------

Co-authored-by: Milan Freml <kopancek@users.noreply.github.com>
Co-authored-by: Bolaji Olajide <25608335+BolajiOlajide@users.noreply.github.com>
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
@courier-new
Merge branch 'main' into batches/commit-signing
5eab7f8
@BolajiOlajide
batches: Specify domain when creating a Github App (#52347)
98bc90f 
Closes [#52346](https://github.com/sourcegraph/sourcegraph/issues/52346)

This PR allows the domain to be specified when creating a GitHub app.
Right now it's hard coded to only work with the `repos` domain till we
figure out the UX for creating a Batch Changes GitHub App.

## Test plan

<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
* Create a GitHub app.
* inspect the `github_apps` table for the newly created GitHub app - the
app should have the value of `repos` in the `domain` column.
@st0nebreaker
Resolve merge conflict
b74198c
@st0nebreaker st0nebreaker self-assigned this May 31, 2023
@cla-bot cla-bot Bot added the cla-signed label May 31, 2023
@st0nebreaker st0nebreaker changed the base branch from main to batches/commit-signing May 31, 2023 21:23
@sourcegraph-buildkite

sourcegraph-buildkite commented May 31, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Bundle size report 📦

Initial size Total size Async size Modules
0.01% (+0.22 kb) 0.00% (+0.65 kb) 0.00% (+0.43 kb) 0.00% (0)

Look at the Statoscope report for a full comparison between the commits 74a2524 and 9ee59be or learn more.

Open explanation
  • Initial size is the size of the initial bundle (the one that is loaded when you open the page)
  • Total size is the size of the initial bundle + all the async loaded chunks
  • Async size is the size of all the async loaded chunks
  • Modules is the number of modules in the initial bundle

@st0nebreaker st0nebreaker mentioned this pull request Jun 1, 2023
Open
@st0nebreaker st0nebreaker force-pushed the becca/new-BatchChangesCodeHost-property branch from 703446d to ef1c773 Compare June 1, 2023 19:27
st0nebreaker
st0nebreaker commented Jun 1, 2023
View reviewed changes
Comment thread enterprise/cmd/frontend/internal/auth/githubappauth/middleware.go Outdated
@st0nebreaker st0nebreaker marked this pull request as ready for review June 1, 2023 22:56
@st0nebreaker st0nebreaker requested a review from a team June 1, 2023 22:56
@sourcegraph-bot

sourcegraph-bot commented Jun 1, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 74767e7...91839f2.

Notify File(s)
@courier-new cmd/frontend/graphqlbackend/batches.go
@eseliger cmd/frontend/graphqlbackend/batches.go

@sourcegraph-bot

sourcegraph-bot commented Jun 1, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

📖 Storybook live preview

courier-new
courier-new approved these changes Jun 2, 2023
View reviewed changes

@courier-new courier-new left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very nice 👍

Comment thread cmd/frontend/graphqlbackend/batches.graphql Outdated
Comment thread cmd/frontend/graphqlbackend/batches.graphql Outdated
Comment thread enterprise/cmd/frontend/internal/batches/resolvers/code_host.go Outdated
Comment thread enterprise/cmd/frontend/internal/batches/resolvers/code_host.go Outdated
Comment thread enterprise/internal/github_apps/store/store.go Outdated
Comment thread enterprise/internal/github_apps/store/store.go Outdated
Comment thread enterprise/internal/github_apps/store/store_test.go Outdated
st0nebreaker and others added 7 commits June 2, 2023 16:15
@st0nebreaker @courier-new
Update cmd/frontend/graphqlbackend/batches.graphql
c6c1cfc 
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
@st0nebreaker @courier-new
Update cmd/frontend/graphqlbackend/batches.graphql
6568162 
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
@st0nebreaker @courier-new
Update enterprise/cmd/frontend/internal/batches/resolvers/code_host.go
20808e7 
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
@st0nebreaker @courier-new
Update enterprise/cmd/frontend/internal/batches/resolvers/code_host.go
90d00b9 
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
@st0nebreaker @courier-new
Update enterprise/internal/github_apps/store/store.go
95a09f3 
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
@st0nebreaker @courier-new
Update enterprise/internal/github_apps/store/store.go
b2b25aa 
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
@st0nebreaker
Add does not exist test
c50c309
courier-new
courier-new reviewed Jun 2, 2023
View reviewed changes
Comment thread enterprise/internal/github_apps/store/store_test.go Outdated
courier-new
courier-new reviewed Jun 3, 2023
View reviewed changes
Comment thread enterprise/internal/github_apps/store/store.go
Comment thread enterprise/internal/github_apps/store/store.go Outdated
Comment thread enterprise/internal/github_apps/store/store.go
courier-new
courier-new reviewed Jun 3, 2023
View reviewed changes

@courier-new courier-new left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added a couple small suggestions around the error handling of the unique batches app check!

@courier-new courier-new mentioned this pull request Jun 5, 2023
Merged
st0nebreaker and others added 4 commits June 5, 2023 09:51
@st0nebreaker @courier-new
Update enterprise/internal/github_apps/store/store.go
0f5ba5a 
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
@st0nebreaker @courier-new
Update enterprise/internal/github_apps/store/store.go
bcbd3c4 
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
@st0nebreaker @courier-new
Update enterprise/internal/github_apps/store/store.go
8d1a496 
Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
@st0nebreaker
Fix error concatenation
d64184c
@st0nebreaker st0nebreaker force-pushed the becca/new-BatchChangesCodeHost-property branch from 37a8ec5 to d64184c Compare June 5, 2023 16:14
@st0nebreaker st0nebreaker merged commit b76c993 into batches/commit-signing Jun 5, 2023
@st0nebreaker st0nebreaker deleted the becca/new-BatchChangesCodeHost-property branch June 5, 2023 16:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@courier-new courier-new courier-new approved these changes

@BolajiOlajide BolajiOlajide BolajiOlajide left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@st0nebreaker st0nebreaker

Labels

cla-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Surface commitSigningConfiguration on BatchChangesCodeHost type

5 participants

@st0nebreaker @sourcegraph-buildkite @sourcegraph-bot @courier-new @BolajiOlajide