Skip to content

fix(admin): finalize round via DAO close path - #491

Closed
userAdityaa wants to merge 1 commit into
hatnote:masterfrom
userAdityaa:finalization-logic
Closed

userAdityaa wants to merge 1 commit into
hatnote:masterfrom
userAdityaa:finalization-logic

Conversation

@userAdityaa

Copy link
Copy Markdown

Description

The admin finalize round endpoint was directly mutating round status, which skipped round-close side effects. This change introduces a dedicated DAO close-round method and delegates the endpoint to it.

Verification

  • Full pytest suite run
  • New regression test included and passing
  • Manual Validation

Solution directly follows the feedback given in #463 (comment)

Closes #462

Screenshots of UI Validation

Before Finalization After Finaliation
Screenshot 2026-04-11 at 10 39 01 PM Screenshot 2026-04-11 at 10 39 40 PM

@userAdityaa

Copy link
Copy Markdown
Author

@lgelauff, would appreciate your thoughts on the improvements.

ayushshukla1807 pushed a commit to ayushshukla1807/montage that referenced this pull request Apr 12, 2026
- Moved RoundResultsSummary creation from finalize_ranking_round to finalize_campaign as requested by maintainer.
- Created generic CoordinatorDAO.finalize_round method to ensure consistent status updates, close_date injection, and audit logging.
- Updated admin_endpoints.py to delegate to the new DAO method, resolving the bypass issue.
- Integrated regression test from PR hatnote#491 to ensure technical parity and future stability.
@userAdityaa

Copy link
Copy Markdown
Author

Hey @lgelauff, this fix now directly resolves #462. I’ve updated the issue’s title and description to remove unrelated details. I’ve tested the full flow in the application, and it’s ready for review or merge.

lgelauff added a commit that referenced this pull request Oct 31, 2026
#491 dropped it because finalize_round no longer uses it, but the
follow-up for #644 uses it in finalize_campaign. The two branches merge
without a conflict and then fail with a NameError (500 on campaign
finalize). Keeping the import leaves the import block as on master.

Refs #491, #644

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lgelauff

Copy link
Copy Markdown
Collaborator

Thank you for this fix and the test. To get it merged without another round trip, I've rebased it onto current master as #683, keeping your commit and your authorship. I added a few small things on top: timezone-aware now() instead of utcnow(), the two-step import in the test (#510), a check so that only active or paused rounds can be finalized, and keeping the FINALIZED_STATUS import, which #684 needs. Closing this one in favour of #683.

Written by Claude (Claude Code) on behalf of @lgelauff.

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.

[Bug]: finalize_round endpoint bypasses DAO finalization logic (missing close_date, audit log)

2 participants