fix: fix fail test cases

[karm1000]

No description provided.

[greptile-apps]

Confidence Score: 5/5

Safe to merge — all changes are targeted test fixes and a minor defensive guard, with no logic regressions.

Both P0/P1 concerns from the prior review round are resolved: the invalid message= keyword is now msg= (valid against Frappe's throw(msg, exc, title) signature), and the file mock is correctly wrapped in a list. The DuplicateEntryError assertion matches what validate_po_no explicitly raises. No remaining findings rise above P2.

No files require special attention.

Important Files Changed

Filename	Overview
transaction_parser/transaction_parser/controllers/transaction.py	Adds empty-files guard with correct frappe.throw(msg=...) call (addressing the prior message= bug), adds i18n import, and simplifies _attach_file() since files is now always a list.
transaction_parser/tests/test_sales_order.py	Fixes failing tests: wraps file mock in a list to match the API contract, tightens spec with Mock(spec=File), and corrects exception assertion from ValidationError to DuplicateEntryError to match what validate_po_no actually throws.

_{Reviews (4): Last reviewed commit: "fix: update error message for missing fi..." | Re-trigger Greptile}

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR changes Transaction.generate to require a list for its files parameter (type-annotated as list) and removes runtime normalization of single-file inputs. The method now raises an error if files is falsy and directly assigns/iterates over the provided list. Tests were updated to mock an uploaded/parsed File as a list (files_mock = [file_mock]) and to call controller.generate(self.files_mock). A test expectation was modified to expect frappe.DuplicateEntryError instead of frappe.ValidationError.

🚥 Pre-merge checks | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	No description was provided by the author, making it impossible to assess whether it relates to the changeset.	Add a meaningful description explaining the changes to file validation, error types, and test updates to help reviewers understand the fix.
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'fix: fix fail test cases' is vague and repetitive, using generic terms that don't clearly convey the main change despite being related to the changeset.	Use a more descriptive title that explains the specific fix, such as 'fix: require files list in transaction generation and update validation errors'.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2b573786-30e6-4d5f-9592-1051cda720f4

📥 Commits

Reviewing files that changed from the base of the PR and between fbb96b6 and 73c19f8.

📒 Files selected for processing (2)

• transaction_parser/tests/test_sales_order.py
• transaction_parser/transaction_parser/controllers/transaction.py

[Abdeali099]

@greptileai

[Abdeali099]

@greptileai

[Abdeali099]

@karm1000

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 321454c8-35ec-41e3-8d80-cc47d3b57dab

📥 Commits

Reviewing files that changed from the base of the PR and between 73c19f8 and 187ed53.

📒 Files selected for processing (2)

• transaction_parser/tests/test_sales_order.py
• transaction_parser/transaction_parser/controllers/transaction.py