Skip to content

Pick up the three things the rebase left behind - #125

Open
openipc-ai wants to merge 2 commits into
feat/ia-cutoverfrom
fix/missed-from-snapshot
Open

Pick up the three things the rebase left behind#125
openipc-ai wants to merge 2 commits into
feat/ia-cutoverfrom
fix/missed-from-snapshot

Conversation

@openipc-ai

Copy link
Copy Markdown
Collaborator

You asked whether the whole working tree was covered. It wasn't — this is what
the audit found.

Stacked on #124. Method: diff the June tree's 106 changed files against
everything the five PRs landed. 20 unaccounted for; 17 deliberate and
explained in those PRs (the concern upstream rewrote better, locale files for
languages no longer served, a Yarn 1 lockfile, a schema dump). Three were
not.

1. The merchandise page shows one T-shirt three times

Same image, same .shirt_1 keys, same link — pasted into three cards. It has
read as a catalogue of one product in triplicate on the live site for as long as
the file has existed. One card now, and a test that counts them.

2. The Open Wall didn't say what it is

It's in the navigation and the footer since #124, so it now gets visitors who
have never seen it and can't tell from a grid of stills whether these images
were collected or volunteered. A line under the heading says they were
volunteered, and that they're deleted after two days — which is
PurgeImagesJob::RETENTION, checked rather than assumed. en/ru/zh.

3. Six surviving pages had no tests at all

/green_life, /merchandise, /stages-of-firmware-development, /utilities,
and the partition and timer calculators are served by the app and were rendered
by no test whatsoever.

They're the pages nobody thinks about — and they render through the layout,
navbar and footer that #122 and #124 rewrote, so a mistake in any of those shows
up there first, or not at all. Ten pages are rendered now, each checked for its
own title and for translation missing, and held to the layout wrapper — which
also pins the other side of the content_for(:fullwidth) switch against pages
that depend on it.

Verification

check result
bin/rails test 303 runs, 1390 assertions, 0 failures (289 → 303)
i18n-tasks missing none
i18n-tasks unused 72, unchanged
rubocop on the new test 0 offences

The merchandise test fails on master (finds 3, expects 1), which is the check
worth repeating.

🤖 Generated with Claude Code

https://claude.ai/code/session_015hvzXBErEjEGMyRme2K3hi

openipc-ai and others added 2 commits August 25, 2026 15:49
An audit of the June working tree against what the five preceding changes
actually landed found 20 files unaccounted for. Seventeen were deliberate and
are explained in those changes -- the concern upstream rewrote better, the
locale files for languages no longer served, a Yarn 1 lockfile, a schema dump.
Three were not deliberate. They were missed.

MERCHANDISE. The page advertises a single T-shirt three times: the same image,
the same .shirt_1 keys and the same link, pasted into three cards. It has read
as a catalogue of one product in triplicate on the live site for as long as the
file has existed. One card now, and a test that counts them.

THE OPEN WALL SAYS WHAT IT IS. The wall is in the navigation and the footer
since the cutover, so it now gets visitors who have never seen it and cannot
tell from a grid of stills whether these images were collected or volunteered.
A line under the heading says they were volunteered, and that they are deleted
after two days -- which is PurgeImagesJob::RETENTION, checked rather than
assumed.

THE SURVIVING PAGES HAD NO TESTS. /green_life, /merchandise,
/stages-of-firmware-development, /utilities and the partition and timer
calculators are served by the app and were rendered by no test at all. They are
the pages nobody thinks about, and they render through the layout, navbar and
footer that the relaunch rewrote -- so a mistake in any of those shows up there
first, or not at all. Ten of them are rendered now, checked for their own title
and for translation-missing, and held to the layout wrapper, which pins the
other side of the content_for(:fullwidth) switch against pages that depend on
it.

303 runs, 1390 assertions, 0 failures. i18n-tasks reports no missing
translations and 72 unused, unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015hvzXBErEjEGMyRme2K3hi
The existing link test walks whatever the page renders and requires each href to
resolve. That cannot notice a link which stopped being rendered at all -- a
pillar card losing its href, or a whole section dropped in a refactor -- because
a page with fewer links still passes.

This is the one assertion from the June test file that the two replacements did
not carry between them.

304 runs, 1404 assertions, 0 failures.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015hvzXBErEjEGMyRme2K3hi
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Restore missed page fixes and legacy coverage

🐞 Bug fix 🧪 Tests ✨ Enhancement 🕐 20-40 Minutes

Grey Divider

AI Description

