Send device identifier to PersonalID #3492

OrangeAndGreen · 2026-01-14T21:01:32Z

https://dimagi.atlassian.net/browse/CCCT-2024

Product Description

No user visible change

Technical Summary

CCCT-2024: Added device identifier string to start_configuration call (manufacturer + model).

Feature Flag

None

Safety Assurance

Safety story

Very simple change with no user-facing effect and no real way of breaking.
Dev tested that the call still succeeds.

Automated test coverage

None

QA Plan

Start configuring a PersonalID account and verify that there is no error when pressing the button to continue after entering the phone number.

… (manufacturer + model).

coderabbitai · 2026-01-14T21:03:46Z

📝 Walkthrough

Walkthrough

This pull request adds device information to the configuration request body in PersonalIdPhoneFragment. A new import for android.os.Build is added, and the configuration request now includes a "device" field populated with the device manufacturer and model in "MANUFACTURER MODEL" format. This enables the backend to receive device identification details when the configuration request is made.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Pm Device configuration api #3117: Adds the same "device" field (Build.MANUFACTURER + Build.MODEL) to the configuration request in PersonalIdPhoneFragment.
Send Firebase user ID to PersonalID #3237: Also modifies PersonalIdPhoneFragment.startConfigurationRequest to add additional key-value pairs to the configuration request body.

Suggested labels

skip-integration-tests, QA Note

Suggested reviewers

shubham1g5
conroy-ricketts
Jignesh-dimagi

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding a device identifier to the PersonalID configuration call.
Description check	✅ Passed	The description covers the required sections with adequate detail: product description, technical summary with ticket link, feature flag status, and safety assurance including testing approach.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch ccct-2024/send_device_string

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8f3c950 and 331ffb6.

📒 Files selected for processing (1)

app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3108
File: app/src/org/commcare/activities/StandardHomeActivityUIController.java:0-0
Timestamp: 2025-06-20T13:21:20.908Z
Learning: User OrangeAndGreen prefers to handle code issues in separate PRs rather than immediately fixing them in the current PR when they are not directly related to the main changes.

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/connect/network/ApiPersonalId.java:0-0
Timestamp: 2025-07-29T14:56:40.681Z
Learning: User OrangeAndGreen prefers to defer ApiPersonalId refactoring (specifically InputStream resource management improvements) to separate PRs rather than mixing them with Connect feature work, even when the code is already in master.

📚 Learning: 2025-04-22T15:48:29.346Z

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3037
File: app/src/org/commcare/connect/ConnectIDManager.java:233-243
Timestamp: 2025-04-22T15:48:29.346Z
Learning: Never instantiate Android Activity classes directly with 'new'. Activities should only be created through the Android framework using Intents.

Applied to files:

app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java

🔇 Additional comments (2)

app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java (2)

7-7: LGTM!

The android.os.Build import is appropriate for accessing device manufacturer and model information.

285-291: LGTM - Device identifier added correctly.

The implementation correctly uses Build.MANUFACTURER and Build.MODEL to create the device identifier string. Both fields are guaranteed non-null by Android (returning "unknown" if unavailable), so no null-safety concerns here.

One minor consideration: some devices may have redundant manufacturer names in the model string (e.g., "Samsung Samsung Galaxy S21"). If the backend needs cleaner data, you could consider trimming the manufacturer from the model if present, but this is optional and may not be necessary depending on backend requirements.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

shubham1g5 · 2026-01-15T13:25:09Z

app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java

        body.put("application_id", requireContext().getPackageName());
        body.put("gps_location", GeoUtils.locationToString(location));
        body.put("cc_device_id", ReportingUtils.getDeviceId());
+        body.put("device", String.format("%s %s", Build.MANUFACTURER, Build.MODEL));


Should we null check these fields before adding ?

My understanding is that these values should never be null as they are initialized by the system with empty strings. But that being said, empty strings shouldn't happen with any mainstream devices but are technically possible. So we should probably at least check for that and easy enough to check for null also.

But now the question is, what do we do if one or both of these values are empty/null? Should I create a localized placeholder like "unidentifiable device"?

But now the question is, what do we do if one or both of these values are empty/null? Should I create a localized placeholder like "unidentifiable device"?

I think we just should not send this value in these cases so that server has a clear indication that mobile was not able to get this detail and can code the notification messaging accordingly (by skipping the device details)

Sounds good. I also heard rumor that some manufacturers include their name in the MODEL field, such that we would show something like "Samsung Samsung Galaxy 8".
So I created a helper for getting a clean model string and only prepend the manufacturer if it isn't already part of the model. The helper returns null if no model info is present (I chose to ignore cases where MANUFACTURER is provided but MODEL isn't), and the Phone fragment only sends the model to the server if it's available.
0dcabec

…vailable (including manufacturer). Only sending model string to server on start_configuration when available.

CCCT-2024: Added device identifier string to start_configuration call…

331ffb6

… (manufacturer + model).

OrangeAndGreen requested review from Jignesh-dimagi, conroy-ricketts and shubham1g5 January 14, 2026 21:01

OrangeAndGreen self-assigned this Jan 14, 2026

shubham1g5 reviewed Jan 15, 2026

View reviewed changes

conroy-ricketts previously approved these changes Jan 15, 2026

View reviewed changes

Added helper method for getting clean device model description when a…

0dcabec

…vailable (including manufacturer). Only sending model string to server on start_configuration when available.

OrangeAndGreen dismissed conroy-ricketts’s stale review via 0dcabec January 15, 2026 19:50

OrangeAndGreen requested review from conroy-ricketts and shubham1g5 January 15, 2026 19:53

Combined a couple ifs.

0d606bf

shubham1g5 approved these changes Jan 15, 2026

View reviewed changes

conroy-ricketts approved these changes Jan 15, 2026

View reviewed changes

OrangeAndGreen merged commit a406272 into master Jan 16, 2026
7 checks passed

OrangeAndGreen deleted the ccct-2024/send_device_string branch January 16, 2026 14:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Send device identifier to PersonalID #3492

Send device identifier to PersonalID #3492

Uh oh!

OrangeAndGreen commented Jan 14, 2026

Uh oh!

coderabbitai bot commented Jan 14, 2026

Walkthrough

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Uh oh!

shubham1g5 Jan 15, 2026

Uh oh!

OrangeAndGreen Jan 15, 2026

Uh oh!

shubham1g5 Jan 15, 2026

Uh oh!

OrangeAndGreen Jan 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Send device identifier to PersonalID #3492

Send device identifier to PersonalID #3492

Uh oh!

Conversation

OrangeAndGreen commented Jan 14, 2026

Product Description

Technical Summary

Feature Flag

Safety Assurance

Safety story

Automated test coverage

QA Plan

Uh oh!

coderabbitai bot commented Jan 14, 2026

Walkthrough

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Uh oh!

shubham1g5 Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

OrangeAndGreen Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

shubham1g5 Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

OrangeAndGreen Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants