feat: Connector-wise config the settings. Moved the India Banking Settings to the Connector Level.#108
feat: Connector-wise config the settings. Moved the India Banking Settings to the Connector Level.#108l0gesh29 wants to merge 5 commits intoversion-16-devfrom
Conversation
Bhavan23
left a comment
There was a problem hiding this comment.
Bank Payment Request is no longer required.
There was a problem hiding this comment.
Pull request overview
Moves India Banking configuration from a singleton settings DocType to a connector-specific DocType, updates client/server integrations to use the renamed connector, and removes the legacy Bank Payment Request/Bank Connector doctypes.
Changes:
- Introduces the new India Banking Connector DocType (connector-wise configuration surface).
- Updates scheduled tasks and client calls to use the new connector controller path.
- Removes legacy Bank Connector and Bank Payment Request doctypes and trims India Banking Settings fields accordingly.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| india_banking/tasks.py | Updates cron logic to iterate connectors and adjusts payment status query logic. |
| india_banking/public/js/payment_order.js | Points Payment Order actions to the renamed connector backend methods. |
| india_banking/public/js/bank_account.js | Points Bank Account actions to the renamed connector backend methods. |
| india_banking/overrides/payment_request/payment_request.py | Adjusts Payment Request override behavior (incl. TDS calculation logic). |
| india_banking/india_banking/doctype/india_banking_settings/india_banking_settings.json | Removes many settings fields to reflect migration to connector-level config. |
| india_banking/india_banking/doctype/india_banking_connector/india_banking_connector.py | Renames controller class, adds onload state, and updates connector DocType references. |
| india_banking/india_banking/doctype/india_banking_connector/india_banking_connector.json | Adds the new connector DocType schema, including moved settings fields. |
| india_banking/india_banking/doctype/india_banking_connector/india_banking_connector.js | Updates client script to bind to “India Banking Connector”. |
| india_banking/india_banking/doctype/india_banking_connector/test_india_banking_connector.py | Adds a new (currently empty) integration test scaffold. |
| india_banking/india_banking/doctype/bank_payment_request/test_bank_payment_request.py | Removes Bank Payment Request tests (doctype removed). |
| india_banking/india_banking/doctype/bank_payment_request/bank_payment_request_list.js | Removes Bank Payment Request list view script (doctype removed). |
| india_banking/india_banking/doctype/bank_payment_request/bank_payment_request.py | Removes Bank Payment Request server logic (doctype removed). |
| india_banking/india_banking/doctype/bank_payment_request/bank_payment_request.json | Removes Bank Payment Request schema (doctype removed). |
| india_banking/india_banking/doctype/bank_payment_request/bank_payment_request.js | Removes Bank Payment Request form script (doctype removed). |
| india_banking/india_banking/doctype/bank_connector/test_bank_connector.py | Removes Bank Connector tests (doctype removed/renamed). |
| india_banking/india_banking/doctype/bank_connector/bank_connector.json | Removes Bank Connector schema (doctype removed/renamed). |
Comments suppressed due to low confidence (2)
india_banking/india_banking/doctype/india_banking_connector/india_banking_connector.py:551
_notify_partystill reads notification config fromIndia Banking Settings, but this PR moves notification settings to the connector (and the Settings DocType JSON no longer containsnotify_party/payment_notification). This will prevent notifications (and may error if the fields are missing). Use the current connector’snotify_party/payment_notificationfields instead (or provide a backward-compatible fallback).
india_banking/india_banking/doctype/india_banking_connector/india_banking_connector.py:748- The error message still says “Bank Connector is not initialized” even though the DocType was renamed to “India Banking Connector”. Updating the message will make troubleshooting clearer and consistent with the new connector naming.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| frappe.db.get_value("India Banking Connector", connector, "status_check") | ||
| == "Every 20 Minutes" | ||
| ): | ||
| update_payment_date_as_posting_date() | ||
| update_payment_status() |
There was a problem hiding this comment.
India Banking Connector DocType defines the field as status_check_at, but this job checks status_check. As a result, the condition will never match and the status cron won’t run. Also, these calls don’t pass the connector context, so with multiple connectors enabled you can enqueue duplicate updates and you can’t respect connector-specific flags—pass the connector through and scope updates to that connector’s bank_account/company.
| frappe.db.get_value("India Banking Connector", connector, "status_check") | ||
| == "Every Day at Midnight" | ||
| ): | ||
| update_payment_date_as_posting_date() | ||
| update_payment_status() |
There was a problem hiding this comment.
India Banking Connector DocType field is status_check_at, but this check uses status_check, so the midnight schedule won’t run. Fix the fieldname and ensure updates are scoped per connector (otherwise multiple connectors can enqueue the same payment orders multiple times).
| def update_payment_status(check_processed_payments=False, connector=None): | ||
| if connector and frappe.db.get_value( | ||
| "India Banking Connector", connector, "auto_update_payment_status" | ||
| ): | ||
| update_payment_date_as_posting_date() | ||
| update_payment_status() | ||
|
|
||
|
|
||
| def update_payment_status(check_processed_payments=False): | ||
| if not frappe.get_single("India Banking Settings").update_payment_status: | ||
| return |
There was a problem hiding this comment.
The connector flag check is inverted: update_payment_status currently returns early when auto_update_payment_status is truthy, which prevents updates when auto-update is enabled. This should return early when the flag is disabled (or when the connector is missing), and callers should consistently pass connector so the toggle is actually honored.
| @@ -292,11 +288,6 @@ def calculate_pr_tds(self, amount): | |||
| doc.base_tax_withholding_net_total = amount | |||
| doc.tax_withholding_net_total = amount | |||
| doc.taxes = [] | |||
There was a problem hiding this comment.
calculate_pr_tds no longer returns a numeric TDS amount. Since validate() uses its return value to set taxes_deducted and compute grand_total, this will produce None and can break arithmetic. Re-introduce the TDS calculation (or explicitly return 0) and keep the return type consistent.
| doc.taxes = [] | |
| doc.taxes = [] | |
| # Ensure this method returns a numeric TDS amount so callers can safely use it in arithmetic. | |
| # This override currently does not compute any TDS, so we explicitly return 0.0. | |
| return flt(0) |
| frappe.db.get_value("India Banking Connector", connector, "status_check") | ||
| == "Every Hour" | ||
| ): | ||
| update_payment_date_as_posting_date() | ||
| update_payment_status() |
There was a problem hiding this comment.
India Banking Connector uses status_check_at (per the new DocType), but this code reads status_check, so the hourly schedule will never trigger for any connector. Update the fieldname and ensure the subsequent update functions are executed for the specific connector only (to avoid duplicate enqueues across connectors).
| orders = ( | ||
| frappe.qb.from_(PaymentOrderSummary) | ||
| frappe.qb.from_(PaymentOrder) | ||
| .join(PaymentOrderSummary) | ||
| .on(PaymentOrder.name == PaymentOrderSummary.parent) | ||
| .select(PaymentOrderSummary.parent) | ||
| .where( |
There was a problem hiding this comment.
This query selects pending/initiated payment orders across the entire site. If update_payment_status is intended to be connector-specific (as implied by the new connector parameter and connector-wise settings), you need to filter by the connector’s company/bank_account (e.g., Payment Order.company and Payment Order.company_bank_account) so a connector cron run doesn’t enqueue updates for unrelated payment orders.
| "field_order": [ | ||
| "basic_configuration_tab", | ||
| "basic_configuration_section", | ||
| "summarise_payment_based_on", | ||
| "use_payment_order_date_as_payment_entry_date", | ||
| "allow_future_date_payment_order", |
There was a problem hiding this comment.
This settings DocType JSON removes multiple fields that are still referenced elsewhere in the app (e.g., notify_party, payment_notification, retry_interval_minutes, auto_post_payments, retry_period, update_posting_date_as_payment_date, doctype_naming_series). For example, india_banking/india_banking/doctype/india_banking_settings/india_banking_settings.py accesses these at lines 19, 42, 51, 57, and 80, and india_banking/tasks.py and india_banking_connector.py also read removed fields. Either keep these fields in Settings (and migrate later), or update the codepaths to read from India Banking Connector and remove the stale references to avoid runtime AttributeError/meta issues.
2304f16 to
a6112a6
Compare
Bank Connector -> India Bank Connector
Connector-wise config the settings. Moved the India Banking Settings to the Connector Level.
Remove Bank Payment Request
Home Page for India Banking Flow