Skip to content

An edit can add a flash dump to a report sent without one - #461

Merged
openipc-ai merged 2 commits into
masterfrom
edit-adds-a-dump-456
Oct 11, 2026
Merged

openipc-ai merged 2 commits into
masterfrom
edit-adds-a-dump-456

Conversation

@openipc-ai

Copy link
Copy Markdown
Collaborator

A member editing a report on /club could add photos and text, never a dump
(#456): the server refused any backup in an edit, because a backup is what the
camera said and stays as sent. That is about changing one; a report sent
without a dump had nothing to change, and its owner had no way to add the
chip they read later short of sending a new report.

  • An edit may carry one backup, with its consent (private by default, or
    public), to a report that has none: its consent was "none" and becomes the
    one given, in the edit's transaction. A report that has a dump keeps it, and
    a second is refused.
  • The dump is what one sent with the report would be: an ipctool backup taken
    with the report's own ipctool output (another's, or one on a report without
    ipctool's output, is refused -- that output an edit cannot add), or a whole
    chip read with a programmer. The report already says which camera it is.
  • A decided report goes back for review, and the dump is paid then, as any
    dump is (Stars for a flash dump: a programmer or TFTP dump earns what an ipctool backup earns #458 is about what that should be).
  • The editor offers "Full flash dump" with the send form's line and public
    checkbox, only when the report has none.
  • Someone else's report answers 404 before its contents are looked at, so a
    refusal never says whether it holds ipctool's output.
  • A backup no longer counts toward the files limit in an edit, as on sending.

Closes #456

A member editing a report on /club could add photos and text, never a dump
(#456): the server refused any backup in an edit, because a backup is what the
camera said and stays as sent. That is about changing one; a report sent
without a dump had nothing to change, and its owner had no way to add the
chip they read later short of sending a new report.

- An edit may carry one backup, with its consent (private by default, or
  public), to a report that has none: its consent was "none" and becomes the
  one given, in the edit's transaction. A report that has a dump keeps it, and
  a second is refused.
- The dump is what one sent with the report would be: an ipctool backup taken
  with the report's own ipctool output (another's, or one on a report without
  ipctool's output, is refused -- that output an edit cannot add), or a whole
  chip read with a programmer. The report already says which camera it is.
- A decided report goes back for review, and the dump is paid then, as any
  dump is (#458 is about what that should be).
- The editor offers "Full flash dump" with the send form's line and public
  checkbox, only when the report has none.
- Someone else's report answers 404 before its contents are looked at, so a
  refusal never says whether it holds ipctool's output.
- A backup no longer counts toward the files limit in an edit, as on sending.

Closes #456
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Allow members to add a flash dump when editing a report without one

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Let report owners add one flash dump later without submitting a new report.
• Validate dump provenance and consent while preserving existing backups and ownership privacy.
• Return decided reports for review and award dump points when they are approved.
Diagram

sequenceDiagram
    actor Member
    participant Editor as Edit form
    participant API as Edit API
    participant Files as File storage
    participant Store as Report store
    participant DB as Database
    actor Reviewer
    Member->>Editor: Select dump and consent
    Editor->>API: Submit multipart edit
    API->>Store: Check ownership
    Store->>DB: Read report owner
    DB-->>API: Owner and report facts
    API->>API: Validate dump against report
    API->>Files: Prepare dump
    API->>Store: Apply edit
    Store->>DB: Save dump and consent atomically
    Store->>DB: Queue decided report for review
    Reviewer->>Store: Approve report
    Store->>DB: Award dump points
Loading
High-Level Assessment

Using the existing edit transaction keeps the dump, consent, audit entry, and return-to-review behavior together. A separate dump-upload endpoint would duplicate ownership and review handling without a clear benefit.

Files changed (6) +204 / -14

Enhancement (3) +104 / -13
EditReport.tsxOffer a single flash dump and consent choice during editing +21/-7

Offer a single flash dump and consent choice during editing

• Adds the backup option only when the report has none. Reuses the send-form guidance and public-consent checkbox, defaulting submissions to private.

frontend/apps/site/src/components/club/EditReport.tsx

edit.goValidate and persist backup additions in report edits +80/-3

Validate and persist backup additions in report edits

• Checks ownership before inspecting private report contents, validates a dump against the report, and prepares it for storage. The edit transaction rejects a second backup, saves consent with the new file, and excludes backups from the ordinary file limit.

service/internal/reports/edit.go

handler.goAccept backup and consent parts in edit requests +3/-3

Accept backup and consent parts in edit requests

• Allows multipart edits to carry a backup and consent while continuing to reject changes to ipctool output. Existing backup parsing and upload limits remain available to the edit path.

service/internal/reports/handler.go

Tests (2) +97 / -0
EditReport.test.tsxTest dump availability and default consent in the editor +21/-0

Test dump availability and default consent in the editor

• Checks that an eligible editor submits a backup with private consent and that a report already holding a backup does not offer another.

frontend/apps/site/src/components/club/EditReport.test.tsx

edit_test.goCover flash-dump edits and review outcomes +76/-0

Cover flash-dump edits and review outcomes

• Exercises valid programmer images and matching ipctool backups, consent defaults, and rejection of invalid, mismatched, duplicate, or unauthorized uploads. Verifies that a decided report returns to review and earns dump points on approval.

service/internal/club/edit_test.go

Documentation (1) +3 / -1
CLAUDE.mdDocument the new report-edit allowance +3/-1

Document the new report-edit allowance

• Clarifies that an owner may add a flash dump to a report without one, subject to consent and compatibility with its existing ipctool output.

CLAUDE.md

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Database errors appear as missing reports ✓ Resolved
Description
Store.Owner returns an empty owner with a nil error when its query fails before scanning a member
ID. When the new ownership check in API.Edit encounters that failure, it returns 404 instead of
logging the database error and returning a retryable server error.
Code

service/internal/reports/edit.go[R354-358]

+	if owner, err := st.Owner(ctx, id); err != nil || owner != member {
+		if err != nil {
+			a.fail(w, "the report", err)
+		} else {
+			a.refuse(w, http.StatusNotFound, "no report of yours has this id")
Evidence
Owner checks m == nil before returning err, so a failed query that leaves m nil becomes
("", nil). The newly added edit check treats that result as an ownership mismatch and calls
refuse(404); unlike fail, refuse does not log the database error.

service/internal/reports/club.go[85-93]
service/internal/reports/edit.go[351-360]
service/internal/reports/handler.go[692-701]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new edit ownership check depends on `Store.Owner`, which can suppress a database query error and make the edit return 404.
## Fix Focus Areas
- service/internal/reports/club.go[85-93]
- service/internal/reports/edit.go[351-360]
## Recommended Fix
In `Store.Owner`, return non-`ErrNoRows` query errors before checking whether the scanned member is nil. Preserve the empty-owner result for a genuinely missing or unowned report, so `API.Edit` can send query failures through `a.fail`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can tweak Display settings with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread service/internal/reports/edit.go
…ut an owner

Qodo on #461: the nil-member check came before the error's, so a query that
failed read as "no owner" -- a 404 from the edit's new ownership check, and
an ownerless report to Decide -- instead of an error logged and answered 500.
@openipc-ai
openipc-ai merged commit 03738fd into master Oct 11, 2026
4 checks passed
@openipc-ai
openipc-ai deleted the edit-adds-a-dump-456 branch October 11, 2026 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Editing a sent report: no way to add a flash dump, and the page doesn't say where one goes

1 participant