Pulling in latest model changes for Connect #3084
Conversation
📝 WalkthroughWalkthroughThis change introduces two new data model classes: Sequence Diagram(s)sequenceDiagram
participant Client
participant ConnectAppRecord
participant ConnectDeliveryDetails
participant ConnectDeliveryPaymentSummaryInfo
Client->>ConnectAppRecord: setName(), setDescription(), setOrganization(), setLearnModules(), setLastUpdate()
Client->>ConnectAppRecord: getName(), getDescription(), getOrganization(), getLearnModules(), getInstallUrl(), getPassingScore()
Client->>ConnectDeliveryDetails: setDeliveryName(), setApprovedCount(), setPendingCount(), setTotalAmount(), setRemainingDays(), setApprovedPercentage(), setUnitId()
Client->>ConnectDeliveryDetails: getDeliveryName(), getApprovedCount(), getPendingCount(), getTotalAmount(), getRemainingDays(), getApprovedPercentage(), getUnitId()
Client->>ConnectDeliveryPaymentSummaryInfo: setPaymentUnitName(), setPaymentUnitAmount(), setPaymentUnitMaxDaily()
Client->>ConnectDeliveryPaymentSummaryInfo: getPaymentUnitName(), getPaymentUnitAmount(), getPaymentUnitMaxDaily()
Possibly related PRs
Suggested labels
Suggested reviewers
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (12)
app/src/org/commcare/android/database/connect/models/ConnectDeliveryPaymentSummaryInfo.java (7)
1-3: Add class-level JavaDoc.Providing a brief description of the class’s responsibility improves maintainability and IDE support.
/** * Represents a summary of payment information for a specific delivery unit, * including unit name, amount, and maximum daily limit. */
4-7: Annotate fields for null-safety or immutability.Consider marking
paymentUnitNameas@NonNull(or usingObjects.requireNonNullin the constructor) to enforce non-null usage. Alternatively, declare fields asfinaland remove setters to make the class immutable if mutation is not required.
8-12: Add input validation in constructor.Ensure constructor parameters are validated to avoid invalid state (e.g., null names or negative amounts). For example:
public ConnectDeliveryPaymentSummaryInfo(String paymentUnitName, int paymentUnitAmount, int paymentUnitMaxDaily) { + Objects.requireNonNull(paymentUnitName, "paymentUnitName cannot be null"); + if (paymentUnitAmount < 0 || paymentUnitMaxDaily < 0) { + throw new IllegalArgumentException("Amounts must be non-negative"); + } this.paymentUnitName = paymentUnitName; this.paymentUnitAmount = paymentUnitAmount; this.paymentUnitMaxDaily = paymentUnitMaxDaily; }(Note: add
import java.util.Objects;if usingrequireNonNull.)
15-21: Enforce non-null in setter.In
setPaymentUnitName, validate null input or annotate the parameter to maintain consistent object state. For example:public void setPaymentUnitName(String paymentUnitName) { + Objects.requireNonNull(paymentUnitName, "paymentUnitName cannot be null"); this.paymentUnitName = paymentUnitName; }
23-29: Validate non-negative in setter.Add guards in
setPaymentUnitAmountto prevent negative values:public void setPaymentUnitAmount(int paymentUnitAmount) { + if (paymentUnitAmount < 0) { + throw new IllegalArgumentException("paymentUnitAmount must be non-negative"); + } this.paymentUnitAmount = paymentUnitAmount; }
31-37: Validate non-negative daily maximum in setter.Ensure
paymentUnitMaxDailycannot be set to a negative number:public void setPaymentUnitMaxDaily(int paymentUnitMaxDaily) { + if (paymentUnitMaxDaily < 0) { + throw new IllegalArgumentException("paymentUnitMaxDaily must be non-negative"); + } this.paymentUnitMaxDaily = paymentUnitMaxDaily; }
1-38: Implementequals(),hashCode(), andtoString().Overriding these methods can be valuable for using instances in collections, logging, and testing:
+@Override +public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof ConnectDeliveryPaymentSummaryInfo)) return false; + ConnectDeliveryPaymentSummaryInfo that = (ConnectDeliveryPaymentSummaryInfo) o; + return paymentUnitAmount == that.paymentUnitAmount + && paymentUnitMaxDaily == that.paymentUnitMaxDaily + && Objects.equals(paymentUnitName, that.paymentUnitName); +} + +@Override +public int hashCode() { + return Objects.hash(paymentUnitName, paymentUnitAmount, paymentUnitMaxDaily); +} + +@Override +public String toString() { + return "ConnectDeliveryPaymentSummaryInfo{" + + "paymentUnitName='" + paymentUnitName + '\'' + + ", paymentUnitAmount=" + paymentUnitAmount + + ", paymentUnitMaxDaily=" + paymentUnitMaxDaily + + '}'; +}Don’t forget to import
java.util.Objects.app/src/org/commcare/android/database/connect/models/ConnectDeliveryDetails.java (5)
3-4: Add class-level Javadoc.Provide a concise description of
ConnectDeliveryDetails, its role in the Connect feature, and any serialization/ORM usage expectations to improve maintainability.
4-10: Consider stronger types for monetary and percentage fields.
totalAmountis currently aString, which can lead to parsing inconsistencies; consider usingBigDecimal(or a dedicated money type) for safe arithmetic.approvedPercentageis adoublefield; if it represents a percentage (0–100), you may also validate its range in setter/constructor.
12-14: Document or remove the no-args constructor.If the default constructor exists solely for ORM/serialization frameworks, annotate or comment its purpose. Otherwise, consider removing it to enforce immutability.
16-24: Add input validation in constructor.Guard against invalid state by checking for null/empty names and non-negative counts:
if (deliveryName == null || deliveryName.isEmpty()) { throw new IllegalArgumentException("deliveryName cannot be null or empty"); } if (approvedCount < 0 || pendingCount < 0 || remainingDays < 0) { throw new IllegalArgumentException("Counts and remainingDays cannot be negative"); }
1-81: Overrideequals(),hashCode(), andtoString().Implementing these methods (or using
Objects/IDE-generated stubs) will improve debugging, logging, and collection usage:@Override public boolean equals(Object o) { /* ... */ } @Override public int hashCode() { /* ... */ } @Override public String toString() { /* ... */ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/src/org/commcare/android/database/connect/models/ConnectAppRecord.java(1 hunks)app/src/org/commcare/android/database/connect/models/ConnectDeliveryDetails.java(1 hunks)app/src/org/commcare/android/database/connect/models/ConnectDeliveryPaymentSummaryInfo.java(1 hunks)app/src/org/commcare/android/database/connect/models/ConnectJobAssessmentRecord.java(0 hunks)app/src/org/commcare/android/database/connect/models/ConnectJobDeliveryFlagRecord.java(1 hunks)app/src/org/commcare/android/database/connect/models/ConnectJobLearningRecord.java(0 hunks)app/src/org/commcare/android/database/connect/models/ConnectJobPaymentRecord.java(0 hunks)app/src/org/commcare/android/database/connect/models/ConnectJobPaymentRecordV3.java(0 hunks)app/src/org/commcare/android/database/connect/models/ConnectJobRecord.java(2 hunks)app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java(1 hunks)app/src/org/commcare/connect/ConnectIDManager.java(1 hunks)
💤 Files with no reviewable changes (4)
- app/src/org/commcare/android/database/connect/models/ConnectJobAssessmentRecord.java
- app/src/org/commcare/android/database/connect/models/ConnectJobPaymentRecord.java
- app/src/org/commcare/android/database/connect/models/ConnectJobLearningRecord.java
- app/src/org/commcare/android/database/connect/models/ConnectJobPaymentRecordV3.java
🧰 Additional context used
🧠 Learnings (1)
app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java (1)
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#2912
File: app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java:66-71
Timestamp: 2025-01-21T17:28:09.007Z
Learning: The ConnectUserRecord class in CommCare Android uses @Persisting annotations with sequential indices for field persistence, and @MetaField annotations for fields that have corresponding META constants. Fields should be documented with Javadoc comments explaining their purpose, format requirements, and relationships with other fields.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (9)
app/src/org/commcare/android/database/connect/models/ConnectJobRecord.java (1)
17-17: Duplicate import removal approved
The duplicateimport java.text.SimpleDateFormat;has been correctly removed, cleaning up the import list without impacting functionality.app/src/org/commcare/android/database/connect/models/ConnectJobDeliveryFlagRecord.java (1)
67-67: Add trailing newline for POSIX compliance.Including a newline at end-of-file prevents potential warnings from linters and aligns with POSIX conventions.
app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java (2)
218-219: Method return type simplifiedThe method now returns the raw token string rather than an AuthInfo.TokenAuth object. This simplifies the interface and moves token validation responsibility to the caller.
222-223: LGTM: Minor formatting changeThe method formatting has been updated but maintains the same functionality.
app/src/org/commcare/connect/ConnectIDManager.java (1)
243-243:Details
✅ Verification successful
Updated token construction to match ConnectUserRecord changes
The method now correctly constructs an AuthInfo.TokenAuth object from the raw token string returned by ConnectUserRecord.getConnectToken(). This aligns with the changes made to the ConnectUserRecord class.
🏁 Script executed:
#!/bin/bash # Check for any other places in the codebase that might rely on the old token behavior echo "Checking for other token usage patterns that might be affected by the ConnectUserRecord.getConnectToken() change..." # Look for places where getConnectToken is used rg -B 1 -A 3 "getConnectToken\(\)" --type javaLength of output: 3423
TokenAuth instantiation aligns with ConnectUserRecord changes; no other impacts found
- Verified via ripgrep that the only raw token usage (
user.getConnectToken()) occurs in ConnectIDManager, with ConnectSsoHelper simply consuming the resulting AuthInfo.TokenAuth.- No other call sites rely on the previous token behavior.
Approving these changes.
app/src/org/commcare/android/database/connect/models/ConnectAppRecord.java (4)
119-125: Accessors fordescriptionare correct
ThegetDescription()andsetDescription(...)methods properly encapsulate thedescriptionfield without issues.
127-133: Accessors fororganizationfollow convention
ThegetOrganization()andsetOrganization(...)methods correctly handle theorganizationfield.
135-141: Accessors fornameare implemented correctly
The getter and setter fornameadhere to the established JavaBean pattern.
143-145:getPassingScore()is unchanged and correct
Moving it to the bottom is purely organizational; the method itself remains accurate.
| public ConnectDeliveryDetails(String deliveryName, int approvedCount, int pendingCount, String totalAmount, long remainingDays, long approvedPercentage, int unitId) { | ||
| this.deliveryName = deliveryName; | ||
| this.approvedCount = approvedCount; | ||
| this.pendingCount = pendingCount; | ||
| this.remainingDays = remainingDays; | ||
| this.totalAmount = totalAmount; | ||
| this.approvedPercentage = approvedPercentage; | ||
| this.unitId = unitId; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix approvedPercentage parameter type in the all-args constructor.
The constructor parameter is declared as long but assigns to a double field, risking precision loss. Change it to double to match the field:
- public ConnectDeliveryDetails(String deliveryName, int approvedCount, int pendingCount,
- String totalAmount, long remainingDays, long approvedPercentage,
- int unitId) {
+ public ConnectDeliveryDetails(String deliveryName, int approvedCount, int pendingCount,
+ String totalAmount, long remainingDays, double approvedPercentage,
+ int unitId) {📝 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.
| public ConnectDeliveryDetails(String deliveryName, int approvedCount, int pendingCount, String totalAmount, long remainingDays, long approvedPercentage, int unitId) { | |
| this.deliveryName = deliveryName; | |
| this.approvedCount = approvedCount; | |
| this.pendingCount = pendingCount; | |
| this.remainingDays = remainingDays; | |
| this.totalAmount = totalAmount; | |
| this.approvedPercentage = approvedPercentage; | |
| this.unitId = unitId; | |
| } | |
| public ConnectDeliveryDetails(String deliveryName, int approvedCount, int pendingCount, | |
| String totalAmount, long remainingDays, double approvedPercentage, | |
| int unitId) { | |
| this.deliveryName = deliveryName; | |
| this.approvedCount = approvedCount; | |
| this.pendingCount = pendingCount; | |
| this.remainingDays = remainingDays; | |
| this.totalAmount = totalAmount; | |
| this.approvedPercentage = approvedPercentage; | |
| this.unitId = unitId; | |
| } |
🤖 Prompt for AI Agents (early access)
In
app/src/org/commcare/android/database/connect/models/ConnectDeliveryDetails.java
lines 16 to 24, the constructor parameter approvedPercentage is declared as long
but assigned to a double field, causing a type mismatch and potential precision
loss. Change the approvedPercentage parameter type from long to double in the
constructor signature to match the field type and ensure correct assignment.
app/src/org/commcare/android/database/connect/models/ConnectAppRecord.java
Show resolved
Hide resolved
Product Description
Pulls in Connect Models from #3048
Labels and Review