Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 18 additions & 0 deletions app/res/layout/loading.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?xml version="1.0" encoding="utf-8"?>
<FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/loadingContainer"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:contentDescription="@string/please_wait"
android:elevation="8dp"
android:gravity="center"
android:importantForAccessibility="noHideDescendants"
android:visibility="gone">

<ProgressBar
android:id="@+id/progressBar"
style="?android:attr/progressBarStyleLarge"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="center" />
</FrameLayout>
78 changes: 78 additions & 0 deletions app/src/org/commcare/fragments/base/BaseConnectFragment.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package org.commcare.fragments.base

import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import android.widget.FrameLayout
import androidx.fragment.app.Fragment
import androidx.viewbinding.ViewBinding
import org.commcare.dalvik.databinding.LoadingBinding
import org.commcare.interfaces.base.BaseConnectView

abstract class BaseConnectFragment<B : ViewBinding> :
Fragment(),
BaseConnectView {
private var _binding: B? = null
val binding get() = _binding!!

private lateinit var loadingBinding: LoadingBinding
private lateinit var rootView: View
Copy link
Contributor

Choose a reason for hiding this comment

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

@jaypanchal-13 Please remove this


/**
* Implement this method in child fragments to inflate their specific binding.
*/
protected abstract fun inflateBinding(
inflater: LayoutInflater,
container: ViewGroup?,
): B

override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
savedInstanceState: Bundle?,
): View {
// Inflate child fragment's main view
_binding = inflateBinding(inflater, container)
val mainView = binding.root

// Inflate loading layout
loadingBinding = LoadingBinding.inflate(inflater, container, false)
val loadingView = loadingBinding.root

// Create a parent container that holds both mainView and loadingView
val mergedLayout =
FrameLayout(
requireContext(),
).apply {
layoutParams =
ViewGroup.LayoutParams(
ViewGroup.LayoutParams.MATCH_PARENT,
ViewGroup.LayoutParams.MATCH_PARENT,
)
addView(mainView)
addView(loadingView)
}

// Hide loading by default
hideLoading()

rootView = mergedLayout
return rootView
}

override fun onDestroyView() {
super.onDestroyView()
_binding = null
// No need to nullify loadingBinding since it's lateinit — but safe practice to hide it
loadingBinding.root.visibility = View.GONE
}

override fun showLoading() {
loadingBinding.root.visibility = View.VISIBLE
}

