marked unused database values as depreciated #3225

pm-dimagi · 2025-06-30T19:58:22Z

Product Description

https://dimagi.atlassian.net/browse/CCCT-1241
Marked secondary pone number and related fields depreciated

Technical Summary

Feature Flag

Safety Assurance

Safety story

Automated test coverage

QA Plan

Labels and Review

Do we need to enhance the manual QA test coverage ? If yes, the "QA Note" label is set correctly
Does the PR introduce any major changes worth communicating ? If yes, the "Release Note" label is set and a "Release Note" is specified in PR description.
Risk label is set correctly
The set of people pinged as reviewers is appropriate for the level of risk of the change

coderabbitai · 2025-06-30T20:00:11Z

📝 Walkthrough

Walkthrough

This change removes the getter and setter methods for the alternatePhone field from both ConnectUserRecord and ConnectUserRecordV14 classes, and marks the alternatePhone, secondaryPhoneVerified, and verifySecondaryPhoneByDate fields as deprecated. The shouldForcePassword() method is also removed from ConnectUserRecord. In addition, the storeAlternatePhone method is removed from the PhoneNumberHelper class. The assignment of alternatePhone in the fromV14 method is replaced with an empty string. No new logic or features are introduced.

Possibly related PRs

remove secondry number handling #3194: Removes or deprecates code related to handling secondary or alternate phone numbers, including associated methods and fields.
Code clean up for Secondry Phone number #3193: Focuses on deprecating and removing code related to alternate and secondary phone number fields and their associated methods in ConnectUserRecord classes.

Suggested reviewers

shubham1g5
OrangeAndGreen

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e5c2345 and be87743.

📒 Files selected for processing (3)

app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java (3 hunks)
app/src/org/commcare/android/database/connect/models/ConnectUserRecordV14.java (2 hunks)
app/src/org/commcare/utils/PhoneNumberHelper.java (0 hunks)

💤 Files with no reviewable changes (1)

app/src/org/commcare/utils/PhoneNumberHelper.java

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: shubham1g5
PR: dimagi/commcare-android#0
File: :0-0
Timestamp: 2025-05-08T11:08:18.530Z
Learning: PR #3048 "Phase 4 Connect PR" introduces a substantial feature called "Connect" to the CommCare Android app, which includes messaging, job management, delivery tracking, payment processing, authentication flows, and learning modules. It follows a modern architecture using Navigation Components with three navigation graphs, segregated business logic in Manager classes, and proper database persistence.

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.

Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#2912
File: app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java:81-89
Timestamp: 2025-01-21T17:27:58.754Z
Learning: In the CommCare Android codebase, use org.jetbrains.annotations.NotNull for null-safety annotations. Empty strings are acceptable for unspecified values, but null values should be prevented using @NotNull.

Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#2912
File: app/src/org/commcare/fragments/connectId/ConnectIdPasswordVerificationFragment.java:214-215
Timestamp: 2025-01-21T18:19:05.799Z
Learning: In ConnectIdPasswordVerificationFragment, when creating a ConnectUserRecord, it's acceptable for payment information (paymentName and paymentPhone) to be empty strings if the server response doesn't include payment info in the CONNECT_PAYMENT_INFO field.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectLinkedAppRecordV3.java:44-68
Timestamp: 2025-01-23T21:38:24.443Z
Learning: The ConnectLinkedAppRecord classes are intentionally designed with simple getters/setters without additional validation or defensive copies.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectUserRecordV5.java:72-76
Timestamp: 2025-01-26T14:32:09.266Z
Learning: In ConnectUserRecordV5, initializing lastPasswordDate and connectTokenExpiration to new Date() in the constructor is intentional.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectLinkedAppRecordV8.java:107-107
Timestamp: 2025-01-28T08:41:32.380Z
Learning: When upgrading from ConnectLinkedAppRecordV3 to ConnectLinkedAppRecordV8, the connectIdLinked field is intentionally set to true as part of the database migration strategy since this field was not available from the server in V3 and is introduced in V8.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectLinkedAppRecordV9.java:44-44
Timestamp: 2025-01-26T14:33:04.310Z
Learning: The password field in ConnectLinkedAppRecordV9 is intentionally stored in plain text as it is not linked to any important update and doesn't require encryption.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectJobRecordV2.java:120-121
Timestamp: 2025-01-26T19:02:16.066Z
Learning: The date fields (projectEndDate, lastUpdate, lastLearnUpdate, lastDeliveryUpdate) in ConnectJobRecordV2 and its newer versions are guaranteed to be non-null through the application's initialization logic and data flow. The newer version (ConnectJobRecord) explicitly initializes these in the constructor, while V2 maintains this guarantee through database upgrades and JSON parsing.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectLinkedAppRecordV3.java:25-39
Timestamp: 2025-01-23T21:38:38.376Z
Learning: The password field in ConnectLinkedAppRecordV3 is intentionally stored without encryption as per the team's design decision.