• Removes duplicate merchandise cards so the single T-shirt appears once.
• Explains Open Wall consent and two-day retention across all supported languages.
• Adds rendering, localization, layout, navigation, and footer coverage for ten legacy pages.
Diagram

graph TD
  U["Site visitor"] --> R["Rails routes"] --> V["Legacy views"] --> L["Shared layout"]
  R --> W["Open Wall"] --> I["Locale copy"]
  T["Integration tests"] --> V
  T --> W
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Derive retention copy from policy
  • ➕ Prevents localized copy from drifting when image retention changes.
  • ➕ Makes the stated two-day policy traceable to PurgeImagesJob::RETENTION.
  • ➖ Couples presentation code to a background-job constant.
  • ➖ Requires localized interpolation and duration formatting across languages.

Recommendation: The current fixes and request-level regression coverage are appropriate for this focused recovery PR. Keep the simple static copy, but add an explicit policy contract test or interpolate the retention value if the retention period is expected to change; the new Open Wall assertion alone does not bind “two days” to PurgeImagesJob::RETENTION.

Files changed (6) +87 / -26

Enhancement (4) +8 / -0
index.html.erbExplain Open Wall participation and retention +5/-0

Explain Open Wall participation and retention

• Adds a visually secondary introduction beneath the Open Wall heading explaining voluntary uploads, automatic deletion, and how camera owners can participate.

app/views/snapshots/index.html.erb

en.ymlAdd English Open Wall introduction +1/-0

Add English Open Wall introduction

• Adds English copy describing voluntary snapshot uploads, two-day retention, and camera-side opt-in.

config/locales/en.yml

ru.ymlAdd Russian Open Wall introduction +1/-0

Add Russian Open Wall introduction

• Adds the Russian translation of the new Open Wall participation and retention explanation.

config/locales/ru.yml

zh.ymlAdd Chinese Open Wall introduction +1/-0

Add Chinese Open Wall introduction

• Adds the Chinese translation of the new Open Wall participation and retention explanation.

config/locales/zh.yml

Bug fix (1) +3 / -26
merchandise.html.erbRemove duplicate T-shirt cards +3/-26

Remove duplicate T-shirt cards

• Deletes two identical product cards so the merchandise page lists its sole T-shirt once. Adds context documenting the prior duplication and regression expectation.

app/views/pages/merchandise.html.erb

Tests (1) +76 / -0
legacy_pages_test.rbAdd legacy page integration coverage +76/-0

Add legacy page integration coverage

• Adds request tests for ten surviving pages, validating successful rendering, localized titles, translation completeness, layout wrappers, navigation, and footer presence. Includes focused regressions for one merchandise product link and the Open Wall introduction.

test/controllers/legacy_pages_test.rb

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

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Unverified upload consent claim 🐞 Bug ≡ Correctness
Description
The new intro presents every wall image as voluntarily uploaded by an OpenIPC camera, but
SnapshotsController#create accepts unauthenticated uploads and neither requires OpenIPC firmware
nor records/verifies an Open Wall opt-in. Arbitrary clients can therefore publish images that the
page falsely represents as owner-volunteered camera submissions.
Code

app/views/snapshots/index.html.erb[16]

+    <p class="text-body-secondary small" style="max-width: 70ch"><i class="bi bi-camera-video me-1"></i><%= t('.intro_html') %></p>
Evidence
The create action accepts permitted public request data without authentication or opt-in
verification, and firmware is only an optional permitted parameter; the model's validations do not
establish the provenance asserted by the new copy.

app/controllers/snapshots_controller.rb[3-24]
app/controllers/snapshots_controller.rb[92-95]
app/models/snapshot.rb[61-64]
config/locales/en.yml[191-191]

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 Open Wall intro asserts consent and OpenIPC-camera provenance that the upload endpoint does not verify. Either add enforceable uploader authentication/opt-in provenance or revise every locale to describe only what the system can guarantee.

## Issue Context
`SnapshotsController#create` is CSRF-exempt, permits firmware as optional metadata, and saves snapshots without authentication or a consent/opt-in field. Model validations only cover the image, MAC address, blacklist, and upload interval.

## Fix Focus Areas
- app/views/snapshots/index.html.erb[12-16]
- config/locales/en.yml[191-191]
- config/locales/ru.yml[193-193]
- config/locales/zh.yml[191-191]
- app/controllers/snapshots_controller.rb[15-24]
- app/models/snapshot.rb[61-64]

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


