Skip to content
Merged
4 changes: 4 additions & 0 deletions RELEASES.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ This file is meant as an easy way for us to collate notes and change logs across
- PersonalID sign-up, account recovery and profile screens now share consistent headings, text styles, spacing and colors.
- Messaging and Work History now show the sidebar, and opening another sidebar section replaces the current one instead of stacking on top of it.
- After an incorrect email or phone OTP code, PersonalID now shows how many attempts remain before a new code must be requested.
- Opportunities whose visits have no daily limit now show today's visit count without a daily progress bar or "daily limit reached" warning.

#### Important Bug Fixes

Expand All @@ -38,6 +39,9 @@ This file is meant as an easy way for us to collate notes and change logs across
- Enter wrong email OTP codes during sign-up, profile email edit, and forgot-backup-code recovery, and confirm the error shows the attempts remaining (2, then 1) before the "request a new code" message appears.
- For an invited user receiving the phone OTP by SMS through PersonalID, confirm a wrong code likewise shows the attempts remaining.
- Gregorian Date Widget: set a date in portrait, rotate to landscape, and update it with the keyboard. Tapping the day or the year field should bring up the keyboard's own full-width editor with a DONE key, and the value typed there should apply to the widget once DONE is pressed.
- On an opportunity with no daily visit limit, confirm the delivery dashboard and the job tile on the app home screen show only today's visit count with no progress bar, and no daily-limit warning appears.
- Confirm the opportunity intro and learning-complete screens omit the "Up to N per day" text for such opportunities.
- On an opportunity that still has daily limits, confirm the daily progress bar and daily-limit warnings work as before.

## CommCare 2.64.1

Expand Down
5 changes: 4 additions & 1 deletion app/res/layout/item_progress_job_summary_visit.xml
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@
tools:text="2/2"
android:textColor="@color/connect_dark_blue_color"
android:textSize="14sp"
android:layout_marginEnd="20dp"
app:layout_goneMarginEnd="0dp"
app:layout_constraintBottom_toBottomOf="@+id/tv_primary_visit_title"
app:layout_constraintEnd_toStartOf="@+id/guideline3"
app:layout_constraintEnd_toStartOf="@+id/lp_primary_visit_progress"
app:layout_constraintTop_toTopOf="@+id/tv_primary_visit_title" />

