feat: Phase 3 - Quick Sales, GRN, Petty Cash, UI consistency - #4
Merged
Conversation
- PosState gets billDiscountPct, discountAmount, updated total - setBillDiscount() and setLineDiscount() on PosNotifier - Cart items show long-press sheet for per-item % discount - Tap 'Add Discount' in bottom panel for bill-level % - discountAmount saved on invoice at checkout - Receipt PDF shows Subtotal + Discount rows when discount > 0
iamvirul
disabled auto-merge
June 15, 2026 18:30
hiranyasemindi
approved these changes
Jun 15, 2026
Contributor
|
✅ Action performedReview finished.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds four new features—GRN (goods receipt note), Quick Sales (no-invoice sales), POS line/bill discounts, and Petty Cash receipt photos—alongside a shared filter bar widget library, sidebar navigation grouping with SVG branding, a sales-returns database v2 schema with migration, global input UI density and em-dash cleanup, and complete repository scaffolding (issue templates, CI, licensing, documentation). ChangesBMS Feature Release
Sequence DiagramssequenceDiagram
participant GrnScreen
participant GrnNotifier
participant SuppliersDao
participant InventoryDao
participant AuditLogDao
GrnScreen->>GrnNotifier: confirm()
GrnNotifier->>SuppliersDao: nextGrnNumber()
SuppliersDao-->>GrnNotifier: GRN-00001
GrnNotifier->>SuppliersDao: insertPurchase(entry)
GrnNotifier->>SuppliersDao: insertPurchaseItems(items)
loop each cart item
GrnNotifier->>InventoryDao: upsertStock(productId, qty)
GrnNotifier->>InventoryDao: insertStockMovement(in)
GrnNotifier->>InventoryDao: updateCostPrice(productId)
end
GrnNotifier->>SuppliersDao: updateSupplierBalance()
GrnNotifier->>AuditLogDao: insert(grn_event)
GrnNotifier-->>GrnScreen: success with lastGrnNo
sequenceDiagram
participant QuickSalesScreen
participant QuickSaleActions
participant InvoicesDao
participant InventoryDao
participant AuditLogDao
QuickSalesScreen->>QuickSaleActions: sell(product, qty, price, notes)
QuickSaleActions->>InvoicesDao: insertNoInvoiceSale(entry)
QuickSaleActions->>InventoryDao: getCurrentStock(productId)
QuickSaleActions->>InventoryDao: upsertStock(productId, clampedQty)
QuickSaleActions->>InventoryDao: insertStockMovement(out, saleId)
QuickSaleActions->>AuditLogDao: insert(quick_sale_event)
QuickSaleActions-->>QuickSalesScreen: success or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![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: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
lib/features/settings/presentation/settings_screen.dart (3)
13-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused import.
The
audit_log_dao.dartimport is unused and flagged by the analyzer.🧹 Proposed fix
-import '../../../data/database/daos/audit_log_dao.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/features/settings/presentation/settings_screen.dart` at line 13, Remove the unused import statement for audit_log_dao.dart from the file. This import is not referenced anywhere in the settings_screen.dart file and is causing an analyzer warning, so it should be deleted to keep the imports clean and improve code quality.Source: Pipeline failures
140-140:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit type argument to
showDialog.The analyzer requires explicit type arguments to avoid inference failures.
🔧 Proposed fix
- showDialog( + showDialog<void>(🤖 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/settings/presentation/settings_screen.dart` at line 140, The showDialog function call at line 140 in settings_screen.dart is missing explicit type arguments, which causes analyzer inference failures. Add an explicit type argument to the showDialog call by specifying the return type in angle brackets, such as showDialog<T>() where T represents the expected return type of the dialog (typically void, bool, or another appropriate type based on what the dialog should return).Source: Pipeline failures
96-96:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit type argument to
MaterialPageRoute.The analyzer requires explicit type arguments to avoid inference failures.
🔧 Proposed fix
- MaterialPageRoute(builder: (_) => const _AuditLogScreen()), + MaterialPageRoute<void>(builder: (_) => const _AuditLogScreen()),🤖 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/settings/presentation/settings_screen.dart` at line 96, The MaterialPageRoute constructor in the navigation call is missing an explicit type argument, which the analyzer requires to avoid inference failures. Add an explicit type argument to MaterialPageRoute, such as MaterialPageRoute<void>, to specify the return type of the route. This applies to the MaterialPageRoute that wraps the _AuditLogScreen widget.Source: Pipeline failures
lib/features/invoices/presentation/invoice_pdf.dart (1)
230-230:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused
colWidthsvariable.Line 230 defines
colWidthsbut the columnWidths map (lines 258–265) uses hardcoded FixedColumnWidth/FlexColumnWidth values instead. This unused variable triggers a pipeline warning.🗑️ Proposed fix
) { const colWidths = [30.0, null, 40.0, 60.0, 50.0, 65.0]; - pw.Widget cell(Remove line 230 entirely.
🤖 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/invoices/presentation/invoice_pdf.dart` at line 230, The colWidths constant defined in the invoice_pdf.dart file is not being used anywhere since the columnWidths map uses hardcoded FixedColumnWidth and FlexColumnWidth values instead. Remove the unused colWidths variable declaration entirely to eliminate the pipeline warning.lib/features/petty_cash/presentation/petty_cash_screen.dart (1)
142-152:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove dead
_statusColor/_statusBghelpers to clear analyzer warnings.Line 142 and Line 148 define unused methods already flagged by CI analyze output.
Suggested fix
- Color _statusColor(String status) => switch (status) { - 'approved' => AppColors.success, - 'rejected' => AppColors.error, - _ => AppColors.warning, - }; - - Color _statusBg(String status) => switch (status) { - 'approved' => AppColors.successLight, - 'rejected' => AppColors.errorLight, - _ => AppColors.warningLight, - };🤖 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/petty_cash/presentation/petty_cash_screen.dart` around lines 142 - 152, Remove the two unused helper methods `_statusColor` and `_statusBg` from the petty cash screen class, as they are not being called anywhere in the codebase and are causing analyzer warnings. Simply delete both method definitions entirely to clean up the dead code and resolve the CI analyzer output warnings.Source: Pipeline failures
lib/providers/petty_cash_provider.dart (2)
52-68:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce petty-cash amount/type invariants before insert.
addEntrypersistsamountandtypeas-is. Negative/zero/non-finite amounts (or unexpectedtype) can be stored and corrupt balance math downstream. Validate in the action layer before DAO writes.Suggested fix
Future<void> addEntry({ required String description, required double amount, required String type, required String category, String? receiptPhotoPath, }) async { + if (!amount.isFinite || amount <= 0) { + throw ArgumentError.value(amount, 'amount', 'Must be a positive number'); + } + if (type != 'in' && type != 'out') { + throw ArgumentError.value(type, 'type', "Must be 'in' or 'out'"); + } final id = _uuid.v7();🤖 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/petty_cash_provider.dart` around lines 52 - 68, In the addEntry method, add validation logic before the DAO insert call to enforce invariants on the amount and type parameters. Validate that amount is positive and finite (not negative, zero, or NaN/infinity), and validate that type contains an expected/valid value. If validation fails, throw an appropriate exception to prevent corrupt data from being persisted to the database.
60-109:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefresh
pettyCashEntriesProviderafter successful mutations.
addEntry/approve/rejectwrite data but never invalidatepettyCashEntriesProvider, so the current screen can keep stale entries/status until another dependency changes.Suggested fix
class PettyCashActions { PettyCashActions(this._ref); final Ref _ref; final _uuid = const Uuid(); + void _refreshEntries() => _ref.invalidate(pettyCashEntriesProvider); @@ await _ref.read(pettyCashDaoProvider).insert(PettyCashCompanion.insert( @@ )); + _refreshEntries(); @@ Future<void> approve(String id) async { await _ref.read(pettyCashDaoProvider).approve(id, _userId); + _refreshEntries(); @@ Future<void> reject(String id) async { await _ref.read(pettyCashDaoProvider).reject(id); + _refreshEntries();🤖 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/petty_cash_provider.dart` around lines 60 - 109, The addEntry, approve, and reject methods in PettyCashProvider perform mutations to the database and log audit events, but they do not invalidate the pettyCashEntriesProvider cache afterward. This causes the UI to display stale data until another dependency triggers a refresh. After the auditLogDaoProvider.log call completes in each of the three methods (addEntry, approve, and reject), add a call to invalidate the pettyCashEntriesProvider by using _ref.invalidate(pettyCashEntriesProvider) to ensure the cached entries are refreshed and the UI displays the latest data.lib/features/pos/presentation/pos_screen.dart (1)
264-264:⚠️ Potential issue | 🟡 MinorAdd explicit generic type arguments for dialog APIs to clear analyzer warnings.
The
showDialogandshowModalBottomSheetcalls at lines 264, 274, and 331 are missing explicit generic type arguments. Add<void>to each call since their return values are not used. Note that line 806 already includes the type argument, creating inconsistency.Suggested patch
- showDialog( + showDialog<void>( context: context, builder: (_) => const _CustomerSearchDialog(), ); - showModalBottomSheet( + showModalBottomSheet<void>( context: context, isScrollControlled: true, builder: (ctx) => Padding( ... ), ); - showModalBottomSheet( + showModalBottomSheet<void>( context: context, isScrollControlled: true, builder: (ctx) => Padding( ... ), );🤖 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/pos/presentation/pos_screen.dart` at line 264, Add explicit generic type argument `<void>` to the `showDialog` and `showModalBottomSheet` calls that are missing them. Specifically, modify the calls at line 264, line 274, and line 331 in the pos_screen.dart file to include `<void>` as the generic type argument (for example, change `showDialog(` to `showDialog<void>(`). This will make these calls consistent with the existing implementation at line 806 which already includes the type argument, and will resolve analyzer warnings since the return values of these calls are not used.Source: Pipeline failures
🧹 Nitpick comments (5)
README.md (2)
61-91: 💤 Low valueAdd language identifier to Project Structure code block.
Line 61 defines a fenced code block without a language identifier. For consistency and proper syntax highlighting, specify a language (e.g.,
textortree).
[minor_issue]✏️ Proposed fix
## Project Structure -``` +```text lib/🤖 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 `@README.md` around lines 61 - 91, The project structure code block in the README starting with the lib/ directory listing is missing a language identifier on the opening fence. Add a language identifier (such as `text` or `tree`) immediately after the opening triple backticks to enable proper syntax highlighting and maintain consistency with other code blocks in the document.Source: Linters/SAST tools
147-154: 💤 Low valueAdd language identifier to web database clearing code block.
Line 151 defines a fenced code block without a language identifier. For markdown consistency, specify a language (e.g.,
textor leave it blank if intended as plain output).
[minor_issue]✏️ Proposed fix
The web build uses IndexedDB. After a schema change, clear the old database: -``` +```text Chrome DevTools -> Application -> IndexedDB -> delete bms_local -> refresh -``` +```text🤖 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 `@README.md` around lines 147 - 154, The fenced code block in the README.md section "Clearing the web database" (the block containing the Chrome DevTools instructions) is missing a language identifier. Add a language identifier such as "text" to the opening triple backticks (change ``` to ```text) to ensure markdown consistency and proper syntax highlighting compliance.Source: Linters/SAST tools
lib/shared/widgets/sidebar_nav.dart (1)
133-133: ⚡ Quick winExtract the repeated narrow-width threshold to a shared constant.
The
< 120threshold is duplicated across_Header,_NavTile, and_UserFooter. Extracting it to a top-level constant (e.g.,_kNarrowThreshold = 120.0) improves maintainability.♻️ Proposed refactor
Add the constant near the other layout constants:
const double _kExpandedWidth = 224; const double _kCollapsedWidth = 56; const Duration _kAnimDuration = Duration(milliseconds: 200); +const double _kNarrowThreshold = 120;Then replace each occurrence:
- final narrow = constraints.maxWidth < 120; + final narrow = constraints.maxWidth < _kNarrowThreshold;Also applies to: 214-214, 290-290
🤖 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/shared/widgets/sidebar_nav.dart` at line 133, Extract the magic number 120 that appears in the narrow-width threshold comparisons across the file into a top-level constant named _kNarrowThreshold with a value of 120.0. Then replace all three occurrences of the hardcoded comparison (< 120) with comparisons using this constant in the _Header widget at line 133, the _NavTile widget at line 214, and the _UserFooter widget at line 290 to improve maintainability and reduce duplication.lib/features/inventory/presentation/inventory_screen.dart (1)
28-28: ⚡ Quick winAdd explicit type arguments to all
showModalBottomSheetcalls to silence analyzer warnings.All three screen files trigger Flutter analyzer
inference_failure_on_function_invocationwarnings onshowModalBottomSheetcalls. Adding an explicit<void>type argument resolves these.
lib/features/inventory/presentation/inventory_screen.dart#L28: Add<void>type argumentlib/features/suppliers/presentation/suppliers_screen.dart#L14,L85,L158: Add<void>type argument to all three callslib/features/users/presentation/users_screen.dart#L64,L145,L154,L235: Add<void>type argument to all four calls🤖 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/inventory/presentation/inventory_screen.dart` at line 28, Add explicit `<void>` type arguments to all `showModalBottomSheet` calls to resolve Flutter analyzer inference warnings. In lib/features/inventory/presentation/inventory_screen.dart at line 28, change `showModalBottomSheet(` to `showModalBottomSheet<void>(`. In lib/features/suppliers/presentation/suppliers_screen.dart, add `<void>` type argument to the `showModalBottomSheet` calls at lines 14, 85, and 158. In lib/features/users/presentation/users_screen.dart, add `<void>` type argument to the `showModalBottomSheet` calls at lines 64, 145, 154, and 235. Each change follows the same pattern: insert `<void>` immediately after the method name `showModalBottomSheet`.Source: Pipeline failures
lib/data/database/tables/returns_table.dart (1)
7-8: ⚡ Quick winIndex lookup keys used by returns queries.
invoiceIdandreturnIdare hot filter paths in the DAO; without indexes, these queries degrade to full scans as return history grows.Also applies to: 24-24
🤖 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/tables/returns_table.dart` around lines 7 - 8, Add indexes to the columns used as hot filter paths in DAO queries to prevent full table scans as data grows. In the ReturnsTable class, add indexing to the invoiceId column (line 7) and the column at line 24 to enable efficient lookups. Use the appropriate indexing method provided by the drift library to mark these columns as indexed in the table schema definition.
🤖 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 @.github/workflows/codeql.yml:
- Line 15: The actions/checkout action on Line 15 is pinned to a version tag
(v4) instead of a specific commit hash, which creates a supply-chain attack
vulnerability since version tags can be reassigned. Replace the version tag
reference with a specific commit hash from the latest stable release in the
actions/checkout repository. Additionally, add the parameter
persist-credentials: false to the actions/checkout action to explicitly disable
credential persistence, which is a security best practice. Use the latest stable
commit hash available from the actions/checkout releases page rather than the
example hash mentioned in the comment.
- Line 17: The subosito/flutter-action is currently pinned to a version tag (v2)
instead of a specific commit hash, which poses a supply-chain security risk.
Replace the version tag reference with a specific commit hash by changing the
uses directive from the v2 tag format to use the format with a commit hash.
Check the subosito/flutter-action releases page to find the latest stable commit
hash and use that instead of the version tag.
In `@lib/data/database/daos/returns_dao.dart`:
- Around line 29-31: The nextReturnNumber() method in ReturnsDAO uses row count
for sequence generation, which produces duplicate return numbers after record
deletions and fails under concurrent requests, violating the unique constraint
on returnNo. Since the method is currently unused, either remove it entirely, or
if it needs to be retained, replace the row count logic with a proper sequence
generation approach that queries the maximum existing return number suffix and
increments it, while wrapping the operation in a transaction with conflict retry
handling to prevent duplicates under concurrent access.
- Around line 12-16: The `insertReturn` and `insertItems` methods are separate
operations that can lead to partial writes and database inconsistency if one
succeeds while the other fails. Combine both operations into a single
transactional method that executes the header insertion via `insertReturn` and
the items insertion via `insertItems` within a unified transaction block,
ensuring atomicity. Use the batch transaction pattern (similar to how
`insertItems` currently uses batch) to wrap both the return header and items
insertions in a single atomic operation so either both succeed or both fail
together.
In `@lib/data/database/daos/suppliers_dao.dart`:
- Around line 57-60: The nextGrnNumber() method has a race condition and
performance problem: it loads all purchases to count them (O(n) operation)
without transaction protection, allowing concurrent requests to generate the
same GRN number. Replace the current count-based approach with a database-level
sequence strategy using either a dedicated counter row that gets atomically
incremented within a transaction, or implement a retry-on-conflict pattern where
you attempt to insert a candidate number and regenerate if it conflicts. This
ensures atomicity at the database level and eliminates the need to load all
records just to get a count.
In `@lib/data/database/tables/returns_table.dart`:
- Around line 10-12: The type field in the returns table lacks database-level
validation for allowed values, which can lead to invalid data being stored. Add
a CHECK constraint to the type TextColumn getter method to enforce that only the
supported values ('refund', 'credit', 'exchange') can be inserted or updated.
Apply the constraint directly in the column definition to ensure data integrity
at the database layer.
- Around line 27-29: The qty, unitPrice, and subtotal real column definitions in
the ReturnItems table lack CHECK constraints, allowing invalid negative or zero
values that compromise financial data integrity. Modify each of these three
column definitions to add validation constraints using Drift's check method: for
the qty field, enforce that it must be greater than 0; for the unitPrice field,
enforce that it must be greater than or equal to 0; and for the subtotal field,
enforce that it must also be greater than or equal to 0. Chain the check method
to each real() call before the final parentheses to add these database-level
constraints.
In `@lib/features/grn/presentation/grn_screen.dart`:
- Around line 460-463: The TextButton in GRN screen (calling
ref.read(grnProvider.notifier).reset()) is currently always enabled, which
allows users to clear the form while a submission is in progress, causing state
desynchronization. Disable this button by setting its onPressed to null when a
submission is active. Check the GRN provider for an existing loading/submitting
state indicator, then conditionally set onPressed to null when submission is in
progress and keep the current reset callback when idle. Optionally, update the
button's styling (opacity or color) to visually indicate the disabled state to
users.
- Around line 224-225: The itemBuilder in the ListView.builder for _GrnItemRow
(lines 224-225) lacks a stable key, and the text controllers for quantity and
cost fields are initialized only once in initState (lines 331-337). When item
values update from the provider, these controllers do not reflect the changes,
causing drift between the UI and the provider state. Add a stable key to the
itemBuilder using a unique identifier from each item (such as item id), and
refactor the controller initialization logic in initState to either add
listeners that respond to item value changes from the provider, or implement a
mechanism that updates controller values whenever the underlying item data
changes, ensuring the qty and cost fields remain synchronized with the provider
state.
In `@lib/features/invoices/presentation/invoices_screen.dart`:
- Around line 67-73: The onDatePick and onSearch callbacks are using a captured
filter snapshot that can become stale if other fields change before the callback
executes, causing race conditions where later callbacks overwrite other filter
criteria. Instead of using the captured filter variable in the copyWith calls,
read the latest filter state directly from invoiceFilterProvider within each
callback by replacing the filter snapshot with a call to
ref.read(invoiceFilterProvider) at callback execution time, ensuring each update
always merges with the most current state rather than a stale snapshot.
In `@lib/features/petty_cash/presentation/petty_cash_screen.dart`:
- Around line 205-213: The _viewPhoto method uses Image.file(File(path)) without
verifying that the file still exists on disk, which can cause runtime failures
when persisted receipt paths reference deleted files. Before creating the
Image.file widget, check if the file exists using File(path).existsSync(). If
the file doesn't exist, display a fallback message or placeholder widget instead
of attempting to load the image. Apply this same defensive check at all
locations where receipt image files are directly loaded for display.
In `@lib/features/quick_sales/presentation/quick_sales_screen.dart`:
- Around line 167-170: The validation check for qty and price in the
quick_sales_screen.dart file currently returns silently when either value is
invalid (less than or equal to zero), providing no user feedback. Instead of
returning without action when the if condition (qty <= 0 || price <= 0) is true,
display an error message to the user using an appropriate feedback mechanism
such as a SnackBar or dialog to explain that both quantity and price must be
greater than zero. This ensures users receive clear feedback about why their
action was not processed.
In `@lib/providers/grn_provider.dart`:
- Around line 181-182: After resetting the state with the lastGrnNo assignment,
the grnListProvider is not being invalidated, which causes the History tab to
display stale data since the provider remains alive in the tab view. Add an
invalidation call to grnListProvider immediately after the state reset to ensure
the GRN history is refreshed and synchronized with the latest changes.
- Around line 119-179: Wrap all database operations in a single transaction to
ensure atomicity and prevent partial state commits on failure. In
lib/providers/grn_provider.dart (lines 119-179), wrap all operations from
insertPurchase through the final auditDao.log call in a database transaction. In
lib/providers/quick_sale_provider.dart (lines 62-104), wrap the sale insert,
stock upsert, stock movement insert, and audit log operations in a database
transaction. Use the Drift database transaction API to ensure that if any
operation fails, the entire sequence is rolled back, maintaining cross-table
consistency for inventory, supplier balances, and audit logs.
- Around line 104-179: The submit method reads state.supplier, state.items, and
state.total multiple times across awaits, risking inconsistent snapshots if
state mutates during the operation. Immediately after setting isSubmitting to
true, capture immutable local variables for supplier, items, and total from
state. Then replace all subsequent references to state.supplier, state.items,
and state.total (used in suppliersDao.insertPurchase, the for loop iterating
items, inventoryDao calls, suppliersDao.updateBalance, and auditDao.log) with
the corresponding local variables to ensure a consistent snapshot throughout the
entire async operation.
In `@lib/providers/pos_provider.dart`:
- Around line 134-140: The setLineDiscount method correctly updates the
discountPct in state, but the discountAmount is not being calculated and
persisted when items are inserted into the database during checkout. Find where
invoice items are being inserted during checkout (likely in a method that
persists the cart items to the database) and ensure that for each item, you
calculate the discountAmount based on the item's discountPct and price, then
persist both discountPercent and the calculated discountAmount to the
InvoiceItems table. This ensures discount analytics and exports reflect the
actual discount amounts applied to each line item.
In `@lib/providers/quick_sale_provider.dart`:
- Around line 51-56: The sell method in quick_sale_provider.dart accepts qty and
price parameters but does not validate them at the action layer, relying instead
only on UI-side guards which can be bypassed by non-UI callers. Add input
validation at the beginning of the sell method to reject non-positive values for
both qty and price parameters; throw an appropriate exception or error for
invalid inputs to ensure data integrity and prevent invalid writes from any
caller source.
- Around line 72-84: The issue is that when the requested qty exceeds available
stock, the newQty is clamped to zero but the recordMovement method still records
the full requested qty, creating a mismatch between inventory and the stock
ledger. Instead of recording the original requested qty in the recordMovement
call within the StockMovementsCompanion.insert, calculate and record the actual
quantity that was deducted from inventory, which is the difference between the
current stock and newQty (the actual amount that was removed).
In `@lib/shared/widgets/bms_filter_bar.dart`:
- Around line 24-26: The bms_filter_bar.dart widget contains hard-coded date
bounds (firstDate as DateTime(2020) and lastDate as DateTime(2035)) that
restrict valid date selections across all consuming screens. Replace these
hard-coded values with parameterized constructor parameters for the widget or
calculate them dynamically based on the current date and business requirements.
This allows different screens to use appropriate date ranges for their specific
needs without being constrained by fixed historical and future limits.
In `@lib/shared/widgets/sidebar_nav.dart`:
- Around line 288-331: The logout functionality is only available in the wide
layout (in the IconButton at the end of the Row), but is completely missing from
the narrow layout (the Tooltip-wrapped avatar). To fix this, add a
GestureDetector or InkWell around the avatar in the narrow mode branch (where
constraints.maxWidth < 120) and wire its onTap callback to trigger the same
logout action as the IconButton: ref.read(authStateProvider.notifier).logout().
This ensures users on narrow displays can still access the logout functionality
by tapping the avatar.
---
Outside diff comments:
In `@lib/features/invoices/presentation/invoice_pdf.dart`:
- Line 230: The colWidths constant defined in the invoice_pdf.dart file is not
being used anywhere since the columnWidths map uses hardcoded FixedColumnWidth
and FlexColumnWidth values instead. Remove the unused colWidths variable
declaration entirely to eliminate the pipeline warning.
In `@lib/features/petty_cash/presentation/petty_cash_screen.dart`:
- Around line 142-152: Remove the two unused helper methods `_statusColor` and
`_statusBg` from the petty cash screen class, as they are not being called
anywhere in the codebase and are causing analyzer warnings. Simply delete both
method definitions entirely to clean up the dead code and resolve the CI
analyzer output warnings.
In `@lib/features/pos/presentation/pos_screen.dart`:
- Line 264: Add explicit generic type argument `<void>` to the `showDialog` and
`showModalBottomSheet` calls that are missing them. Specifically, modify the
calls at line 264, line 274, and line 331 in the pos_screen.dart file to include
`<void>` as the generic type argument (for example, change `showDialog(` to
`showDialog<void>(`). This will make these calls consistent with the existing
implementation at line 806 which already includes the type argument, and will
resolve analyzer warnings since the return values of these calls are not used.
In `@lib/features/settings/presentation/settings_screen.dart`:
- Line 13: Remove the unused import statement for audit_log_dao.dart from the
file. This import is not referenced anywhere in the settings_screen.dart file
and is causing an analyzer warning, so it should be deleted to keep the imports
clean and improve code quality.
- Line 140: The showDialog function call at line 140 in settings_screen.dart is
missing explicit type arguments, which causes analyzer inference failures. Add
an explicit type argument to the showDialog call by specifying the return type
in angle brackets, such as showDialog<T>() where T represents the expected
return type of the dialog (typically void, bool, or another appropriate type
based on what the dialog should return).
- Line 96: The MaterialPageRoute constructor in the navigation call is missing
an explicit type argument, which the analyzer requires to avoid inference
failures. Add an explicit type argument to MaterialPageRoute, such as
MaterialPageRoute<void>, to specify the return type of the route. This applies
to the MaterialPageRoute that wraps the _AuditLogScreen widget.
In `@lib/providers/petty_cash_provider.dart`:
- Around line 52-68: In the addEntry method, add validation logic before the DAO
insert call to enforce invariants on the amount and type parameters. Validate
that amount is positive and finite (not negative, zero, or NaN/infinity), and
validate that type contains an expected/valid value. If validation fails, throw
an appropriate exception to prevent corrupt data from being persisted to the
database.
- Around line 60-109: The addEntry, approve, and reject methods in
PettyCashProvider perform mutations to the database and log audit events, but
they do not invalidate the pettyCashEntriesProvider cache afterward. This causes
the UI to display stale data until another dependency triggers a refresh. After
the auditLogDaoProvider.log call completes in each of the three methods
(addEntry, approve, and reject), add a call to invalidate the
pettyCashEntriesProvider by using _ref.invalidate(pettyCashEntriesProvider) to
ensure the cached entries are refreshed and the UI displays the latest data.
---
Nitpick comments:
In `@lib/data/database/tables/returns_table.dart`:
- Around line 7-8: Add indexes to the columns used as hot filter paths in DAO
queries to prevent full table scans as data grows. In the ReturnsTable class,
add indexing to the invoiceId column (line 7) and the column at line 24 to
enable efficient lookups. Use the appropriate indexing method provided by the
drift library to mark these columns as indexed in the table schema definition.
In `@lib/features/inventory/presentation/inventory_screen.dart`:
- Line 28: Add explicit `<void>` type arguments to all `showModalBottomSheet`
calls to resolve Flutter analyzer inference warnings. In
lib/features/inventory/presentation/inventory_screen.dart at line 28, change
`showModalBottomSheet(` to `showModalBottomSheet<void>(`. In
lib/features/suppliers/presentation/suppliers_screen.dart, add `<void>` type
argument to the `showModalBottomSheet` calls at lines 14, 85, and 158. In
lib/features/users/presentation/users_screen.dart, add `<void>` type argument to
the `showModalBottomSheet` calls at lines 64, 145, 154, and 235. Each change
follows the same pattern: insert `<void>` immediately after the method name
`showModalBottomSheet`.
In `@lib/shared/widgets/sidebar_nav.dart`:
- Line 133: Extract the magic number 120 that appears in the narrow-width
threshold comparisons across the file into a top-level constant named
_kNarrowThreshold with a value of 120.0. Then replace all three occurrences of
the hardcoded comparison (< 120) with comparisons using this constant in the
_Header widget at line 133, the _NavTile widget at line 214, and the _UserFooter
widget at line 290 to improve maintainability and reduce duplication.
In `@README.md`:
- Around line 61-91: The project structure code block in the README starting
with the lib/ directory listing is missing a language identifier on the opening
fence. Add a language identifier (such as `text` or `tree`) immediately after
the opening triple backticks to enable proper syntax highlighting and maintain
consistency with other code blocks in the document.
- Around line 147-154: The fenced code block in the README.md section "Clearing
the web database" (the block containing the Chrome DevTools instructions) is
missing a language identifier. Add a language identifier such as "text" to the
opening triple backticks (change ``` to ```text) to ensure markdown consistency
and proper syntax highlighting compliance.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c15f774f-95d3-4245-b7f8-c170b2232613
⛔ Files ignored due to path filters (8)
assets/images/bms_logo.svgis excluded by!**/*.svgdocs/banner.pngis excluded by!**/*.pngdocs/banner.svgis excluded by!**/*.svgdocs/logo.pngis excluded by!**/*.pngdocs/org-banner.pngis excluded by!**/*.pngdocs/org-banner.svgis excluded by!**/*.svgpubspec.lockis excluded by!**/*.lockweb/favicon.svgis excluded by!**/*.svg
📒 Files selected for processing (50)
.github/ISSUE_TEMPLATE/bug_report.md.github/ISSUE_TEMPLATE/feature_request.md.github/ISSUE_TEMPLATE/task.md.github/pull_request_template.md.github/workflows/codeql.ymlCHANGELOG.mdLICENSEREADME.mdanalysis_options.yamldocs/org-profile/README.mdlib/core/router/app_router.dartlib/core/router/route_guard.dartlib/core/theme/app_theme.dartlib/data/database/app_database.dartlib/data/database/daos/inventory_dao.dartlib/data/database/daos/returns_dao.dartlib/data/database/daos/suppliers_dao.dartlib/data/database/tables/returns_table.dartlib/features/cheques/presentation/cheque_screen.dartlib/features/customers/presentation/customers_screen.dartlib/features/dashboard/presentation/dashboard_screen.dartlib/features/grn/presentation/grn_screen.dartlib/features/inventory/presentation/inventory_screen.dartlib/features/invoices/presentation/invoice_detail_screen.dartlib/features/invoices/presentation/invoice_pdf.dartlib/features/invoices/presentation/invoices_screen.dartlib/features/petty_cash/presentation/petty_cash_screen.dartlib/features/pos/presentation/pos_screen.dartlib/features/pos/presentation/receipt_pdf.dartlib/features/quick_sales/presentation/quick_sales_screen.dartlib/features/reports/presentation/reports_screen.dartlib/features/settings/presentation/settings_screen.dartlib/features/suppliers/presentation/suppliers_screen.dartlib/features/users/presentation/users_screen.dartlib/providers/cheques_provider.dartlib/providers/customers_provider.dartlib/providers/database_provider.dartlib/providers/grn_provider.dartlib/providers/inventory_provider.dartlib/providers/petty_cash_provider.dartlib/providers/pos_provider.dartlib/providers/quick_sale_provider.dartlib/providers/settings_provider.dartlib/providers/suppliers_provider.dartlib/shared/widgets/bms_filter_bar.dartlib/shared/widgets/sidebar_nav.dartpubspec.yamlweb/index.htmlwindows/flutter/generated_plugin_registrant.ccwindows/flutter/generated_plugins.cmake
💤 Files with no reviewable changes (2)
- lib/features/invoices/presentation/invoice_detail_screen.dart
- analysis_options.yaml
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 12 file(s) based on 20 unresolved review comments. A stacked PR containing fixes has been created.
Time taken:
Lines 12–20 runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v4
+ - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+ with:
+ persist-credentials: false
- - uses: subosito/flutter-action@v2
+ - uses: subosito/flutter-action@f2c4f6686ca8e8d6e6d0f28410eeef506ed66aef # v2.16.0
with:
channel: stable |
Fixed 12 file(s) based on 20 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
fix: CodeRabbit auto-fixes for PR #4
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 (2)
lib/providers/quick_sale_provider.dart (2)
103-109:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAudit log records requested qty, not actual deducted quantity.
The audit log at lines 107-108 uses the original
qtyandqty * pricefor the total. If an oversell scenario occurs (partially fulfilled), this audit entry will misrepresent what actually happened. Ensure the audit reflects reality by usingactualDeductedif partial fulfillment is allowed, or this becomes moot if you fail on insufficient stock per the previous comment.🤖 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/quick_sale_provider.dart` around lines 103 - 109, The audit log's newValue dictionary is recording the requested quantity (qty) instead of the actual deducted quantity in an oversell scenario where partial fulfillment is allowed. Update the 'qty' field and the 'total' calculation in the newValue dictionary to use actualDeducted instead of qty to ensure the audit log accurately reflects what was actually deducted from inventory rather than what was requested.
66-74:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSale record should use
actualDeductedto stay consistent with stock movement.The fix at line 89 correctly records the actual deducted quantity in the stock movement. However, the sale record at line 70 still uses the original
qty. Whenqty > currentQty, this creates a mismatch: the sale claims X units were sold, but only Y units were actually removed from inventory.Consider either:
- Using
actualDeductedfor the sale record (and adjusting price/total accordingly), or- Failing the sale when insufficient stock exists (throw before line 66)
Option 2: Fail early on insufficient stock
if (qty <= 0 || price <= 0) { throw ArgumentError('Quantity and price must be greater than zero'); } + final current = await _ref.read(inventoryDaoProvider).getStock(product.id); + final currentQty = current?.qty ?? 0; + if (qty > currentQty) { + throw StateError('Insufficient stock: requested $qty but only $currentQty available'); + } + final id = _uuid.v7(); final inventoryDao = _ref.read(inventoryDaoProvider);Then simplify lines 76-79:
- final current = await inventoryDao.getStock(product.id); - final currentQty = current?.qty ?? 0; - final newQty = (currentQty - qty).clamp(0.0, double.infinity); - final actualDeducted = currentQty - newQty; + final newQty = currentQty - qty; await inventoryDao.upsertStock(StockCompanion( productId: Value(product.id), qty: Value(newQty),And at line 89:
- qty: actualDeducted, + qty: qty,🤖 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/quick_sale_provider.dart` around lines 66 - 74, The NoInvoiceSalesCompanion.insert call in the insertNoInvoiceSale method is recording the requested qty quantity, but when insufficient stock exists, only actualDeducted units are removed from inventory (as recorded at line 89), creating a mismatch between the sale record and the actual stock movement. You must align these by either: (1) passing actualDeducted to the qty parameter in NoInvoiceSalesCompanion.insert and adjusting the price calculation accordingly to reflect only the quantity actually deducted, or (2) validating that qty does not exceed currentQty before the insertNoInvoiceSale call and throwing an exception to fail the sale early if stock is insufficient. Choose the approach that best fits your business requirements and implement it consistently.
🧹 Nitpick comments (1)
lib/providers/quick_sale_provider.dart (1)
61-94: Wrap multi-step writes in a transaction to ensure atomicity.The
sell()method performs four sequential writes: invoice insert, inventory upsert, stock movement insert, and audit log. If any operation fails mid-sequence, the database is left in an inconsistent state (e.g., sale created but stock not deducted).Other providers in this codebase (e.g.,
grn_provider.dart) follow the established pattern of wrapping multi-step operations indb.transaction(() async { ... }). Access the database instance via_ref.read(appDatabaseProvider)and apply the same pattern here.🤖 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/quick_sale_provider.dart` around lines 61 - 94, The sell() method performs four sequential database operations (insertNoInvoiceSale, upsertStock, recordMovement, and the audit log write) without transactional protection, risking database inconsistency if any operation fails. Wrap all these database writes in a transaction by first accessing the database instance via _ref.read(appDatabaseProvider), then using its transaction() method with an async callback to execute all operations atomically. This follows the established pattern used in other providers like grn_provider.dart and ensures that either all writes complete successfully or none of them do, maintaining database integrity.
🤖 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/providers/quick_sale_provider.dart`:
- Around line 103-109: The audit log's newValue dictionary is recording the
requested quantity (qty) instead of the actual deducted quantity in an oversell
scenario where partial fulfillment is allowed. Update the 'qty' field and the
'total' calculation in the newValue dictionary to use actualDeducted instead of
qty to ensure the audit log accurately reflects what was actually deducted from
inventory rather than what was requested.
- Around line 66-74: The NoInvoiceSalesCompanion.insert call in the
insertNoInvoiceSale method is recording the requested qty quantity, but when
insufficient stock exists, only actualDeducted units are removed from inventory
(as recorded at line 89), creating a mismatch between the sale record and the
actual stock movement. You must align these by either: (1) passing
actualDeducted to the qty parameter in NoInvoiceSalesCompanion.insert and
adjusting the price calculation accordingly to reflect only the quantity
actually deducted, or (2) validating that qty does not exceed currentQty before
the insertNoInvoiceSale call and throwing an exception to fail the sale early if
stock is insufficient. Choose the approach that best fits your business
requirements and implement it consistently.
---
Nitpick comments:
In `@lib/providers/quick_sale_provider.dart`:
- Around line 61-94: The sell() method performs four sequential database
operations (insertNoInvoiceSale, upsertStock, recordMovement, and the audit log
write) without transactional protection, risking database inconsistency if any
operation fails. Wrap all these database writes in a transaction by first
accessing the database instance via _ref.read(appDatabaseProvider), then using
its transaction() method with an async callback to execute all operations
atomically. This follows the established pattern used in other providers like
grn_provider.dart and ensures that either all writes complete successfully or
none of them do, maintaining database integrity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 52f5ca4a-e87e-4f99-91c3-bc0db25a4b38
📒 Files selected for processing (12)
lib/data/database/daos/returns_dao.dartlib/data/database/daos/suppliers_dao.dartlib/data/database/tables/returns_table.dartlib/features/grn/presentation/grn_screen.dartlib/features/invoices/presentation/invoices_screen.dartlib/features/petty_cash/presentation/petty_cash_screen.dartlib/features/quick_sales/presentation/quick_sales_screen.dartlib/providers/grn_provider.dartlib/providers/pos_provider.dartlib/providers/quick_sale_provider.dartlib/shared/widgets/bms_filter_bar.dartlib/shared/widgets/sidebar_nav.dart
🚧 Files skipped from review as they are similar to previous changes (10)
- lib/data/database/daos/suppliers_dao.dart
- lib/features/quick_sales/presentation/quick_sales_screen.dart
- lib/providers/pos_provider.dart
- lib/data/database/tables/returns_table.dart
- lib/shared/widgets/sidebar_nav.dart
- lib/features/invoices/presentation/invoices_screen.dart
- lib/shared/widgets/bms_filter_bar.dart
- lib/providers/grn_provider.dart
- lib/features/grn/presentation/grn_screen.dart
- lib/features/petty_cash/presentation/petty_cash_screen.dart
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@analysis_options.yaml`:
- Around line 6-7: Remove the global ignore directives for
inference_failure_on_function_invocation and
inference_failure_on_instance_creation from the analysis_options.yaml file
(lines 6-7). These global suppressions conflict with the strict-inference: true
setting and mask actual type inference issues. Instead, identify the specific
callsites where these diagnostics are triggered and either fix the type
inference problems directly at those locations or apply narrowly-scoped //
ignore comments only where absolutely necessary.
In `@test/widget_test.dart`:
- Around line 4-5: Replace the tautological placeholder test (which only
verifies true == true) with a real smoke test using testWidgets that verifies
app initialization and critical UI rendering. Remove the placeholder test and
add a testWidgets block that creates the app widget, pumps it, and includes
assertions to verify that key UI elements render correctly (such as checking for
presence of critical widgets or text on the main screen). This will restore
meaningful integration coverage to catch regressions in app boot and core
workflows.
🪄 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: 60a8905c-aefc-4167-80ac-b600ff9cba74
📒 Files selected for processing (8)
analysis_options.yamllib/features/invoices/presentation/invoice_pdf.dartlib/features/petty_cash/presentation/petty_cash_screen.dartlib/features/pos/presentation/pos_screen.dartlib/features/settings/presentation/settings_screen.darttest/unit/auth/auth_repository_test.darttest/unit/inventory/inventory_repository_test.darttest/widget_test.dart
💤 Files with no reviewable changes (6)
- test/unit/auth/auth_repository_test.dart
- test/unit/inventory/inventory_repository_test.dart
- lib/features/settings/presentation/settings_screen.dart
- lib/features/invoices/presentation/invoice_pdf.dart
- lib/features/petty_cash/presentation/petty_cash_screen.dart
- lib/features/pos/presentation/pos_screen.dart
Contributor
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
hiranyasemindi
approved these changes
Jun 16, 2026
This was referenced 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
3 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
Completes Phase 3 of BMS development - adds three missing core features, fixes UI consistency across the entire app, and ships the brand identity (logo, banner, README).
Type
Changes
Screenshots
See README banner and individual screen descriptions above.
Test plan
Related issues
Phase 3 scope completion.
Summary by CodeRabbit
Release Notes