Skip to content

feat: Credit Sales Summary + per-customer Items on Credit tab - #323

Open
NotAbdelrahmanelsayed wants to merge 2 commits into
BrainWise-DEV:developfrom
NotAbdelrahmanelsayed:feature/credit-sales-summary
Open

feat: Credit Sales Summary + per-customer Items on Credit tab#323
NotAbdelrahmanelsayed wants to merge 2 commits into
BrainWise-DEV:developfrom
NotAbdelrahmanelsayed:feature/credit-sales-summary

Conversation

@NotAbdelrahmanelsayed

Copy link
Copy Markdown
Contributor

Credit Sales Summary + Per-Customer "Items on Credit" tab

Adds a fast way to answer "who owes the shop money?" and "what did this customer take on credit?" — without opening customers one by one.

What's new

  • Credit Sales button in the POS side rail → opens a summary of every company customer with a net outstanding balance, each row clickable.
  • Clicking a customer drills into the existing Customer Account statement (reusing all existing dues/payment logic).
  • A new "Items on Credit" tab in the Customer Account dialog that aggregates line items across all unpaid invoices, grouped by item, with the net balance as a headline.

Backend

  • pos_next/api/customer_dues.py: new get_credit_customers_summary(pos_profile, company) — one grouped Sales Invoice query (mirrors credit_sales.get_customer_balance math, GROUP BY customer), returns customers with net_balance > 0, their due-invoice counts, and a company-wide total. Permission-guarded + frappe.log_error wrapped.
  • pos_next/api/test_customer_dues.py: unit tests (mixed regular + return invoices; permission guard).

Frontend

  • CreditSalesSummaryDialog.vue (new), CustomerDuesDialog.vue (Statement / Items-on-Credit tabs + items aggregation), ManagementSlider.vue (rail button), posUI.js (dialog registration), POSSale.vue (wiring).
  • Full Arabic localization added to translations/ar.csv; verified end-to-end in EN and AR/RTL.

ℹ️ Screenshots below are from the live build with customer names and amounts redacted (Customer 1…N, E£ •••).


English

Credit Sales summary

summary-en

Customer Account → Statement tab

statement-en

Customer Account → Items on Credit tab

items-en

Arabic (RTL)

ملخص المبيعات الآجلة

summary-ar

كشف الحساب

statement-ar

الأصناف الآجلة

items-ar


Note on scope

This feature extends files (the dues dialog, the main POS page, the UI store, ar.csv) that already carried related in-progress local work, and the page's import graph ties them together. To keep the branch building/running as tested, it therefore also includes that related POS work rather than an artificially split, non-building diff.

🤖 Generated with Claude Code

@NotAbdelrahmanelsayed
NotAbdelrahmanelsayed force-pushed the feature/credit-sales-summary branch from 0ce3799 to dd0db84 Compare June 20, 2026 10:01
@github-actions

Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had recent activity for 21 days.

To keep this PR open, please:

  • Rebase on the latest develop branch
  • Address any pending review comments
  • Reply with an update on the PR status

If no further activity occurs within the next 14 days, this PR will be automatically closed.

@github-actions github-actions Bot added the stale label Jul 12, 2026
@engahmed1190

Copy link
Copy Markdown
Contributor

Nice Feature..

@github-actions github-actions Bot removed the stale label Jul 24, 2026
@NotAbdelrahmanelsayed

Copy link
Copy Markdown
Contributor Author

Nice Feature..

Why are my PRs never merged? I'd like to contribute to this repo — I have a lot of ideas to help improve the UX, but I'm getting discouraged after more than 5 of my PRs went unreviewed

@engahmed1190

Copy link
Copy Markdown
Contributor

@MohamedAliSmk

Copy link
Copy Markdown
Collaborator

Thanks @NotAbdelrahmanelsayed for your contribute, this is code review for your feature

