spikerj/spikersoft-issues
1
0 Fork 0
Code Issues 23 Pull Requests Actions Packages Projects Releases Wiki Activity

Epic: build the full parent-approval book model (upload approval chain, ApprovedBy audit, parent family/staff-uploads view, auto-Public-on-approval) #621

New Issue
Closed
opened 2026-07-16 23:00:27 +00:00 by spikerj · 3 comments
spikerj commented 2026-07-16 23:00:27 +00:00
Owner
Copy link
Copy source

Split out from #618. While closing the book read-authorization hole, a code audit found that the intended parent-approval model is mostly not implemented. The intended model (per product owner):

  • Anyone with privileges can upload a book.
  • If the uploader is a child: the parent AND staff must approve the upload.
  • If the uploader is a parent: staff must approve.
  • If the uploader is staff: staff approval (even self), with audit info.
  • Staff see all books, with uploader attribution.
  • Children see their parental-approved books.
  • Parents see all books they / their children / staff uploaded, and toggle each on/off for their children.

What actually exists today (audit findings)

  1. Upload approval is staff/admin-only. A child's (or anyone's) upload creates an UploadWorkflowState with Status = "pending_approval" that notifies staff (BookController.ApproveUpload is guarded by IsAdminOrStaff() → Forbid()). There is no parent-approval step and no parent notification.
  2. No approval fields on Book. Book has only UploadedBy, Visibility, Status, DeletedAt/By. No ApprovedBy, ApprovedAt, or parental-approval state. Approval audit lives only on the transient UploadWorkflowState.Events list.
  3. No parent "family + staff uploads" view. No query lists "books my children / staff / I uploaded" with uploader attribution. Parents only get a per-child FeaturePermissions.ApprovedBookIds toggle (PUT profile/parental/permissions/{childUserId}).
  4. Nothing auto-makes a book Public. Ingest only flips Status → Available; Visibility is never set to Public in code (appears to be a manual/edit step). So the parent "enable for child" toggle (ApprovedBookIds) only takes effect once a book is independently made Public+Available.
  5. Child parental-gating is enforced in exactly one endpoint (my-books). Direct book-content endpoints historically applied only Public+Available — see the follow-up under #618 that adds the child ApprovedBookIds gate to those endpoints.

Scope of this epic (to design before building)

  • Parent-approval step in the upload workflow (child upload → parent + staff; parent upload → staff), with role resolution (parent/child are profile flags IsParent/IsChildAccount, not Keycloak roles).
  • Persist approval + audit on the Book (ApprovedBy, ApprovedAt, approval state) rather than only on the workflow doc.
  • Parent-facing book list with uploader attribution + the enable-for-child toggle in one place.
  • Define/automate the Visibility → Public transition and how it relates to approval.
  • Frontend: parent approval UI, parent family-books view, staff audit view.

This is multi-PR (backend + frontend). #618 is being finished narrowly (close the read hole + child gate on direct access) independently of this epic.

Child issues (per-repo, auto-close on merge)

No open child issues were produced by the migration — either this epic's
work was already complete, or its scope needs to be broken down into
per-repo issues before it can progress.

Checklist generated by the umbrella-tracker migration, 2026-08-07 — Opus 5 Agent

