-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
updated attributes , added additionalDetails #586
Conversation
WalkthroughThe changes introduce a new nullable column, Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
packages/digit_data_model/lib/models/entities/attributes.dart (1)
72-72
: LGTM: Addition of additionalDetails fieldThe addition of the
additionalDetails
field, its inclusion in the constructor, and its handling in thecompanion
getter are well-implemented. This change aligns with the PR objectives and provides flexibility for storing additional information.A minor suggestion for improvement:
Consider adding a comment explaining the purpose and expected usage of the
additionalDetails
field. This would help future developers understand its intended use case.Also applies to: 75-75, 118-119
packages/digit_data_model/lib/models/entities/service.mapper.dart (1)
Line range hint
388-420
: LGTM! CopyWith implementation updated correctlyThe
call
method signature and theadditionalDetails
getter in thecopyWith
implementation have been correctly updated to handle the newMap<String, dynamic>?
type foradditionalDetails
. The changes are consistent and necessary for the proper functioning of thecopyWith
method with the new type.Consider simplifying the
additionalDetails
getter implementation using the null-aware operator:- MapCopyWith<$R, String, dynamic, ObjectCopyWith<$R, dynamic, dynamic>>? - get additionalDetails => $value.additionalDetails != null - ? MapCopyWith( - $value.additionalDetails!, - (v, t) => ObjectCopyWith(v, $identity, t), - (v) => call(additionalDetails: v)) - : null; + MapCopyWith<$R, String, dynamic, ObjectCopyWith<$R, dynamic, dynamic>>? + get additionalDetails => $value.additionalDetails?.let((v) => MapCopyWith( + v, + (v, t) => ObjectCopyWith(v, $identity, t), + (v) => call(additionalDetails: v)));This change makes the code more concise and easier to read while maintaining the same functionality.
packages/digit_data_model/lib/models/entities/service.dart (1)
55-55
: Update unit tests to cover 'additionalDetails' fieldWith the change of
additionalDetails
toMap<String, dynamic>?
, please ensure that unit tests are updated or added to cover serialization, deserialization, and any logic involving this field.Would you like assistance in generating unit tests for the
additionalDetails
field?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
apps/health_campaign_field_worker_app/pubspec.lock
is excluded by!**/*.lock
,!**/*.lock
packages/digit_scanner/pubspec.lock
is excluded by!**/*.lock
,!**/*.lock
📒 Files selected for processing (6)
- packages/digit_data_model/lib/data/local_store/sql_store/sql_store.g.dart (24 hunks)
- packages/digit_data_model/lib/data/local_store/sql_store/tables/attributes.dart (1 hunks)
- packages/digit_data_model/lib/models/entities/attributes.dart (8 hunks)
- packages/digit_data_model/lib/models/entities/attributes.mapper.dart (9 hunks)
- packages/digit_data_model/lib/models/entities/service.dart (6 hunks)
- packages/digit_data_model/lib/models/entities/service.mapper.dart (4 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/digit_data_model/lib/data/local_store/sql_store/tables/attributes.dart
🧰 Additional context used
🔇 Additional comments (42)
packages/digit_data_model/lib/data/local_store/sql_store/sql_store.g.dart (24)
25191-25196
: LGTM: New columnadditionalDetails
added correctly.The
additionalDetails
column is properly defined as a nullableString
column, consistent with the existingadditionalFields
column. This addition enhances the data model by allowing for more flexible attribute storage.
25220-25221
: LGTM:additionalDetails
added to$columns
list.The new
additionalDetails
column is correctly added to the$columns
list, ensuring it's included in all relevant database operations.
25344-25349
: LGTM: Validation logic added foradditionalDetails
.The validation logic for the
additionalDetails
column is correctly implemented, following the same pattern as other columns. This ensures data integrity when working with this new field.
25403-25404
: LGTM: Data mapping updated foradditionalDetails
.The
additionalDetails
field is correctly added to the data mapping logic, ensuring proper reading of the 'additional_details' field from the database.
25437-25437
: LGTM:additionalDetails
field added toAttribute
class.The
additionalDetails
field is correctly added to theAttribute
class as a nullable String, consistent with the database schema changes.Also applies to: 25460-25461
25531-25533
: LGTM:toColumns
method updated foradditionalDetails
.The
toColumns
method is correctly updated to include theadditionalDetails
field, properly handling null values.
25598-25600
: LGTM:copyWith
method updated foradditionalDetails
.The
copyWith
method is correctly updated to include theadditionalDetails
field, properly handling null values and absent cases.
25631-25632
: LGTM:fromJson
method updated foradditionalDetails
.The
fromJson
method is correctly updated to deserialize theadditionalDetails
field from JSON, maintaining consistency with other fields.
25661-25661
: LGTM:toJson
method updated foradditionalDetails
.The
toJson
method is correctly updated to serialize theadditionalDetails
field to JSON, maintaining consistency with other fields.
25687-25688
: LGTM:AttributesCompanion
constructor updated foradditionalDetails
.The
AttributesCompanion
constructor is correctly updated to include theadditionalDetails
field with a defaultValue.absent()
, consistent with other nullable fields.
25731-25733
: LGTM:Attribute
classcopyWith
method updated foradditionalDetails
.The
copyWith
method in theAttribute
class is correctly updated to include theadditionalDetails
field, maintaining consistency with other fields.
25759-25760
: LGTM:toString
method updated foradditionalDetails
.The
toString
method is correctly updated to include theadditionalDetails
field in the string representation of the object.
25788-25789
: LGTM:hashCode
getter updated foradditionalDetails
.The
hashCode
getter is correctly updated to include theadditionalDetails
field in the hash code calculation, maintaining consistency with other fields.
25816-25817
: LGTM: Equality operator updated foradditionalDetails
.The equality operator is correctly updated to include the
additionalDetails
field in the comparison, ensuring proper equality checks.
25843-25843
: LGTM:AttributesCompanion
class updated foradditionalDetails
.The
AttributesCompanion
class is correctly updated to include theadditionalDetails
field as aValue<String?>
, consistent with other nullable fields.
25868-25868
: LGTM:AttributesCompanion
constructor updated foradditionalDetails
.The
AttributesCompanion
constructor is correctly updated to include theadditionalDetails
field with a default value ofValue.absent()
, consistent with other optional fields.
25894-25894
: LGTM:AttributesCompanion.insert
constructor updated foradditionalDetails
.The
AttributesCompanion.insert
constructor is correctly updated to include theadditionalDetails
field with a default value ofValue.absent()
, consistent with other optional fields.
25920-25920
: LGTM:custom
method updated foradditionalDetails
.The
custom
method is correctly updated to include anExpression<String>?
parameter foradditionalDetails
, consistent with other nullable String fields.
25948-25948
: LGTM:RawValuesInsertable
updated foradditionalDetails
incustom
method.The
RawValuesInsertable
map in thecustom
method is correctly updated to include theadditionalDetails
field when it's not null, consistent with other fields.
25976-25976
: LGTM:copyWith
method inAttributesCompanion
updated foradditionalDetails
.The
copyWith
method inAttributesCompanion
is correctly updated to include aValue<String?>?
parameter foradditionalDetails
, consistent with other optional, nullable fields.
26001-26001
: LGTM:copyWith
method implementation inAttributesCompanion
updated foradditionalDetails
.The
copyWith
method implementation inAttributesCompanion
is correctly updated to include theadditionalDetails
field, maintaining consistency with other fields.
26075-26077
: LGTM:toColumns
method inAttributesCompanion
updated foradditionalDetails
.The
toColumns
method inAttributesCompanion
is correctly updated to include logic for handling theadditionalDetails
field, consistent with other fields.
26109-26109
: LGTM:toString
method inAttributesCompanion
updated foradditionalDetails
.The
toString
method inAttributesCompanion
is correctly updated to include theadditionalDetails
field in the string representation, maintaining consistency with other fields.
Line range hint
25191-26109
: Summary: Successful integration ofadditionalDetails
fieldThe changes to add the
additionalDetails
field to theAttributes
table and related classes have been implemented consistently and correctly throughout the file. This includes:
- Adding the column definition
- Updating data mapping and validation logic
- Modifying the
Attribute
class and its methods- Updating the
AttributesCompanion
class and its methodsThese changes enhance the data model by allowing for additional, flexible attribute storage. The implementation follows best practices and maintains consistency with existing code patterns.
packages/digit_data_model/lib/models/entities/attributes.dart (6)
2-2
: LGTM: Import added for JSON encodingThe addition of
import 'dart:convert';
is appropriate as it's used later in the file for JSON encoding of theadditionalDetails
field.
14-15
: LGTM: Improved formatting for AttributesSearchModelThe reformatting of the
AttributesSearchModel
class declaration improves readability without affecting functionality.
Line range hint
25-38
: LGTM: Improved formatting for AttributesSearchModel constructorThe reformatting of the
AttributesSearchModel
constructor improves readability and maintains consistent styling without affecting functionality.
Line range hint
41-52
: LGTM: Improved formatting for AttributesSearchModel.ignoreDeleted constructorThe reformatting of the
AttributesSearchModel.ignoreDeleted
constructor improves readability and maintains consistent styling without affecting functionality.
124-125
: LGTM: Improved formatting for AttributesAdditionalFieldsThe reformatting of the
AttributesAdditionalFields
class declaration improves readability and maintains consistent styling with other classes in the file.
Line range hint
1-131
: Overall assessment: Changes improve code structure and align with PR objectivesThe modifications to this file successfully introduce the
additionalDetails
field to theAttributesModel
class, aligning with the PR objectives. The changes are well-implemented, maintaining consistency with the existing code structure. The formatting improvements enhance readability throughout the file.Key points:
- Addition of
additionalDetails
field provides flexibility for storing extra information.- Constructor and
companion
getter updates correctly handle the new field.- Formatting changes improve overall code readability.
These changes enhance the data model's capability to store and manage additional details, which should improve the overall functionality of the system.
packages/digit_data_model/lib/models/entities/service.mapper.dart (2)
368-369
: LGTM! CopyWith method updated correctlyThe
copyWith
method foradditionalDetails
has been properly updated to useMapCopyWith
, which is consistent with the newMap<String, dynamic>?
type. This ensures that thecopyWith
functionality works correctly with the new Map type foradditionalDetails
.
232-234
: LGTM! Verify impact of type change for additionalDetailsThe change from
String?
toMap<String, dynamic>?
foradditionalDetails
is a good improvement, allowing for more structured and flexible additional data. This change enhances the model's capability to store complex information.To ensure this change doesn't introduce any breaking changes, please run the following script to check for any usage of
additionalDetails
as a String in the codebase:If the script returns any results, those occurrences may need to be updated to handle the new Map type.
packages/digit_data_model/lib/models/entities/service.dart (1)
2-2
: Appropriate import added for JSON encodingThe addition of
import 'dart:convert';
is necessary for usingjsonEncode
.packages/digit_data_model/lib/models/entities/attributes.mapper.dart (9)
245-249
: Addition ofadditionalDetails
field is correctly implemented.The
additionalDetails
field has been added appropriately, including the getter method and field definition. The implementation aligns with existing code patterns and ensures that the field is correctly mapped.
307-307
:additionalDetails
field included infields
map correctly.Including
_f$additionalDetails
in thefields
map ensures that theadditionalDetails
field is properly recognized and handled during serialization and deserialization processes.
338-338
: Deserialization ofadditionalDetails
in_instantiate
method is correct.The line
additionalDetails: data.dec(_f$additionalDetails),
correctly decodes theadditionalDetails
field, ensuring it is properly instantiated when creating anAttributesModel
object from map data.
412-413
: AddedadditionalDetails
getter for copy functionality.The getter for
additionalDetails
in theAttributesModelCopyWith
interface allows for copying and chaining operations that include the new field. This enhances the flexibility of the copy functionality.
424-425
: Updatedcall
method signature to includeadditionalDetails
.Including
additionalDetails
in the parameters of thecall
method ensures that this field can be modified during copy operations, maintaining consistency with the other fields.
454-461
: Implementation ofadditionalDetails
copy logic is correct.The logic for copying
additionalDetails
handles the nullableMap<String, dynamic>
correctly usingMapCopyWith
, enabling proper copying of map entries when the field is not null.
481-482
: Default parameters added foradditionalDetails
incall
method.Providing a default value for
additionalDetails
in thecall
method maintains consistency with the handling of optional parameters and ensures that the field remains optional during copy operations.
499-499
: Conditional field inclusion foradditionalDetails
.The conditional check
if (additionalDetails != $none)
ensures thatadditionalDetails
is only included in theFieldCopyWithData
when explicitly provided, preventing unintended overwrites.
521-522
: Assignment ofadditionalDetails
in$make
method is correct.Assigning
additionalDetails
usingdata.get(#additionalDetails, or: $value.additionalDetails)
correctly retrieves the new value or retains the existing one, ensuring accurate object creation.
@@ -92,21 +95,21 @@ class ServiceModel extends EntityModel with ServiceModelMappable { | |||
serviceDefId: Value(serviceDefId), | |||
isActive: Value(isActive), | |||
accountId: Value(accountId), | |||
additionalDetails: Value(additionalDetails), | |||
additionalDetails: Value(jsonEncode(additionalDetails)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle null additionalDetails
to prevent storing 'null'
string
When additionalDetails
is null
, jsonEncode(additionalDetails)
returns the string 'null'
, which may not be desired. To avoid storing 'null'
as a string in the database, consider handling null values explicitly.
Apply this diff to handle null
values appropriately:
-additionalDetails: Value(jsonEncode(additionalDetails)),
+additionalDetails: additionalDetails != null ? Value(jsonEncode(additionalDetails)) : const Value(null),
📝 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.
additionalDetails: Value(jsonEncode(additionalDetails)), | |
additionalDetails: additionalDetails != null ? Value(jsonEncode(additionalDetails)) : const Value(null), |
static const schemaName = 'Service'; | ||
|
||
final String? id; | ||
final String clientId; | ||
final String? serviceDefId; | ||
final bool? isActive; | ||
final String? accountId; | ||
final String? additionalDetails; | ||
final Map<String, dynamic>? additionalDetails; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Issues Found: Inconsistent Handling of additionalDetails
After Type Change
Some usages of additionalDetails
in the codebase still treat it as a string instead of the updated Map<String, dynamic>?
type. For example:
-
File:
packages/referral_reconciliation/lib/pages/create_referral/record_reason_checklist.dart
additionalDetails: null
-
File:
packages/attendance_management/lib/pages/session_select.dart
if (selectedRegister.additionalDetails?["sessions"].toValue() == 2)
-
File:
apps/health_campaign_field_worker_app/lib/utils/extensions/context_utility.dart
projectState.selectedProject?.additionalDetails?.projectType?.cycles
Action Required:
- Review and update all instances where
additionalDetails
is compared to or treated as a string. - Ensure that all usages correctly handle
additionalDetails
as aMap<String, dynamic>?
, including proper null checks and key-value accesses.
🔗 Analysis chain
Ensure 'additionalDetails' type change is reflected throughout the codebase
By changing additionalDetails
to Map<String, dynamic>?
, please verify that all usages of additionalDetails
in the codebase are updated to handle the new type.
Run the following script to identify usages of additionalDetails
:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all usages of 'additionalDetails' reflect the new type.
# Test: Search for 'additionalDetails' declarations and usages.
# Expect: All occurrences are updated to `Map<String, dynamic>?`.
rg --type dart --word-regexp 'additionalDetails' -A 5 -B 5
Length of output: 251931
No description provided.