Skip to content

Fix ten issues from a full code review - #4

Merged
Loyale merged 1 commit into
mainfrom
claude/pensive-rubin-9orytb
Oct 16, 2026
Merged

Loyale merged 1 commit into
mainfrom
claude/pensive-rubin-9orytb

Conversation

@Loyale

@Loyale Loyale commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Fixes all ten findings from a full review of main. Three of them came from #3: the reporting schedule, the notice date and the funding totals. No migrations, and no changes to stored records.

Reports and awards

  • Current & Pending PD/PI. The report now names the grant's lead: the contact PI, else the PI/MPI on the team, else the Lab PI when "My role" is PI or MPI. Before, it listed whoever the report was run for, so a postdoc's report named the postdoc as PD/PI.
  • Reporting schedule for ongoing awards. The award form now pre-ticks the reporting schedule for any active award that doesn't have one, so an ongoing grant entered as Awarded gets its RPPR and close-out reminders. Closed awards still aren't offered it.
  • Notice date. A new award's notice date is its decision date when one is recorded. An award entered late through the status dialog now lands in the right year in Funding awarded.
  • Funding totals. Funding awarded and Total awarded count only applications marked Awarded, matching the success rate. Money no longer shows up for an application moved to Not funded that still has an award record.

Crashes and validation

  • Leap day. The dashboard no longer fails on Feb 29; the 5-year window now uses relativedelta.
  • Tasks on closed applications. Tasks on Withdrawn or Not pursued applications can be edited again. Those applications still aren't offered when creating a new task.
  • CSV import. Problems are now reported row by row instead of failing the whole import with a server error. That covers values beyond the header, numbers too large for their column, NaN or Infinity, and probabilities outside 0–100. Extra decimal places are still accepted and rounded, as before.
  • Long tags. Tags and critique themes longer than 60 characters are looked up by their stored (truncated) name, so reusing one no longer hits the unique constraint.
  • Redirect. After claiming the Lab PI role, the redirect only follows a same-site next URL, via safe_next.

Testing

  • New tests. 19 new tests across test_analytics, test_import (new), test_people and test_views. 15 of them fail on the previous code, covering all ten findings; the other 4 guard behavior that must not change.
  • Full suite. 136 tests pass on PostgreSQL 16. check --deploy and makemigrations --check are clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SAPwFhRyU83ey74tK1cYbJ


Generated by Claude Code

Reports and awards
- Current & Pending names the grant's PD/PI (contact PI, else the PI/MPI on
  the team, else the Lab PI when "My role" is PI or MPI). It no longer lists
  whichever person the report is for, which put postdocs down as PD/PI.
- The award form offers the reporting schedule to any active award that has
  none yet, so an ongoing grant entered as Awarded gets its RPPR and
  close-out reminders. Closed awards still aren't offered it.
- A new award's notice date is its decision date when one is recorded, so an
  award entered late through the status dialog lands in the right year.
- Funding awarded and Total awarded count only applications marked Awarded,
  matching the success rate, even if an award record lingers elsewhere.

Crashes and validation
- The dashboard no longer fails on Feb 29 (5-year window via relativedelta).
- Tasks on Withdrawn or Not pursued applications stay editable; those
  applications are still not offered for new tasks.
- CSV import reports, row by row, extra values beyond the header, numbers too
  large for their column, non-finite numbers, and probabilities outside 0-100,
  instead of failing the whole import with a server error.
- Tags and critique themes longer than 60 characters are matched by their
  stored name, so reusing one no longer hits the unique constraint.
- Claiming the Lab PI role only redirects to a same-site "next" URL.

Each fix has tests that fail without it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SAPwFhRyU83ey74tK1cYbJ
@Loyale
Loyale merged commit d67ee15 into main Oct 16, 2026
4 checks passed
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.

2 participants