Repository navigation
Conversation
f06d54c to
6dd6f0c
Compare
Three people you might want to follow sit under the fifth commit, each
with why they're suggested, Follow and Not interested. Devnull sends
three spares, so one dismissed gives their place to the next, and the
round is asked again to top the spares up. The refresh moves to the next
page of the day's picks. Don't show again hides it with an Undo, and a
switch in settings brings it back.
Needs GET /api/v1/me/suggested-users and POST /api/v1/users/{username}/dismiss
from dev-null-sh/dev-null.sh#990.
6dd6f0c to
7b64f62
Compare
|
@xHeaven @nunomaduro can we get this into the API so we can show "Who to follow?" |
The "Who to follow" algo changed a bit in #1021, can you guys check if this PR needs to be adjusted? |
|
Yep. I'll check it later |
7b64f62 to
43fee92
Compare
Three people you might want to follow sit under the fifth commit, each
with why they're suggested, Follow and Not interested. Devnull sends
three spares, so one dismissed gives their place to the next, and the
round is asked again to top the spares up. The refresh moves to the next
page of the day's picks. Don't show again hides it with an Undo, and a
switch in settings brings it back.
Needs GET /api/v1/me/suggested-users and POST /api/v1/users/{username}/dismiss
from dev-null-sh/dev-null.sh#990.
@xHeaven Thanks for the hint! I updated the branch for #1021.
Without these parameters, nothing changes. A test checks that the API returns the same suggestions as the web for the same page. |
The mobile app can show who to follow as the web does: the same three people and three spares, a new page of the day's picks each round, with why each one is suggested and whether they follow you. Not interested keeps someone out of the suggestions for good, as on the web.
Takes the context the web passes since dev-null-sh#1021: `username` for a profile, `commit_id` for a commit the user can see, as its page requires. Without either, the suggestions stay as they were.
9fe7624 to
b646908
Compare
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
List suggestions app/Http/Requests/ListApiSuggestedUsersRequest.php, app/Http/Controllers/Api/SuggestedUserController.php, app/Http/Resources/UserResource.php, routes/api.php, tests/Feature/Controllers/Api/SuggestedUserControllerTest.php, KNOWLEDGE_BASE.md |
The request validates pagination and optional profile or visible commit context. The controller retrieves suggestions and returns a resource collection. The resource conditionally includes followed_by_following_count. Routes and feature tests cover the signed API behavior. |
Dismiss a suggestion app/Http/Requests/CreateApiSuggestionDismissalRequest.php, app/Http/Controllers/Api/SuggestionDismissalController.php, tests/Feature/Controllers/Api/SuggestionDismissalControllerTest.php |
The request rejects self-dismissal. The controller records dismissal and returns no content. Tests cover repeated dismissal, follow and mute behavior, unknown usernames, and unsigned requests. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant ListApiSuggestedUsersRequest
participant SuggestedUserController
participant SuggestedUsersQuery
participant UserResource
ListApiSuggestedUsersRequest->>SuggestedUserController: validated round and context
SuggestedUserController->>SuggestedUsersQuery: user, round, context, and 3 spares
SuggestedUsersQuery-->>SuggestedUserController: suggested users
SuggestedUserController->>UserResource: return suggestions as a resource collection
Possibly related issues
dev-null-sh/dev-null.sh#938— This adds suggested-user API access, but does not add organizations to suggestions.dev-null-sh/dev-null.sh#1095— This exposes the suggestion flow through an API, but does not establish GitHub as a suggestion source.dev-null-sh/dev-null.sh#897— This exposes the same suggestion flow, but the changes do not establish a fix for the reported list behavior.dev-null-sh/dev-null.sh#857— The PR adds API endpoints, but does not add API documentation.
Possibly related PRs
- dev-null-sh/dev-null.sh#1156 — Both changes use the username identity contract for API route binding and lookups.
- dev-null-sh/dev-null.sh#1148 — Both changes cover the lifecycle of suggested users, including dismissal behavior.
Suggested reviewers: xheaven
Merge Risk: 
b6469
-
The suggested-users API is mergeable after normal checks; no actionable merge-blocking issue is established.
Security architecture risk: 
b6469
-
The endpoints reuse existing suggestion and dismissal logic and keep mutations tied to the signed-in account. No authorization bypass was identified in the reviewed paths. Concurrent duplicate-request responses remain unverified.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- inferred — An authenticated device can request public-profile suggestions and viewer-relative relationship counts, or add dismissal preferences owned by its associated account. The traced mutation does not modify the target account, follow relationships, or mute relationships.
Trust Boundaries and Controls
- observed — Both new routes are inside the required signed-request group, not the optional guest group. Successful authentication binds the verified device owner to Auth; the requests and CurrentUser controller parameters use that identity.
- observed — Commit context does not grant visibility independently of the caller. The visibility rule and context lookup both use visibleTo for the authenticated user; that policy admits approved or caller-owned commits while excluding authors across blocks.
Resilience and Maintainability Implications
- inferred — Dismissal has no new multi-resource transition: it writes one actor-to-target pivot relationship. The composite primary key prevents duplicate persisted pairs, and cascading foreign keys remove preferences when either user is deleted. Interruption or repetition can affect completion or response status, but cannot create duplicate preference ownership. Concurrent duplicate success responses are not established by the available framework evidence.
-
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the primary change: adding suggested-user functionality to the API. Docstring Coverage Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 8 files. (1 skipped: 1 u… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 8 files. (1 skipped: 1 unsupported.)
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
@xHeaven Sorry to bother you! Just following up to see if you’ve had a chance to review this PR. Let me know if there’s anything you need from me. Thanks! |
Lets the mobile app show who to follow as the web does. It follows #130 and is for the app's roadmap item, NativePHP/dev-null-app#72.
The web gets its suggestions as the
suggestedUsersInertia prop and dismisses them throughPOST /@{user}/dismiss, neither of which a signed request can reach. Both endpoints reuse what the web runs:SuggestedUsersQueryandCreateSuggestionDismissal.Endpoints
GET /api/v1/me/suggested-usersanswers with the people the web would suggest, asUserResources: three to show and three spares to fill the place of one dismissed, as the web asks for them. Each one hasfollowers_count,followed_by_following_count(new onUserResource, only set here) andfollows_youfor the reason line and the follow-back, andis_followed: false, as other lists of people say it.?round=Nis the next page of the day's picks, as the web's refresh button moves through them with itswho_to_follow_roundcookie. A missing or empty round is the first.?username=or?commit_id=asks for the suggestions as the web shows them beside that profile or commit since #1021: the people the profile follows, or the thread's other participants, come first, and the regular suggestions fill the rest. The commit has to be one the user can see, as its page requires, otherwise the answer is422with "That commit is no longer available." An unknown username is422too, and so is passing both. Across a block, a profile is ignored and the suggestions are the regular ones, as on the web. Without either, nothing changes.{"data": [{"id": "…", "username": "octocat", "name": "The Octocat", "lane": 3, "followers_count": 12, "followed_by_following_count": 2, "follows_you": false, "is_followed": false, "…": "…"}]}POST /api/v1/users/{username}/dismissmarks someone Not interested and answers204. As on the web, they are never suggested again, nothing else changes, and dismissing yourself is403.Muted people, anyone across a block, and dismissed people are left out, as the query already does for the web.
Testing
Pint, Rector, PHPStan, type coverage at 100%, and all 4,149 tests pass locally (without the coverage driver, so line coverage is CI's to confirm). The new tests cover both halves of the mix, rounds, dismissals, mutes and blocks, the profile and commit context, commits the user can't see, validation, and unsigned requests, and that the endpoint answers what the web does for the same profile or commit page.
The app side is NativePHP/dev-null-app#74. It ran against this branch on an iOS simulator: following, dismissing with the spares moving up, and the next round. The app only shows who to follow in the Log, as the web does on smaller screens, so it doesn't pass a context.
Summary by CodeRabbit