Skip to content

Dispute Penalty - Harrison individual M4 - #165

Closed
frasermuller wants to merge 1 commit into
mainfrom
Issue-#25-Dispute-penalty-(Individual-Issue-M4-Harrison)
Closed

Dispute Penalty - Harrison individual M4#165
frasermuller wants to merge 1 commit into
mainfrom
Issue-#25-Dispute-penalty-(Individual-Issue-M4-Harrison)

Conversation

@frasermuller

Copy link
Copy Markdown
Collaborator

No description provided.

@frasermuller

Copy link
Copy Markdown
Collaborator Author

Hey, I’m confused by this PR and need clarification.

I don’t understand why report_output/report.html and final_report.json were added. They look like generated test artifacts and don’t belong with a backend feature. Are they meant to be committed? What are they for?

The dispute feature still isn’t usable. The service relies on print() and input() for approval, which doesn’t fit an API workflow. It doesn’t save disputes or store user reasons, and there’s no proper admin review endpoint. Even though CI passes, the feature won’t actually work when used.

What you should actually add:

User endpoint to file a dispute:
POST /penalties/{penalty_id}/dispute
body: { "user_id": "...", "reason": "..." }
validate user owns the penalty and create a Dispute record with status "pending"
Dispute persistence and audit

Add Dispute model with fields like dispute_id, penalty_id, user_id, reason, status, reviewer_id, reviewer_comment, created_at, reviewed_at
Non-interactive admin review endpoint

POST /disputes/{dispute_id}/review or PUT /disputes/{dispute_id}
body: { "admin_id": "...", "decision": "approve" | "deny", "comment": "..." }
validate admin permissions, update dispute.status and timestamps
if approved, call resolve_penalty(penalty_id) programmatically
Remove console I/O

Delete all print()/input() from service code. Decisions must come from API calls or parameters.
Tests

Replace interactive tests with HTTP-driven tests:
create a dispute and assert status == pending
simulate admin review via the review endpoint and assert dispute status and penalty state

Security
Only the penalty owner can create a dispute
Only admins can review disputes

Please either explain why the report files are here or remove them, and update the code and tests to implement the API-driven dispute flow above. Right now this is incomplete and won’t work if implemented in our actual project. If you want, I can complete this feature, but it is meant to be your individual feature. Good luck. Let me know if you need help.

Tip: if you look at the refund system that I implemented for my individual feature, the logic should be fairly similar.

@KitheK KitheK closed this Dec 5, 2025
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.

3 participants