Skip to content

Code clean up for Secondry Phone number #3193

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.

Sign up for GitHub

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

Merged
shubham1g5 merged 6 commits into from
Jun 12, 2025
Merged

Code clean up for Secondry Phone number #3193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c3a00e3
mark secondry phone number related fields deprecated and clean any usage
shubham1g5 Jun 12, 2025
de1ebd5
remove handleRedirection and call beginRegistration directly instead
shubham1g5 Jun 12, 2025
9c85669
remove unused handling from onActivityResult
shubham1g5 Jun 12, 2025
5bc9ac0
remove beginRegistration as nav graph should take care of starting fr…
shubham1g5 Jun 12, 2025
38f630e
remove relevant xml for secondry no update
shubham1g5 Jun 12, 2025
98e86c3
Merge branch 'commcare_2.57' into codeCleanUpAndFixes
shubham1g5 Jun 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 0 additions & 68 deletions app/res/layout-land/home_screen.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,74 +13,6 @@
android:paddingRight="@dimen/content_start"
tools:viewBindingIgnore="true">

<androidx.cardview.widget.CardView
android:id="@+id/connect_alert_tile"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="16dp"
android:layout_marginTop="10dp"
android:layout_marginEnd="16dp"
android:layout_marginBottom="2dp"
android:clickable="true"
android:focusable="true"
android:visibility="gone"
app:cardBackgroundColor="@color/white"
app:cardCornerRadius="10dp"
app:cardElevation="5dp"
app:layout_constraintStart_toStartOf="parent"
>

<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:padding="10dp"
android:visibility="visible">

<ImageView
android:id="@+id/connect_secondary_phone_icon"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:src="@drawable/phone_heart"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
app:tint="@color/black" />

<TextView
android:id="@+id/connect_phone_label"
android:layout_width="0dp"
android:layout_height="match_parent"
android:textColor="@color/black"
android:textSize="16sp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toEndOf="@id/connect_secondary_phone_icon"
app:layout_constraintTop_toTopOf="parent" />

<com.google.android.material.button.MaterialButton
android:id="@+id/connect_phone_yes_button"
android:layout_width="wrap_content"
android:layout_height="35dp"
android:layout_marginTop="10dp"
android:text="@string/connect_payment_confirm_yes"
android:textColor="@color/blue"
android:textSize="18sp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintTop_toBottomOf="@id/connect_phone_label" />

<com.google.android.material.button.MaterialButton
style="@style/NegativeButtonStyle"
android:id="@+id/connect_phone_no_button"
android:layout_width="wrap_content"
android:layout_height="35dp"
android:layout_marginTop="10dp"
android:layout_marginEnd="10dp"
android:text="@string/connect_payment_confirm_no"
app:layout_constraintEnd_toStartOf="@id/connect_phone_yes_button"
app:layout_constraintTop_toBottomOf="@id/connect_phone_label" />
</androidx.constraintlayout.widget.ConstraintLayout>

</androidx.cardview.widget.CardView>

<androidx.core.widget.NestedScrollView
android:id="@+id/nsv_home_screen"
android:layout_width="match_parent"
Expand Down
73 changes: 0 additions & 73 deletions app/res/layout/home_screen.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,79 +8,6 @@
android:orientation="vertical"
android:padding="4dp"
tools:viewBindingIgnore="true">
<androidx.cardview.widget.CardView
android:id="@+id/connect_alert_tile"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="16dp"
android:layout_marginTop="10dp"
android:layout_marginEnd="16dp"
android:layout_marginBottom="2dp"
android:clickable="true"
android:focusable="true"
android:visibility="gone"
android:shadowColor="@color/connect_light_grey_transparent"
android:shadowDx="30"
android:shadowDy="30"
android:shadowRadius="50"
app:cardBackgroundColor="@color/white"
app:cardCornerRadius="10dp"
app:cardElevation="5dp"
app:layout_constraintStart_toStartOf="parent"
>

<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:padding="10dp"
android:visibility="visible">

<ImageView
android:id="@+id/connect_secondary_phone_icon"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:src="@drawable/phone_heart"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
app:tint="@color/black" />

<TextView
android:id="@+id/connect_phone_label"
android:layout_width="0dp"
android:layout_height="match_parent"
android:textColor="@color/black"
android:textSize="16sp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toEndOf="@id/connect_secondary_phone_icon"
app:layout_constraintTop_toTopOf="parent" />

<com.google.android.material.button.MaterialButton
android:id="@+id/connect_phone_yes_button"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="@string/connect_payment_confirm_yes"
android:layout_marginTop="10dp"
android:textSize="18sp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintTop_toBottomOf="@id/connect_phone_label" />

<com.google.android.material.button.MaterialButton
style="@style/NegativeButtonStyle"
android:id="@+id/connect_phone_no_button"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginTop="10dp"
android:layout_marginEnd="10dp"
android:text="@string/connect_payment_confirm_no"
android:textSize="18sp"
app:layout_constraintEnd_toStartOf="@id/connect_phone_yes_button"
app:layout_constraintTop_toBottomOf="@id/connect_phone_label"
/>
</androidx.constraintlayout.widget.ConstraintLayout>