override fun hideLoading() {
loadingBinding.root.visibility = View.GONE
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional: Since we are already touching a lot of Java code, what if we converted these files to Kotlin?

This comment goes for all the other Java files in this PR as well.

Copy link
Contributor

@shubham1g5 shubham1g5 Oct 27, 2025

Choose a reason for hiding this comment

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

@conroy-ricketts Good suggestions although don't think we have talked about converting the existing Java code to Kotling as part of functional PRs. Few thoughts on this -

  • We have mixed confidence on team with Kotlin vs Java and we are still in the phase where we are trying to build the Kotlin competency of team by writing new code in Kotlin.
  • I do think it's a good one for us to do for Connect speicifc code but we probably need a separate tech-debt effort to do so rather than making it part of functional PRs to facilititate easy change management and reviews on PRs.
  • We have also struggled to quantatize benefits of converting existing Java code to Kotlin (other than ease of development) so that it makes sense for the product management team to put Sprint time into this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@shubham1g5 Makes sense! I appreciate you sharing your thoughts on this. I really like the idea of making this a separate tech-debt effort because of the reasons you described. I also think it would be good to convert our existing code to Kotlin at some point in the future if we ever want to convert our UI to Jetpack Compose and reap the full benefit of that - I think that may be the best reason to pitch to the Product Management team. All good stuff to think about!

Copy link
Contributor

Choose a reason for hiding this comment

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

@OrangeAndGreen I'm really curious what your thoughts are here?

Original file line number Diff line number Diff line change
Expand Up @@ -25,33 +25,29 @@
import org.commcare.dalvik.R;
import org.commcare.dalvik.databinding.FragmentConnectDeliveryDetailsBinding;
import org.commcare.google.services.analytics.FirebaseAnalyticsUtil;
import org.jetbrains.annotations.NotNull;

public class ConnectDeliveryDetailsFragment extends ConnectJobFragment {

private FragmentConnectDeliveryDetailsBinding binding;
public class ConnectDeliveryDetailsFragment extends ConnectJobFragment<FragmentConnectDeliveryDetailsBinding> {

public static ConnectDeliveryDetailsFragment newInstance() {
return new ConnectDeliveryDetailsFragment();
}

@Override
public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
binding = FragmentConnectDeliveryDetailsBinding.inflate(inflater, container, false);

public @NotNull View onCreateView(@NotNull LayoutInflater inflater, @Nullable ViewGroup container, @Nullable Bundle savedInstanceState) {
View view = super.onCreateView(inflater, container, savedInstanceState);
ConnectDeliveryDetailsFragmentArgs args = ConnectDeliveryDetailsFragmentArgs.fromBundle(getArguments());

requireActivity().setTitle(getString(R.string.connect_job_info_title));
setupJobDetailsUI(job);
setupButtonBehavior(job, args.getIsButtonVisible());

return binding.getRoot();
return view;
}

private void setupJobDetailsUI(ConnectJobRecord job) {
binding.connectDeliveryTotalVisitsText.setText(getString(R.string.connect_job_info_visit, job.getMaxPossibleVisits()));
binding.connectDeliveryDaysText.setText(getString(R.string.connect_job_info_days, job.getDaysRemaining()));
binding.connectDeliveryMaxDailyText.setText(getString(R.string.connect_job_info_max_visit, job.getMaxDailyVisits()));
binding.connectDeliveryBudgetText.setText(buildPaymentInfoText(job));
getBinding().connectDeliveryTotalVisitsText.setText(getString(R.string.connect_job_info_visit, job.getMaxPossibleVisits()));
getBinding().connectDeliveryDaysText.setText(getString(R.string.connect_job_info_days, job.getDaysRemaining()));
getBinding().connectDeliveryMaxDailyText.setText(getString(R.string.connect_job_info_max_visit, job.getMaxDailyVisits()));
getBinding().connectDeliveryBudgetText.setText(buildPaymentInfoText(job));
}

private String buildPaymentInfoText(ConnectJobRecord job) {
Expand All @@ -76,12 +72,12 @@ private void setupButtonBehavior(ConnectJobRecord job, boolean isButtonVisible)
? (appInstalled ? R.string.connect_delivery_go : R.string.connect_job_info_download)
: R.string.connect_job_info_download;

binding.cardButtonLayout.setVisibility(isButtonVisible ? View.VISIBLE : View.GONE);
binding.connectDeliveryButton.setText(buttonTextId);
getBinding().cardButtonLayout.setVisibility(isButtonVisible ? View.VISIBLE : View.GONE);
getBinding().connectDeliveryButton.setText(buttonTextId);

binding.connectDeliveryButton.setOnClickListener(v -> {
getBinding().connectDeliveryButton.setOnClickListener(v -> {
if (jobClaimed) {
proceedAfterJobClaimed(binding.connectDeliveryButton, job, appInstalled);
proceedAfterJobClaimed(getBinding().connectDeliveryButton, job, appInstalled);
} else {
claimJob(job, appInstalled);
}
Expand All @@ -99,7 +95,7 @@ private void claimJob(ConnectJobRecord job, boolean appInstalled) {

@Override
public void onSuccess(Boolean data) {
proceedAfterJobClaimed(binding.connectDeliveryButton, job, appInstalled);
proceedAfterJobClaimed(getBinding().connectDeliveryButton, job, appInstalled);
FirebaseAnalyticsUtil.reportCccApiClaimJob(true);
}

Expand All @@ -111,7 +107,7 @@ public void onFailure(@NonNull PersonalIdOrConnectApiErrorCodes errorCode, @Null
} else {
message = PersonalIdApiErrorHandler.handle(requireActivity(), errorCode, t);
}
showSnackBarWithDismissAction(binding.getRoot(), message);
showSnackBarWithDismissAction(getBinding().getRoot(), message);
FirebaseAnalyticsUtil.reportCccApiClaimJob(false);
}
}.claimJob(requireContext(), user, job.getJobId());
Expand All @@ -137,4 +133,9 @@ private NavDirections navigateToDownloadingPage() {
return ConnectDeliveryDetailsFragmentDirections.actionConnectJobDeliveryDetailsFragmentToConnectDownloadingFragment(
getString(R.string.connect_downloading_delivery), false);
}

@Override
protected @NotNull FragmentConnectDeliveryDetailsBinding inflateBinding(@NotNull LayoutInflater inflater, @Nullable ViewGroup container) {
return FragmentConnectDeliveryDetailsBinding.inflate(inflater, container, false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
import android.widget.LinearLayout;
import android.widget.TextView;

import com.google.common.base.Strings;

import androidx.annotation.NonNull;
import androidx.cardview.widget.CardView;
import androidx.core.content.ContextCompat;
Expand All @@ -24,18 +22,22 @@
import androidx.recyclerview.widget.LinearLayoutManager;
import androidx.recyclerview.widget.RecyclerView;

import com.google.common.base.Strings;

import org.commcare.android.database.connect.models.ConnectJobDeliveryFlagRecord;
import org.commcare.android.database.connect.models.ConnectJobDeliveryRecord;
import org.commcare.connect.ConnectDateUtils;
import org.commcare.connect.ConnectJobHelper;
import org.commcare.dalvik.R;
import org.commcare.dalvik.databinding.FragmentConnectDeliveryListBinding;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.util.ArrayList;
import java.util.List;
import java.util.Locale;

public class ConnectDeliveryListFragment extends ConnectJobFragment {
public class ConnectDeliveryListFragment extends ConnectJobFragment<FragmentConnectDeliveryListBinding> {
private static final String ALL_IDENTIFIER = "all";
private static final String APPROVED_IDENTIFIER = "approved";
private static final String REJECTED_IDENTIFIER = "rejected";
Expand All @@ -44,7 +46,6 @@ public class ConnectDeliveryListFragment extends ConnectJobFragment {
ALL_IDENTIFIER, APPROVED_IDENTIFIER, REJECTED_IDENTIFIER, PENDING_IDENTIFIER
};

private FragmentConnectDeliveryListBinding binding;
private String currentFilter = ALL_IDENTIFIER;
private String unitName;
private DeliveryAdapter adapter;
Expand All @@ -54,35 +55,29 @@ public static ConnectDeliveryListFragment newInstance() {
}

@Override
public View onCreateView(@NonNull LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
binding = FragmentConnectDeliveryListBinding.inflate(inflater, container, false);
public @NotNull View onCreateView(@NotNull LayoutInflater inflater, @Nullable ViewGroup container, @Nullable Bundle savedInstanceState) {
View view = super.onCreateView(inflater, container, savedInstanceState);
unitName = ConnectDeliveryListFragmentArgs.fromBundle(getArguments()).getUnitId();
requireActivity().setTitle(getString(R.string.connect_visit_type_title, unitName));
setupRecyclerView();
setupFilterControls();
setupMenuProvider();
return binding.getRoot();
}

@Override
public void onDestroyView() {
super.onDestroyView();
binding = null;
return view;
}

private void setupRecyclerView() {
binding.deliveryList.setLayoutManager(new LinearLayoutManager(getContext()));
getBinding().deliveryList.setLayoutManager(new LinearLayoutManager(getContext()));
adapter = new DeliveryAdapter(getContext(), getFilteredDeliveries());
binding.deliveryList.setAdapter(adapter);
getBinding().deliveryList.setAdapter(adapter);
}

private void setupFilterControls() {
CardView[] filterCards = {
binding.allFilterButton, binding.approvedFilterButton, binding.rejectedFilterButton, binding.pendingFilterButton
getBinding().allFilterButton, getBinding().approvedFilterButton, getBinding().rejectedFilterButton, getBinding().pendingFilterButton
};

TextView[] filterLabels = {
binding.allTextView, binding.approvedTextView, binding.rejectedTextView, binding.pendingTextView
getBinding().allTextView, getBinding().approvedTextView, getBinding().rejectedTextView, getBinding().pendingTextView
};

for (int i = 0; i < filterCards.length; i++) {
Expand Down Expand Up @@ -158,6 +153,11 @@ private List<ConnectJobDeliveryRecord> getFilteredDeliveries() {
return filteredList;
}

@Override
protected @NotNull FragmentConnectDeliveryListBinding inflateBinding(@NotNull LayoutInflater inflater, @Nullable ViewGroup container) {
return FragmentConnectDeliveryListBinding.inflate(inflater, container, false);
}

private static class DeliveryAdapter extends RecyclerView.Adapter<DeliveryAdapter.VerificationViewHolder> {
private List<ConnectJobDeliveryRecord> deliveries;
private final Context context;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import android.view.ViewGroup;

import androidx.annotation.RequiresApi;
import androidx.core.content.ContextCompat;
import androidx.navigation.Navigation;
import androidx.recyclerview.widget.LinearLayoutManager;
import androidx.recyclerview.widget.RecyclerView;
Expand All @@ -17,18 +16,18 @@
import org.commcare.android.database.connect.models.ConnectJobDeliveryRecord;
import org.commcare.android.database.connect.models.ConnectJobRecord;
import org.commcare.android.database.connect.models.ConnectPaymentUnitRecord;
import org.commcare.connect.PersonalIdManager;
import org.commcare.dalvik.R;
import org.commcare.dalvik.databinding.FragmentConnectProgressDeliveryBinding;
import org.commcare.views.connect.CircleProgressBar;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;

public class ConnectDeliveryProgressDeliveryFragment extends ConnectJobFragment {
private FragmentConnectProgressDeliveryBinding binding;
public class ConnectDeliveryProgressDeliveryFragment extends ConnectJobFragment<FragmentConnectProgressDeliveryBinding> {
private RecyclerView recyclerView;
private ConnectDeliveryProgressReportAdapter adapter;

Expand All @@ -38,10 +37,9 @@ public static ConnectDeliveryProgressDeliveryFragment newInstance() {

@RequiresApi(api = Build.VERSION_CODES.N)
@Override
public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
binding = FragmentConnectProgressDeliveryBinding.inflate(inflater, container, false);

binding.btnSync.setOnClickListener(view -> {
public @NotNull View onCreateView(@NotNull LayoutInflater inflater, @Nullable ViewGroup container, @Nullable Bundle savedInstanceState) {
View view = super.onCreateView(inflater, container, savedInstanceState);
getBinding().btnSync.setOnClickListener(v -> {
ConnectDeliveryProgressFragment parentFragment = (ConnectDeliveryProgressFragment)getParentFragment();
if (parentFragment != null) {
parentFragment.refresh();
Expand All @@ -51,17 +49,17 @@ public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle sa

updateProgressSummary();
populateDeliveryProgress();
return binding.getRoot();
return view;
}

public void updateProgressSummary() {
int completed = job.getCompletedVisits();
int total = job.getMaxVisits();
int percent = total > 0 ? (100 * completed / total) : 100;

CircleProgressBar progress = binding.connectProgressProgressBar;
CircleProgressBar progress = getBinding().connectProgressProgressBar;
progress.setProgress(percent);
binding.connectProgressProgressText.setText(String.format(Locale.getDefault(), "%d%%", percent));
getBinding().connectProgressProgressText.setText(String.format(Locale.getDefault(), "%d%%", percent));

Comment on lines 58 to 63
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Clamp computed values to safe ranges; avoid negative remaining and >100%.

Guard against data inconsistencies and division rounding quirks.

-        int percent = total > 0 ? (100 * completed / total) : 100;
+        int percent = total > 0 ? (100 * completed / total) : 100;
+        if (percent > 100) percent = 100;
@@
-            int remaining = unit.getMaxTotal() - approved;
+            int remaining = Math.max(0, unit.getMaxTotal() - approved);

Also applies to: 80-83

🤖 Prompt for AI Agents
In
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressDeliveryFragment.java
around lines 44-49 (and also apply same change to lines 80-83): the computed
percent and any remaining/derived values must be clamped to the 0–100 range to
avoid negative values or >100 due to division/rounding errors; after computing
percent (and any remaining), bound it with Math.max(0, Math.min(100, percent))
before passing to setProgress and formatting the text, and ensure any remaining
or completed counts shown are similarly clamped to valid ranges to prevent
UI/logic errors.

StringBuilder completedText = new StringBuilder(
getString(R.string.connect_progress_status, completed, total));
Expand All @@ -76,7 +74,7 @@ public void updateProgressSummary() {
}
}

binding.connectProgressStatusText.setText(completedText.toString());
getBinding().connectProgressStatusText.setText(completedText.toString());
}

@RequiresApi(api = Build.VERSION_CODES.N)
Expand All @@ -103,7 +101,7 @@ private void populateDeliveryProgress() {
));
}

recyclerView = binding.rvDeliveryProgressReport;
recyclerView = getBinding().rvDeliveryProgressReport;
recyclerView.setLayoutManager(new LinearLayoutManager(getContext()));
if (adapter == null) {
adapter = new ConnectDeliveryProgressReportAdapter(
Expand Down Expand Up @@ -152,6 +150,11 @@ private HashMap<String, HashMap<String, Integer>> getStatusMap(ConnectJobRecord
@Override
public void onResume() {
super.onResume();
binding.getRoot().requestLayout();
getBinding().getRoot().requestLayout();
}

@Override
protected @NotNull FragmentConnectProgressDeliveryBinding inflateBinding(@NotNull LayoutInflater inflater, @Nullable ViewGroup container) {
return FragmentConnectProgressDeliveryBinding.inflate(inflater, container, false);
}
}
Loading