added response folders and updated toolkit
This commit is contained in:
250
sources/01-summarization.md
Normal file
250
sources/01-summarization.md
Normal file
@@ -0,0 +1,250 @@
|
||||
● Here's the synopsis.
|
||||
|
||||
What it is
|
||||
|
||||
Stocks in the Future
|
||||
(rubyforgood/stocks-in-the-future) — a Rails 8.1 /
|
||||
Ruby 3.4.4 app for a nonprofit that teaches
|
||||
middle-schoolers financial literacy. Students earn
|
||||
real-ledger, fake-money by attending class and
|
||||
getting good grades, then invest that money in a
|
||||
simulated stock market tracking real prices. Teachers
|
||||
enter grades; admins run everything.
|
||||
|
||||
Architecture
|
||||
|
||||
Standard Rails-with-extras, no API layer —
|
||||
server-rendered Hotwire (Turbo + Stimulus),
|
||||
Tailwind/shadcn, Propshaft, importmap. Postgres.
|
||||
Solid Queue for jobs. Devise for auth, Pundit for
|
||||
authorization, Discard for soft deletes. Deployment
|
||||
is Capistrano → AWS Lightsail (Terraform-managed),
|
||||
with SES for mail; Docker for local dev.
|
||||
|
||||
Layering is more disciplined than typical:
|
||||
app/services (business operations), app/policies,
|
||||
app/presenters, app/facades, plus a PortfolioPosition
|
||||
PORO aggregate. Models stay reasonably thin.
|
||||
|
||||
Domain shape:
|
||||
School → SchoolYear → Quarter(1-4) → GradeBook →
|
||||
GradeEntry
|
||||
↓
|
||||
Classroom ← TeacherClassroom → Teacher
|
||||
↓
|
||||
ClassroomEnrollment → Student ─ Portfolio ─┬
|
||||
PortfolioTransaction (ledger)
|
||||
├
|
||||
PortfolioStock (lots)
|
||||
└
|
||||
PortfolioSnapshot (monthly)
|
||||
Student → Order → Stock
|
||||
|
||||
User is STI (User/Student/Teacher) with an admin
|
||||
boolean orthogonal to type.
|
||||
|
||||
Two design decisions worth knowing:
|
||||
|
||||
1. Transactions are an append-only ledger. portfolios
|
||||
deliberately has no cash column — balance is derived
|
||||
by summing transactions
|
||||
(app/models/portfolio.rb:88), and it subtracts
|
||||
pending buy orders plus a pending fee so students
|
||||
can't double-spend money already committed.
|
||||
2. Orders are batch-executed, not immediate. Students
|
||||
queue buy/sell orders; OrderExecutionJob drains
|
||||
them, then TransactionFeeProcessor charges a flat
|
||||
$1/student/day regardless of order count. Prices
|
||||
update once daily from Alpha Vantage, so the price a
|
||||
student sees is the price they get.
|
||||
|
||||
Earnings: $0.20/day attended, $1 perfect-attendance
|
||||
bonus, $3 for an A / $2 for a B in reading and math,
|
||||
$2 per grade improvement over the prior quarter.
|
||||
Admin "finalizes" a gradebook, which is what actually
|
||||
mints the deposits (DistributeEarnings).
|
||||
|
||||
docs/ is unusually good for a volunteer project —
|
||||
orders-and-transactions.md and gradebook-earnings.md
|
||||
explain the why.
|
||||
|
||||
State of the code
|
||||
|
||||
I ran the suite: 675 tests, 1 failure, 2 skips, 0
|
||||
errors (~18s).
|
||||
|
||||
The one failure is not a product bug — it's a time
|
||||
bomb. test/controllers/admin/teachers_controller_test
|
||||
.rb:143 hardcodes school year "2025 - 2026", but
|
||||
Year.current_school_year returns "2026 - 2027" for
|
||||
any date after June 2026, so the classroom never
|
||||
renders. It began failing on 2026-07-01 and will keep
|
||||
failing until the literal is replaced with a
|
||||
computed current year.
|
||||
|
||||
Things I'd flag
|
||||
|
||||
Two authorization gaps — I verified both with
|
||||
throwaway integration tests, then deleted them.
|
||||
|
||||
1. IDOR on order updates
|
||||
(app/controllers/orders_controller.rb:41). set_order
|
||||
does an unscoped Order.find(params[:id]), and update
|
||||
never calls authorize. OrderPolicy#update? exists and
|
||||
is unit-tested, but nothing invokes it — only cancel
|
||||
does. Confirmed: an unrelated student changed
|
||||
another student's pending order from 1 share to 42
|
||||
and got a 302 success. Ownership can't be reassigned
|
||||
(order_params excludes user_id) and only pending
|
||||
orders qualify, but one student can still rewrite
|
||||
another's trade. One-line fix: authorize @order in
|
||||
update.
|
||||
2. Teachers aren't scoped to their own classrooms in
|
||||
StudentsController and
|
||||
ClassroomEnrollmentsController. Both use
|
||||
ensure_teacher_or_admin — any teacher — then
|
||||
Classroom.find(params[:classroom_id]) with no
|
||||
membership check. Confirmed: a teacher of classroom A
|
||||
reset the password of a student in classroom B.
|
||||
Notably
|
||||
ClassroomsController#check_classroom_eligibility does
|
||||
check membership, so this is an inconsistency rather
|
||||
than a deliberate policy.
|
||||
|
||||
Neither is remotely exploitable — both require an
|
||||
authenticated account — and the blast radius is one
|
||||
school district's students, but #2 crosses a
|
||||
classroom boundary that the app elsewhere treats as
|
||||
real.
|
||||
|
||||
Lower-priority:
|
||||
|
||||
- Admin::PortfolioTransactionsController has six
|
||||
commented-out authorize calls with TODO: FIX.
|
||||
Practically fine (BaseController gates admins), but
|
||||
the ledger doc says transactions are immutable, and
|
||||
this controller offers edit/update/destroy on them —
|
||||
that's a stated-invariant violation more than an
|
||||
authz one.
|
||||
- docs/scheduling.md has drifted from
|
||||
config/recurring.yml. The doc says OrderExecutionJob
|
||||
runs weekdays at 1 AM and auto-triggers the price
|
||||
update; actually it runs every 15 minutes and the
|
||||
price job is independently scheduled. It also
|
||||
references two docs that don't exist.
|
||||
- Admin::BaseController#apply_sorting interpolates
|
||||
params[:sort] into reorder. Rails quotes hash keys as
|
||||
identifiers so it isn't injectable, but an unknown
|
||||
column produces a 500 rather than a fallback.
|
||||
- Mid-migration duality: students link to classrooms
|
||||
via both legacy users.classroom_id and the newer
|
||||
ClassroomEnrollment. ClassroomFacade#students unions
|
||||
both. Worth finishing, since some scopes (e.g.
|
||||
- docs/scheduling.md has drifted from config/recurring.yml. The doc says OrderExecutionJob runs weekdays at 1
|
||||
AM and auto-triggers the price update; actually it runs every 15 minutes and the price job is independently
|
||||
scheduled. It also references two docs that don't exist.
|
||||
- Admin::BaseController#apply_sorting interpolates params[:sort] into reorder. Rails quotes hash keys as
|
||||
identifiers so it isn't injectable, but an unknown column produces a 500 rather than a fallback.
|
||||
- Mid-migration duality: students link to classrooms via both legacy users.classroom_id and the newer
|
||||
ClassroomEnrollment. ClassroomFacade#students unions both. Worth finishing, since some scopes (e.g.
|
||||
Classroom.order_by_student_count) still read only the legacy column.
|
||||
- Order#destroy is defined below private, so DELETE /orders/:id routes to a nonexistent action — dead code
|
||||
plus a phantom route.
|
||||
$2 per grade improvement over the prior quarter.
|
||||
Admin "finalizes" a gradebook, which is what actually
|
||||
mints the deposits (DistributeEarnings).
|
||||
|
||||
docs/ is unusually good for a volunteer project —
|
||||
orders-and-transactions.md and gradebook-earnings.md
|
||||
explain the why.
|
||||
|
||||
State of the code
|
||||
|
||||
I ran the suite: 675 tests, 1 failure, 2 skips, 0
|
||||
errors (~18s).
|
||||
|
||||
The one failure is not a product bug — it's a time
|
||||
bomb. test/controllers/admin/teachers_controller_test
|
||||
.rb:143 hardcodes school year "2025 - 2026", but
|
||||
Year.current_school_year returns "2026 - 2027" for
|
||||
any date after June 2026, so the classroom never
|
||||
renders. It began failing on 2026-07-01 and will keep
|
||||
failing until the literal is replaced with a
|
||||
computed current year.
|
||||
|
||||
Things I'd flag
|
||||
|
||||
Two authorization gaps — I verified both with
|
||||
throwaway integration tests, then deleted them.
|
||||
|
||||
1. IDOR on order updates
|
||||
(app/controllers/orders_controller.rb:41). set_order
|
||||
does an unscoped Order.find(params[:id]), and update
|
||||
never calls authorize. OrderPolicy#update? exists and
|
||||
is unit-tested, but nothing invokes it — only cancel
|
||||
does. Confirmed: an unrelated student changed
|
||||
another student's pending order from 1 share to 42
|
||||
and got a 302 success. Ownership can't be reassigned
|
||||
(order_params excludes user_id) and only pending
|
||||
orders qualify, but one student can still rewrite
|
||||
another's trade. One-line fix: authorize @order in
|
||||
update.
|
||||
2. Teachers aren't scoped to their own classrooms in
|
||||
StudentsController and
|
||||
ClassroomEnrollmentsController. Both use
|
||||
ensure_teacher_or_admin — any teacher — then
|
||||
Classroom.find(params[:classroom_id]) with no
|
||||
membership check. Confirmed: a teacher of classroom A
|
||||
reset the password of a student in classroom B.
|
||||
Notably
|
||||
ClassroomsController#check_classroom_eligibility does
|
||||
check membership, so this is an inconsistency rather
|
||||
than a deliberate policy.
|
||||
|
||||
Neither is remotely exploitable — both require an
|
||||
authenticated account — and the blast radius is one
|
||||
school district's students, but #2 crosses a
|
||||
classroom boundary that the app elsewhere treats as
|
||||
real.
|
||||
|
||||
Lower-priority:
|
||||
|
||||
- Admin::PortfolioTransactionsController has six
|
||||
commented-out authorize calls with TODO: FIX.
|
||||
Practically fine (BaseController gates admins), but
|
||||
the ledger doc says transactions are immutable, and
|
||||
this controller offers edit/update/destroy on them —
|
||||
that's a stated-invariant violation more than an
|
||||
authz one.
|
||||
- docs/scheduling.md has drifted from
|
||||
config/recurring.yml. The doc says OrderExecutionJob
|
||||
runs weekdays at 1 AM and auto-triggers the price
|
||||
update; actually it runs every 15 minutes and the
|
||||
price job is independently scheduled. It also
|
||||
references two docs that don't exist.
|
||||
- Admin::BaseController#apply_sorting interpolates
|
||||
params[:sort] into reorder. Rails quotes hash keys as
|
||||
identifiers so it isn't injectable, but an unknown
|
||||
column produces a 500 rather than a fallback.
|
||||
- Mid-migration duality: students link to classrooms
|
||||
via both legacy users.classroom_id and the newer
|
||||
ClassroomEnrollment. ClassroomFacade#students unions
|
||||
both. Worth finishing, since some scopes (e.g.
|
||||
Classroom.order_by_student_count) still read only the
|
||||
legacy column.
|
||||
- Order#destroy is defined below private, so DELETE
|
||||
/orders/:id routes to a nonexistent action — dead
|
||||
code plus a phantom route.
|
||||
- MemorablePasswordGenerator builds passwords from
|
||||
Faker::Superhero + rand(1..99) — the file's own TODO
|
||||
acknowledges this. Fine for classroom accounts, low
|
||||
entropy in principle.
|
||||
|
||||
Working tree: a 15 MB GITFOLDER.zip containing a full
|
||||
.git directory is sitting untracked in the repo
|
||||
root, and two .DS_Store files show as deleted.
|
||||
.gitignore covers .DS_Store but not the zip. Probably
|
||||
a stray artifact from someone's backup — worth
|
||||
removing before it gets committed.
|
||||
|
||||
|
||||
56
sources/02-branches.md
Normal file
56
sources/02-branches.md
Normal file
@@ -0,0 +1,56 @@
|
||||
# Branch Survey
|
||||
|
||||
**Repo:** `rubyforgood/stocks-in-the-future`
|
||||
**`main` at:** `63732df` (2026-06-30)
|
||||
**Surveyed:** 2026-08-14
|
||||
**Scope:** all 11 local branches other than `main`.
|
||||
|
||||
---
|
||||
|
||||
## Priority legend
|
||||
|
||||
| P | Meaning |
|
||||
|---|---------|
|
||||
| 1 | Merge-ready and valuable now. Zero commits behind `main`, so it fast-forwards. `main` is currently *missing* this work. |
|
||||
| 2 | High value, current, but a large review. Will rot quickly if `main` moves. |
|
||||
| 3 | Small, self-contained, cheap to land. |
|
||||
| 4 | Real unmerged work, but far behind `main` — needs a rebase or a product decision before it is worth anything. |
|
||||
| 5 | Superseded, stale, or already merged. Housekeeping: delete or consciously abandon. |
|
||||
|
||||
---
|
||||
|
||||
## Branches
|
||||
|
||||
| Priority | Name | Description |
|
||||
|---|---|---|
|
||||
| 1 | `dependabot/bundler/solid_queue-1.6.0` | Despite the name, not a single dependency bump — this is the de-facto integration branch that `main` has fallen behind. 13 unique commits spanning Jun–Aug 2026: solid_queue 1.4→1.6, **Rails 8.1.3→8.1.3.1** (patch release), csv, simplecov 0.22→1.0.3, rubocop/rubocop-rails, selenium-webdriver, and image_processing 1.14→2.0.2 — the last accompanied by libvips provisioning for staging/production (`config/deploy.rb`, CI workflows, `Dockerfile.dev`) and a new Active Storage image-processing test. Also carries the "show reset password notice once" fix (#1150) and a rotted-test repair. 19 ahead / **0 behind**, so it fast-forwards cleanly. |
|
||||
| 1 | `pr-1150` | The original home of the "show reset password notice once" fix (removes duplicate flash markup from `classrooms/show`). Now roughly 95% duplicated by the solid_queue branch above, but holds **one commit that branch lacks**: a test-setup fix creating the `teacher_classrooms` join row so the teacher actually passes `ClassroomsController#check_classroom_eligibility` (the factory only set the `belongs_to`, leaving the join table empty, so the teacher was redirected to root and the notice never rendered). Reconcile the two branches rather than merging both. 19 ahead / 0 behind. |
|
||||
| 2 | `stocksdesign` | A full UI and design-system overhaul, and by far the largest branch: 95 commits, 251 files, +14,028/−3,536. Adds `design.md` (the design system, adapted from the Ruby for Good **CASA** project and re-reconciled for this app), `design-instructions.md` (process), and `design-todo.md` (an automated audit of 117 templates flagging WCAG and design-token violations — hex colours, off-tier breakpoints, faint text, missing `alt`, removed focus outlines, `div`-as-button, `th` without `scope`). Self-hosts the Figtree variable font, unifies buttons/cards/tables/badges onto shared primitives, flattens navigation and adds a mobile drawer, converts copy to sentence case, adds `AdminDashboard`, `EarningsCalculator` and `PopulateGradeBook`, and brings a substantial new system/integration test suite. Most recently active branch (2026-08-04) and **0 behind** `main`. |
|
||||
| 3 | `increase_rate_limit` | **Name does not match content.** Adds three lines to `config/application.rb` setting `config.solid_queue.recurring_tasks_file` to `config/recurring.yml`, so the recurring-job schedule is loaded explicitly rather than relying on Solid Queue's default lookup. Nothing in the diff concerns rate limiting; the name most likely refers to the job cadence in `recurring.yml` that this change activates. 2 ahead / 268 behind, but the diff is 3 lines and trivially re-appliable. |
|
||||
| 3 | `script_updates` | Hardens `script/migrate_returning_students.rb`, the one-off production script that imports returning students' prior balances, stock holdings and quarterly grades from a spreadsheet. Replaces hardcoded 0-indexed CSV column positions (`COL = { username: 0, earnings: 6, ... }`) with **named case-sensitive headers**, and replaces the hardcoded `TARGET_CLASSROOM_ID = 1` with a required `--classroom=ID` flag. Net −22 lines but a near-total rewrite of the parsing layer. Only 38 commits behind. |
|
||||
| 3 | `ah/argument-alignment` | Pure lint. Enables the `Layout/FirstMethodArgumentLineBreak` RuboCop cop in `.rubocop.yml` and reformats the 11 files that then violate it (5 app, 7 test). 558 behind, so the reformatting would conflict, but the branch is regenerable in a minute by enabling the cop and running `rubocop -a`. Value is the decision, not the diff. |
|
||||
| 4 | `student-grade-import-to-transaction` | First pass at `BulkGradeImportService` — 271 lines of service plus 466 lines of tests, and nothing else. Imports student grades from CSV (school/classroom/quarter/math/reading/absences) so that grade data can flow into gradebook entries and, on finalisation, into earnings transactions. Complements the existing `BulkStudentImportService`, which only creates student accounts. Genuinely useful and absent from `main`, but 693 commits behind and never wired into a controller, route or admin UI. |
|
||||
| 4 | `feature/kamal-deployment` | Migrates deployment to **Kamal 2**: base/staging/production `config/deploy*.yml`, `.kamal/secrets*` files, secrets documentation, and GitHub Actions deploy workflows for both environments. Additive only (224 insertions, 0 deletions). Appears to be an abandoned alternative direction — `main` stayed on Capistrano + AWS Lightsail and has kept investing there (the P1 branch above adds libvips provisioning to `config/deploy.rb`). Needs a product/infra decision before any rebase is worthwhile. 525 behind. |
|
||||
| 5 | `feature/multiple-classroom-memberships` | **Superseded.** Lets a student belong to several classrooms simultaneously via a new `Enrollment` join model, three migrations, and updates to `Classroom`/`Student`/`ImportStudentService`. `main` has since shipped the same concept under a different name and a richer design — `ClassroomEnrollment`, with `enrolled_at`/`unenrolled_at`, a `primary` flag, `current`/`historical` scopes, and a dedicated controller. 48 ahead / 338 behind, and the abandoned dual-write approach ("maintain dual relationship") survives in `main` as the legacy `users.classroom_id` / `ClassroomEnrollment` duality. Keep only as historical context. |
|
||||
| 5 | `resolve-testing-issues-admin-v2` | **Stale — targets code that no longer exists.** Un-skips and repairs tests across the `admin_v2` namespace (base, grades, portfolio_transactions, school_years, students, teachers controllers) plus `SchoolYear` and a form builder. `main` renamed that whole namespace from `admin_v2` (`/admin-new`) to `admin` (`/admin`), so **zero** `admin_v2` paths remain — every one of the 12 files touched is gone. The underlying intent (no skipped admin tests) may still be worth redoing against the current namespace; the diff itself is unusable. 245 behind. |
|
||||
| 5 | `feature/make-check-box-lable-clickable` | **Already merged — delete.** Made the label clickable on the shared checkbox component (`a81e914`), plus a README touch-up. The branch tip is a direct ancestor of `main`: 0 commits ahead, 1,430 behind. Nothing to recover; it is pure branch-list clutter. (Note the typo in the branch name, `lable`.) |
|
||||
|
||||
---
|
||||
|
||||
## Cross-cutting observations
|
||||
|
||||
**`main` is behind its own development.** `main`'s tip is 2026-06-30, but two branches (`dependabot/bundler/solid_queue-1.6.0`, `pr-1150`) and `stocksdesign` are all **0 commits behind** with work running through early August. The most consequential item sitting unmerged is the **Rails 8.1.3 → 8.1.3.1** patch bump. Landing the P1 branches is the cheapest high-value action available.
|
||||
|
||||
**Two branches duplicate each other.** `dependabot/bundler/solid_queue-1.6.0` and `pr-1150` share six commits verbatim and a seventh in squashed form. Merging both would be redundant; the solid_queue branch is very nearly a superset, so the practical move is to merge it and cherry-pick `2783b49` from `pr-1150`.
|
||||
|
||||
**Two branch names actively mislead.** `increase_rate_limit` contains no rate-limiting code, and `dependabot/bundler/solid_queue-1.6.0` is not a lone Dependabot bump but the project's real integration branch. Anyone triaging by name alone will mis-rank both.
|
||||
|
||||
**Three branches are dead weight** (`feature/make-check-box-lable-clickable`, `resolve-testing-issues-admin-v2`, `feature/multiple-classroom-memberships`) — one already merged, two overtaken by renames or reimplementation in `main`.
|
||||
|
||||
**Related to the earlier code review:** `pr-1150`'s unique commit documents that `Classroom#teachers` (the `teacher_classrooms` join) is the real gate for classroom access, while `users.classroom_id` is not. That is the same legacy/enrollment duality flagged in the synopsis, and the same join that `StudentsController` fails to check — the branch confirms the distinction matters in practice.
|
||||
|
||||
---
|
||||
|
||||
## Method
|
||||
|
||||
For each branch: `git rev-list --count` in both directions against `main`; `git log --no-merges main..<branch>` for unique commits; `git diff --stat <merge-base> <branch>` for scope; then targeted reads of the actual diffs, and `git ls-tree`/`git merge-base --is-ancestor` checks against `main` to establish which branches had been superseded or already merged. No branch was checked out and no branch was modified.
|
||||
Reference in New Issue
Block a user