</androidx.cardview.widget.CardView>

<androidx.core.widget.NestedScrollView
android:id="@+id/nsv_home_screen"
android:layout_width="match_parent"
Expand Down
7 changes: 0 additions & 7 deletions app/src/org/commcare/activities/StandardHomeActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@

import org.commcare.CommCareApplication;
import org.commcare.CommCareNoficationManager;
import org.commcare.connect.ConnectConstants;
import org.commcare.connect.PersonalIdManager;
import org.commcare.dalvik.R;
import org.commcare.google.services.analytics.AnalyticsParamValue;
import org.commcare.google.services.analytics.FirebaseAnalyticsUtil;
Expand Down Expand Up @@ -49,7 +47,6 @@ public void onCreateSessionSafe(Bundle savedInstanceState) {
@Override
public void onResumeSessionSafe() {
super.onResumeSessionSafe();
uiController.updateSecondaryPhoneConfirmationTile();
}

void enterRootModule() {
Expand Down Expand Up @@ -268,8 +265,4 @@ public void refreshUI() {
void refreshCCUpdateOption() {
invalidateOptionsMenu();
}

public void performSecondaryPhoneVerification() {
PersonalIdManager.getInstance().beginSecondaryPhoneVerification(this, ConnectConstants.STANDARD_HOME_CONNECT_LAUNCH_REQUEST_CODE);
}
}
28 changes: 0 additions & 28 deletions app/src/org/commcare/activities/StandardHomeActivityUIController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,32 +110,4 @@ protected void updateSyncButtonMessage(String message) {
adapter.setMessagePayload(adapter.getSyncButtonPosition(), message);
adapter.notifyItemChanged(adapter.getSyncButtonPosition());
}

public void updateSecondaryPhoneConfirmationTile() {
boolean show = activity.getIntent().getBooleanExtra(LoginActivity.PERSONALID_MANAGED_LOGIN, false)
&& PersonalIdManager.getInstance().shouldShowSecondaryPhoneConfirmationTile(activity);
View connectTile = activity.findViewById(R.id.connect_alert_tile);
updateSecondaryPhoneConfirmationTile(activity, connectTile, show);
}

private void updateSecondaryPhoneConfirmationTile(Context context, View tile, boolean show) {
tile.setVisibility(show ? View.VISIBLE : View.GONE);

if (show) {
ConnectUserRecord user = PersonalIdManager.getInstance().getUser(context);
String dateStr = formatDate(user.getSecondaryPhoneVerifyByDate());
String message = context.getString(R.string.login_connect_secondary_phone_message, dateStr);

TextView view = tile.findViewById(R.id.connect_phone_label);
view.setText(message);

MaterialButton yesButton = tile.findViewById(R.id.connect_phone_yes_button);
yesButton.setOnClickListener(v -> activity.performSecondaryPhoneVerification());

MaterialButton noButton = tile.findViewById(R.id.connect_phone_no_button);
noButton.setOnClickListener(v -> {
tile.setVisibility(View.GONE);
});
}
}
}
31 changes: 0 additions & 31 deletions app/src/org/commcare/activities/connect/PersonalIdActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,18 @@
import org.commcare.activities.NavigationHostCommCareActivity;
import org.commcare.fragments.personalId.PersonalIdBiometricConfigFragment;
import org.commcare.connect.ConnectConstants;
import org.commcare.connect.PersonalIdManager;
import org.commcare.dalvik.R;
import org.commcare.fragments.personalId.PersonalIdPhoneFragmentDirections;
import org.commcare.views.dialogs.CustomProgressDialog;

import androidx.appcompat.app.ActionBar;
import androidx.fragment.app.Fragment;
import androidx.navigation.NavDirections;
import androidx.navigation.fragment.NavHostFragment;

public class PersonalIdActivity extends NavigationHostCommCareActivity<PersonalIdActivity> {

@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
handleRedirection(getIntent());
updateBackButton();
}

Expand All @@ -32,18 +28,6 @@ protected void onActivityResult(int requestCode, int resultCode, Intent data) {
|| requestCode == ConnectConstants.CONFIGURE_BIOMETRIC_REQUEST_CODE) {
//PIN unlock should only be requested while BiometricConfig fragment is active, else this will crash
getCurrentFragment().handleFinishedPinActivity(requestCode, resultCode, data);
} else if (requestCode == ConnectConstants.CONNECT_JOB_INFO) {
handleRedirection(data);
}
if (requestCode == RESULT_OK) {
finish();
}
}

private void handleRedirection(Intent intent) {
String value = intent.getStringExtra(ConnectConstants.TASK);
if (ConnectConstants.BEGIN_REGISTRATION.equals(value)) {
beginRegistration();
}
}