Code review: dd0db844 (PR #323)

Feature shape is solid (summary list → per-customer statement/items → FIFO pay), with real reuse of get_customer_balance / partial payments. Several issues will break or mislead the pay / summary flows.

Severity Location Finding
High pos_next/api/customer_dues.py:401-407 Lump-sum Pay Due crashes. create_payment_entry() does not accept pos_opening_shift. Every pay_customer_due call raises TypeError after PaymentDialog completes.
High customer_dues.py:278-295 + CreditSalesSummaryDialog.vue API errors look like “no debtors”. get_credit_customers_summary catches all exceptions and returns empty customers. UI treats that as success (“Everyone is settled up”), not failure.
High CustomerDuesDialog.vue:133-145 + pay_customer_due Pay Due ignores return credit. Button / PaymentDialog target is total_outstanding, not net_balance. Customer with 382 due + 133 credit is asked to pay 382. Applying “Customer Credit” in PaymentDialog is also unsafe: those rows are sent to create_payment_entry as a normal MoP instead of redeem_customer_credit.
Medium CustomerDuesDialog.vue:526-548 Items on Credit overstates amounts for partly paid invoices: aggregates full line qty/amount, not the unpaid portion. Headline uses net_balance while rows can sum much higher.
Medium POSSale.vue @payment-completed After payment, only invoice history refreshes. Credit Sales summary underneath stays stale until manual refresh/reopen.
Medium CustomerDuesDialog.vue:439-444 useDialogSubmit enables Enter/Ctrl+S whenever dialog is open; it does not guard total_outstanding > 0, so Enter can open Pay Due on a settled customer.
Medium customer_dues.py:108-113 Due invoices are unbounded and each is enriched via payment history → N+1 risk for heavy debtors.
Low customer_dues.py:186-201 If both company and pos_profile are missing, query is all companies. Frontend usually passes company, but API should require one.
Low CustomerDuesDialog.vue:246 :key="item.item_code" breaks if the same item appears twice on one invoice.
Low test_customer_dues.py Tests cover summary netting + permission only; no coverage for the broken pay_customer_due path or statement behavior.

Confirmed break (Pay Due)

				pe_name = create_payment_entry(
					invoice_name=inv_name,
					amount=alloc_amount,
					mode_of_payment=mode,
					payment_account=account,
					pos_opening_shift=pos_opening_shift,
				)

create_payment_entry only accepts: invoice_name, amount, mode_of_payment, payment_account, reference_no, remarks, posting_date.

What’s good

  • Balance math matches credit_sales.get_customer_balance (positive outstanding vs return credit).
  • Batch item fetch for statements; Arabic search helper; rail wiring and translations look coherent.
  • Savepoint around FIFO allocation is the right pattern (once the call signature is fixed).

Suggested merge blockers

  1. Remove invalid pos_opening_shift= (or extend create_payment_entry and actually link PE to the shift).
  2. Re-raise or return an error from get_credit_customers_summary instead of empty success.
  3. Align Pay Due with net balance / credit redemption (or hide Apply Credit in this PaymentDialog context).
  4. Add a regression test that pay_customer_due can create allocations without TypeError.

@MostafaKadry

MostafaKadry commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Hi @NotAbdelrahmanelsayed ,

Thank you for reaching out and sharing your thoughts. We completely understand your perspective, and we truly appreciate your passion for improving on of open source projects.

We are incredibly grateful to have skilled developers like you actively participating in our open-source ecosystem. Regarding the review times, please rest assured that the delay is not a reflection of your work or its quality. As we continuously drive the project forward, our maintainers are actively balancing a high volume of core roadmap priorities. This strategic scheduling sometimes means that community PRs take a bit longer to be evaluated and integrated as we sequence our upcoming updates.

Your contributions are highly effective and deeply valued by our team. in fact, we recently merged your PR #299!

Please don't let the natural ebb and flow of the review cycle discourage you. We absolutely want you to keep your ideas and pull requests coming. We strongly encourage you to continue supporting the BrainWise's open source repositories and the broader open-source community, as collaborative efforts from developers like you are exactly what push technology forward and make a real difference in the world!

Thank you again for your patience, your understanding, and your outstanding contributions. We will review your pending PRs as soon as they align with our current integration cycle.

@MostafaKadry

Copy link
Copy Markdown
Collaborator

@NotAbdelrahmanelsayed , also please fix checks (auto test) failings

Add a "who owes the shop money" view reachable from the POS side rail,
plus a per-customer breakdown of items taken on credit.

Backend
- pos_next/api/customer_dues.py: get_credit_customers_summary() (one grouped
  Sales Invoice query, GROUP BY customer, mirrors credit_sales.get_customer_balance
  math), get_customer_due_statement(), and pay_customer_due(). Permission-guarded.
- pos_next/api/test_customer_dues.py: unit tests (returns netting, permission guard).

Frontend
- CreditSalesSummaryDialog.vue: debtor list with total-owed card and
  Arabic-aware client search; rows drill into the customer statement.
- CustomerDuesDialog.vue: Statement / Items-on-Credit tabs; items tab
  aggregates due-invoice line items by item (qty + amount).
- ManagementSlider.vue: new "Credit Sales" rail button.
- posUI.js: register the creditSalesSummary dialog (shortcut suppression).
- POSSale.vue: mount the dialogs + wire the rail action and drill-in.
- utils/searchText.js, composables/useDialogSubmit.js: helpers the dialogs use.
- translations/ar.csv: Arabic for all new strings.

Rebased onto develop so the PR contains only the credit-sales feature.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0193VupL1jxR1L3ShCCqMjfy
- pay_customer_due no longer passes pos_opening_shift to
  create_payment_entry, which doesn't accept it and raised TypeError
  on every lump-sum payment.
- get_credit_customers_summary no longer swallows exceptions into an
  empty "no debtors" response; real errors now propagate.
- Pay Due targets net_balance instead of total_outstanding, so a
  customer with return credit isn't asked to pay more than they
  actually owe.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PV3utGBMZNgC1QAiKEHWpv
@NotAbdelrahmanelsayed
NotAbdelrahmanelsayed force-pushed the feature/credit-sales-summary branch from dd0db84 to c0eee54 Compare September 8, 2026 06:33
@NotAbdelrahmanelsayed

Copy link
Copy Markdown
Contributor Author

@MohamedAliSmk pushed fixes for the three High findings:

  1. pay_customer_due no longer passes pos_opening_shift to create_payment_entry — that call takes it now, matching the accepted signature.
  2. get_credit_customers_summary no longer swallows exceptions into an empty "no debtors" response; errors propagate normally.
  3. Pay Due now targets net_balance instead of total_outstanding, so a customer with return credit isn't asked to pay more than they actually owe. (Left "Apply Customer Credit" as-is in this dialog — it's already off by default here since allowCreditSale/allowCustomerCreditPayment aren't passed to the lump-sum PaymentDialog, so that half of the finding wasn't reachable.)

Also rebased onto develop — CI is green and the branch is conflict-free. Medium/Low items (Items on Credit overstating partial invoices, stale summary after payment, N+1 on heavy debtors, etc.) are still open; happy to take a pass at those too if useful.

@MohamedAliSmk

Copy link
Copy Markdown
Collaborator

@NotAbdelrahmanelsayed Thanks for the quick fixes on the high findings — the pay_customer_due / create_payment_entry signature fix, the error propagation in get_credit_customers_summary, and switching Pay Due to net_balance all look good from the technical side.

We’ll do a business review next, and merge once business approves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants