fix: check all successful EBICS requests

[barredterra]

Summary

• Check every matching successful EBICS Request when deciding whether a request date was already covered.
• Add a regression test for multiple successful requests where only a later row might be returned first.

Test plan

• ruff check banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.py banking/klarna_kosma_integration/doctype/banking_settings/test_banking_settings.py
• ruff format --check banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.py banking/klarna_kosma_integration/doctype/banking_settings/test_banking_settings.py
• PYTHONPATH=<split worktree> bench --site frappe-16 run-tests --module banking.klarna_kosma_integration.doctype.banking_settings.test_banking_settings --test TestBankingSettings.test_successful_request_exists_checks_all_matching_requests

Greptile Summary

This PR fixes successful_request_exists to iterate over all matching successful EBICS Requests instead of only the first one returned by frappe.db.get_value, and adds a regression test that covers the ordering-dependent failure case.

• banking_settings.py: Replaces frappe.db.get_value (single-row) with frappe.get_all + a loop so every matching record is checked before concluding no prior sync covers request_date.
• test_banking_settings.py: Adds test_successful_request_exists_checks_all_matching_requests, which inserts a later-dated record first and confirms the function still finds the earlier-dated one.

Confidence Score: 3/5

The fix is directionally correct but leaves a gap: frappe.get_all() caps results at 20 by default, so any installation that has accumulated more than ~20 successful EBICS requests will see the same category of miss the PR was meant to eliminate.

The intent of the change is sound but omitting limit=0 means only the first 20 records are ever examined, reintroducing the original failure mode for production systems running daily syncs for more than a few weeks.

banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.py — the frappe.get_all() call needs limit=0.

Important Files Changed

Filename	Overview
banking/klarna_kosma_integration/doctype/banking_settings/banking_settings.py	Switches from get_value (returns 1 record) to get_all (returns up to 20 by default) to fix multi-record check — but omits limit=0, so the fix is incomplete for users with more than 20 historical requests.
banking/klarna_kosma_integration/doctype/banking_settings/test_banking_settings.py	Adds a regression test with two nested successful requests to confirm all matching records are checked; test correctly exercises the newly introduced loop.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[daily_sync_ebics] --> B{successful_request_exists?}
    B --> C[frappe.get_all EBICS Request\nfilters: ebics_user, order_type, status=Successful\n default limit=20]
    C --> D{more records?}
    D -- yes --> E[json.loads parameters]
    E --> F{start_date <= request_date <= end_date?}
    F -- yes --> G[return True\nskip sync]
    F -- no --> D
    E -- JSONDecodeError --> D
    D -- no --> H[return False\nenqueue sync]

Loading

_{Reviews (1): Last reviewed commit: "fix: check all successful EBICS requests" | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[greptile-apps]

Missing limit on frappe.get_all() — only the first 20 records are returned by default. For any EBICS user who has been syncing daily for more than ~20 days (or has multiple requests per day), the function will silently stop checking records beyond that cutoff. If the record covering request_date happens to be beyond position 20 in the result set, successful_request_exists returns False and a redundant sync is enqueued. Pass limit=0 to fetch all matching rows.

Suggested change

	existing_requests = frappe.get_all(
	"EBICS Request",
	{
	filters={
	"ebics_user": ebics_user,
	"order_type": order_type,
	"status": "Successful",
	},
	["name", "parameters"],
	as_dict=True,
	fields=["parameters"],
	)
	existing_requests = frappe.get_all(
	"EBICS Request",
	filters={
	"ebics_user": ebics_user,
	"order_type": order_type,
	"status": "Successful",
	},
	fields=["parameters"],
	limit=0,
	)