app/src/org/commcare/android/database/connect/models/ConnectUserRecordV14.java (10)

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.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectJobRecordV2.java:120-121
Timestamp: 2025-01-26T19:02:16.066Z
Learning: The date fields (projectEndDate, lastUpdate, lastLearnUpdate, lastDeliveryUpdate) in ConnectJobRecordV2 and its newer versions are guaranteed to be non-null through the application's initialization logic and data flow. The newer version (ConnectJobRecord) explicitly initializes these in the constructor, while V2 maintains this guarantee through database upgrades and JSON parsing.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectLinkedAppRecordV3.java:1-68
Timestamp: 2025-01-23T21:38:17.427Z
Learning: The versioned database model classes (e.g., ConnectLinkedAppRecordV3) intentionally maintain their original design patterns, including their security implementation, to preserve backward compatibility.

Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3040
File: app/src/org/commcare/android/database/connect/models/ConnectJobDeliveryFlagRecord.java:39-55
Timestamp: 2025-04-18T20:13:29.655Z
Learning: In the CommCare Android Connect feature, the JSON object passed to `ConnectJobDeliveryFlagRecord.fromJson()` method should never be null based on the implementation design.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectLinkedAppRecordV8.java:107-107
Timestamp: 2025-01-28T08:41:32.380Z
Learning: When upgrading from ConnectLinkedAppRecordV3 to ConnectLinkedAppRecordV8, the connectIdLinked field is intentionally set to true as part of the database migration strategy since this field was not available from the server in V3 and is introduced in V8.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectJobRecordV2.java:120-121
Timestamp: 2025-01-26T19:02:16.066Z
Learning: The date fields in ConnectJobRecordV2 (projectEndDate, lastUpdate, lastLearnUpdate, lastDeliveryUpdate) are guaranteed to be non-null as part of the core database model design, despite not having explicit initialization in the constructor.

Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#2912
File: app/src/org/commcare/fragments/connectId/ConnectIdPasswordVerificationFragment.java:214-215
Timestamp: 2025-01-21T18:19:05.799Z
Learning: In ConnectIdPasswordVerificationFragment, when creating a ConnectUserRecord, it's acceptable for payment information (paymentName and paymentPhone) to be empty strings if the server response doesn't include payment info in the CONNECT_PAYMENT_INFO field.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectLinkedAppRecordV3.java:44-68
Timestamp: 2025-01-23T21:38:24.443Z
Learning: The ConnectLinkedAppRecord classes are intentionally designed with simple getters/setters without additional validation or defensive copies.

Learnt from: shubham1g5
PR: dimagi/commcare-android#0
File: :0-0
Timestamp: 2025-05-08T11:08:18.530Z
Learning: PR #3048 "Phase 4 Connect PR" introduces a substantial feature called "Connect" to the CommCare Android app, which includes messaging, job management, delivery tracking, payment processing, authentication flows, and learning modules. It follows a modern architecture using Navigation Components with three navigation graphs, segregated business logic in Manager classes, and proper database persistence.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectUserRecordV5.java:72-76
Timestamp: 2025-01-26T14:32:09.266Z
Learning: In ConnectUserRecordV5, initializing lastPasswordDate and connectTokenExpiration to new Date() in the constructor is intentional.

