From f9c4ca075732ce6605193bd018b5eb4dc03f2245 Mon Sep 17 00:00:00 2001 From: Simon Hamp Date: Fri, 16 Oct 2026 11:12:25 +0100 Subject: [PATCH 1/2] feat: star and unstar commits from the API Adds POST and DELETE /api/v1/commits/{commit}/star for signed requests. They use the web's CreateCommitStar and DeleteCommitStar actions, so starring twice or unstarring a commit that isn't starred is fine, and the author gets the same grouped star notification. Both answer with the commit as GET /api/v1/commits/{commit} shows it, with the new stars_count and starred. The user may star any commit they may see but their own, like on the web, which answers their own with a 403 and the web's message. A commit they may not see, including one across a block, or a deleted one, is a 404 like on GET /api/v1/commits/{commit}. The web answers 403 there and lets anyone unstar any commit, but the API sends the commit back, so unstarring needs a commit the user may see too. Muting someone doesn't stop starring their commits, as on the web. Starring and unstarring share the web's limit of 60 a minute, answered with rate_limited like the API's other limits. --- PRODUCT.md | 2 +- README.md | 4 +- .../Controllers/Api/CommitStarController.php | 35 ++++ .../Requests/CreateApiCommitStarRequest.php | 40 ++++ .../Requests/DeleteApiCommitStarRequest.php | 35 ++++ routes/api.php | 9 + .../Api/CommitStarControllerTest.php | 186 ++++++++++++++++++ 7 files changed, 308 insertions(+), 3 deletions(-) create mode 100644 app/Http/Controllers/Api/CommitStarController.php create mode 100644 app/Http/Requests/CreateApiCommitStarRequest.php create mode 100644 app/Http/Requests/DeleteApiCommitStarRequest.php create mode 100644 tests/Feature/Controllers/Api/CommitStarControllerTest.php diff --git a/PRODUCT.md b/PRODUCT.md index 273c1ab5e..a34a7eca8 100644 --- a/PRODUCT.md +++ b/PRODUCT.md @@ -23,7 +23,7 @@ Identity is GitHub. Every account is a real GitHub user, and the profile (avatar - Sign-in is GitHub OAuth only. Profile changes happen on GitHub ("Edit on GitHub"), then sync at next login or with "Sync from GitHub" (once a minute). - Everything except the landing page, the public read API, and profile feeds requires login. - Public read API: read-only JSON under `/api/v1`, no authentication, for public profiles (with counts), a user's commits or replies, and single commits. It returns exactly what a guest's link preview may show: approved, non-deleted commits only (with their images), approved reply counts, and a parent only while it is approved. Never emails or other hidden profile fields. 60 requests a minute per IP. -- Signed API: the mobile app uses the same `/api/v1`, signing every request with an SSH key it keeps on the device, which the user approved as a device (no passwords or API tokens). The app signs in like an OAuth app with PKCE: it opens `/mobile/authorize` in a browser, the user signs in with GitHub if they need to and approves it, and the app redeems the one-time code for the device with a request signed by that key. The approval is the proof, so the key doesn't need to be on GitHub, and a key that is on GitHub, like one made for the GitHub Action, can't use the API without it. Devices are listed, with when each was last used, and revoked under Settings → Devices. A signed request to a public endpoint gets what the web shows that user, such as their own pending commits. Signed-only endpoints read the log, replies, and notifications, and commit, reply, delete, and follow, with the web's validation, moderation, and rate limits. A bad or expired signature is a 401, never the public view. 120 requests a minute per user. +- Signed API: the mobile app uses the same `/api/v1`, signing every request with an SSH key it keeps on the device, which the user approved as a device (no passwords or API tokens). The app signs in like an OAuth app with PKCE: it opens `/mobile/authorize` in a browser, the user signs in with GitHub if they need to and approves it, and the app redeems the one-time code for the device with a request signed by that key. The approval is the proof, so the key doesn't need to be on GitHub, and a key that is on GitHub, like one made for the GitHub Action, can't use the API without it. Devices are listed, with when each was last used, and revoked under Settings → Devices. A signed request to a public endpoint gets what the web shows that user, such as their own pending commits. Signed-only endpoints read the log, replies, and notifications, and commit, reply, delete, follow, and star, with the web's validation, moderation, and rate limits. A bad or expired signature is a 401, never the public view. 120 requests a minute per user. - Profile feeds: every profile has an Atom feed of its latest 50 approved top-level commits at `/@{username}/feed.atom`, their images as enclosures, advertised with `` on the profile's guest preview. - First sign-in leads to a short setup at `/onboarding`: confirm the mirrored profile, pick a color, follow a few suggested developers, learn the composer, `?` shortcuts and terminal commits, then land on the log (or the page the user was headed to). Skippable at any step (Skip setup or Esc); finishing or skipping ends it for good. Accounts that existed before it never see it. - Profile links (`/@{username}`) and links to approved commits (`/@{username}/commits/{id}`) unfurl with a link preview (the author and their bio or counts, or the commit body and reply count) for guests and crawlers; a guest opening one sees the landing page, and the profile or commit itself stays behind login. Pending and rejected commits are a 404 for guests. diff --git a/README.md b/README.md index 53718050e..ceda5547d 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,7 @@ Every profile also has an Atom feed of its latest 50 top-level commits at `/@{us ## Signed requests -The mobile app signs its requests to `/api/v1` with an SSH key it keeps on the device, which the user approved on the web (see [Signing in](#signing-in)). A signed request to the endpoints above gets what the web shows that user. The log, replies, notifications, committing, deleting and following only take signed requests. Signed requests are limited to 120 a minute per user. +The mobile app signs its requests to `/api/v1` with an SSH key it keeps on the device, which the user approved on the web (see [Signing in](#signing-in)). A signed request to the endpoints above gets what the web shows that user. The log, replies, notifications, committing, deleting, following and starring only take signed requests. Signed requests are limited to 120 a minute per user. Commits in a signed response carry what the web's cards show: `preview_segments`, the excerpt a list shows of a long commit (or `null`), the `poll` with the user's view of its votes and each option's `percentage`, the `quoted_commit` (a tombstone once deleted, or `null` when the user may not see it, while `quoted_commit_id` still says a commit was quoted), and in lists the `reply_preview` the web shows under each commit. Its `next_cursor` continues with `GET /api/v1/commits/{id}/replies?newest=1&cursor=…`, as on the web. @@ -124,7 +124,7 @@ An error is JSON with a `message` to show the user. The errors the app has to te | `401` | `device_not_approved` | Nobody approved the key as a device, or it was revoked. | | `403` | `user_blocked` | The user blocked the profile they asked for. Neither of them sees the other's commits. | | `403` | `user_unavailable` | The profile isn't available to the user, because it blocked them. | -| `429` | `rate_limited` | Over 120 signed requests a minute for the user, 10 commits a minute, 30 follows and unfollows a minute, or 60 failed signatures a minute from one IP. `Retry-After` says when to try again. | +| `429` | `rate_limited` | Over 120 signed requests a minute for the user, 10 commits a minute, 30 follows and unfollows a minute, 60 stars and unstars a minute, or 60 failed signatures a minute from one IP. `Retry-After` says when to try again. | Other errors have no `code`, like a `422` for a commit that fails validation or a `404` for one that doesn't exist. Unsigned requests to the public endpoints get the same errors as before, without codes. diff --git a/app/Http/Controllers/Api/CommitStarController.php b/app/Http/Controllers/Api/CommitStarController.php new file mode 100644 index 000000000..da0b4b5f9 --- /dev/null +++ b/app/Http/Controllers/Api/CommitStarController.php @@ -0,0 +1,35 @@ +handle($user, $commit); + + return CommitResource::make($page->get($commit, $user)); + } + + public function destroy(DeleteApiCommitStarRequest $request, #[CurrentUser] User $user, Commit $commit, DeleteCommitStar $action, CommitPageQuery $page): CommitResource + { + $action->handle($user, $commit); + + return CommitResource::make($page->get($commit, $user)); + } +} diff --git a/app/Http/Requests/CreateApiCommitStarRequest.php b/app/Http/Requests/CreateApiCommitStarRequest.php new file mode 100644 index 000000000..cb9e69dbc --- /dev/null +++ b/app/Http/Requests/CreateApiCommitStarRequest.php @@ -0,0 +1,40 @@ +user(); + assert($user instanceof User); + + $commit = $this->route('commit'); + assert($commit instanceof Commit); + + return match (true) { + $commit->user_id === $user->id => Response::deny(__('You cannot star your own commit.')), + $commit->isVisibleTo($user) => Response::allow(), + default => Response::denyAsNotFound(), + }; + } + + /** + * @return array{} + */ + public function rules(): array + { + return []; + } +} diff --git a/app/Http/Requests/DeleteApiCommitStarRequest.php b/app/Http/Requests/DeleteApiCommitStarRequest.php new file mode 100644 index 000000000..a96020bfa --- /dev/null +++ b/app/Http/Requests/DeleteApiCommitStarRequest.php @@ -0,0 +1,35 @@ +user(); + assert($user instanceof User); + + $commit = $this->route('commit'); + assert($commit instanceof Commit); + + return $commit->isVisibleTo($user) ? Response::allow() : Response::denyAsNotFound(); + } + + /** + * @return array{} + */ + public function rules(): array + { + return []; + } +} diff --git a/routes/api.php b/routes/api.php index 4721e9d9e..1f5ecc7fc 100644 --- a/routes/api.php +++ b/routes/api.php @@ -4,6 +4,7 @@ use App\Http\Controllers\Api\CommitController as ApiCommitController; use App\Http\Controllers\Api\CommitReplyController; +use App\Http\Controllers\Api\CommitStarController; use App\Http\Controllers\Api\CurrentUserController; use App\Http\Controllers\Api\DeviceController; use App\Http\Controllers\Api\LogController; @@ -63,6 +64,14 @@ ->withTrashed() ->name('api-commit-reply.index'); + // Commit Star... + Route::post('commits/{commit}/star', [CommitStarController::class, 'store']) + ->middleware('throttle:stars') + ->name('api-commit-star.store'); + Route::delete('commits/{commit}/star', [CommitStarController::class, 'destroy']) + ->middleware('throttle:stars') + ->name('api-commit-star.destroy'); + // User Follow... Route::post('users/{user:username}/follow', [UserFollowController::class, 'store']) ->middleware('throttle:follows') diff --git a/tests/Feature/Controllers/Api/CommitStarControllerTest.php b/tests/Feature/Controllers/Api/CommitStarControllerTest.php new file mode 100644 index 000000000..77632e403 --- /dev/null +++ b/tests/Feature/Controllers/Api/CommitStarControllerTest.php @@ -0,0 +1,186 @@ +user = User::factory()->create(); + $this->keyPair = apiKeyPairFor($this->user); +}); + +it('stars a commit and answers with its new stars', function (): void { + $commit = Commit::factory()->create(); + $commit->stars()->attach(User::factory()->create()); + + $response = signedApiRequest($this->keyPair, 'POST', route('api-commit-star.store', $commit)); + + $response->assertOk() + ->assertJsonPath('data.id', $commit->id) + ->assertJsonPath('data.stars_count', 2) + ->assertJsonPath('data.starred', true); + + expect($this->user->hasStarred($commit))->toBeTrue() + ->and($commit->user->notifications()->sole()->type)->toBe(NotificationType::Starred); +}); + +it('answers with the commit as the commit endpoint shows it', function (): void { + $commit = Commit::factory()->create(); + Commit::factory()->for($commit, 'parent')->create(); + + $response = signedApiRequest($this->keyPair, 'POST', route('api-commit-star.store', $commit)); + + $response->assertOk() + ->assertExactJson(['data' => signedApiRequest($this->keyPair, 'GET', route('api-commit.show', $commit))->json('data')]); +}); + +it('stars a commit only once', function (): void { + $commit = Commit::factory()->create(); + + signedApiRequest($this->keyPair, 'POST', route('api-commit-star.store', $commit))->assertOk(); + + $response = signedApiRequest($this->keyPair, 'POST', route('api-commit-star.store', $commit)); + + $response->assertOk() + ->assertJsonPath('data.stars_count', 1) + ->assertJsonPath('data.starred', true); + + expect($this->user->starredCommits()->count())->toBeOne(); +}); + +it('may not star their own commit, as on the web', function (Closure $commit): void { + $commit = $commit($this->user); + + $response = signedApiRequest($this->keyPair, 'POST', route('api-commit-star.store', $commit)); + + $response->assertForbidden() + ->assertJsonPath('message', 'You cannot star your own commit.'); + + expect($this->user->hasStarred($commit))->toBeFalse(); +})->with([ + 'approved' => fn (User $user): Commit => Commit::factory()->for($user)->create(), + 'pending' => fn (User $user): Commit => Commit::factory()->for($user)->pending()->create(), +]); + +it('may not star a commit across a block', function (Closure $block): void { + $commit = Commit::factory()->create(); + + $block($this->user, $commit->user); + + signedApiRequest($this->keyPair, 'POST', route('api-commit-star.store', $commit))->assertNotFound(); + + expect($this->user->hasStarred($commit))->toBeFalse(); +})->with([ + 'blocking' => fn (User $user, User $author) => $user->blocking()->attach($author), + 'blocked' => fn (User $user, User $author) => $author->blocking()->attach($user), +]); + +it('may star the commits of someone the user mutes, as on the web', function (): void { + $commit = Commit::factory()->create(); + + $this->user->muting()->attach($commit->user); + + signedApiRequest($this->keyPair, 'POST', route('api-commit-star.store', $commit)) + ->assertOk() + ->assertJsonPath('data.starred', true); +}); + +it('may not star a commit the user may not see', function (string $state): void { + $commit = Commit::factory()->{$state}()->create(); + + $response = signedApiRequest($this->keyPair, 'POST', route('api-commit-star.store', $commit)); + + $response->assertNotFound(); + + expect($this->user->hasStarred($commit))->toBeFalse(); +})->with(['pending', 'rejected']); + +it('may not star a deleted commit', function (): void { + $commit = Commit::factory()->create(); + $commit->delete(); + + $response = signedApiRequest($this->keyPair, 'POST', route('api-commit-star.store', $commit)); + + $response->assertNotFound(); + + expect($commit->stars()->count())->toBe(0); +}); + +it('unstars a commit and answers with its new stars', function (): void { + $commit = Commit::factory()->create(); + $commit->stars()->attach([$this->user->id, User::factory()->create()->id]); + + $response = signedApiRequest($this->keyPair, 'DELETE', route('api-commit-star.destroy', $commit)); + + $response->assertOk() + ->assertJsonPath('data.id', $commit->id) + ->assertJsonPath('data.stars_count', 1) + ->assertJsonPath('data.starred', false); + + expect($this->user->hasStarred($commit))->toBeFalse(); +}); + +it('unstars a commit that was not starred', function (): void { + $commit = Commit::factory()->create(); + $commit->stars()->attach(User::factory()->create()); + + $response = signedApiRequest($this->keyPair, 'DELETE', route('api-commit-star.destroy', $commit)); + + $response->assertOk() + ->assertJsonPath('data.stars_count', 1) + ->assertJsonPath('data.starred', false); +}); + +it('may not unstar a commit the user may not see', function (): void { + $commit = Commit::factory()->create(); + $commit->stars()->attach($this->user); + $commit->update(['status' => CommitStatus::Rejected]); + + $response = signedApiRequest($this->keyPair, 'DELETE', route('api-commit-star.destroy', $commit)); + + $response->assertNotFound() + ->assertDontSee($commit->body); + + expect($this->user->hasStarred($commit))->toBeTrue(); +}); + +it('may not unstar a deleted commit', function (): void { + $commit = Commit::factory()->create(); + $commit->stars()->attach($this->user); + $commit->delete(); + + $response = signedApiRequest($this->keyPair, 'DELETE', route('api-commit-star.destroy', $commit)); + + $response->assertNotFound(); +}); + +it('returns not found for unknown commits', function (string $method): void { + $response = signedApiRequest($this->keyPair, $method, 'api/v1/commits/0199e3a4-0000-7000-8000-000000000000/star'); + + $response->assertNotFound(); +})->with(['POST', 'DELETE']); + +it('requires a signed request', function (string $method): void { + $commit = Commit::factory()->create(); + + $response = $this->json($method, "api/v1/commits/{$commit->id}/star"); + + $response->assertUnauthorized(); + + expect($commit->stars()->count())->toBe(0); +})->with(['POST', 'DELETE']); + +it('limits stars and unstars to 60 a minute, like the web, with a code', function (): void { + $commit = Commit::factory()->create(); + + foreach (range(1, 60) as $attempt) { + signedApiRequest($this->keyPair, $attempt % 2 === 0 ? 'DELETE' : 'POST', route('api-commit-star.store', $commit))->assertOk(); + } + + signedApiRequest($this->keyPair, 'POST', route('api-commit-star.store', $commit)) + ->assertTooManyRequests() + ->assertExactJson(['message' => 'Too Many Attempts.', 'code' => 'rate_limited']); +}); From e3464f15960ed718452390b18a1e729d8e143cec Mon Sep 17 00:00:00 2001 From: Simon Hamp Date: Fri, 16 Oct 2026 11:12:57 +0100 Subject: [PATCH 2/2] feat: save commits and list saved commits from the API Adds PUT and DELETE /api/v1/commits/{commit}/bookmark and GET /api/v1/saved for signed requests. Saving uses the web's CreateBookmark and DeleteBookmark actions with the same rules: the user may save any commit they may see, saving twice keeps one save, and unsaving something that isn't saved is fine. A commit they may not see, or a deleted one, is a 404. Both answer with the commit as GET /api/v1/commits/{commit} shows it, with the new is_saved. GET /api/v1/saved lists the user's own saves through the web's SavedCommitsQuery, last saved first, 20 at a time with a cursor like the other lists. Saved replies come with the parent the user may see, or its tombstone, and every saved commit with the reply preview the web shows. Commits across a block drop out of the list, as on the web. Saving and unsaving share the web's limit of 60 a minute, answered with rate_limited. --- PRODUCT.md | 2 +- README.md | 4 +- .../Controllers/Api/BookmarkController.php | 50 +++ .../Requests/CreateApiBookmarkRequest.php | 32 ++ .../Requests/DeleteApiBookmarkRequest.php | 32 ++ routes/api.php | 10 + .../Api/BookmarkControllerTest.php | 371 ++++++++++++++++++ 7 files changed, 498 insertions(+), 3 deletions(-) create mode 100644 app/Http/Controllers/Api/BookmarkController.php create mode 100644 app/Http/Requests/CreateApiBookmarkRequest.php create mode 100644 app/Http/Requests/DeleteApiBookmarkRequest.php create mode 100644 tests/Feature/Controllers/Api/BookmarkControllerTest.php diff --git a/PRODUCT.md b/PRODUCT.md index a34a7eca8..6da5dce30 100644 --- a/PRODUCT.md +++ b/PRODUCT.md @@ -23,7 +23,7 @@ Identity is GitHub. Every account is a real GitHub user, and the profile (avatar - Sign-in is GitHub OAuth only. Profile changes happen on GitHub ("Edit on GitHub"), then sync at next login or with "Sync from GitHub" (once a minute). - Everything except the landing page, the public read API, and profile feeds requires login. - Public read API: read-only JSON under `/api/v1`, no authentication, for public profiles (with counts), a user's commits or replies, and single commits. It returns exactly what a guest's link preview may show: approved, non-deleted commits only (with their images), approved reply counts, and a parent only while it is approved. Never emails or other hidden profile fields. 60 requests a minute per IP. -- Signed API: the mobile app uses the same `/api/v1`, signing every request with an SSH key it keeps on the device, which the user approved as a device (no passwords or API tokens). The app signs in like an OAuth app with PKCE: it opens `/mobile/authorize` in a browser, the user signs in with GitHub if they need to and approves it, and the app redeems the one-time code for the device with a request signed by that key. The approval is the proof, so the key doesn't need to be on GitHub, and a key that is on GitHub, like one made for the GitHub Action, can't use the API without it. Devices are listed, with when each was last used, and revoked under Settings → Devices. A signed request to a public endpoint gets what the web shows that user, such as their own pending commits. Signed-only endpoints read the log, replies, and notifications, and commit, reply, delete, follow, and star, with the web's validation, moderation, and rate limits. A bad or expired signature is a 401, never the public view. 120 requests a minute per user. +- Signed API: the mobile app uses the same `/api/v1`, signing every request with an SSH key it keeps on the device, which the user approved as a device (no passwords or API tokens). The app signs in like an OAuth app with PKCE: it opens `/mobile/authorize` in a browser, the user signs in with GitHub if they need to and approves it, and the app redeems the one-time code for the device with a request signed by that key. The approval is the proof, so the key doesn't need to be on GitHub, and a key that is on GitHub, like one made for the GitHub Action, can't use the API without it. Devices are listed, with when each was last used, and revoked under Settings → Devices. A signed request to a public endpoint gets what the web shows that user, such as their own pending commits. Signed-only endpoints read the log, replies, notifications, and saved commits, and commit, reply, delete, follow, star, and save, with the web's validation, moderation, and rate limits. A bad or expired signature is a 401, never the public view. 120 requests a minute per user. - Profile feeds: every profile has an Atom feed of its latest 50 approved top-level commits at `/@{username}/feed.atom`, their images as enclosures, advertised with `` on the profile's guest preview. - First sign-in leads to a short setup at `/onboarding`: confirm the mirrored profile, pick a color, follow a few suggested developers, learn the composer, `?` shortcuts and terminal commits, then land on the log (or the page the user was headed to). Skippable at any step (Skip setup or Esc); finishing or skipping ends it for good. Accounts that existed before it never see it. - Profile links (`/@{username}`) and links to approved commits (`/@{username}/commits/{id}`) unfurl with a link preview (the author and their bio or counts, or the commit body and reply count) for guests and crawlers; a guest opening one sees the landing page, and the profile or commit itself stays behind login. Pending and rejected commits are a 404 for guests. diff --git a/README.md b/README.md index ceda5547d..5be63e798 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,7 @@ Every profile also has an Atom feed of its latest 50 top-level commits at `/@{us ## Signed requests -The mobile app signs its requests to `/api/v1` with an SSH key it keeps on the device, which the user approved on the web (see [Signing in](#signing-in)). A signed request to the endpoints above gets what the web shows that user. The log, replies, notifications, committing, deleting, following and starring only take signed requests. Signed requests are limited to 120 a minute per user. +The mobile app signs its requests to `/api/v1` with an SSH key it keeps on the device, which the user approved on the web (see [Signing in](#signing-in)). A signed request to the endpoints above gets what the web shows that user. The log, replies, notifications, saved commits, committing, deleting, following, starring and saving only take signed requests. Signed requests are limited to 120 a minute per user. Commits in a signed response carry what the web's cards show: `preview_segments`, the excerpt a list shows of a long commit (or `null`), the `poll` with the user's view of its votes and each option's `percentage`, the `quoted_commit` (a tombstone once deleted, or `null` when the user may not see it, while `quoted_commit_id` still says a commit was quoted), and in lists the `reply_preview` the web shows under each commit. Its `next_cursor` continues with `GET /api/v1/commits/{id}/replies?newest=1&cursor=…`, as on the web. @@ -124,7 +124,7 @@ An error is JSON with a `message` to show the user. The errors the app has to te | `401` | `device_not_approved` | Nobody approved the key as a device, or it was revoked. | | `403` | `user_blocked` | The user blocked the profile they asked for. Neither of them sees the other's commits. | | `403` | `user_unavailable` | The profile isn't available to the user, because it blocked them. | -| `429` | `rate_limited` | Over 120 signed requests a minute for the user, 10 commits a minute, 30 follows and unfollows a minute, 60 stars and unstars a minute, or 60 failed signatures a minute from one IP. `Retry-After` says when to try again. | +| `429` | `rate_limited` | Over 120 signed requests a minute for the user, 10 commits a minute, 30 follows and unfollows a minute, 60 stars and unstars a minute, 60 saves and unsaves a minute, or 60 failed signatures a minute from one IP. `Retry-After` says when to try again. | Other errors have no `code`, like a `422` for a commit that fails validation or a `404` for one that doesn't exist. Unsigned requests to the public endpoints get the same errors as before, without codes. diff --git a/app/Http/Controllers/Api/BookmarkController.php b/app/Http/Controllers/Api/BookmarkController.php new file mode 100644 index 000000000..4673341a0 --- /dev/null +++ b/app/Http/Controllers/Api/BookmarkController.php @@ -0,0 +1,50 @@ +builder($user)->cursorPaginate(20); + + $replyPreviews->load($commits->items(), $user); + + return CommitResource::collection($commits); + } + + /** + * Answers with the commit as its page shows it, so the app gets the new is_saved. + */ + public function store(CreateApiBookmarkRequest $request, #[CurrentUser] User $user, Commit $commit, CreateBookmark $action, CommitPageQuery $page): CommitResource + { + $action->handle($user, $commit); + + return CommitResource::make($page->get($commit, $user)); + } + + public function destroy(DeleteApiBookmarkRequest $request, #[CurrentUser] User $user, Commit $commit, DeleteBookmark $action, CommitPageQuery $page): CommitResource + { + $action->handle($user, $commit); + + return CommitResource::make($page->get($commit, $user)); + } +} diff --git a/app/Http/Requests/CreateApiBookmarkRequest.php b/app/Http/Requests/CreateApiBookmarkRequest.php new file mode 100644 index 000000000..383f22b39 --- /dev/null +++ b/app/Http/Requests/CreateApiBookmarkRequest.php @@ -0,0 +1,32 @@ +user(); + assert($user instanceof User); + + $commit = $this->route('commit'); + assert($commit instanceof Commit); + + return $commit->isVisibleTo($user) ? Response::allow() : Response::denyAsNotFound(); + } + + /** + * @return array{} + */ + public function rules(): array + { + return []; + } +} diff --git a/app/Http/Requests/DeleteApiBookmarkRequest.php b/app/Http/Requests/DeleteApiBookmarkRequest.php new file mode 100644 index 000000000..1700d2d4b --- /dev/null +++ b/app/Http/Requests/DeleteApiBookmarkRequest.php @@ -0,0 +1,32 @@ +user(); + assert($user instanceof User); + + $commit = $this->route('commit'); + assert($commit instanceof Commit); + + return $commit->isVisibleTo($user) ? Response::allow() : Response::denyAsNotFound(); + } + + /** + * @return array{} + */ + public function rules(): array + { + return []; + } +} diff --git a/routes/api.php b/routes/api.php index 1f5ecc7fc..225425dad 100644 --- a/routes/api.php +++ b/routes/api.php @@ -2,6 +2,7 @@ declare(strict_types=1); +use App\Http\Controllers\Api\BookmarkController; use App\Http\Controllers\Api\CommitController as ApiCommitController; use App\Http\Controllers\Api\CommitReplyController; use App\Http\Controllers\Api\CommitStarController; @@ -64,6 +65,15 @@ ->withTrashed() ->name('api-commit-reply.index'); + // Private saved commits... + Route::get('saved', [BookmarkController::class, 'index'])->name('api-bookmark.index'); + Route::put('commits/{commit}/bookmark', [BookmarkController::class, 'store']) + ->middleware('throttle:bookmarks') + ->name('api-bookmark.store'); + Route::delete('commits/{commit}/bookmark', [BookmarkController::class, 'destroy']) + ->middleware('throttle:bookmarks') + ->name('api-bookmark.destroy'); + // Commit Star... Route::post('commits/{commit}/star', [CommitStarController::class, 'store']) ->middleware('throttle:stars') diff --git a/tests/Feature/Controllers/Api/BookmarkControllerTest.php b/tests/Feature/Controllers/Api/BookmarkControllerTest.php new file mode 100644 index 000000000..60d4be3fe --- /dev/null +++ b/tests/Feature/Controllers/Api/BookmarkControllerTest.php @@ -0,0 +1,371 @@ +user = User::factory()->create(); + $this->keyPair = apiKeyPairFor($this->user); +}); + +it('saves a commit and answers with it saved', function (): void { + $commit = Commit::factory()->create(); + + $response = signedApiRequest($this->keyPair, 'PUT', route('api-bookmark.store', $commit)); + + $response->assertOk() + ->assertJsonPath('data.id', $commit->id) + ->assertJsonPath('data.is_saved', true); + + $this->assertDatabaseHas('bookmarks', ['user_id' => $this->user->id, 'commit_id' => $commit->id]); +}); + +it('answers with the commit as the commit endpoint shows it', function (): void { + $commit = Commit::factory()->create(); + $commit->stars()->attach(User::factory()->create()); + + $response = signedApiRequest($this->keyPair, 'PUT', route('api-bookmark.store', $commit)); + + $response->assertOk() + ->assertExactJson(['data' => signedApiRequest($this->keyPair, 'GET', route('api-commit.show', $commit))->json('data')]); +}); + +it('saves a commit only once', function (): void { + $commit = Commit::factory()->create(); + + signedApiRequest($this->keyPair, 'PUT', route('api-bookmark.store', $commit))->assertOk(); + + $response = signedApiRequest($this->keyPair, 'PUT', route('api-bookmark.store', $commit)); + + $response->assertOk() + ->assertJsonPath('data.is_saved', true); + + $this->assertDatabaseCount('bookmarks', 1); +}); + +it('saves for the user who signed the request, whatever the body says', function (): void { + $other = User::factory()->create(); + $commit = Commit::factory()->create(); + + signedApiRequest($this->keyPair, 'PUT', route('api-bookmark.store', $commit), ['user_id' => $other->id])->assertOk(); + + $this->assertDatabaseCount('bookmarks', 1); + $this->assertDatabaseHas('bookmarks', ['user_id' => $this->user->id, 'commit_id' => $commit->id]); +}); + +it('may save their own commits in every moderation state', function (CommitStatus $status): void { + $commit = Commit::factory()->for($this->user)->create(['status' => $status]); + + $response = signedApiRequest($this->keyPair, 'PUT', route('api-bookmark.store', $commit)); + + $response->assertOk() + ->assertJsonPath('data.status', $status->value) + ->assertJsonPath('data.is_saved', true); +})->with(CommitStatus::cases()); + +it('may not save a commit the user may not see', function (string $state): void { + $commit = Commit::factory()->{$state}()->create(); + + $response = signedApiRequest($this->keyPair, 'PUT', route('api-bookmark.store', $commit)); + + $response->assertNotFound(); + + $this->assertDatabaseCount('bookmarks', 0); +})->with(['pending', 'rejected']); + +it('may not save a deleted commit', function (): void { + $commit = Commit::factory()->for($this->user)->create(); + $commit->delete(); + + $response = signedApiRequest($this->keyPair, 'PUT', route('api-bookmark.store', $commit)); + + $response->assertNotFound(); + + $this->assertDatabaseCount('bookmarks', 0); +}); + +it('unsaves a commit and answers with it unsaved', function (): void { + $commit = Commit::factory()->create(); + Bookmark::factory()->create(['user_id' => $this->user->id, 'commit_id' => $commit->id]); + $theirs = Bookmark::factory()->create(['commit_id' => $commit->id]); + + $response = signedApiRequest($this->keyPair, 'DELETE', route('api-bookmark.destroy', $commit)); + + $response->assertOk() + ->assertJsonPath('data.id', $commit->id) + ->assertJsonPath('data.is_saved', false); + + $this->assertDatabaseCount('bookmarks', 1); + $this->assertModelExists($theirs); +}); + +it('unsaves a commit that was not saved', function (): void { + $commit = Commit::factory()->create(); + + $response = signedApiRequest($this->keyPair, 'DELETE', route('api-bookmark.destroy', $commit)); + + $response->assertOk() + ->assertJsonPath('data.is_saved', false); +}); + +it('may not unsave a commit the user may not see', function (): void { + $commit = Commit::factory()->create(); + $bookmark = Bookmark::factory()->create(['user_id' => $this->user->id, 'commit_id' => $commit->id]); + $commit->update(['status' => CommitStatus::Pending]); + + $response = signedApiRequest($this->keyPair, 'DELETE', route('api-bookmark.destroy', $commit)); + + $response->assertNotFound() + ->assertDontSee($commit->body); + + $this->assertModelExists($bookmark); +}); + +it('may not unsave a deleted commit', function (): void { + $commit = Commit::factory()->create(); + $bookmark = Bookmark::factory()->create(['user_id' => $this->user->id, 'commit_id' => $commit->id]); + $commit->delete(); + + $response = signedApiRequest($this->keyPair, 'DELETE', route('api-bookmark.destroy', $commit)); + + $response->assertNotFound(); + + $this->assertModelExists($bookmark); +}); + +it('returns not found for unknown commits', function (string $method): void { + $response = signedApiRequest($this->keyPair, $method, 'api/v1/commits/0199e3a4-0000-7000-8000-000000000000/bookmark'); + + $response->assertNotFound(); +})->with(['PUT', 'DELETE']); + +it('lists the saved commits of the user, last saved first', function (): void { + $author = User::factory()->create(); + $older = Commit::factory()->for($author)->create(['body' => 'It was DNS.', 'created_at' => now()->subYear()]); + $newer = Commit::factory()->create(); + $image = CommitImage::factory()->for($older)->create(['width' => 640, 'height' => 480]); + $reply = Commit::factory()->for($older, 'parent')->create(); + $older->stars()->attach($this->user); + + Bookmark::factory()->create(['user_id' => $this->user->id, 'commit_id' => $newer->id]); + Bookmark::factory()->create(['user_id' => $this->user->id, 'commit_id' => $older->id]); + + $response = signedApiRequest($this->keyPair, 'GET', route('api-bookmark.index')); + + $response->assertOk() + ->assertJsonCount(2, 'data') + ->assertJsonPath('data.0.reply_preview.data.*.id', [$reply->id]) + ->assertJsonPath('data.0.reply_preview.next_cursor', null) + ->assertJsonPath('data.1.id', $newer->id) + ->assertJsonPath('data.1.is_saved', true) + ->assertJsonPath('meta.next_cursor', null) + ->assertJsonPath('meta.per_page', 20); + + expect(collect($response->json('data.0'))->except('reply_preview')->all())->toBe([ + 'id' => $older->id, + 'short_hash' => mb_substr(str_replace('-', '', $older->id), -7), + 'body' => 'It was DNS.', + 'parent_id' => null, + 'html_url' => route('commit.show', [$author, $older]), + 'created_at' => now()->subYear()->toIso8601ZuluString(), + 'stars_count' => 1, + 'replies_count' => 1, + 'link_preview' => null, + 'images' => [[ + 'id' => $image->id, + 'width' => 640, + 'height' => 480, + 'description' => null, + 'url' => route('commit-image.show', $image), + ]], + 'author' => [ + 'id' => $author->id, + 'username' => $author->username, + 'name' => $author->name, + 'avatar_url' => $author->avatar_url, + 'bio' => $author->bio, + 'company' => $author->company, + 'location' => $author->location, + 'website' => $author->website, + 'github_url' => "https://github.com/{$author->username}", + 'html_url' => route('user.show', $author), + 'feed_url' => route('user-feed.show', $author), + 'created_at' => now()->toIso8601ZuluString(), + 'lane' => $author->lane(), + ], + 'body_segments' => [['type' => 'text', 'text' => 'It was DNS.']], + 'preview_segments' => null, + 'mentions' => [], + 'status' => 'approved', + 'is_pinned' => false, + 'starred' => true, + 'is_saved' => true, + 'deleted_at' => null, + 'parent' => null, + 'quoted_commit_id' => null, + 'quoted_commit' => null, + 'poll' => null, + ]); +}); + +it('never lists the saves of others', function (): void { + $other = User::factory()->create(); + Bookmark::factory()->create(['user_id' => $other->id]); + Bookmark::factory()->create(['user_id' => $other->id, 'commit_id' => Commit::factory()->for($this->user)]); + + $response = signedApiRequest($this->keyPair, 'GET', route('api-bookmark.index', ['user_id' => $other->id])); + + $response->assertOk() + ->assertJsonCount(0, 'data'); +}); + +it('lists saved commits the user may still see', function (): void { + $mine = Commit::factory()->for($this->user)->pending()->create(); + $hidden = Commit::factory()->create(); + $deleted = Commit::factory()->for($this->user)->create(); + Commit::factory()->for($deleted, 'parent')->create(); + + foreach ([$mine, $hidden, $deleted] as $commit) { + Bookmark::factory()->create(['user_id' => $this->user->id, 'commit_id' => $commit->id]); + } + + $hidden->update(['status' => CommitStatus::Rejected]); + $deleted->delete(); + + $response = signedApiRequest($this->keyPair, 'GET', route('api-bookmark.index')); + + $response->assertOk() + ->assertJsonCount(1, 'data') + ->assertJsonPath('data.0.id', $mine->id) + ->assertJsonPath('data.0.status', 'pending'); +}); + +it('lists saved replies with the parent the user may see', function (): void { + $parent = Commit::factory()->create(); + $reply = Commit::factory()->for($parent, 'parent')->create(); + $hiddenParent = Commit::factory()->pending()->create(); + $hiddenReply = Commit::factory()->for($hiddenParent, 'parent')->create(['created_at' => now()->subMinute()]); + $deletedParent = Commit::factory()->create(['body' => 'Gone']); + $orphanedReply = Commit::factory()->for($deletedParent, 'parent')->create(['created_at' => now()->subMinutes(2)]); + $deletedParent->delete(); + + foreach ([$orphanedReply, $hiddenReply, $reply] as $commit) { + Bookmark::factory()->create(['user_id' => $this->user->id, 'commit_id' => $commit->id]); + } + + $response = signedApiRequest($this->keyPair, 'GET', route('api-bookmark.index')); + + $response->assertOk() + ->assertJsonCount(3, 'data') + ->assertJsonPath('data.0.id', $reply->id) + ->assertJsonPath('data.0.parent.id', $parent->id) + ->assertJsonPath('data.0.parent.author.id', $parent->user_id) + ->assertJsonPath('data.1.id', $hiddenReply->id) + ->assertJsonPath('data.1.parent', null) + ->assertJsonPath('data.2.id', $orphanedReply->id) + ->assertJsonPath('data.2.parent', [ + 'id' => $deletedParent->id, + 'parent_id' => null, + 'deleted_at' => now()->toIso8601ZuluString(), + ]) + ->assertDontSee('Gone'); +}); + +it('lists saved commits 20 at a time with a cursor to the next page', function (): void { + foreach (range(0, 21) as $index) { + Bookmark::factory()->create([ + 'user_id' => $this->user->id, + 'commit_id' => Commit::factory()->create(['body' => "Commit {$index}"]), + ]); + } + + $first = signedApiRequest($this->keyPair, 'GET', route('api-bookmark.index')); + + $first->assertOk() + ->assertJsonCount(20, 'data') + ->assertJsonPath('data.0.body', 'Commit 21') + ->assertJsonPath('data.19.body', 'Commit 2'); + + $second = signedApiRequest($this->keyPair, 'GET', route('api-bookmark.index', ['cursor' => $first->json('meta.next_cursor')])); + + $second->assertOk() + ->assertJsonCount(2, 'data') + ->assertJsonPath('data.0.body', 'Commit 1') + ->assertJsonPath('data.1.body', 'Commit 0') + ->assertJsonPath('meta.next_cursor', null); +}); + +it('does not query each saved commit or author', function (): void { + signedApiRequest($this->keyPair, 'GET', route('api-bookmark.index'))->assertOk(); + + $queries = function (): int { + DB::flushQueryLog(); + DB::enableQueryLog(); + + signedApiRequest($this->keyPair, 'GET', route('api-bookmark.index'))->assertOk(); + + return count(DB::getQueryLog()); + }; + + Bookmark::factory()->count(2)->create(['user_id' => $this->user->id]); + $few = $queries(); + + Bookmark::factory()->count(8)->create(['user_id' => $this->user->id]); + + expect($queries())->toBe($few); +}); + +it('requires a signed request', function (string $method, string $uri): void { + $commit = Commit::factory()->create(); + + $response = $this->json($method, str_replace('{commit}', $commit->id, $uri)); + + $response->assertUnauthorized(); + + $this->assertDatabaseCount('bookmarks', 0); +})->with([ + 'saved' => ['GET', 'api/v1/saved'], + 'save' => ['PUT', 'api/v1/commits/{commit}/bookmark'], + 'unsave' => ['DELETE', 'api/v1/commits/{commit}/bookmark'], +]); + +it('may not save a commit across a block, and lists no saved commits across one', function (): void { + $saved = Commit::factory()->create(); + Bookmark::factory()->create(['user_id' => $this->user->id, 'commit_id' => $saved->id]); + $other = Commit::factory()->for($saved->user)->create(); + + $saved->user->blocking()->attach($this->user); + + signedApiRequest($this->keyPair, 'PUT', route('api-bookmark.store', $other))->assertNotFound(); + signedApiRequest($this->keyPair, 'GET', route('api-bookmark.index')) + ->assertOk() + ->assertJsonCount(0, 'data'); +}); + +it('lists saved commits with the newest reply under each, as the web does', function (): void { + $commit = Commit::factory()->create(); + $reply = Commit::factory()->for($commit, 'parent')->create(); + Bookmark::factory()->create(['user_id' => $this->user->id, 'commit_id' => $commit->id]); + + signedApiRequest($this->keyPair, 'GET', route('api-bookmark.index')) + ->assertOk() + ->assertJsonPath('data.0.reply_preview.data.*.id', [$reply->id]); +}); + +it('limits saves and unsaves to 60 a minute, like the web, with a code', function (): void { + $commit = Commit::factory()->create(); + + foreach (range(1, 60) as $attempt) { + signedApiRequest($this->keyPair, $attempt % 2 === 0 ? 'DELETE' : 'PUT', route('api-bookmark.store', $commit))->assertOk(); + } + + signedApiRequest($this->keyPair, 'PUT', route('api-bookmark.store', $commit)) + ->assertTooManyRequests() + ->assertExactJson(['message' => 'Too Many Attempts.', 'code' => 'rate_limited']); +});