Split out from #618. While closing the book read-authorization hole, a code audit found that the intended **parent-approval model** is mostly **not implemented**. The intended model (per product owner): - Anyone with privileges can upload a book. - If the uploader is a **child**: the **parent AND staff** must approve the upload. - If the uploader is a **parent**: **staff** must approve. - If the uploader is **staff**: staff approval (even self), with audit info. - **Staff** see all books, with uploader attribution. - **Children** see their parental-approved books. - **Parents** see all books they / their children / staff uploaded, and toggle each on/off for their children. ## What actually exists today (audit findings) 1. **Upload approval is staff/admin-only.** A child's (or anyone's) upload creates an `UploadWorkflowState` with `Status = "pending_approval"` that notifies **staff** (`BookController.ApproveUpload` is guarded by `IsAdminOrStaff()` → `Forbid()`). There is **no parent-approval step** and no parent notification. 2. **No approval fields on `Book`.** `Book` has only `UploadedBy`, `Visibility`, `Status`, `DeletedAt/By`. No `ApprovedBy`, `ApprovedAt`, or parental-approval state. Approval audit lives only on the transient `UploadWorkflowState.Events` list. 3. **No parent "family + staff uploads" view.** No query lists "books my children / staff / I uploaded" with uploader attribution. Parents only get a per-child `FeaturePermissions.ApprovedBookIds` toggle (`PUT profile/parental/permissions/{childUserId}`). 4. **Nothing auto-makes a book `Public`.** Ingest only flips `Status → Available`; `Visibility` is never set to `Public` in code (appears to be a manual/edit step). So the parent "enable for child" toggle (`ApprovedBookIds`) only takes effect once a book is independently made Public+Available. 5. **Child parental-gating is enforced in exactly one endpoint** (`my-books`). Direct book-content endpoints historically applied only Public+Available — see the follow-up under #618 that adds the child `ApprovedBookIds` gate to those endpoints. ## Scope of this epic (to design before building) - Parent-approval step in the upload workflow (child upload → parent + staff; parent upload → staff), with role resolution (parent/child are profile flags `IsParent`/`IsChildAccount`, not Keycloak roles). - Persist approval + audit on the Book (`ApprovedBy`, `ApprovedAt`, approval state) rather than only on the workflow doc. - Parent-facing book list with uploader attribution + the enable-for-child toggle in one place. - Define/automate the `Visibility → Public` transition and how it relates to approval. - Frontend: parent approval UI, parent family-books view, staff audit view. This is multi-PR (backend + frontend). #618 is being finished narrowly (close the read hole + child gate on direct access) independently of this epic. <!-- BEGIN MIGRATED-CHILDREN --> --- ## Child issues (per-repo, auto-close on merge) _No open child issues were produced by the migration — either this epic's work was already complete, or its scope needs to be broken down into per-repo issues before it can progress._ <sub>Checklist generated by the umbrella-tracker migration, 2026-08-07 — Opus 5 Agent</sub> <!-- END MIGRATED-CHILDREN -->
spikerj commented 2026-07-22 03:16:33 +00:00
Author
Owner
Copy link
Copy source

Reconciliation check 2026-07-22 (code vs. epic): partial overlap shipped via #729, core epic items not built — stays open.