app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java (10)

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.

Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3040
File: app/src/org/commcare/android/database/connect/models/ConnectJobDeliveryFlagRecord.java:39-55
Timestamp: 2025-04-18T20:13:29.655Z
Learning: In the CommCare Android Connect feature, the JSON object passed to `ConnectJobDeliveryFlagRecord.fromJson()` method should never be null based on the implementation design.

Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#2912
File: app/src/org/commcare/fragments/connectId/ConnectIdPasswordVerificationFragment.java:214-215
Timestamp: 2025-01-21T18:19:05.799Z
Learning: In ConnectIdPasswordVerificationFragment, when creating a ConnectUserRecord, it's acceptable for payment information (paymentName and paymentPhone) to be empty strings if the server response doesn't include payment info in the CONNECT_PAYMENT_INFO field.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectLinkedAppRecordV8.java:107-107
Timestamp: 2025-01-28T08:41:32.380Z
Learning: When upgrading from ConnectLinkedAppRecordV3 to ConnectLinkedAppRecordV8, the connectIdLinked field is intentionally set to true as part of the database migration strategy since this field was not available from the server in V3 and is introduced in V8.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectUserRecordV5.java:72-76
Timestamp: 2025-01-26T14:32:09.266Z
Learning: In ConnectUserRecordV5, initializing lastPasswordDate and connectTokenExpiration to new Date() in the constructor is intentional.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectLinkedAppRecordV3.java:44-68
Timestamp: 2025-01-23T21:38:24.443Z
Learning: The ConnectLinkedAppRecord classes are intentionally designed with simple getters/setters without additional validation or defensive copies.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectJobRecordV2.java:120-121
Timestamp: 2025-01-26T19:02:16.066Z
Learning: The date fields (projectEndDate, lastUpdate, lastLearnUpdate, lastDeliveryUpdate) in ConnectJobRecordV2 and its newer versions are guaranteed to be non-null through the application's initialization logic and data flow. The newer version (ConnectJobRecord) explicitly initializes these in the constructor, while V2 maintains this guarantee through database upgrades and JSON parsing.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectLinkedAppRecordV3.java:1-68
Timestamp: 2025-01-23T21:38:17.427Z
Learning: The versioned database model classes (e.g., ConnectLinkedAppRecordV3) intentionally maintain their original design patterns, including their security implementation, to preserve backward compatibility.

Learnt from: shubham1g5
PR: dimagi/commcare-android#2949
File: app/src/org/commcare/fragments/connectId/ConnectIdPasswordVerificationFragment.java:173-247
Timestamp: 2025-03-10T08:16:29.416Z
Learning: In the ConnectIdPasswordVerificationFragment, password comparisons should use MessageDigest.isEqual() rather than equals() to prevent timing attacks, and empty password validation should be implemented before verification attempts.

Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectLinkedAppRecordV9.java:44-44
Timestamp: 2025-01-26T14:33:04.310Z
Learning: The password field in ConnectLinkedAppRecordV9 is intentionally stored in plain text as it is not linked to any important update and doesn't require encryption.

⏰ Context from checks skipped due to timeout of 90000ms (1)

GitHub Check: Lint Code Base

🔇 Additional comments (4)

app/src/org/commcare/android/database/connect/models/ConnectUserRecordV14.java (2)

32-34: Deprecation approach is consistent with database versioning patterns.

The alternatePhone field is properly marked as deprecated while maintaining the @Persisting annotation for backward compatibility.

51-58: Consistent deprecation of secondary phone verification fields.

Both secondaryPhoneVerified and verifySecondaryPhoneByDate fields are properly deprecated with @Deprecated annotations while preserving their persistence and metadata configurations.

app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java (2)

38-40: Proper deprecation of alternatePhone field.

The field is correctly marked as deprecated while maintaining database persistence for compatibility.

199-199: Correct migration strategy for deprecated field.

Setting alternatePhone to an empty string instead of copying from the old record is the appropriate approach when deprecating a field. This prevents carrying forward deprecated data while maintaining database compatibility.

✨ Finishing Touches

📝 Generate Docstrings

🧪 Generate Unit Tests

Create PR with Unit Tests
Post Copyable Unit Tests in Comment
Commit Unit Tests in branch pm_ccct_1241

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.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai auto-generate unit tests to generate unit tests for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

- marked unused database values as depreciated

be87743

pm-dimagi requested review from Jignesh-dimagi, OrangeAndGreen and shubham1g5 June 30, 2025 19:58

pm-dimagi added the skip-integration-tests Skip android tests. label Jun 30, 2025

shubham1g5 approved these changes Jul 1, 2025

View reviewed changes

pm-dimagi merged commit 668e4b3 into master Jul 8, 2025
5 of 7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

marked unused database values as depreciated #3225

marked unused database values as depreciated #3225

Uh oh!

pm-dimagi commented Jun 30, 2025

Uh oh!

coderabbitai bot commented Jun 30, 2025

Walkthrough

Possibly related PRs

Suggested reviewers

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

marked unused database values as depreciated #3225

marked unused database values as depreciated #3225

Uh oh!

Conversation

pm-dimagi commented Jun 30, 2025

Product Description

Technical Summary

Feature Flag

Safety Assurance

Safety story

Automated test coverage

QA Plan

Labels and Review

Uh oh!

coderabbitai bot commented Jun 30, 2025

Walkthrough

Possibly related PRs

Suggested reviewers

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CodeRabbit Configuration File (`.coderabbit.yaml`)