feat: Phase 4 - reports, sales returns, enterprise dashboard, POS improvements#17
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds a complete sales-returns flow on invoice detail (per-item quantity selection, inventory stock restoration, return history), introduces a ChangesSales Returns
Reports and Dashboard Analytics
POS Unit-Aware Quantities
UI Polish and Infrastructure
Sequence Diagram(s)sequenceDiagram
participant InvoiceDetailScreen
participant _DetailBody
participant _ProcessReturnSheet
participant ReturnsDao
participant InventoryRepository
participant invoiceReturnsProvider
InvoiceDetailScreen->>_DetailBody: canReturn=true
_DetailBody->>invoiceReturnsProvider: watch(invoiceId)
invoiceReturnsProvider-->>_DetailBody: List of SalesReturn
_DetailBody->>_ProcessReturnSheet: showModalBottomSheet
_ProcessReturnSheet->>ReturnsDao: nextReturnNumber()
ReturnsDao-->>_ProcessReturnSheet: RET-00042
_ProcessReturnSheet->>ReturnsDao: insertReturn + insertReturnItems
loop each returned item
_ProcessReturnSheet->>InventoryRepository: adjustStock(movementType: return_in)
end
_ProcessReturnSheet-->>_DetailBody: success
_DetailBody->>invoiceReturnsProvider: invalidate(invoiceId)
sequenceDiagram
participant ReportsScreen
participant PLTab
participant StockTab
participant AgingTab
participant ReportsDao
ReportsScreen->>PLTab: TabBarView render
PLTab->>ReportsDao: getDailySales(from, to)
ReportsDao-->>PLTab: List<DailySales>
ReportsScreen->>StockTab: TabBarView render
StockTab->>ReportsDao: getStockValuation()
ReportsDao-->>StockTab: List<StockValuationRow>
ReportsScreen->>AgingTab: TabBarView render
AgingTab->>ReportsDao: getDebtorAging()
ReportsDao-->>AgingTab: List<DebtorAgingRow>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
hiranyasemindi
approved these changes
Jun 16, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
lib/providers/reports_provider.dart (1)
7-17: 💤 Low valueConsider using
ref.watchinstead ofref.readfor consistency.While
ref.readworks here becausereportsDaoProvideriskeepAliveand the DAO is stateless, usingref.watchis the idiomatic Riverpod pattern and ensures proper dependency tracking if the DAO provider ever changes behavior.`@riverpod` Future<List<DailySales>> dailySales(Ref ref, DateTime from, DateTime to) => - ref.read(reportsDaoProvider).getDailySales(from, to); + ref.watch(reportsDaoProvider).getDailySales(from, to); `@riverpod` Future<List<StockValuationRow>> stockValuation(Ref ref) => - ref.read(reportsDaoProvider).getStockValuation(); + ref.watch(reportsDaoProvider).getStockValuation(); `@riverpod` Future<List<DebtorAgingRow>> debtorAging(Ref ref) => - ref.read(reportsDaoProvider).getDebtorAging(); + ref.watch(reportsDaoProvider).getDebtorAging();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/providers/reports_provider.dart` around lines 7 - 17, Replace all instances of `ref.read(reportsDaoProvider)` with `ref.watch(reportsDaoProvider)` in the three provider functions: dailySales, stockValuation, and debtorAging. This ensures consistency with idiomatic Riverpod patterns and maintains proper dependency tracking in case the DAO provider behavior changes in the future.lib/data/database/daos/reports_dao.dart (1)
152-174: 💤 Low valueN+1 query pattern in
getDebtorAging.The method executes one query per debtor to fetch the oldest unpaid invoice. For businesses with many debtors, this could degrade performance.
Consider batching with a single query using a correlated subquery or window function if Drift/SQLite supports it, or fetch all qualifying invoices grouped by customer in one query and compute the oldest in Dart.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/data/database/daos/reports_dao.dart` around lines 152 - 174, The getDebtorAging method executes an N+1 query pattern by making one database query per customer to fetch their oldest unpaid invoice. Instead of looping through each customer and querying invoices individually, refactor the method to fetch all qualifying invoices (with status 'open' or 'partial') in a single query, then group them by customerId in Dart to find the oldest invoice date for each customer. This eliminates the per-customer loop query and consolidates database access into two queries total: one for customers and one for all relevant invoices, significantly improving performance for scenarios with many debtors.lib/features/dashboard/presentation/dashboard_screen.dart (1)
698-701: 💤 Low valueUnused
totalSalesparameter.The
totalSalesparameter is passed to_PaymentMixCardbut never used. Line 716 recomputes the total frommix.valuesinstead.Either remove the unused parameter or use it directly:
class _PaymentMixCard extends StatelessWidget { - const _PaymentMixCard({required this.mix, required this.totalSales}); + const _PaymentMixCard({required this.mix}); final Map<String, double> mix; - final double totalSales;Also applies to: 716-716
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/dashboard/presentation/dashboard_screen.dart` around lines 698 - 701, The `totalSales` parameter in the `_PaymentMixCard` constructor is declared but never used in the class implementation. Remove the unused `totalSales` parameter from both the constructor definition and any call sites where `_PaymentMixCard` is instantiated, or alternatively, use the existing `totalSales` parameter directly instead of recomputing the total from `mix.values` at the location where it is currently being recalculated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/data/database/daos/returns_dao.dart`:
- Around line 12-23: The nextReturnNumber() method has a race condition where
multiple concurrent calls can read the same MAX returnNo value and generate
duplicate return numbers. Move the return number generation logic from
nextReturnNumber() into the insertReturnWithItems transaction so that reading
the current maximum and generating the next number happen atomically within a
single database transaction. Then update all callers to remove the
pre-population of returnNo in the companion object, allowing the transaction to
generate and assign the unique return number during insert.
In `@lib/features/auth/presentation/login_screen.dart`:
- Around line 51-148: The current fixed Column + Expanded layout in the body of
LoginScreen will overflow when the soft keyboard appears, hiding the submit
button and making the form unreachable. Wrap the entire body content (the Column
with Expanded and Footer children) inside a SafeArea widget to protect from
system UI overlays, and then wrap the main Column's children within a
SingleChildScrollView to enable scrolling when the keyboard is active. This
ensures the login form remains accessible and the submit button stays reachable
even on short screens or with the keyboard displayed.
In `@lib/features/invoices/presentation/invoice_detail_screen.dart`:
- Around line 610-623: The return record insertion via
returnsDao.insertReturnWithItems and the subsequent stock adjustments in the
adjustStock loop are not wrapped in a single database transaction, creating a
risk of inconsistent state if any adjustStock call fails after the return is
already inserted. Wrap both the insertReturnWithItems call and the entire loop
of adjustStock calls within a single database transaction to ensure atomicity,
so that if any stock adjustment fails, the return record insertion is also
rolled back.
- Around line 544-551: The return amount calculations in the
`_totalReturnAmount` getter incorrectly use `unitPrice * qty` to calculate
refunds, but this does not account for discounts applied to invoice items. When
an item has `discountPercent > 0`, the customer actually paid the `subtotal`
(the discounted amount), not the full unit price. Replace the line calculating
`total += widget.detail.items[i].unitPrice * qty;` with `total +=
widget.detail.items[i].subtotal;` since subtotal already represents the
customer's actual charge. Apply the same fix to the other return calculation
elsewhere in the file (around line 585) by using `e.item.subtotal` instead of
calculating from unitPrice.
In `@lib/features/pos/presentation/pos_screen.dart`:
- Around line 928-944: The _stepFor function allows fine-grained unit steps
(0.25 for kg/l, 50 for g/ml) and _formatQty properly formats quantities to three
decimals with trailing zeros trimmed, but the receipt PDF formatting in
receipt_pdf.dart (lines 264-310) only formats fractional quantities to one
decimal place, causing 0.25 to display as 0.3 in the printed receipt. Update the
receipt quantity formatting in receipt_pdf.dart to use the same precision logic
as _formatQty, formatting to three decimals and trimming trailing zeros, so that
selected quantities match what appears on the printed receipt.
In `@lib/providers/dashboard_provider.dart`:
- Around line 48-54: The mtdGrossProfit getter is summing all 30 days from the
salesTrend list without filtering for the current calendar month, which causes
it to include data from the previous month. Fix this by filtering the salesTrend
list to only include entries with dates that fall within the current month
before folding to calculate the sum, or alternatively compute the gross profit
directly from the already-fetched MTD invoice data instead of relying on the
full 30-day trend data.
In `@lib/providers/pos_provider.dart`:
- Around line 88-95: The addItem method in lib/providers/pos_provider.dart does
not validate that the qty parameter is positive before processing, allowing zero
or negative quantities to enter the cart state and affect downstream checkout
calculations and stock movements. Add a guard clause at the start of the addItem
method that rejects non-positive quantities (check if qty <= 0) and either
returns early or throws an appropriate exception to prevent invalid quantities
from entering the state model.
In `@lib/shared/widgets/stat_card.dart`:
- Around line 26-44: The decorated Container inside the InkWell widget is
blocking the ripple/splash feedback from rendering properly, weakening the tap
affordance. Replace the Container widget (which contains the BoxDecoration with
borderRadius and boxShadow) with an Ink widget instead, preserving the same
decoration property and all its child widgets. This allows the InkWell's ripple
effect to display properly while maintaining the visual styling.
---
Nitpick comments:
In `@lib/data/database/daos/reports_dao.dart`:
- Around line 152-174: The getDebtorAging method executes an N+1 query pattern
by making one database query per customer to fetch their oldest unpaid invoice.
Instead of looping through each customer and querying invoices individually,
refactor the method to fetch all qualifying invoices (with status 'open' or
'partial') in a single query, then group them by customerId in Dart to find the
oldest invoice date for each customer. This eliminates the per-customer loop
query and consolidates database access into two queries total: one for customers
and one for all relevant invoices, significantly improving performance for
scenarios with many debtors.
In `@lib/features/dashboard/presentation/dashboard_screen.dart`:
- Around line 698-701: The `totalSales` parameter in the `_PaymentMixCard`
constructor is declared but never used in the class implementation. Remove the
unused `totalSales` parameter from both the constructor definition and any call
sites where `_PaymentMixCard` is instantiated, or alternatively, use the
existing `totalSales` parameter directly instead of recomputing the total from
`mix.values` at the location where it is currently being recalculated.
In `@lib/providers/reports_provider.dart`:
- Around line 7-17: Replace all instances of `ref.read(reportsDaoProvider)` with
`ref.watch(reportsDaoProvider)` in the three provider functions: dailySales,
stockValuation, and debtorAging. This ensures consistency with idiomatic
Riverpod patterns and maintains proper dependency tracking in case the DAO
provider behavior changes in the future.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6bc12ec7-d07a-4651-bafe-f6864c3f3210
📒 Files selected for processing (17)
CHANGELOG.mdlib/core/theme/app_theme.dartlib/data/database/daos/reports_dao.dartlib/data/database/daos/returns_dao.dartlib/data/repositories/inventory_repository.dartlib/features/auth/presentation/login_screen.dartlib/features/dashboard/presentation/dashboard_screen.dartlib/features/invoices/presentation/invoice_detail_screen.dartlib/features/pos/presentation/pos_screen.dartlib/features/reports/presentation/reports_screen.dartlib/providers/dashboard_provider.dartlib/providers/database_provider.dartlib/providers/invoices_provider.dartlib/providers/pos_provider.dartlib/providers/reports_provider.dartlib/shared/widgets/stat_card.dartwindows/flutter/generated_plugins.cmake
…d overflow and polish
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/data/database/daos/returns_dao.dart (1)
15-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFilter
MAX(returnNo)to validRET-\d+values before parsing.Current logic takes the global lexicographic max, then falls back to
0on parse failure. If the max row is malformed (legacy/manual value), this can regenerateRET-00001and collide with existing records. Limit the query to strictly valid return numbers before computing the next suffix.Suggested direction
- final maxExpr = salesReturns.returnNo.max(); - final row = - await (selectOnly(salesReturns)..addColumns([maxExpr])).getSingle(); - final maxVal = row.read(maxExpr); + // Only consider return numbers that match the RET numeric format + // (e.g., RET-00001), then compute max from that filtered set. + // This avoids resetting to 1 when malformed values exist. + final row = await /* filtered query for valid RET numeric values */; int maxNumber = 0; if (maxVal != null) { - final match = RegExp(r'RET-(\d+)').firstMatch(maxVal); + final match = RegExp(r'^RET-(\d+)$').firstMatch(maxVal); if (match != null) maxNumber = int.tryParse(match.group(1)!) ?? 0; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/data/database/daos/returns_dao.dart` around lines 15 - 23, The current logic in the ReturnsDao method computes the global lexicographic MAX of the returnNo column and then attempts to parse it, but if the maximum value is malformed (doesn't match RET-\d+), parsing fails and defaults to 0, risking collision with existing records. Modify the selectOnly query that computes maxExpr to filter the returnNo column to only include values matching the RET-\d+ pattern BEFORE computing the MAX aggregation. This ensures only valid return numbers are considered when determining the next return number suffix, eliminating the risk of regenerating numbers that already exist in the database.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@lib/data/database/daos/returns_dao.dart`:
- Around line 15-23: The current logic in the ReturnsDao method computes the
global lexicographic MAX of the returnNo column and then attempts to parse it,
but if the maximum value is malformed (doesn't match RET-\d+), parsing fails and
defaults to 0, risking collision with existing records. Modify the selectOnly
query that computes maxExpr to filter the returnNo column to only include values
matching the RET-\d+ pattern BEFORE computing the MAX aggregation. This ensures
only valid return numbers are considered when determining the next return number
suffix, eliminating the risk of regenerating numbers that already exist in the
database.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c658763-44d2-4f56-8e7f-af9728070902
📒 Files selected for processing (9)
lib/data/database/daos/returns_dao.dartlib/features/auth/presentation/login_screen.dartlib/features/dashboard/presentation/dashboard_screen.dartlib/features/invoices/presentation/invoice_detail_screen.dartlib/features/pos/presentation/receipt_pdf.dartlib/providers/dashboard_provider.dartlib/providers/pos_provider.dartlib/providers/reports_provider.dartlib/shared/widgets/stat_card.dart
✅ Files skipped from review due to trivial changes (1)
- lib/providers/reports_provider.dart
🚧 Files skipped from review as they are similar to previous changes (6)
- lib/providers/pos_provider.dart
- lib/features/auth/presentation/login_screen.dart
- lib/shared/widgets/stat_card.dart
- lib/features/dashboard/presentation/dashboard_screen.dart
- lib/features/invoices/presentation/invoice_detail_screen.dart
- lib/providers/dashboard_provider.dart
hiranyasemindi
approved these changes
Jun 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 4 delivers the Reports suite, Sales Returns flow, a full enterprise dashboard redesign, POS fractional quantity support, and a series of polish fixes across the app.
Type
Changes
Reports
Sales Returns
nextReturnNumber()using MAX() aggregate for O(log n) generationinvoiceReturnsProviderfor per-invoice return watchingmovementType: return_inon each returned itemDashboard
POS
Login
DateTime.now().yearGlobal
Screenshots
UI changes cover Dashboard, Reports, POS, Invoice Detail, and Login screens.
Test plan
Related issues
Phase 4 scope.
Summary by CodeRabbit
New Features
Improvements