Add Indian Bonds module and Flutter preview app with mock NSE CBRICS transport#1
Open
Add Indian Bonds module and Flutter preview app with mock NSE CBRICS transport#1
Conversation
📝 WalkthroughWalkthroughIntroduces a complete Flutter Bond Trading Module for Indian markets (NSE CBRICS), implementing clean architecture with domain entities, repository pattern, use cases, a presentation layer with UI components, HTTP data source abstraction, mock transport for local preview, and comprehensive documentation. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant BondPage
participant BondController
participant PlaceBondOrder
participant BondRepository
participant NseCbricsDataSource
participant HttpTransport
User->>BondPage: Load page / Tap refresh
BondPage->>BondController: loadBonds()
BondController->>BondRepository: getAvailableBonds()
BondRepository->>NseCbricsDataSource: fetchBonds()
NseCbricsDataSource->>HttpTransport: get('/bonds')
HttpTransport-->>NseCbricsDataSource: JSON response
NseCbricsDataSource->>NseCbricsDataSource: Deserialize to BondModel list
NseCbricsDataSource-->>BondRepository: List<Bond>
BondRepository-->>BondController: List<Bond>
BondController-->>BondPage: List<Bond>
BondPage->>BondPage: Render bonds in ListView
User->>BondPage: Expand bond tile
BondPage->>BondOrderTicket: Show with isin
User->>BondOrderTicket: Enter quantity, price, submit
BondOrderTicket->>BondController: placeLimitBuyOrder(isin, qty, price)
BondController->>PlaceBondOrder: call(isin, BUY, LIMIT, qty, price)
PlaceBondOrder->>PlaceBondOrder: Validate inputs
PlaceBondOrder->>BondRepository: placeOrder(...)
BondRepository->>NseCbricsDataSource: submitOrder(...)
NseCbricsDataSource->>HttpTransport: post('/orders', body)
HttpTransport-->>NseCbricsDataSource: JSON response with orderId
NseCbricsDataSource->>NseCbricsDataSource: Deserialize to BondOrderModel
NseCbricsDataSource-->>BondRepository: BondOrder
BondRepository-->>PlaceBondOrder: BondOrder
PlaceBondOrder-->>BondController: BondOrder
BondController-->>BondOrderTicket: BondOrder
BondOrderTicket->>BondOrderTicket: Display orderId in message
sequenceDiagram
participant BondPreviewPage
participant MockNseCbricsTransport
participant NseCbricsDataSource
participant BondRepositoryImpl
participant BondController
participant BondPage
BondPreviewPage->>MockNseCbricsTransport: new()
BondPreviewPage->>NseCbricsDataSource: new(baseUrl, apiKey, MockTransport)
BondPreviewPage->>BondRepositoryImpl: new(dataSource)
BondPreviewPage->>BondController: new(repository)
BondPreviewPage->>BondPage: new(controller)
BondPreviewPage-->>BondPreviewPage: Build with mock-backed BondPage
Note over MockNseCbricsTransport: In-memory orders list
Note over NseCbricsDataSource: Uses mock transport instead of real HTTP
Note over BondRepositoryImpl: Delegates to mock data source
Note over BondController: Operates on mock data
Note over BondPage: Displays mock bonds & accepts mock orders
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8df8258bf3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+47
to
+49
| setState(() { | ||
| _message = 'Order placed: ${order.orderId}'; | ||
| }); |
There was a problem hiding this comment.
After awaiting placeLimitBuyOrder, _submit calls setState unconditionally in both success and error paths. If the user navigates away (or the widget is otherwise disposed) before the network call completes, this triggers setState() on a disposed State, which can raise a runtime exception in production. Add a mounted guard before updating _message after the await (and in the catch path as well).
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/features/bonds/data/datasources/nse_cbrics_datasource.dart`:
- Around line 35-37: The code assumes jsonDecode(raw) yields a Map with a List
under key 'data' (variables payload and items); add defensive checks to validate
the envelope before casting: ensure jsonDecode(raw) is a Map<String, dynamic>,
that payload.containsKey('data'), and that payload['data'] is a List<dynamic>
(otherwise return a safe default, throw a descriptive exception, or handle the
API error envelope); apply the same guard/validation to the other parsing sites
referenced (the other payload/data casts and the submitOrder parsing) so all
casts are protected and errors include contextual messages indicating which
endpoint/operation failed.
- Around line 7-15: Update the HttpTransport contract to return a response
object containing both statusCode and body (instead of Future<String>) and
adjust callers to check status before parsing: modify the abstract class
HttpTransport methods get/post to return a type (e.g., HttpResponse { int
statusCode; String body; }) and then in NseCbricsDatasource.fetchBonds(),
submitOrder(), and fetchOpenOrders() inspect response.statusCode (accept
200/2xx) and only call jsonDecode(response.body) on success; for non-OK statuses
throw or return a meaningful error including response.statusCode and
response.body so error responses aren’t passed to jsonDecode.
In `@lib/features/bonds/data/models/bond_model.dart`:
- Around line 14-23: The BondModel.fromJson deserialization uses unsafe casts
and DateTime.parse that can throw opaque runtime errors; update
BondModel.fromJson to validate each field (isin, symbol, issuer, maturityDate,
couponRate, faceValue, category) using helper validators similar to the
BondOrderModel enum decoders: check types (is String / is num), parse numbers to
double safely, wrap DateTime.parse in try/catch and validate format, and throw
FormatException with a clear contextual message for each invalid field; also
ensure callers in nse_cbrics_datasource.dart propagate or handle these
FormatExceptions instead of letting raw TypeError/FormatException surface.
In `@lib/features/bonds/data/models/bond_order_model.dart`:
- Around line 23-24: The current JSON decode for the BondOrderModel fields
quantity and filledQuantity uses .toInt(), which silently truncates fractional
numbers; instead validate the incoming num values before converting and fail
loudly on malformed data: check (json['quantity'] as num) and
(json['filledQuantity'] as num?) to ensure they are whole numbers (e.g., num % 1
== 0) and throw a FormatException (or return an error) if not, then safely
convert to int; apply this validation in the BondOrderModel JSON
constructor/fromJson where quantity and filledQuantity are parsed to prevent
silent truncation.
In `@lib/features/bonds/domain/usecases/place_bond_order.dart`:
- Around line 16-34: The ISIN should be normalized and the limitPrice must be
checked for finiteness before calling the repository: create a trimmedIsin =
isin.trim() and use trimmedIsin in the call to _repository.placeOrder (replace
usages of raw isin), and when type == BondOrderType.limit validate that
limitPrice is not null, greater than 0 and isFinite (reject NaN/Infinity) before
proceeding; keep the existing behavior of passing null for market orders
(limitPrice: type == BondOrderType.market ? null : limitPrice) but use the
validated/normalized values.
In `@lib/features/bonds/presentation/pages/bond_page.dart`:
- Around line 25-30: The _refresh method should update _bondsFuture with
widget.controller.loadBonds() but catch and swallow any errors so they don't
propagate to the RefreshIndicator; change _refresh to assign _bondsFuture inside
setState then await it inside a try/catch (e.g., try { await _bondsFuture; }
catch (_) {}), leaving error rendering to the existing FutureBuilder that
watches _bondsFuture; reference _refresh, _bondsFuture, and
widget.controller.loadBonds when making the change.
In `@lib/features/bonds/presentation/pages/bond_preview_page.dart`:
- Around line 14-24: The build method currently constructs NseCbricsDataSource,
BondRepositoryImpl, and BondController every build, causing a new controller to
be created and stale UI state; convert BondPreviewPage from a StatelessWidget to
a StatefulWidget, move creation of NseCbricsDataSource (the instance with
baseUrl 'https://mock.nse-cbrics.local'), BondRepositoryImpl, and BondController
into the State object's initState(), store the controller as a field on the
State and pass that field to BondPage in build(), and implement dispose() on the
State to clean up the controller if it exposes a dispose/close method so the UI
uses a stable controller instance across rebuilds.
In `@lib/features/bonds/presentation/widgets/bond_order_ticket.dart`:
- Around line 38-57: The try/catch blocks update state without checking mounted,
which can throw after dispose; wrap the setState calls that assign _message in
both the success path (after awaiting widget.controller.placeLimitBuyOrder) and
the error path (inside catch) with an if (mounted) guard before calling setState
so the widget is still mounted; keep the existing mounted check in the finally
for _submitting and reference the same members (_qtyController,
_priceController, widget.controller.placeLimitBuyOrder, _message, _submitting)
when applying the guards.
🪄 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
Run ID: 1094ad10-bbd4-4ed7-b2ce-618460cead66
📒 Files selected for processing (17)
README.mdlib/features/bonds/README.mdlib/features/bonds/data/datasources/mock_nse_cbrics_transport.dartlib/features/bonds/data/datasources/nse_cbrics_datasource.dartlib/features/bonds/data/models/bond_model.dartlib/features/bonds/data/models/bond_order_model.dartlib/features/bonds/data/repositories/bond_repository_impl.dartlib/features/bonds/domain/entities/bond.dartlib/features/bonds/domain/entities/bond_order.dartlib/features/bonds/domain/repositories/bond_repository.dartlib/features/bonds/domain/usecases/place_bond_order.dartlib/features/bonds/presentation/controllers/bond_controller.dartlib/features/bonds/presentation/pages/bond_page.dartlib/features/bonds/presentation/pages/bond_preview_page.dartlib/features/bonds/presentation/widgets/bond_order_ticket.dartlib/main.dartpubspec.yaml
Comment on lines
+7
to
+15
| abstract class HttpTransport { | ||
| Future<String> get(String path, {Map<String, String>? headers}); | ||
|
|
||
| Future<String> post( | ||
| String path, { | ||
| Map<String, String>? headers, | ||
| Object? body, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n lib/features/bonds/data/datasources/nse_cbrics_datasource.dartRepository: SumitUni7/Test-web
Length of output: 2697
🏁 Script executed:
# Search for HttpTransport interface and implementations
rg "class HttpTransport|abstract class HttpTransport" -A 10Repository: SumitUni7/Test-web
Length of output: 984
🏁 Script executed:
# Search for usages of HttpTransport methods
rg "\.get\(|\.post\(" lib/features/bonds/data/datasources/ -B 2 -A 3Repository: SumitUni7/Test-web
Length of output: 1824
🏁 Script executed:
# Check if there are any response handling or status checks in the datasource
rg "statusCode|status|200|400|500" lib/features/bonds/data/datasources/ -A 2 -B 2Repository: SumitUni7/Test-web
Length of output: 548
🏁 Script executed:
# Search for HttpResponse or similar response wrapper classes
rg "class HttpResponse|statusCode.*body|response.*statusCode" --type dartRepository: SumitUni7/Test-web
Length of output: 44
🏁 Script executed:
# Find all implementations of HttpTransport
rg "class.*HttpTransport|implements HttpTransport" --type dart -A 5Repository: SumitUni7/Test-web
Length of output: 1246
🏁 Script executed:
# Search for any error handling or try-catch blocks around HTTP calls
rg "try|catch|Exception|Error" lib/features/bonds/data/datasources/ -B 2 -A 2Repository: SumitUni7/Test-web
Length of output: 972
🏁 Script executed:
# Check if there are any repository implementations that handle errors
rg "class.*Repository|implements.*Repository" lib/features/bonds/data/ --type dart -A 10Repository: SumitUni7/Test-web
Length of output: 1068
🏁 Script executed:
# Search for any file that might implement the real HTTP transport
fd ".*transport.*\.dart" lib/features/bonds/Repository: SumitUni7/Test-web
Length of output: 128
🏁 Script executed:
# Check pubspec for HTTP dependencies
cat pubspec.yaml | grep -A 5 -B 5 "http\|dio"Repository: SumitUni7/Test-web
Length of output: 44
🏁 Script executed:
# Search for http.Client or similar HTTP implementations
rg "http\.Client|Dio|HttpClient" --type dartRepository: SumitUni7/Test-web
Length of output: 44
🏁 Script executed:
# Check the repository structure
find . -type f -name "*.dart" | grep -E "(test|spec|transport)" | head -20Repository: SumitUni7/Test-web
Length of output: 130
🏁 Script executed:
# Search for any integration or real HTTP implementation
find . -type f -name "*.dart" | head -30Repository: SumitUni7/Test-web
Length of output: 851
🏁 Script executed:
# Check if there's a pubspec.yaml at all and look for HTTP dependencies
find . -name "pubspec.yaml" -exec cat {} \;Repository: SumitUni7/Test-web
Length of output: 308
HttpTransport must expose HTTP status codes, not only response body.
The datasource cannot currently distinguish successful HTTP responses from error responses (4xx/5xx), so failed requests will be passed to jsonDecode() as if they were valid. This affects all three methods: fetchBonds() (line 34), submitOrder() (line 50), and fetchOpenOrders() (line 68).
Change the interface to return a response object with both status code and body:
Suggested fix
abstract class HttpTransport {
- Future<String> get(String path, {Map<String, String>? headers});
+ Future<HttpResponse> get(String path, {Map<String, String>? headers});
- Future<String> post(
+ Future<HttpResponse> post(
String path, {
Map<String, String>? headers,
Object? body,
});
}
+
+class HttpResponse {
+ final int statusCode;
+ final String body;
+
+ const HttpResponse({required this.statusCode, required this.body});
+}Then validate status before parsing:
- final raw = await http.get('$baseUrl/bonds', headers: _headers);
- final payload = jsonDecode(raw) as Map<String, dynamic>;
+ final response = await http.get('$baseUrl/bonds', headers: _headers);
+ if (response.statusCode < 200 || response.statusCode >= 300) {
+ throw StateError('fetchBonds failed: ${response.statusCode}');
+ }
+ final payload = jsonDecode(response.body) as Map<String, dynamic>;Apply the same pattern to submitOrder() and fetchOpenOrders().
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| abstract class HttpTransport { | |
| Future<String> get(String path, {Map<String, String>? headers}); | |
| Future<String> post( | |
| String path, { | |
| Map<String, String>? headers, | |
| Object? body, | |
| }); | |
| } | |
| abstract class HttpTransport { | |
| Future<HttpResponse> get(String path, {Map<String, String>? headers}); | |
| Future<HttpResponse> post( | |
| String path, { | |
| Map<String, String>? headers, | |
| Object? body, | |
| }); | |
| } | |
| class HttpResponse { | |
| final int statusCode; | |
| final String body; | |
| const HttpResponse({required this.statusCode, required this.body}); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/bonds/data/datasources/nse_cbrics_datasource.dart` around lines
7 - 15, Update the HttpTransport contract to return a response object containing
both statusCode and body (instead of Future<String>) and adjust callers to check
status before parsing: modify the abstract class HttpTransport methods get/post
to return a type (e.g., HttpResponse { int statusCode; String body; }) and then
in NseCbricsDatasource.fetchBonds(), submitOrder(), and fetchOpenOrders()
inspect response.statusCode (accept 200/2xx) and only call
jsonDecode(response.body) on success; for non-OK statuses throw or return a
meaningful error including response.statusCode and response.body so error
responses aren’t passed to jsonDecode.
Comment on lines
+35
to
+37
| final payload = jsonDecode(raw) as Map<String, dynamic>; | ||
| final items = payload['data'] as List<dynamic>; | ||
|
|
There was a problem hiding this comment.
Guard response envelope shape before casting data.
payload['data'] is assumed to always exist and match expected types. If API returns an error envelope or schema drift, this throws at runtime (or parses a non-order map in submitOrder).
Suggested defensive parsing
+ Map<String, dynamic> _decodeObject(String raw) {
+ final decoded = jsonDecode(raw);
+ if (decoded is! Map<String, dynamic>) {
+ throw const FormatException('Expected JSON object response');
+ }
+ return decoded;
+ }
+
+ List<Map<String, dynamic>> _readDataList(Map<String, dynamic> payload) {
+ final data = payload['data'];
+ if (data is! List) {
+ throw const FormatException('Expected `data` to be a list');
+ }
+ return data.map((e) {
+ if (e is! Map<String, dynamic>) {
+ throw const FormatException('Expected list items to be JSON objects');
+ }
+ return e;
+ }).toList(growable: false);
+ }
...
- final payload = jsonDecode(raw) as Map<String, dynamic>;
- final items = payload['data'] as List<dynamic>;
+ final payload = _decodeObject(raw);
+ final items = _readDataList(payload);
...
- final payload = jsonDecode(raw) as Map<String, dynamic>;
- final orderMap = (payload['data'] ?? payload) as Map<String, dynamic>;
+ final payload = _decodeObject(raw);
+ final data = payload['data'];
+ if (data is! Map<String, dynamic>) {
+ throw const FormatException('Expected `data` to be an order object');
+ }
+ final orderMap = data;
...
- final payload = jsonDecode(raw) as Map<String, dynamic>;
- final items = payload['data'] as List<dynamic>;
+ final payload = _decodeObject(raw);
+ final items = _readDataList(payload);Also applies to: 62-64, 69-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/bonds/data/datasources/nse_cbrics_datasource.dart` around lines
35 - 37, The code assumes jsonDecode(raw) yields a Map with a List under key
'data' (variables payload and items); add defensive checks to validate the
envelope before casting: ensure jsonDecode(raw) is a Map<String, dynamic>, that
payload.containsKey('data'), and that payload['data'] is a List<dynamic>
(otherwise return a safe default, throw a descriptive exception, or handle the
API error envelope); apply the same guard/validation to the other parsing sites
referenced (the other payload/data casts and the submitOrder parsing) so all
casts are protected and errors include contextual messages indicating which
endpoint/operation failed.
Comment on lines
+14
to
+23
| factory BondModel.fromJson(Map<String, dynamic> json) { | ||
| return BondModel( | ||
| isin: json['isin'] as String, | ||
| symbol: json['symbol'] as String, | ||
| issuer: json['issuer'] as String, | ||
| maturityDate: DateTime.parse(json['maturityDate'] as String), | ||
| couponRate: (json['couponRate'] as num).toDouble(), | ||
| faceValue: (json['faceValue'] as num).toDouble(), | ||
| category: json['category'] as String, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n lib/features/bonds/data/models/bond_model.dart | head -50Repository: SumitUni7/Test-web
Length of output: 974
🏁 Script executed:
# Search for other model files with fromJson patterns
find lib -name "*_model.dart" -type f | head -20Repository: SumitUni7/Test-web
Length of output: 161
🏁 Script executed:
# Search for usages of BondModel.fromJson
rg "BondModel\.fromJson|\.fromJson.*bond" lib/Repository: SumitUni7/Test-web
Length of output: 302
🏁 Script executed:
# Check the Bond entity
find lib -name "bond.dart" -path "*/domain/entities/*" | xargs catRepository: SumitUni7/Test-web
Length of output: 469
🏁 Script executed:
cat -n lib/features/bonds/data/models/bond_order_model.dartRepository: SumitUni7/Test-web
Length of output: 2642
🏁 Script executed:
cat -n lib/features/bonds/data/datasources/nse_cbrics_datasource.dart | head -100Repository: SumitUni7/Test-web
Length of output: 2697
Harden JSON parsing in BondModel.fromJson to match BondOrderModel patterns.
The current implementation uses unsafe casts that fail with opaque runtime errors on malformed API payloads. BondOrderModel in the same codebase demonstrates the pattern: validate fields and throw contextual errors (see enum decoders). BondModel should follow suit—add validation for strings, dates, and numeric fields so failures are diagnosable rather than cryptic TypeError or FormatException from runtime casts.
Specifically:
- Direct
as Stringandas numcasts will crash on type mismatches DateTime.parse()has no try-catch for malformed datesnse_cbrics_datasource.darthas no error handling, so failures propagate uncontrolled
Add field-level validation helpers (following BondOrderModel's pattern) to throw FormatException with context before returning BondModel.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/bonds/data/models/bond_model.dart` around lines 14 - 23, The
BondModel.fromJson deserialization uses unsafe casts and DateTime.parse that can
throw opaque runtime errors; update BondModel.fromJson to validate each field
(isin, symbol, issuer, maturityDate, couponRate, faceValue, category) using
helper validators similar to the BondOrderModel enum decoders: check types (is
String / is num), parse numbers to double safely, wrap DateTime.parse in
try/catch and validate format, and throw FormatException with a clear contextual
message for each invalid field; also ensure callers in
nse_cbrics_datasource.dart propagate or handle these FormatExceptions instead of
letting raw TypeError/FormatException surface.
Comment on lines
+23
to
+24
| quantity: (json['quantity'] as num).toInt(), | ||
| filledQuantity: ((json['filledQuantity'] as num?) ?? 0).toInt(), |
There was a problem hiding this comment.
Prevent silent quantity truncation during JSON decode.
Line 23 and Line 24 use .toInt(), which will silently truncate fractional payloads (e.g., 10.9 -> 10). This can corrupt order state instead of rejecting malformed data.
Proposed fix
factory BondOrderModel.fromJson(Map<String, dynamic> json) {
return BondOrderModel(
orderId: json['orderId'] as String,
isin: json['isin'] as String,
side: _decodeSide(json['side'] as String),
type: _decodeType(json['type'] as String),
status: _decodeStatus((json['status'] as String?) ?? 'PENDING'),
- quantity: (json['quantity'] as num).toInt(),
- filledQuantity: ((json['filledQuantity'] as num?) ?? 0).toInt(),
+ quantity: _asStrictInt(json['quantity'], 'quantity'),
+ filledQuantity: _asStrictInt(json['filledQuantity'] ?? 0, 'filledQuantity'),
limitPrice: (json['limitPrice'] as num?)?.toDouble(),
createdAt: DateTime.parse(json['createdAt'] as String),
);
}
+
+ static int _asStrictInt(Object? value, String field) {
+ if (value is int) return value;
+ if (value is num && value == value.roundToDouble()) return value.toInt();
+ throw ArgumentError.value(value, field, 'Expected an integer value');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| quantity: (json['quantity'] as num).toInt(), | |
| filledQuantity: ((json['filledQuantity'] as num?) ?? 0).toInt(), | |
| factory BondOrderModel.fromJson(Map<String, dynamic> json) { | |
| return BondOrderModel( | |
| orderId: json['orderId'] as String, | |
| isin: json['isin'] as String, | |
| side: _decodeSide(json['side'] as String), | |
| type: _decodeType(json['type'] as String), | |
| status: _decodeStatus((json['status'] as String?) ?? 'PENDING'), | |
| quantity: _asStrictInt(json['quantity'], 'quantity'), | |
| filledQuantity: _asStrictInt(json['filledQuantity'] ?? 0, 'filledQuantity'), | |
| limitPrice: (json['limitPrice'] as num?)?.toDouble(), | |
| createdAt: DateTime.parse(json['createdAt'] as String), | |
| ); | |
| } | |
| static int _asStrictInt(Object? value, String field) { | |
| if (value is int) return value; | |
| if (value is num && value == value.roundToDouble()) return value.toInt(); | |
| throw ArgumentError.value(value, field, 'Expected an integer value'); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/bonds/data/models/bond_order_model.dart` around lines 23 - 24,
The current JSON decode for the BondOrderModel fields quantity and
filledQuantity uses .toInt(), which silently truncates fractional numbers;
instead validate the incoming num values before converting and fail loudly on
malformed data: check (json['quantity'] as num) and (json['filledQuantity'] as
num?) to ensure they are whole numbers (e.g., num % 1 == 0) and throw a
FormatException (or return an error) if not, then safely convert to int; apply
this validation in the BondOrderModel JSON constructor/fromJson where quantity
and filledQuantity are parsed to prevent silent truncation.
Comment on lines
+16
to
+34
| if (isin.trim().isEmpty) { | ||
| throw ArgumentError.value(isin, 'isin', 'ISIN is required'); | ||
| } | ||
|
|
||
| if (quantity <= 0) { | ||
| throw ArgumentError.value(quantity, 'quantity', 'Quantity must be positive'); | ||
| } | ||
|
|
||
| if (type == BondOrderType.limit && (limitPrice == null || limitPrice <= 0)) { | ||
| throw ArgumentError.value(limitPrice, 'limitPrice', 'Limit price must be provided for limit orders'); | ||
| } | ||
|
|
||
| return _repository.placeOrder( | ||
| isin: isin, | ||
| side: side, | ||
| type: type, | ||
| quantity: quantity, | ||
| limitPrice: type == BondOrderType.market ? null : limitPrice, | ||
| ); |
There was a problem hiding this comment.
Normalize ISIN and reject non-finite limit prices before repository call.
Line 16 validates trimmed ISIN, but Line 29 still forwards the raw value. Also, Line 24 allows NaN/Infinity because only <= 0 is checked.
Proposed fix
Future<BondOrder> call({
required String isin,
required BondOrderSide side,
required BondOrderType type,
required int quantity,
double? limitPrice,
}) {
- if (isin.trim().isEmpty) {
+ final normalizedIsin = isin.trim();
+ if (normalizedIsin.isEmpty) {
throw ArgumentError.value(isin, 'isin', 'ISIN is required');
}
@@
- if (type == BondOrderType.limit && (limitPrice == null || limitPrice <= 0)) {
+ if (type == BondOrderType.limit &&
+ (limitPrice == null || !limitPrice.isFinite || limitPrice <= 0)) {
throw ArgumentError.value(limitPrice, 'limitPrice', 'Limit price must be provided for limit orders');
}
return _repository.placeOrder(
- isin: isin,
+ isin: normalizedIsin,
side: side,
type: type,
quantity: quantity,
limitPrice: type == BondOrderType.market ? null : limitPrice,
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/bonds/domain/usecases/place_bond_order.dart` around lines 16 -
34, The ISIN should be normalized and the limitPrice must be checked for
finiteness before calling the repository: create a trimmedIsin = isin.trim() and
use trimmedIsin in the call to _repository.placeOrder (replace usages of raw
isin), and when type == BondOrderType.limit validate that limitPrice is not
null, greater than 0 and isFinite (reject NaN/Infinity) before proceeding; keep
the existing behavior of passing null for market orders (limitPrice: type ==
BondOrderType.market ? null : limitPrice) but use the validated/normalized
values.
Comment on lines
+25
to
+30
| Future<void> _refresh() async { | ||
| setState(() { | ||
| _bondsFuture = widget.controller.loadBonds(); | ||
| }); | ||
| await _bondsFuture; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n lib/features/bonds/presentation/pages/bond_page.dartRepository: SumitUni7/Test-web
Length of output: 3591
Wrap refresh in try-catch to avoid redundant error handling.
The _refresh() method currently propagates load errors through the onRefresh callback. Since the FutureBuilder already handles and displays errors (lines 45-54), catching locally prevents duplicate error state handling from both RefreshIndicator and FutureBuilder.
✅ Suggested refresh handling
Future<void> _refresh() async {
setState(() {
_bondsFuture = widget.controller.loadBonds();
});
- await _bondsFuture;
+ try {
+ await _bondsFuture;
+ } catch (_) {
+ // FutureBuilder renders the error state.
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Future<void> _refresh() async { | |
| setState(() { | |
| _bondsFuture = widget.controller.loadBonds(); | |
| }); | |
| await _bondsFuture; | |
| } | |
| Future<void> _refresh() async { | |
| setState(() { | |
| _bondsFuture = widget.controller.loadBonds(); | |
| }); | |
| try { | |
| await _bondsFuture; | |
| } catch (_) { | |
| // FutureBuilder renders the error state. | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/bonds/presentation/pages/bond_page.dart` around lines 25 - 30,
The _refresh method should update _bondsFuture with
widget.controller.loadBonds() but catch and swallow any errors so they don't
propagate to the RefreshIndicator; change _refresh to assign _bondsFuture inside
setState then await it inside a try/catch (e.g., try { await _bondsFuture; }
catch (_) {}), leaving error rendering to the existing FutureBuilder that
watches _bondsFuture; reference _refresh, _bondsFuture, and
widget.controller.loadBonds when making the change.
Comment on lines
+14
to
+24
| Widget build(BuildContext context) { | ||
| final dataSource = NseCbricsDataSource( | ||
| baseUrl: 'https://mock.nse-cbrics.local', | ||
| apiKey: 'preview-key', | ||
| http: MockNseCbricsTransport(), | ||
| ); | ||
|
|
||
| final repository = BondRepositoryImpl(dataSource); | ||
| final controller = BondController(repository); | ||
|
|
||
| return BondPage(controller: controller); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "bond_preview_page.dart" | head -20Repository: SumitUni7/Test-web
Length of output: 124
🏁 Script executed:
cat -n ./lib/features/bonds/presentation/pages/bond_preview_page.dartRepository: SumitUni7/Test-web
Length of output: 1071
🏁 Script executed:
cat -n ./lib/features/bonds/presentation/pages/bond_page.dart | head -50Repository: SumitUni7/Test-web
Length of output: 1854
🏁 Script executed:
find ./lib/features -type f -name "*page.dart" | head -20Repository: SumitUni7/Test-web
Length of output: 179
Avoid constructing controller graph inside build() of StatelessWidget.
Lines 15–23 recreate the data source, repository, and controller on every build. If the parent widget rebuilds BondPreviewPage, a new controller instance is created and passed to BondPage, but BondPage's _bondsFuture (initialized from the old controller) is never updated. This causes the UI to display stale data from a detached controller instance.
Move initialization to initState() in a StatefulWidget:
Proposed fix
-class BondPreviewPage extends StatelessWidget {
+class BondPreviewPage extends StatefulWidget {
const BondPreviewPage({super.key});
+
+ `@override`
+ State<BondPreviewPage> createState() => _BondPreviewPageState();
+}
+
+class _BondPreviewPageState extends State<BondPreviewPage> {
+ late final BondController controller;
+
+ `@override`
+ void initState() {
+ super.initState();
+ final dataSource = NseCbricsDataSource(
+ baseUrl: 'https://mock.nse-cbrics.local',
+ apiKey: 'preview-key',
+ http: MockNseCbricsTransport(),
+ );
+ final repository = BondRepositoryImpl(dataSource);
+ controller = BondController(repository);
+ }
`@override`
Widget build(BuildContext context) {
- final dataSource = NseCbricsDataSource(
- baseUrl: 'https://mock.nse-cbrics.local',
- apiKey: 'preview-key',
- http: MockNseCbricsTransport(),
- );
-
- final repository = BondRepositoryImpl(dataSource);
- final controller = BondController(repository);
-
return BondPage(controller: controller);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/bonds/presentation/pages/bond_preview_page.dart` around lines 14
- 24, The build method currently constructs NseCbricsDataSource,
BondRepositoryImpl, and BondController every build, causing a new controller to
be created and stale UI state; convert BondPreviewPage from a StatelessWidget to
a StatefulWidget, move creation of NseCbricsDataSource (the instance with
baseUrl 'https://mock.nse-cbrics.local'), BondRepositoryImpl, and BondController
into the State object's initState(), store the controller as a field on the
State and pass that field to BondPage in build(), and implement dispose() on the
State to clean up the controller if it exposes a dispose/close method so the UI
uses a stable controller instance across rebuilds.
Comment on lines
+38
to
+57
| try { | ||
| final qty = int.parse(_qtyController.text.trim()); | ||
| final price = double.parse(_priceController.text.trim()); | ||
| final order = await widget.controller.placeLimitBuyOrder( | ||
| isin: widget.isin, | ||
| quantity: qty, | ||
| price: price, | ||
| ); | ||
|
|
||
| setState(() { | ||
| _message = 'Order placed: ${order.orderId}'; | ||
| }); | ||
| } catch (e) { | ||
| setState(() { | ||
| _message = 'Order failed: $e'; | ||
| }); | ||
| } finally { | ||
| if (mounted) { | ||
| setState(() => _submitting = false); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd bond_order_ticket.dartRepository: SumitUni7/Test-web
Length of output: 124
🏁 Script executed:
cat -n lib/features/bonds/presentation/widgets/bond_order_ticket.dart | head -100Repository: SumitUni7/Test-web
Length of output: 3096
Guard setState after async completion with mounted checks.
setState in success/error paths can run after dispose when the user navigates away mid-request, causing setState() called after dispose. The finally block correctly guards the state update, but the try/catch blocks do not.
🛠️ Proposed lifecycle-safe update
Future<void> _submit() async {
try {
final qty = int.parse(_qtyController.text.trim());
final price = double.parse(_priceController.text.trim());
final order = await widget.controller.placeLimitBuyOrder(
isin: widget.isin,
quantity: qty,
price: price,
);
+ if (!mounted) return;
setState(() {
_message = 'Order placed: ${order.orderId}';
});
} catch (e) {
+ if (!mounted) return;
setState(() {
_message = 'Order failed: $e';
});
} finally {
if (mounted) {
setState(() => _submitting = false);
}
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/bonds/presentation/widgets/bond_order_ticket.dart` around lines
38 - 57, The try/catch blocks update state without checking mounted, which can
throw after dispose; wrap the setState calls that assign _message in both the
success path (after awaiting widget.controller.placeLimitBuyOrder) and the error
path (inside catch) with an if (mounted) guard before calling setState so the
widget is still mounted; keep the existing mounted check in the finally for
_submitting and reference the same members (_qtyController, _priceController,
widget.controller.placeLimitBuyOrder, _message, _submitting) when applying the
guards.
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
1 participant
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.
Motivation
Description
lib/main.dart) and app-levelpubspec.yamlplus a top-levelREADME.mdexplaining how to run the preview app withBondPreviewPage.BondandBondOrderentities, andBondRepositoryinterface underlib/features/bonds/domain.NseCbricsDataSource,MockNseCbricsTransport,BondModel, andBondOrderModelfor JSON mapping and translation of NSE CBRICS-style payloads.BondRepositoryImplandPlaceBondOrderfor basic validation and order submission wiring.BondController, UI pagesBondPageandBondPreviewPage, andBondOrderTicketwidget for placing limit buy orders using the mock transport.lib/features/bonds/README.mddescribing endpoints, wiring examples, and next steps for production hardening.Testing
flutter analyzeand ranflutter test(no tests present), with no blocking analyzer errors reported.Codex Task
Summary by CodeRabbit
Release Notes
New Features
Documentation