Repository navigation
Move janitor's PR-review routes onto HttpApi - #133
Merged
Merged
Conversation
Tests only: PullRequestReview stays on Postgres but calls the pullRequestReview group through makeClient instead of the raw handler. Refusals are asserted by tag: RepositoryNotFound for an unknown repository, PrivateRepository for a private or unseen one, RunNotFound for an unknown or foreign run and TranscriptExpired for an expired transcript. An unreachable runner dies. The send helper, the cross-site case (Routes covers it) and the malformed-id 400 cases go. The web transcript test now serves tagged error bodies and maps RunNotFound to the missing-run copy. The cluster tests fail until the pullRequestReview group lands.
The pullRequestReview group (getSettings, updateSettings, listRuns and getRunTranscript) joins JanitorApi, and the cluster serves it through HttpApiBuilder behind the API middleware. The handlers fail with domain errors. Unknown repositories get RepositoryNotFound, and enabling a private or unseen repository gets PrivateRepository. A run outside the repository gets RunNotFound, and an expired transcript gets TranscriptExpired. The three new classes live in PullRequestReview/Settings. Store, read model, config and run entity failures die, so the old 503 with Retry-After and the prose 410 message are gone. The web settings panel, runs list and transcript call api.pullRequestReview.* and own the copy for each tag. The transcript's Download JSON link is an href, so it keeps its hand-built path. lib/api.ts loses its last caller and goes.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Moves the
pullRequestReviewgroup ontoHttpApi:getSettings,updateSettings,listRunsandgetRunTranscript. With that,apps/web/src/lib/api.tshas no callers left, so it's deleted.Api/PullRequestReview.tsdefines the group, andJanitorApiadds it.PullRequestReview/Settings.tsgainsPrivateRepository(409),RunNotFound(404) andTranscriptExpired(410).Ingress/PullRequestReview.tsis now oneHttpApiBuilder.grouplayer behind the API middleware. Same-origin writes, Access and Membership work as before. Unknown repositories fail withRepositoryNotFound. A private or unseen repository fails withPrivateRepository. A run that isn't in the repository fails withRunNotFound. Infrastructure failures die (store, read model, config, an unreachable run entity), so the 503 withRetry-Afterand the prose 410 message are gone.api.pullRequestReview.*and map the tags to their own copy withcatchTag/catchTags.orElseSucceedcovers everything else. A malformed run id in the URL counts as a missing run. The "Download JSON" link keeps its hand-built path because it's anhref.PullRequestReview.test.tsstill runs on Postgres and now drives the typed client. That change landed in the first commit, ahead of the implementation.Known failures: web tests need a follow-up
Four web tests fail. They provide only
HttpClientto the commands, but the commands now needApiClient(Service not found: ApiClient, plus a type error at each call site):pull-request-review-settings.test.ts: "enables with a PUT of the flag…"pull-request-reviews.test.ts: "loads the newest page, then older pages by cursor…" and "searches after the input settles…"pull-request-review-transcript.test.ts: "fetches the run's transcript…"The fix is the EFF-2304 setup:
Effect.provide(ApiClient.layer.pipe(Layer.provide(Layer.succeed(HttpClient.HttpClient, client)))). The two runs-list stubs also have to read query params fromrequest.urlParams, because the derived client doesn't put them inrequest.url. That correction goes in a separate run. Everything else invp checkandvp testpasses.Closes EFF-2519