Conversation
|
| if self.filters.company: | ||
| query = query.where(si.company == self.filters.company) |
There was a problem hiding this comment.
Company Filter Changes Rows
When a company is selected, this new predicate excludes returned invoices from other companies that the previous query included. This changes the MariaDB row count and violates the repository directive that changed database queries must preserve MariaDB values and row counts, even when the narrower result appears more appropriate. This repository requirement must be satisfied before merging.
Context Used: ERPNext runs on both MariaDB and PostgreSQL from one codebase, but the PostgreSQL test job is label-gated and may not run on this PR, so review every new or changed database query (raw frappe.db.sql, frappe.qb, frappe.get_all/get_list/get_value, and ... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: erpnext/accounts/report/gross_profit/gross_profit.py
Line: 756-757
Comment:
**Company Filter Changes Rows**
When a company is selected, this new predicate excludes returned invoices from other companies that the previous query included. This changes the MariaDB row count and violates the repository directive that changed database queries must preserve MariaDB values and row counts, even when the narrower result appears more appropriate. This repository requirement must be satisfied before merging.
**Context Used:** ERPNext runs on both MariaDB and PostgreSQL from one codebase, but the PostgreSQL test job is label-gated and may not run on this PR, so review every new or changed database query (raw frappe.db.sql, frappe.qb, frappe.get_all/get_list/get_value, and ... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
fa1070e to
423aa09
Compare
Manual backport of #59198.
Adds a company filter to
get_returned_invoice_itemsso the Gross Profit report fetches sales returns only for the selected company, instead of loading every company's returns in the date range.Stacked on #59203 — merge that first.