Shipped (merged):

  • Parents can share their uploaded books with their children — BookAccess household-read branch + self-approve family uploads (backend PR #432, angular PR #454, both merged to master ~2026-07-20). This covers a slice of the "parent family-uploads" intent.

Not found in code (the epic's core):

  • No approval-chain fields on the Book model — SpikerSoft.Common/Models/Book.cs has no ApprovedBy/ApprovalStatus; the only "approved" concept in the Books domain is BookAccess's approvedBookIds (wishlist approvals).
  • No parent-facing staff-uploads review view, no auto-Public-on-approval logic (grep across SpikerSoft.Business/Domain/Books/ finds nothing).
Reconciliation check 2026-07-22 (code vs. epic): **partial overlap shipped via #729, core epic items not built** — stays open. Shipped (merged): - Parents can share their uploaded books with their children — BookAccess household-read branch + self-approve family uploads (backend PR #432, angular PR #454, both merged to master ~2026-07-20). This covers a slice of the "parent family-uploads" intent. Not found in code (the epic's core): - No approval-chain fields on the Book model — `SpikerSoft.Common/Models/Book.cs` has no `ApprovedBy`/`ApprovalStatus`; the only "approved" concept in the Books domain is `BookAccess`'s `approvedBookIds` (wishlist approvals). - No parent-facing staff-uploads review view, no auto-Public-on-approval logic (`grep` across `SpikerSoft.Business/Domain/Books/` finds nothing).
spikerj commented 2026-07-29 18:25:11 +00:00
Author
Owner
Copy link
Copy source

Epic status roll-up — audited the five audit findings against origin/master. The read side has been solved by adjacent work; the approval chain this epic exists for has not started.

Audit finding State
1. Upload approval is staff-only, no parent step ❌ unchanged
2. No ApprovedBy/ApprovedAt on Book ❌ unchanged
3. No parent family/staff-uploads view ❌ unchanged
4. Nothing auto-makes a book Public ❌ unchanged
5. Child gating enforced in only one endpoint ✅ solved — see below

Finding 5 is genuinely fixed, and better than the epic proposed. Rather than adding the child gate endpoint-by-endpoint, #618's follow-up introduced BookAccess.CanRead as a single source of truth, now applied at BookController.cs:183, :456 and :609 — with the comment at :609 saying so explicitly ("Single source of truth with the read guards (#618)"). #729 later extended it with the household read branch. So the read-authorization surface this epic worried about is consolidated rather than scattered, which removes the risk of the next endpoint being added without the gate.

Findings 1–4 are untouched. git grep -nE "ApprovedBy|ApprovedAt" over the Book model returns nothing — approval audit still lives only on the transient UploadWorkflowState.Events list. The parental surface that does exist is still just the per-child toggle (ProfileController.cs:394/:398, approved-books/{bookId}), which is the "enable for child" switch, not an approval chain.

So the epic's design question — a child upload requiring parent and staff approval, a parent upload requiring staff — is still entirely unbuilt.

One dependency worth flagging before this is designed. The approval chain will need to read and write parental permission state, and #876 is still open: ApproveParentalLinkCommandHandler.cs:36-46 replaces ParentalControls.Permissions wholesale with a hardcoded object, silently zeroing any field not in that list (currently InfoVault). Any new approval-related permission added before #876 is fixed will be silently reset on the next parental link approval — and because this is a COPPA surface, that failure would be invisible and consequential. Worth fixing #876 first; it's a small change.

Also relevant to finding 4: the Visibility → Public transition is still never set in code, so the existing per-child toggle remains inert until a book is independently made Public+Available. That's arguably the cheapest slice to define, since it's blocking the one piece of parental control that already ships.

**Epic status roll-up** — audited the five audit findings against `origin/master`. **The read side has been solved by adjacent work; the approval chain this epic exists for has not started.** | Audit finding | State | |---|---| | 1. Upload approval is staff-only, no parent step | ❌ unchanged | | 2. No `ApprovedBy`/`ApprovedAt` on `Book` | ❌ unchanged | | 3. No parent family/staff-uploads view | ❌ unchanged | | 4. Nothing auto-makes a book `Public` | ❌ unchanged | | 5. Child gating enforced in only one endpoint | ✅ **solved** — see below | **Finding 5 is genuinely fixed, and better than the epic proposed.** Rather than adding the child gate endpoint-by-endpoint, #618's follow-up introduced `BookAccess.CanRead` as a single source of truth, now applied at `BookController.cs:183`, `:456` and `:609` — with the comment at `:609` saying so explicitly ("Single source of truth with the read guards (#618)"). #729 later extended it with the household read branch. So the read-authorization surface this epic worried about is consolidated rather than scattered, which removes the risk of the next endpoint being added without the gate. **Findings 1–4 are untouched.** `git grep -nE "ApprovedBy|ApprovedAt"` over the Book model returns **nothing** — approval audit still lives only on the transient `UploadWorkflowState.Events` list. The parental surface that does exist is still just the per-child toggle (`ProfileController.cs:394`/`:398`, `approved-books/{bookId}`), which is the "enable for child" switch, not an approval chain. So the epic's design question — a child upload requiring parent **and** staff approval, a parent upload requiring staff — is still entirely unbuilt. **One dependency worth flagging before this is designed.** The approval chain will need to read and write parental permission state, and **#876** is still open: `ApproveParentalLinkCommandHandler.cs:36-46` replaces `ParentalControls.Permissions` wholesale with a hardcoded object, silently zeroing any field not in that list (currently `InfoVault`). Any new approval-related permission added before #876 is fixed will be silently reset on the next parental link approval — and because this is a COPPA surface, that failure would be invisible and consequential. Worth fixing #876 first; it's a small change. Also relevant to finding 4: the `Visibility → Public` transition is still never set in code, so the existing per-child toggle remains inert until a book is independently made Public+Available. That's arguably the cheapest slice to define, since it's blocking the one piece of parental control that already ships.
spikerj added the epic label 2026-08-07 13:43:02 +00:00
spikerj commented 2026-08-07 14:27:50 +00:00
Author
Owner
Copy link
Copy source

Dissolved into per-repo issues as part of the umbrella-tracker breakup. This epic held
implementation work that could never auto-close from a merge; it now lives where the code is.
The unit is tracked by the shared [Book approval] title prefix plus sibling cross-links in
every issue.

Backend (spikerj/spikersoft-backend@98102023):

Angular (spikerj/spikersoft-angular@8e5a4048):

spikerj/spikersoft-backend#612 is deliberately a decision issue rather than an implementation
stub: the product-owner policy is settled but the mechanics genuinely are not, and three of the
four remaining backend issues cannot be started without them.

Already shipped, no issue filed:

  • Audit finding 5 (child gating enforced in only one endpoint) — solved. BookAccess.CanRead
    (SpikerSoft.Business/Domain/Books/BookAccess.cs:92-93) is the single source of truth and is
    applied at BookController.cs:183, :456 and :609; the household read branch from
    spikerj/spikersoft-issues#729 sits at BookAccess.cs:85-89. Confirmed on master today.
  • The per-child grant mutation — PUT|DELETE profile/parental/children/{childUserId}/approved-books/{bookId}
    (ProfileController.cs:393-400 → SetChildBookApprovalCommandHandler) and its clobber-free
    frontend call (profile.service.ts:200-205, parent-dashboard.component.ts:364-380) are done.
    The issues above cover the missing read/attribution side around it, not the mutation.

Findings 1–4 re-confirmed unbuilt on today's master (spikersoft-backend@98102023):
git grep -nE "ApprovedBy|ApprovedAt" over the Book model returns nothing; all four approval
endpoints are still if (!IsAdminOrStaff()) return Forbid(); (BookController.cs:472, :488,
:517, :568); NotifyPendingApprovalCountChangedAsync (:1012-1029) still broadcasts via
SendToAllAsync; and git grep 'Visibility = "Public"' finds comparisons only — never an
assignment, while BookManagementService.cs:90 hard-codes Visibility = "Private".

Closing here — the umbrella tracker is being emptied.

— Opus 5 Agent

Dissolved into per-repo issues as part of the umbrella-tracker breakup. This epic held implementation work that could never auto-close from a merge; it now lives where the code is. The unit is tracked by the shared **`[Book approval]`** title prefix plus sibling cross-links in every issue. **Backend** (`spikerj/spikersoft-backend@98102023`): - spikerj/spikersoft-backend#612 — **decision issue**: the approval-chain state machine. Where approval state lives (Mongo `books` subclass vs `book-upload-workflows` vs both), parallel vs sequential parent/staff approval, the `Username`(`preferred_username`) ↔ `KeycloakUserId`(`sub`) key mismatch, the no-linked-parent case, staff self-approval, revocation, and whether the chain subsumes the spikerj/spikersoft-issues#729 household path. Every other backend issue is blocked on this. Blocking dependency called out: spikerj/spikersoft-backend#534. - spikerj/spikersoft-backend#613 — persist `ApprovedBy`/`ApprovedAt`/approval state on the Book document (audit finding 2). - spikerj/spikersoft-backend#614 — the parent+staff two-step chain in the upload workflow (audit finding 1). - spikerj/spikersoft-backend#615 — parent family/staff-uploads query with server-side uploader attribution (audit finding 3). - spikerj/spikersoft-backend#616 — define and automate the `Visibility → Public` transition (audit finding 4). **Angular** (`spikerj/spikersoft-angular@8e5a4048`): - spikerj/spikersoft-angular#716 — parent approval queue UI for pending child uploads. - spikerj/spikersoft-angular#717 — parent family & staff-uploads view with server-side attribution (replaces the `visibility !== "Public"` heuristic in `parent-dashboard.component.ts:347-349`). - spikerj/spikersoft-angular#718 — approval audit + uploader attribution in admin book management. spikerj/spikersoft-backend#612 is deliberately a **decision issue** rather than an implementation stub: the product-owner policy is settled but the mechanics genuinely are not, and three of the four remaining backend issues cannot be started without them. **Already shipped, no issue filed:** - **Audit finding 5 (child gating enforced in only one endpoint) — solved.** `BookAccess.CanRead` (`SpikerSoft.Business/Domain/Books/BookAccess.cs:92-93`) is the single source of truth and is applied at `BookController.cs:183`, `:456` and `:609`; the household read branch from spikerj/spikersoft-issues#729 sits at `BookAccess.cs:85-89`. Confirmed on master today. - **The per-child grant mutation** — `PUT|DELETE profile/parental/children/{childUserId}/approved-books/{bookId}` (`ProfileController.cs:393-400` → `SetChildBookApprovalCommandHandler`) and its clobber-free frontend call (`profile.service.ts:200-205`, `parent-dashboard.component.ts:364-380`) are done. The issues above cover the missing *read/attribution* side around it, not the mutation. **Findings 1–4 re-confirmed unbuilt on today's master** (`spikersoft-backend@98102023`): `git grep -nE "ApprovedBy|ApprovedAt"` over the Book model returns nothing; all four approval endpoints are still `if (!IsAdminOrStaff()) return Forbid();` (`BookController.cs:472`, `:488`, `:517`, `:568`); `NotifyPendingApprovalCountChangedAsync` (`:1012-1029`) still broadcasts via `SendToAllAsync`; and `git grep 'Visibility = "Public"'` finds comparisons only — never an assignment, while `BookManagementService.cs:90` hard-codes `Visibility = "Private"`. Closing here — the umbrella tracker is being emptied. — Opus 5 Agent
Sign in to join this conversation.
No Branch/Tag Specified
Labels
Clear labels
agentic
Actionable surfaced by the agentic architecture & security review
 bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 epic
Cross-repo epic — children live in per-repo trackers
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 sonarqube
Code quality issue surfaced by SonarQube (bug / code smell)
 wontfix
This won't be fixed
No labels epic
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
spikerj (Joseph Spiker)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#621