fix: finalize_round: delegate to proper DAO methods instead of bare s…#463
fix: finalize_round: delegate to proper DAO methods instead of bare s…#463userAdityaa wants to merge 2 commits intohatnote:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes the admin finalize_round endpoint so round finalization goes through the DAO layer (instead of directly mutating rnd.status), ensuring all expected side-effects (e.g., close date, audit logging, summaries/config) are executed consistently.
Changes:
- Route round finalization through
CoordinatorDAO.finalize_ranking_round()for ranking rounds. - Route round finalization through
CoordinatorDAO.finalize_rating_round()for rating/yesno rounds, requiring athresholdinput. - Remove the unused
FINALIZED_STATUSimport fromadmin_endpoints.py.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
montage/admin_endpoints.py
Outdated
| threshold = float(request_dict['threshold']) | ||
| except (KeyError, TypeError): | ||
| raise InvalidAction('threshold is required to finalize a rating/yesno round') |
There was a problem hiding this comment.
float(request_dict['threshold']) will raise ValueError for non-numeric inputs (e.g. "abc" or "") and currently this bubbles up as a 500. Also, thresholds outside the expected [0.0, 1.0] range will trigger an AssertionError downstream in get_rating_advancing_group() (also a 500). Catch ValueError here and validate the numeric range before calling finalize_rating_round, raising InvalidAction with a 400 instead.
| threshold = float(request_dict['threshold']) | |
| except (KeyError, TypeError): | |
| raise InvalidAction('threshold is required to finalize a rating/yesno round') | |
| raw_threshold = request_dict['threshold'] | |
| except KeyError: | |
| raise InvalidAction('threshold is required to finalize a rating/yesno round') | |
| try: | |
| threshold = float(raw_threshold) | |
| except (TypeError, ValueError): | |
| raise InvalidAction('threshold must be a number between 0.0 and 1.0 to finalize a rating/yesno round') | |
| if not (0.0 <= threshold <= 1.0): | |
| raise InvalidAction('threshold must be between 0.0 and 1.0 to finalize a rating/yesno round') |
| def finalize_round(user_dao, round_id, request_dict): | ||
| coord_dao = CoordinatorDAO.from_round(user_dao, round_id) | ||
| rnd = coord_dao.get_round(round_id) | ||
| rnd.status = FINALIZED_STATUS | ||
|
|
||
| return {'status': 'success'} | ||
| if rnd.vote_method == 'ranking': | ||
| result_summary = coord_dao.finalize_ranking_round(round_id) | ||
| return {'status': 'success', | ||
| 'result_summary_id': result_summary.id} | ||
| elif rnd.vote_method in ('rating', 'yesno'): | ||
| try: | ||
| threshold = float(request_dict['threshold']) | ||
| except (KeyError, TypeError): | ||
| raise InvalidAction('threshold is required to finalize a rating/yesno round') | ||
| advance_group = coord_dao.finalize_rating_round(round_id, threshold=threshold) | ||
| return {'status': 'success', | ||
| 'advancing_count': len(advance_group), | ||
| 'threshold': threshold} | ||
| else: | ||
| raise InvalidAction('unknown vote method: %s' % rnd.vote_method) |
There was a problem hiding this comment.
This endpoint now contains core round-finalization branching logic (ranking vs rating/yesno) and is intended to ensure side-effects like close_date, audit log entries, RoundResultsSummary, and final_threshold. There are currently no integration tests exercising /admin/round/<id>/finalize; add test coverage for both vote methods to prevent regressions (including validating the response fields like result_summary_id / advancing_count).
4ef1941 to
6c9a3d4
Compare
…tatus update
Summary
Fix the
finalize_roundendpoint inadmin_endpoints.pyto ensure all required finalization actions are executed by delegating to the appropriate DAO methods instead of only updating the round status.Solution
The endpoint now properly delegates round finalization logic to the DAO layer:
finalize_rating_roundfor rating roundsfinalize_ranking_roundfor ranking roundsFixes #462
Changes
close_dateis set during finalizationRoundResultsSummaryfinal_thresholdin configFINALIZED_STATUSimportHow to test
Create a campaign and add a round
Activate the round and complete voting
Finalize the round via the endpoint
Verify:
close_dateis set in the databaseRoundResultsSummaryfinal_thresholdin config