<org.commcare.views.connect.LinearProgressBar
Expand All @@ -35,6 +37,7 @@
android:layout_marginStart="20dp"
app:layout_constraintBottom_toBottomOf="@+id/tv_primary_visit_title"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintHorizontal_bias="1"
app:layout_constraintStart_toStartOf="@+id/guideline3"
app:layout_constraintTop_toTopOf="@+id/tv_primary_visit_title" />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,9 @@ public void updateConnectJobProgress() {
list.add(new ConnectDeliveryPaymentSummaryInfo(
activity.getString(R.string.connect_job_tile_daily_visits),
job.numberOfDeliveriesToday(),
job.getMaxDailyVisits()
job.hasDailyLimit()
? job.getMaxDailyVisits()
: ConnectDeliveryPaymentSummaryInfo.NO_DAILY_LIMIT
));

connectProgressJobSummaryAdapter.setDeliverySummaries(list);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ public ViewHolder onCreateViewHolder(@NonNull ViewGroup parent, int viewType) {
public void onBindViewHolder(@NonNull ViewHolder holder, int position) {
ConnectDeliveryPaymentSummaryInfo summary = deliverySummaries.get(position);
holder.tvPrimaryVisitTitle.setText(summary.getPaymentUnitName());
if (!summary.hasDailyLimit()) {
holder.lpPrimaryVisitProgress.setVisibility(View.GONE);
holder.tvPrimaryVisitCount.setText(
String.format(Locale.getDefault(), "%d", summary.getPaymentUnitAmount())
);
return;
}

holder.lpPrimaryVisitProgress.setVisibility(View.VISIBLE);
holder.tvPrimaryVisitCount.setText(String.format(Locale.getDefault(), "%d/%d",
summary.getPaymentUnitAmount(), summary.getPaymentUnitMaxDaily()));

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package org.commcare.android.database.connect.models;

public class ConnectDeliveryPaymentSummaryInfo {
public static final int NO_DAILY_LIMIT = -1;

private String paymentUnitName;
private int paymentUnitAmount;
private int paymentUnitMaxDaily;
Expand Down Expand Up @@ -35,4 +37,8 @@ public int getPaymentUnitMaxDaily() {
public void setPaymentUnitMaxDaily(int paymentUnitMaxDaily) {
this.paymentUnitMaxDaily = paymentUnitMaxDaily;
}

public boolean hasDailyLimit() {
return paymentUnitMaxDaily != NO_DAILY_LIMIT;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ public class ConnectJobRecord extends Persisted implements Serializable {

public static final String META_JOB_UUID = "opportunity_id";

public static final int NO_DAILY_LIMIT = -1;

@Persisting(1)
@MetaField(META_JOB_ID)
Expand Down Expand Up @@ -205,7 +206,7 @@ public static ConnectJobRecord fromJson(JSONObject json) throws JSONException {
job.projectEndDate = JsonExtensions.requireDate(json, META_END_DATE);
job.projectStartDate = JsonExtensions.requireDate(json, META_START_DATE);
job.maxVisits = json.getInt(META_MAX_VISITS_PER_USER);
job.maxDailyVisits = json.getInt(META_MAX_DAILY_VISITS);
job.maxDailyVisits = JsonExtensions.optIntSafe(json, META_MAX_DAILY_VISITS, NO_DAILY_LIMIT);
job.budgetPerVisit = json.getInt(META_BUDGET_PER_VISIT);
String budgetPerUserKey = "budget_per_user";
job.totalBudget = json.getInt(budgetPerUserKey);
Expand Down Expand Up @@ -347,6 +348,14 @@ public int getMaxDailyVisits() {
return maxDailyVisits;
}

public boolean hasDailyLimit() {
return maxDailyVisits != NO_DAILY_LIMIT;
}

private boolean isDailyLimitReached() {
return hasDailyLimit() && numberOfDeliveriesToday() >= maxDailyVisits;
}

public Date getProjectStartDate() {
return projectStartDate;
}
Expand Down Expand Up @@ -778,7 +787,7 @@ public String getCardMessageText(Context context) {
// The job-level caps are checked ahead of the per-unit warnings: once the whole
// opportunity is spent, which individual unit ran out first no longer matters.
return context.getString(R.string.connect_progress_warning_max_reached_single);
} else if (numberOfDeliveriesToday() >= getMaxDailyVisits()) {
} else if (isDailyLimitReached()) {
return context.getString(R.string.connect_progress_warning_daily_max_reached_single);
} else if (!getPaymentUnits().isEmpty()) {
return getMultiVisitWarnings(context);
Expand All @@ -802,7 +811,7 @@ private String getMultiVisitWarnings(Context context) {
totalMaxes.add(unit.getName());
} else {
int todayCount = today.containsKey(key) ? today.get(key) : 0;
if (todayCount >= unit.getMaxDaily()) {
if (unit.isDailyLimitReached(todayCount)) {
dailyMaxes.add(unit.getName());
}
}
Expand Down Expand Up @@ -841,8 +850,7 @@ public boolean isFurtherWorkBlocked() {
}

// The job-level caps bind whatever the payment units allow, so they are checked first.
if (getDeliveries().size() >= getMaxVisits()
|| numberOfDeliveriesToday() >= getMaxDailyVisits()) {
if (getDeliveries().size() >= getMaxVisits() || isDailyLimitReached()) {
return true;
}

Expand All @@ -866,7 +874,7 @@ public Set<String> getPaymentUnitsAtLimit() {
String key = unit.getUnitUUID();
int totalCount = total.containsKey(key) ? total.get(key) : 0;
int todayCount = today.containsKey(key) ? today.get(key) : 0;
if (totalCount >= unit.getMaxTotal() || todayCount >= unit.getMaxDaily()) {
if (totalCount >= unit.getMaxTotal() || unit.isDailyLimitReached(todayCount)) {
atLimit.add(key);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.commcare.models.framework.Persisting;
import org.commcare.modern.database.Table;
import org.commcare.modern.models.MetaField;
import org.commcare.utils.JsonExtensions;
import org.javarosa.core.services.Logger;
import org.json.JSONException;
import org.json.JSONObject;
Expand All @@ -28,6 +29,8 @@ public class ConnectPaymentUnitRecord extends Persisted implements Serializable
public static final String META_JOB_UUID = ConnectJobRecord.META_JOB_UUID;
public static final String META_PAYMENT_UNIT_UUID = "payment_unit_id";

public static final int NO_DAILY_LIMIT = -1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see this constant defined in several places but thinking it should be a single source of truth since changing it in one place alone would break functionality

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I thought it would be good to keep them separate to prevent coupling, and because the job and payment unit records each store their own value and they don't compare the values against each other.

But, I don't mind changing it. Curious in what ways do you foresee the functionality breaking if one of these constants change?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, here they aren't connected that makes sense. I was thinking of ConnectJobRecord falling back to NO_DAILY_LIMIT when failing to parse the field from JSON, then consumers checking against their own version of NO_DAILY_LIMIT when showing the UI and making display decisions.
Not a big deal though, in practice it's not ever likely to change. Putting it in ConnectConstants would make it accessible without coupling any of the higher classes together.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ahh, I almost forgot that ConnectConstants was a thing


@Persisting(1)
@MetaField(META_JOB_ID)
private int jobId;
Expand Down Expand Up @@ -76,7 +79,7 @@ public static ConnectPaymentUnitRecord fromJson(JSONObject json, ConnectJobRecor

paymentUnit.name = json.getString(META_NAME);
paymentUnit.maxTotal = json.getInt(META_TOTAL);
paymentUnit.maxDaily = json.getInt(META_DAILY);
paymentUnit.maxDaily = JsonExtensions.optIntSafe(json, META_DAILY, NO_DAILY_LIMIT);
paymentUnit.amount = json.getInt(META_AMOUNT);

return paymentUnit;
Expand Down Expand Up @@ -123,6 +126,14 @@ public int getMaxDaily() {
return maxDaily;
}

public boolean hasDailyLimit() {
return maxDaily != NO_DAILY_LIMIT;
}

public boolean isDailyLimitReached(int visitsToday) {
return hasDailyLimit() && visitsToday >= maxDaily;
}

public int getAmount() {
return amount;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class ConnectNetworkClient
) {
companion object {
private const val BASE_URL = "https://${BuildConfig.CCC_HOST}"
private const val API_VERSION_CONNECT = "1.0"
private const val API_VERSION_CONNECT = "2.0"
Comment thread
conroy-ricketts marked this conversation as resolved.

@Volatile
private var instance: ConnectNetworkClient? = null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ class ConnectDeliveryDashboardFragment :
}

private fun bindVisitProgress(contentEnabled: Boolean) {
val doneToday = job.numberOfDeliveriesToday()
val remainingToday = (job.maxDailyVisits - doneToday).coerceAtLeast(0)
val visitsDoneToday = job.numberOfDeliveriesToday()
val maxDailyVisits = job.maxDailyVisits.takeIf { job.hasDailyLimit() }
val cardMessage: String? = job.getCardMessageText(requireContext())
binding.deliveryProgressCard.bind(
ConnectProgressCard.State(
Expand All @@ -85,14 +85,17 @@ class ConnectDeliveryDashboardFragment :
linearProgress =
ConnectProgressCard.State.LinearProgress(
label = getString(R.string.connect_delivery_daily_visits),
current = doneToday,
max = job.maxDailyVisits,
current = visitsDoneToday,
max = maxDailyVisits,
caption =
resources.getQuantityString(
R.plurals.connect_delivery_visits_remaining_today,
remainingToday,
remainingToday,
),
maxDailyVisits?.let { max ->
val remainingToday = (max - visitsDoneToday).coerceAtLeast(0)
resources.getQuantityString(
R.plurals.connect_delivery_visits_remaining_today,
remainingToday,
remainingToday,
)
},
),
),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ class ConnectJobIntroFragment : ConnectJobFragment<FragmentConnectJobIntroBindin
private fun populateDeliveryCards() {
binding.cardMaxVisits.valueText = job.maxPossibleVisits.toString()
binding.cardMaxVisits.subtitleText =
getString(R.string.connect_opportunity_visits_per_day, job.maxDailyVisits)
if (job.hasDailyLimit()) getString(R.string.connect_opportunity_visits_per_day, job.maxDailyVisits) else null

binding.cardDays.valueText = job.daysRemaining.toString()

Expand Down
6 changes: 6 additions & 0 deletions app/src/org/commcare/utils/JsonExtensions.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ fun JSONObject.optStringSafe(
/** Returns the value at [key] if it is present and not blank, otherwise null. */
fun JSONObject.optNonBlankStringSafe(key: String): String? = optStringSafe(key, null)?.takeIf { it.isNotBlank() }

/** Returns the int at [key], or [fallback] if it is missing or null. */
fun JSONObject.optIntSafe(
key: String,
fallback: Int,
): Int = if (hasNonNull(key)) getInt(key) else fallback

fun JSONObject.requireDate(key: String): Date =
DateUtils.parseDate(getString(key))
?: throw JSONException("Unparseable date for $key: ${getString(key)}")
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class ConnectLearnCompleteView
private fun bindDeliveryCards(job: ConnectJobRecord) {
binding.cardTotalVisits.valueText = job.maxPossibleVisits.toString()
binding.cardTotalVisits.subtitleText =
context.getString(R.string.connect_opportunity_visits_per_day, job.maxDailyVisits)
if (job.hasDailyLimit()) context.getString(R.string.connect_opportunity_visits_per_day, job.maxDailyVisits) else null

binding.cardDaysToComplete.valueText = job.daysRemaining.toString()

Expand Down
11 changes: 10 additions & 1 deletion app/src/org/commcare/views/connect/ConnectProgressCard.kt
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class ConnectProgressCard
data class LinearProgress(
val label: CharSequence? = null,
val current: Int = 0,
val max: Int = 0,
val max: Int? = 0,
val caption: CharSequence? = null,
)

Expand Down Expand Up @@ -152,11 +152,20 @@ class ConnectProgressCard
bindOptionalText(binding.progressCardBarCaption, linearProgress?.caption)

if (linearProgress == null) {
binding.progressCardLinearBar.visibility = VISIBLE
binding.progressCardLinearBar.setProgress(0f)
binding.progressCardBarCount.visibility = GONE
return
}

if (linearProgress.max == null) {
binding.progressCardLinearBar.visibility = GONE
binding.progressCardBarCount.text = linearProgress.current.coerceAtLeast(0).toString()
binding.progressCardBarCount.visibility = VISIBLE
return
}

binding.progressCardLinearBar.visibility = VISIBLE
val (current, max) = coerceProgress(linearProgress.current, linearProgress.max)
binding.progressCardLinearBar.setProgress(ProgressUtils.calculateProgress(current, max) * 100f)
if (max > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class ConnectJobRecordCardMessageTest {
private fun paymentUnit(
id: Int,
maxTotal: Int,
maxDaily: Int,
maxDaily: Int?,
): ConnectPaymentUnitRecord =
ConnectPaymentUnitRecord.fromJson(
JSONObject(
Expand Down Expand Up @@ -155,6 +155,35 @@ class ConnectJobRecordCardMessageTest {
)
}

@Test
fun `a job without a daily limit never reports the daily maximum`() {
val job = multiPaymentJob(maxDailyVisits = ConnectJobRecord.NO_DAILY_LIMIT)
job.deliveries = List(5) { delivery(it, 1, Date()) }

assertNull(job.getCardMessageText(context))
}

@Test
fun `a payment unit without a daily limit is never named in the daily maximum warning`() {
val job = multiPaymentJob()
job.paymentUnits =
listOf(
paymentUnit(id = 1, maxTotal = 50, maxDaily = null),
paymentUnit(id = 2, maxTotal = 50, maxDaily = 1),
)
job.deliveries =
listOf(
delivery(1, 1, Date()),
delivery(2, 1, Date()),
delivery(3, 2, Date()),
)

assertEquals(
context.getString(R.string.connect_progress_warning_daily_max_reached_multi, "Unit 2"),
job.getCardMessageText(context),
)
}

@Test
fun `a multi-payment job with room everywhere reports nothing`() {
val job = multiPaymentJob()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class ConnectJobRecordWorkBlockedTest {
private fun paymentUnit(
id: Int,
maxTotal: Int,
maxDaily: Int,
maxDaily: Int?,
): ConnectPaymentUnitRecord =
ConnectPaymentUnitRecord.fromJson(
JSONObject(
Expand Down Expand Up @@ -300,6 +300,37 @@ class ConnectJobRecordWorkBlockedTest {
assertTrue(job.isFurtherWorkBlocked)
}

@Test
fun `a job without a daily limit is not blocked by today's visits`() {
val job = job(maxVisits = 10, maxDailyVisits = ConnectJobRecord.NO_DAILY_LIMIT)
job.deliveries = List(5) { delivery(it, 1, Date()) }

assertFalse(job.isFurtherWorkBlocked)
}

@Test
fun `a payment unit without a daily limit is not at its limit from today's visits`() {
val job = job(maxVisits = 100, maxDailyVisits = ConnectJobRecord.NO_DAILY_LIMIT)
job.paymentUnits = listOf(paymentUnit(id = 1, maxTotal = 50, maxDaily = null))
job.deliveries = List(5) { delivery(it, 1, Date()) }

assertTrue(job.paymentUnitsAtLimit.isEmpty())
assertFalse(job.isFurtherWorkBlocked)
}

@Test
fun `a payment unit without a daily limit is still blocked by its total limit`() {
val job = job(maxVisits = 100, maxDailyVisits = ConnectJobRecord.NO_DAILY_LIMIT)
job.paymentUnits = listOf(paymentUnit(id = 1, maxTotal = 2, maxDaily = null))
job.deliveries =
listOf(
delivery(1, 1, daysFromNow(-1)),
delivery(2, 1, daysFromNow(-1)),
)

assertTrue(job.isFurtherWorkBlocked)
}

/** Guards the empty case, where an unguarded `atLimit.size == units.size` compares 0 to 0. */
@Test
fun `a job with no payment units is not blocked while it has visits left`() {
Expand Down
Loading
Loading