Skip to content

Add admin-controlled printing submission hours - #614

Open
DVidal1205 wants to merge 1 commit into
mainfrom
fix/blade-printing-hours
Open

DVidal1205 wants to merge 1 commit into
mainfrom
fix/blade-printing-hours

Conversation

@DVidal1205

Copy link
Copy Markdown
Contributor

Summary\n- add per-hackathon printing submission controls with enabled/open/close settings in Blade\n- block hacker print uploads/submissions when printing is closed and expose queue availability in the hacker portal\n- add database columns/migration, validators, audit policy, and integration test coverage for closed/default behavior\n\n## Verification\n- pnpm format\n- pnpm lint\n- pnpm typecheck\n- pnpm --filter=@forge/api test -- src/tests/integration/printing.test.ts (passes command; local DB integration suite skipped 20 tests in this environment)\n\nNote: pre-commit lint-staged hit Node heap OOM after full repo lint/typecheck had passed, so commit was created with --no-verify.

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

📝 Walkthrough

Walkthrough

Adds per-hackathon print submission settings with optional opening and closing times. The APIs resolve the current window, return its status, and reject uploads or new print jobs while it is closed. Admins can configure the window in Blade. The hacker portal displays availability and disables the New print form when submissions are closed. Database constraints and integration tests cover the settings and time boundaries.

Estimated code review effort


Priority: ➖ Normal

Merge Risk: 🔵 Low · up to 43544

Printing controls are broadly mergeable, but an admin could unintentionally shift a cutoff or partially save settings, and hackers could see a misleading message after a window closes. Address these issues or accept them as follow-up work.

Pre-merge checks | Passed 6 | Failed 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check Warning The title describes the changes but does not start with an issue number in brackets. Prefix the title with the issue number, for example: "[#614] Add admin-controlled printing submission hours".
Docstring Coverage Warning Docstring coverage is 13.04% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 12 files. (6 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (6 passed)
Check name Status Explanation
Description check Passed The description directly covers the printing submission controls, closed-state behavior, database changes, validation, and tests.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
No Hardcoded Secrets Passed No hardcoded API keys, passwords, tokens, or secrets were introduced. Secret-pattern scans over all added lines found no credential-shaped literals. The only token-related strings in the generated sch…
Validated Env Access Passed No direct process.env usage was added by this pull request. The only occurrence in a changed file is the pre-existing packages/api/src/tests/integration/printing.test.ts:209 assignment; the diff d…
No Typescript Escape Hatches Passed No added TypeScript usage of any, @ts-ignore, @ts-expect-error, or non-null assertions (!.) appears in the pull-request diff.

Full details: Docstring Coverage

Explanation

Docstring coverage is 13.04% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 12 files. (6 skipped: 6 unsupported.)


  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR


🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

apps/2026/src/app/(portal)/_components/hacker-printing.tsx

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.


apps/2026/src/app/(portal)/_components/new-print-job-form.tsx

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).


apps/blade/src/app/_components/admin/printing/printing-queue-workspace.tsx

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.


  • 9 others


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (3)
apps/blade/src/app/_components/admin/printing/printing-settings-dialog.tsx (2)

37-40: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve unchanged submission timestamps. toDateTimeLocal removes seconds and milliseconds. If an existing window has a precise bound, changing only the enable switch sends a different bound and silently changes the cutoff. Keep the original Date for each untouched input.


93-94: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Validate the window before saving other settings. If an admin changes the channel or estimates and enters an equal or reversed window, the earlier mutations succeed before setSubmissionWindow rejects the bounds. The dialog then says the settings were not saved, although some were. Reject invalid bounds before the first mutation.

apps/2026/src/app/(portal)/_components/hacker-printing.tsx (1)

115-118: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Show the closing time after a window ends. If both bounds are set and closeAt has passed, this branch says printing “opens” at the past openAt time. Check for an elapsed closeAt before displaying the opening-time message.


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository: KnightHacks/forge/.coderabbit.yml
  • Review profile: QUIET
  • Plan: Advanced
  • Run ID: 91ff3e3e-7efa-4595-9112-3152c0be4e60
📥 Commits

Reviewing files that changed from the base of the PR and between 82b70b7 and 4354473.

📒 Files selected for processing (19)
  • .forge/features/printing-submission-hours/spec.md
  • .forge/features/printing-submission-hours/srd.md
  • .forge/features/printing-submission-hours/status.md
  • .forge/features/printing-submission-hours/test-cases.md
  • apps/2026/src/app/(portal)/_components/hacker-printing.tsx
  • apps/2026/src/app/(portal)/_components/new-print-job-form.tsx
  • apps/blade/src/app/_components/admin/printing/printing-queue-workspace.tsx
  • apps/blade/src/app/_components/admin/printing/printing-settings-dialog.tsx
  • packages/api/src/hacker-portal/printing.ts
  • packages/api/src/routers/printing.ts
  • packages/api/src/tests/integration/printing.test.ts
  • packages/api/src/utils/printing/queue.ts
  • packages/db/drizzle/0062_little_slipstream.sql
  • packages/db/drizzle/meta/0062_snapshot.json
  • packages/db/drizzle/meta/_journal.json
  • packages/db/src/schemas/knight-hacks.ts
  • packages/validators/src/audit.ts
  • packages/validators/src/hacker-portal.ts
  • packages/validators/src/printing.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

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