2. Two-day deletion overpromised 🐞 Bug ≡ Correctness
Description
The intro says images are deleted after two days, but the two-day constant is only a cutoff for a
once-nightly purge, so snapshots can remain published for nearly three days even when cron succeeds
and longer when it fails. This makes the new user-facing retention assurance stricter than the
behavior actually implemented.
Code

app/views/snapshots/index.html.erb[R14-16]

+        these were collected or volunteered. Two days is the retention
+        PurgeImagesJob::RETENTION actually enforces. %>
+    <p class="text-body-secondary small" style="max-width: 70ch"><i class="bi bi-camera-video me-1"></i><%= t('.intro_html') %></p>
Evidence
The purge job removes only snapshots older than its two-day threshold when invoked, while the
repository's deployment script documents a single daily invocation at 01:30; therefore two days is
the minimum age at deletion, not the promised deletion time.

app/jobs/purge_images_job.rb[11-18]
deploy/purge-snapshots.sh[3-13]
app/controllers/snapshots_controller.rb[15-18]
config/locales/en.yml[191-191]

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 copy promises deletion after two days although cleanup runs only once nightly after records cross the two-day threshold. Revise all translations to communicate the real upper-bound/best-effort behavior, or schedule per-snapshot deletion so the stated retention is enforced.

## Issue Context
`PurgeImagesJob::RETENTION` selects records older than two days at execution time. The deployment cron invokes that job only at 01:30 daily, leaving an additional delay of up to almost 24 hours.

## Fix Focus Areas
- app/views/snapshots/index.html.erb[12-16]
- config/locales/en.yml[191-191]
- config/locales/ru.yml[193-193]
- config/locales/zh.yml[191-191]
- app/jobs/purge_images_job.rb[11-18]
- deploy/purge-snapshots.sh[3-13]

ⓘ 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 hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

have not seen it before and cannot tell from a grid of stills whether
these were collected or volunteered. Two days is the retention
PurgeImagesJob::RETENTION actually enforces. %>
<p class="text-body-secondary small" style="max-width: 70ch"><i class="bi bi-camera-video me-1"></i><%= t('.intro_html') %></p>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Unverified upload consent claim 🐞 Bug ≡ Correctness

The new intro presents every wall image as voluntarily uploaded by an OpenIPC camera, but
SnapshotsController#create accepts unauthenticated uploads and neither requires OpenIPC firmware
nor records/verifies an Open Wall opt-in. Arbitrary clients can therefore publish images that the
page falsely represents as owner-volunteered camera submissions.
Agent Prompt
## Issue description
The Open Wall intro asserts consent and OpenIPC-camera provenance that the upload endpoint does not verify. Either add enforceable uploader authentication/opt-in provenance or revise every locale to describe only what the system can guarantee.

## Issue Context
`SnapshotsController#create` is CSRF-exempt, permits firmware as optional metadata, and saves snapshots without authentication or a consent/opt-in field. Model validations only cover the image, MAC address, blacklist, and upload interval.

## Fix Focus Areas
- app/views/snapshots/index.html.erb[12-16]
- config/locales/en.yml[191-191]
- config/locales/ru.yml[193-193]
- config/locales/zh.yml[191-191]
- app/controllers/snapshots_controller.rb[15-24]
- app/models/snapshot.rb[61-64]

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

Comment on lines +14 to +16
these were collected or volunteered. Two days is the retention
PurgeImagesJob::RETENTION actually enforces. %>
<p class="text-body-secondary small" style="max-width: 70ch"><i class="bi bi-camera-video me-1"></i><%= t('.intro_html') %></p>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Two-day deletion overpromised 🐞 Bug ≡ Correctness

The intro says images are deleted after two days, but the two-day constant is only a cutoff for a
once-nightly purge, so snapshots can remain published for nearly three days even when cron succeeds
and longer when it fails. This makes the new user-facing retention assurance stricter than the
behavior actually implemented.
Agent Prompt
## Issue description
The copy promises deletion after two days although cleanup runs only once nightly after records cross the two-day threshold. Revise all translations to communicate the real upper-bound/best-effort behavior, or schedule per-snapshot deletion so the stated retention is enforced.

## Issue Context
`PurgeImagesJob::RETENTION` selects records older than two days at execution time. The deployment cron invokes that job only at 01:30 daily, leaving an additional delay of up to almost 24 hours.

## Fix Focus Areas
- app/views/snapshots/index.html.erb[12-16]
- config/locales/en.yml[191-191]
- config/locales/ru.yml[193-193]
- config/locales/zh.yml[191-191]
- app/jobs/purge_images_job.rb[11-18]
- deploy/purge-snapshots.sh[3-13]

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

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.

1 participant