Skip to content

fix(billing): Update payment mode draft invoice#6343

Open
Bowrna wants to merge 5 commits into
frappe:developfrom
Bowrna:update_payment_mode_draft_invoice
Open

fix(billing): Update payment mode draft invoice#6343
Bowrna wants to merge 5 commits into
frappe:developfrom
Bowrna:update_payment_mode_draft_invoice

Conversation

@Bowrna
Copy link
Copy Markdown
Contributor

@Bowrna Bowrna commented May 6, 2026

Current code directly updates the payment mode alone leaving out relevant changes to do during update, this changes will update the team and invoice related doctype.

@Bowrna Bowrna requested review from shadrak98 and ssiyad as code owners May 6, 2026 07:03
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 6, 2026

Greptile Summary

This PR changes how payment_mode is updated when a Razorpay UPI Autopay mandate is activated or cancelled. Instead of using bare frappe.db.set_value (which bypasses document hooks), it now calls change_payment_mode / team.save() so that Team.on_update fires and update_draft_invoice_payment_mode keeps draft invoices in sync.

  • cancel(): Correctly replaces frappe.db.set_value with a full team.save(ignore_permissions=True), so validate_payment_mode and update_draft_invoice_payment_mode fire on cancellation.
  • set_default(): Removes the inline payment_mode update and moves it to the caller; this leaves set_default incomplete when called directly from the dashboard via its @dashboard_whitelist() decorator.
  • handle_razorpay_mandate_success: Calls change_payment_mode(\"UPI Autopay\") after set_default(), but these two writes are not atomic.

Confidence Score: 3/5

The cancel path is improved, but set_default and the mandate success handler have correctness gaps that can leave team payment_mode out of sync with default_razorpay_mandate.

The cancel() change correctly triggers on_update hooks. However, set_default() is a dashboard-whitelisted method that no longer updates payment_mode, and the two-step write in handle_razorpay_mandate_success has no rollback if team.save() fails.

Both files need attention: razorpay_mandate.py because set_default lost its payment_mode side-effect, and billing.py because the two-step write sequence is not atomic.

Important Files Changed

Filename Overview
press/api/billing.py Adds change_payment_mode("UPI Autopay") after mandate.set_default() to trigger team hooks and update draft invoices; introduces a split-transaction risk if team.save() throws after set_default has already committed
press/press/doctype/razorpay_mandate/razorpay_mandate.py Removes frappe.db.set_value for payment_mode from set_default and cancel; cancel is improved, but set_default loses its payment mode side-effect when called directly from the dashboard

Sequence Diagram

sequenceDiagram
    participant User
    participant API as billing.py
    participant Mandate as RazorpayMandate
    participant TeamDB as Team (DB direct)
    participant TeamDoc as Team (doc save)
    participant Invoice as Draft Invoices

    User->>API: handle_razorpay_mandate_success
    API->>Mandate: mandate.set_default()
    Mandate->>TeamDB: db.set_value(default_razorpay_mandate)
    Note over Mandate,TeamDB: payment_mode NOT set here
    API->>TeamDoc: change_payment_mode(UPI Autopay)
    TeamDoc->>TeamDoc: team.save() on_update()
    TeamDoc->>Invoice: update_draft_invoice_payment_mode()
    API-->>User: success
Loading
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
press/press/doctype/razorpay_mandate/razorpay_mandate.py:190-205
**`set_default` no longer updates `payment_mode`**

`set_default()` is decorated with `@dashboard_whitelist()`, meaning the frontend can call it directly. Before this PR, the method atomically set both `default_razorpay_mandate` and `payment_mode = "UPI Autopay"` on the Team. Now it only updates `default_razorpay_mandate`, and the `payment_mode` update only occurs in the `handle_razorpay_mandate_success` code path. If a user (or admin) explicitly calls `set_default()` from the dashboard on an active mandate while `payment_mode` is, e.g., `"Card"`, the team ends up with a `default_razorpay_mandate` pointing to a live mandate but `payment_mode` still set to `"Card"` — auto-debit will never be attempted.

### Issue 2 of 2
press/api/billing.py:834-837
**Non-atomic two-step write in mandate success handler**

`mandate.set_default()` commits `default_razorpay_mandate` directly to the DB via `frappe.db.set_value`, then `change_payment_mode("UPI Autopay")` runs `team.save()`. If `team.save()` raises (e.g., from `validate_payment_mode`), the mandate is left as default in DB but `payment_mode` is never updated — with no rollback of the first write.

Reviews (1): Last reviewed commit: "Merge branch 'develop' into update_payme..." | Re-trigger Greptile

Comment thread press/api/billing.py Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2026

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.83%. Comparing base (dd7106a) to head (495de9d).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
...press/doctype/razorpay_mandate/razorpay_mandate.py 0.00% 8 Missing ⚠️

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6343      +/-   ##
===========================================
- Coverage    49.84%   49.83%   -0.01%     
===========================================
  Files          944      944              
  Lines        78397    78401       +4     
  Branches       351      351              
===========================================
  Hits         39075    39075              
- Misses       39298    39302       +4     
  Partials        24       24              
Flag Coverage Δ
dashboard 60.65% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Bowrna Bowrna changed the title Update payment mode draft invoice fix(billing): Update payment mode draft invoice May 6, 2026
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