Expand Down Expand Up @@ -75,21 +59,6 @@ protected NavHostFragment getHostFragment() {
return navHostFragment;
}

private void beginRegistration() {
NavDirections navDirections = null;

switch (PersonalIdManager.getInstance().getStatus()) {
case NotIntroduced, Registering:
navDirections = PersonalIdPhoneFragmentDirections.actionPersonalidPhoneFragmentSelf();
break;
}
if (navDirections == null) {
navDirections = PersonalIdPhoneFragmentDirections
.actionPersonalidPhoneFragmentToPersonalidBiometricConfig();
}
navController.navigate(navDirections);
}

private void updateBackButton() {
ActionBar actionBar = getSupportActionBar();
if (actionBar != null) {
Expand Down
30 changes: 7 additions & 23 deletions app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,17 @@ public class ConnectUserRecord extends Persisted {
private String pin;
@Persisting(11)
@MetaField(META_SECONDARY_PHONE_VERIFIED)
/**
* @Deprecated should no longer be used, just here to avoid a db migration for the moment
*/
private boolean secondaryPhoneVerified;

@Persisting(12)
@MetaField(META_VERIFY_SECONDARY_PHONE_DATE)
@Deprecated
/**
* @Deprecated should no longer be used, just here to avoid a db migration for the moment
*/
private Date verifySecondaryPhoneByDate;
Comment on lines +58 to 69
Copy link

@coderabbitai coderabbitai bot Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Stale persisted columns still actively written

The deprecated fields remain persisted and are still assigned in:

  • default constructor (secondaryPhoneVerified = true, verifySecondaryPhoneByDate = new Date();)
  • fromV14() migration method.

If the intent is to freeze these columns, continuing to mutate them is misleading and bloats the on-disk diff set.
Either:

  1. Stop writing to them (simplest), or
  2. Remove them entirely and bump the DB version.

At minimum, add a comment explaining why they are still written while considered deprecated.

🤖 Prompt for AI Agents 
In app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java
around lines 58 to 69, the deprecated persisted fields secondaryPhoneVerified
and verifySecondaryPhoneByDate are still being assigned values in the default
constructor and fromV14() migration method, which is misleading and bloats the
on-disk diff. To fix this, stop assigning values to these deprecated fields in
the constructor and migration method if you want to freeze them, or
alternatively remove these fields entirely and increment the database version.
If you choose to keep writing to them temporarily, add a clear comment
explaining why they are still assigned despite being deprecated.


@Persisting(value = 13, nullable = true)
Expand Down Expand Up @@ -154,21 +161,6 @@ public void setLastPinDate(Date date) {
lastPasswordDate = date;
}

public boolean getSecondaryPhoneVerified() {
return secondaryPhoneVerified;
}

public void setSecondaryPhoneVerified(boolean verified) {
secondaryPhoneVerified = verified;
}

public Date getSecondaryPhoneVerifyByDate() {
return verifySecondaryPhoneByDate;
}

public void setSecondaryPhoneVerifyByDate(Date date) {
verifySecondaryPhoneByDate = date;
}

public boolean shouldForcePin() {
return shouldForceRecoveryLogin() && pin != null && pin.length() > 0;
Expand All @@ -191,14 +183,6 @@ private boolean shouldForceRecoveryLogin() {
return forcePin;
}

public boolean shouldRequireSecondaryPhoneVerification() {
if (secondaryPhoneVerified) {
return false;
}

return (new Date()).after(verifySecondaryPhoneByDate);
}

public void updateConnectToken(String token, Date expirationDate) {
connectToken = token;
connectTokenExpiration = expirationDate;
Expand Down
18 changes: 0 additions & 18 deletions app/src/org/commcare/connect/PersonalIdManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,12 +270,7 @@ public AuthInfo.TokenAuth getConnectToken() {
}

public void launchPersonalId(CommCareActivity<?> parent, int requestCode) {
launchPersonalId(parent, ConnectConstants.BEGIN_REGISTRATION, requestCode);
}

private void launchPersonalId(CommCareActivity<?> parent, String task, int requestCode) {
Intent intent = new Intent(parent, PersonalIdActivity.class);
intent.putExtra(ConnectConstants.TASK, task);
parent.startActivityForResult(intent, requestCode);
}

Expand Down Expand Up @@ -582,19 +577,6 @@ public String getConnectUsername(Context context) {
return ConnectUserDatabaseUtil.getUser(context).getUserId();
}

public boolean shouldShowSecondaryPhoneConfirmationTile(Context context) {
boolean show = false;
if (isloggedIn()) {
ConnectUserRecord user = getUser(context);
show = !user.getSecondaryPhoneVerified();
}
return show;
}

public void beginSecondaryPhoneVerification(CommCareActivity<?> parent, int requestCode) {
launchPersonalId(parent, ConnectConstants.VERIFY_PHONE, requestCode);
}

public void setActiveJob(ConnectJobRecord job) {
activeJob = job;
}
Expand Down