Skip to content

feat(api): auto-approve requests per requester account (#2718) - #2761

Open
tunglambk wants to merge 1 commit into
vavallee:mainfrom
tunglambk:feature/2718-auto-approve-requester
Open

tunglambk wants to merge 1 commit into
vavallee:mainfrom
tunglambk:feature/2718-auto-approve-requester

Conversation

@tunglambk

@tunglambk tunglambk commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A requester can only ask, and every ask waits for an admin. On a lean requester UI that turns into a stream of approvals, which is exactly what the reporter wants to switch off for adults they trust. Closes #2718.

The switch is per account, on the user row (requests_auto_approve, migration 090), and off by default. An admin sets it from a new Auto-approve requests checkbox on the Users page, or PUT /api/v1/auth/users/{id}/auto-approve with {"enabled": true}. It sits on users rather than in the settings table because it is a per-account permission like role, and the admin users API already owns that surface. No existing account changes behaviour.

The existing flow, for reference: RequestHandler.Create stores the row as pending and that is where it waited. Approve claims the row with a compare-and-swap on its status, runs runApproval (which revalidates the stored payload, rechecks "already in the library" and calls the same add cores the Add dialog uses with the requester as owner), then Complete marks it approved. Auto-approval hooks in at that first step: Create now hands the stored row to writeCreated, which reads the owner's flag and, when it is on, claims the row and runs the same add. There is no second pipeline. I split Approve so the claim, the renewer, the payload revalidation, the owner in the context and the release-on-failure are one function (runClaimedApproval) that both callers drive. decided_by stays NULL, so the queue can tell an automatic approval from a human one.

The approval body is the approve form's own starting point, since there is no form to fill in: a book request searches on add, an author request runs the ordinary catalogue sync. The form defaults author search off too, and auto-approving a batch of author requests with search on add would fan out over every monitored book of each author, so I kept it that way. Easy to flip if you'd rather it always searched.

Boundaries worth knowing:

  • An account without the flag still lands pending and touches no adder.
  • Turning the flag off puts the manual path back; an admin can still approve by hand.
  • Turning it on does not sweep the queue. Requests already waiting stay waiting.
  • A failed add releases the claim and leaves the request pending, so a provider hiccup cannot lose the request and the queue still sees it.
  • The requestCreated webhook fires as before, and the pending cap is unchanged since an auto-approved request never sits in pending.

I asked on the issue whether this should be blanket or limited to some other condition; no answer yet, so it is blanket for now, which is what the issue's Seerr comparison implies. I did not touch the wanted/sweep/scoring paths or the narrator token work.

Checklist

  • Commits signed off with git commit -s
  • Tests added or updated
  • docs/DEPLOYMENT.md updated if env vars, config, or upgrade path changed (no env or config change; migration 090 only adds a column with a default)
  • Added a changelog fragment under changelog.d/
  • Wiki pages updated if user-facing behaviour changed (no wiki page covers the requester role; docs/multi-user.md is the reference and is updated)

Test plan

Fail-before as a mutation check. With the flag lookup forced to false on the branch, the auto-approval tests fail:

--- FAIL: TestRequestsCreate_AutoApproveNeedsNoAdmin
    requests_auto_approve_test.go:42: status "pending", want approved without an admin
--- FAIL: TestRequestsCreate_AutoApproveOffRestoresTheManualPath
--- FAIL: TestRequestsCreate_AutoApproveLeavesQueuedRequestsAlone
--- FAIL: TestRequestsCreate_AutoApproveAuthorRequest
--- FAIL: TestRequestsCreate_AutoApproveReopenedRequest

TestRequestsCreate_WithoutAutoApproveStillWaits and the failure-fallback test pass either way, which is the point of both. On a clean main worktree a probe confirms the current behaviour: a requester's request is stored pending and the add core never runs.

  • gofmt -l . clean; go build ./..., go vet ./internal/api/... ./internal/db/... ./cmd/... exit 0
  • go test ./cmd/... ./internal/... passes apart from the twelve hardlink/rename/filesystem tests that fail identically on a clean main worktree in this sandbox: TestHardlinkFile, TestHardlinkDir, TestDrop_EbookHardlink, TestImportMode_DefaultHardlinkSameDevice, TestImportMode_DefaultHardlinkSameDevice_DstNotExist, TestResolveImportMode, TestStagedImport_RollbackRestoresMoveSourceSameFS, TestFlattenAudiobookDir_Hardlink, TestScannerFlatten_HardlinkMode, TestDiagnose_QbittorrentAllPass, TestMigrateFlatCache_MovesFilesAndSidecars, TestCheckSQLiteFile_AwkwardPath. The failure sets are identical on both trees.
  • Frontend under Node 24: npx tsc --noEmit exit 0; npm run lint 0 errors (7 pre-existing exhaustive-deps / unused-disable warnings across the app, one of them UsersPage.tsx); npx vitest run 95 files / 1086 tests passed; npm run build exit 0.

I could not run golangci-lint: the binary available here is v1.64.8 and the repo config is v2, so make lint and make check do not run in this environment. The Go and web steps above are the ones I ran.

A requester can only ask, and every ask waits for an admin. With the lean
requester UI that turns into a stream of approvals, which is the opposite
of what the role is for.

Add a per-account setting, off by default so no existing install changes
behaviour. It lives on the user row (migration 090) rather than in the
settings table because it is a per-account permission like role, and it is
set from the Users page or PUT /auth/users/{id}/auto-approve.

When it is on, Create claims the new request and runs the same add an
admin's Approve runs, with no deciding user recorded. The
already-in-the-library check, the stored-payload revalidation and the
pending cap all still apply. A book request searches on add; an author
request runs the ordinary catalogue sync. If the add fails the request
stays pending, so nothing is lost. The switch only affects the next
request; anything already queued stays for a human.

Signed-off-by: Tung Lam <lamphamabtung96@gmail.com>
@github-actions github-actions Bot added the bindery-notified Discord notification already sent for this PR label Sep 24, 2026
@codecov

codecov Bot commented Sep 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.23288% with 21 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/api/requests.go 60.86% 12 Missing and 6 partials ⚠️
internal/api/auth_users.go 85.71% 2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bindery-notified Discord notification already sent for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add auto-approve setting per requester account

1 participant