From a3c786572d97e123707e6ad9a418030c67919889 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Wed, 30 Oct 2024 13:18:32 +0100 Subject: [PATCH 1/4] fix(Bank Reconciliation Tool): check role perms (cherry picked from commit 6c97bc89d4b2eb5b720e7bdd071b7ced27fc2b1e) # Conflicts: # banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py --- .../bank_reconciliation_tool_beta.py | 45 ++++++++++++++++--- 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py b/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py index 0c2f309..4f65902 100644 --- a/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py +++ b/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py @@ -44,7 +44,8 @@ def get_bank_transactions( filters.append(["date", "<=", to_date]) if from_date: filters.append(["date", ">=", from_date]) - transactions = frappe.get_all( + + return frappe.get_list( "Bank Transaction", fields=[ "date", @@ -66,7 +67,6 @@ def get_bank_transactions( filters=filters, order_by=order_by, ) - return transactions @frappe.whitelist() @@ -87,6 +87,8 @@ def create_journal_entry_bts( allow_edit = sbool(allow_edit) bank_transaction = frappe.get_doc("Bank Transaction", bank_transaction_name) + bank_transaction.check_permission("read") + if bank_transaction.deposit and bank_transaction.withdrawal: frappe.throw( _( @@ -398,8 +400,10 @@ def get_linked_payments( from_reference_date: str = None, to_reference_date: str = None, ) -> list: - # get all matching payments for a bank transaction + """Get all matching payments for a bank transaction""" transaction = frappe.get_doc("Bank Transaction", bank_transaction_name) + transaction.check_permission("read") + gl_account, company = frappe.db.get_value( "Bank Account", transaction.bank_account, ["account", "company"] ) @@ -557,7 +561,13 @@ def get_matching_queries( is_withdrawal = transaction.withdrawal > 0.0 is_deposit = transaction.deposit > 0.0 +<<<<<<< HEAD if "payment_entry" in document_types: +======= + common_filters.exact_party_match = "exact_party_match" in (document_types or []) + + if "payment_entry" in document_types and frappe.has_permission("Payment Entry"): +>>>>>>> 6c97bc8 (fix(Bank Reconciliation Tool): check role perms) query = get_pe_matching_query( exact_match, account_from_to, @@ -571,7 +581,7 @@ def get_matching_queries( ) queries.append(query) - if "journal_entry" in document_types: + if "journal_entry" in document_types and frappe.has_permission("Journal Entry"): query = get_je_matching_query( exact_match, transaction, @@ -596,15 +606,28 @@ def get_matching_queries( if include_unpaid: kwargs["company"] = company for doctype, fn in invoice_queries_map.items(): +<<<<<<< HEAD if doctype != "expense_claim": kwargs["include_only_returns"] = doctype != invoice_dt else: del kwargs["include_only_returns"] +======= + if not frappe.has_permission(frappe.unscrub(doctype)): + continue + + if doctype in ["sales_invoice", "purchase_invoice"]: + kwargs.include_only_returns = doctype != invoice_dt + elif kwargs.include_only_returns is not None: + # Remove the key when doctype == "expense_claim" + del kwargs.include_only_returns + +>>>>>>> 6c97bc8 (fix(Bank Reconciliation Tool): check role perms) queries.append(fn(**kwargs)) - else: - if fn := invoice_queries_map.get(invoice_dt): + elif fn := invoice_queries_map.get(invoice_dt): + if frappe.has_permission(frappe.unscrub(invoice_dt)): queries.append(fn(**kwargs)) +<<<<<<< HEAD if "loan_disbursement" in document_types and is_withdrawal: queries.append(get_ld_matching_query(bank_account, exact_match, transaction)) @@ -613,6 +636,16 @@ def get_matching_queries( if "bank_transaction" in document_types: query = get_bt_matching_query(exact_match, transaction, exact_party_match) +======= + if "loan_disbursement" in document_types and is_withdrawal and frappe.has_permission("Loan Disbursement"): + queries.append(get_ld_matching_query(exact_match, common_filters)) + + if "loan_repayment" in document_types and is_deposit and frappe.has_permission("Loan Repayment"): + queries.append(get_lr_matching_query(exact_match, common_filters)) + + if "bank_transaction" in document_types and frappe.has_permission("Bank Transaction"): + query = get_bt_matching_query(exact_match, common_filters, transaction.name) +>>>>>>> 6c97bc8 (fix(Bank Reconciliation Tool): check role perms) queries.append(query) return queries From 502f84e0cdf9443e1013ffb1f9e25a3d95db780f Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Wed, 30 Oct 2024 13:19:12 +0100 Subject: [PATCH 2/4] refactor: filters (cherry picked from commit 437a53b0580038a5ff28d10c2e356d616a9ac846) --- .../bank_reconciliation_tool_beta.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py b/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py index 4f65902..457c4e6 100644 --- a/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py +++ b/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py @@ -35,13 +35,16 @@ def get_bank_transactions( to_date: str = None, order_by: str = "date asc", ): - # returns bank transactions for a bank account - filters = [] - filters.append(["bank_account", "=", bank_account]) - filters.append(["docstatus", "=", 1]) - filters.append(["unallocated_amount", ">", 0.0]) + """Return bank transactions for a bank account""" + filters = [ + ["bank_account", "=", bank_account], + ["docstatus", "=", 1], + ["unallocated_amount", ">", 0.001], + ] + if to_date: filters.append(["date", "<=", to_date]) + if from_date: filters.append(["date", ">=", from_date]) From 2e3706ce74ffa34fc6453e2777fc1479e07eb29a Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Wed, 30 Oct 2024 13:24:47 +0100 Subject: [PATCH 3/4] refactor: don't silently fail on perm check (cherry picked from commit 61179d9692ba62f1772ff54c81db89aecb307bd2) # Conflicts: # banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py --- .../bank_reconciliation_tool_beta.py | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py b/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py index 457c4e6..776e513 100644 --- a/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py +++ b/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py @@ -569,8 +569,13 @@ def get_matching_queries( ======= common_filters.exact_party_match = "exact_party_match" in (document_types or []) +<<<<<<< HEAD if "payment_entry" in document_types and frappe.has_permission("Payment Entry"): >>>>>>> 6c97bc8 (fix(Bank Reconciliation Tool): check role perms) +======= + if "payment_entry" in document_types: + frappe.has_permission("Payment Entry", throw=True) +>>>>>>> 61179d9 (refactor: don't silently fail on perm check) query = get_pe_matching_query( exact_match, account_from_to, @@ -584,7 +589,8 @@ def get_matching_queries( ) queries.append(query) - if "journal_entry" in document_types and frappe.has_permission("Journal Entry"): + if "journal_entry" in document_types: + frappe.has_permission("Journal Entry", throw=True) query = get_je_matching_query( exact_match, transaction, @@ -609,6 +615,7 @@ def get_matching_queries( if include_unpaid: kwargs["company"] = company for doctype, fn in invoice_queries_map.items(): +<<<<<<< HEAD <<<<<<< HEAD if doctype != "expense_claim": kwargs["include_only_returns"] = doctype != invoice_dt @@ -617,6 +624,9 @@ def get_matching_queries( ======= if not frappe.has_permission(frappe.unscrub(doctype)): continue +======= + frappe.has_permission(frappe.unscrub(doctype), throw=True) +>>>>>>> 61179d9 (refactor: don't silently fail on perm check) if doctype in ["sales_invoice", "purchase_invoice"]: kwargs.include_only_returns = doctype != invoice_dt @@ -627,9 +637,10 @@ def get_matching_queries( >>>>>>> 6c97bc8 (fix(Bank Reconciliation Tool): check role perms) queries.append(fn(**kwargs)) elif fn := invoice_queries_map.get(invoice_dt): - if frappe.has_permission(frappe.unscrub(invoice_dt)): - queries.append(fn(**kwargs)) + frappe.has_permission(frappe.unscrub(invoice_dt), throw=True) + queries.append(fn(**kwargs)) +<<<<<<< HEAD <<<<<<< HEAD if "loan_disbursement" in document_types and is_withdrawal: queries.append(get_ld_matching_query(bank_account, exact_match, transaction)) @@ -641,12 +652,18 @@ def get_matching_queries( query = get_bt_matching_query(exact_match, transaction, exact_party_match) ======= if "loan_disbursement" in document_types and is_withdrawal and frappe.has_permission("Loan Disbursement"): +======= + if "loan_disbursement" in document_types and is_withdrawal: + frappe.has_permission("Loan Disbursement", throw=True) +>>>>>>> 61179d9 (refactor: don't silently fail on perm check) queries.append(get_ld_matching_query(exact_match, common_filters)) - if "loan_repayment" in document_types and is_deposit and frappe.has_permission("Loan Repayment"): + if "loan_repayment" in document_types and is_deposit: + frappe.has_permission("Loan Repayment", throw=True) queries.append(get_lr_matching_query(exact_match, common_filters)) - if "bank_transaction" in document_types and frappe.has_permission("Bank Transaction"): + if "bank_transaction" in document_types: + frappe.has_permission("Bank Transaction", throw=True) query = get_bt_matching_query(exact_match, common_filters, transaction.name) >>>>>>> 6c97bc8 (fix(Bank Reconciliation Tool): check role perms) queries.append(query) From 496b79e9d436cc22cfaa2b699a3da610cbef6c63 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Mon, 18 Nov 2024 18:31:06 +0100 Subject: [PATCH 4/4] chore: resolve conflicts --- .../bank_reconciliation_tool_beta.py | 49 ++----------------- 1 file changed, 5 insertions(+), 44 deletions(-) diff --git a/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py b/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py index 776e513..e4b592b 100644 --- a/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py +++ b/banking/klarna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py @@ -564,18 +564,8 @@ def get_matching_queries( is_withdrawal = transaction.withdrawal > 0.0 is_deposit = transaction.deposit > 0.0 -<<<<<<< HEAD - if "payment_entry" in document_types: -======= - common_filters.exact_party_match = "exact_party_match" in (document_types or []) - -<<<<<<< HEAD - if "payment_entry" in document_types and frappe.has_permission("Payment Entry"): ->>>>>>> 6c97bc8 (fix(Bank Reconciliation Tool): check role perms) -======= if "payment_entry" in document_types: frappe.has_permission("Payment Entry", throw=True) ->>>>>>> 61179d9 (refactor: don't silently fail on perm check) query = get_pe_matching_query( exact_match, account_from_to, @@ -615,57 +605,28 @@ def get_matching_queries( if include_unpaid: kwargs["company"] = company for doctype, fn in invoice_queries_map.items(): -<<<<<<< HEAD -<<<<<<< HEAD + frappe.has_permission(frappe.unscrub(doctype), throw=True) + if doctype != "expense_claim": kwargs["include_only_returns"] = doctype != invoice_dt else: del kwargs["include_only_returns"] -======= - if not frappe.has_permission(frappe.unscrub(doctype)): - continue -======= - frappe.has_permission(frappe.unscrub(doctype), throw=True) ->>>>>>> 61179d9 (refactor: don't silently fail on perm check) - - if doctype in ["sales_invoice", "purchase_invoice"]: - kwargs.include_only_returns = doctype != invoice_dt - elif kwargs.include_only_returns is not None: - # Remove the key when doctype == "expense_claim" - del kwargs.include_only_returns - ->>>>>>> 6c97bc8 (fix(Bank Reconciliation Tool): check role perms) queries.append(fn(**kwargs)) elif fn := invoice_queries_map.get(invoice_dt): frappe.has_permission(frappe.unscrub(invoice_dt), throw=True) queries.append(fn(**kwargs)) -<<<<<<< HEAD -<<<<<<< HEAD - if "loan_disbursement" in document_types and is_withdrawal: - queries.append(get_ld_matching_query(bank_account, exact_match, transaction)) - - if "loan_repayment" in document_types and is_deposit: - queries.append(get_lr_matching_query(bank_account, exact_match, transaction)) - - if "bank_transaction" in document_types: - query = get_bt_matching_query(exact_match, transaction, exact_party_match) -======= - if "loan_disbursement" in document_types and is_withdrawal and frappe.has_permission("Loan Disbursement"): -======= if "loan_disbursement" in document_types and is_withdrawal: frappe.has_permission("Loan Disbursement", throw=True) ->>>>>>> 61179d9 (refactor: don't silently fail on perm check) - queries.append(get_ld_matching_query(exact_match, common_filters)) + queries.append(get_ld_matching_query(bank_account, exact_match, transaction)) if "loan_repayment" in document_types and is_deposit: frappe.has_permission("Loan Repayment", throw=True) - queries.append(get_lr_matching_query(exact_match, common_filters)) + queries.append(get_lr_matching_query(bank_account, exact_match, transaction)) if "bank_transaction" in document_types: frappe.has_permission("Bank Transaction", throw=True) - query = get_bt_matching_query(exact_match, common_filters, transaction.name) ->>>>>>> 6c97bc8 (fix(Bank Reconciliation Tool): check role perms) + query = get_bt_matching_query(exact_match, transaction, exact_party_match) queries.append(query) return queries