fix: corregir doble conteo en verificación de donaciones por saldo#13
Open
cyberplant wants to merge 11 commits into
Open
fix: corregir doble conteo en verificación de donaciones por saldo#13cyberplant wants to merge 11 commits into
cyberplant wants to merge 11 commits into
Conversation
Al usar 'Verificar por saldo', la diferencia de crédito detectada incluía donaciones que ya habían sido confirmadas automáticamente vía SMS (confirmedThisSession). Esto causaba que se agregaran al historial tanto las confirmaciones automáticas como las manuales. Se resta confirmedThisSession al total detectado por diferencia de saldo antes de llamar a manuallyConfirmPending(), evitando el doble conteo. También se actualiza el mensaje del diálogo para mostrar el desglose completo. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR attempts to fix double counting when confirming pending donations via balance verification by subtracting automatically confirmed donations from the balance-derived total.
Changes:
- Computes
pendingToConfirmbefore manual confirmation. - Updates the confirmation dialog to show already-confirmed and pending donation counts.
- Uses the pending count when calling
manuallyConfirmPending().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
El fix anterior restaba confirmedThisSession (total de la sesión) al calcular cuánto confirmar por diferencia de saldo, lo que causaba undercounting si el usuario ya tenía confirmaciones previas en la sesión antes de consultar el saldo base. Se agrega confirmedAtBaselineSave: un snapshot de confirmedThisSession tomado en el momento en que se guarda el saldo base (LAST_BALANCE_KEY), y solo cuando NO se está en modo verificación. Así se restan únicamente las confirmaciones recibidas entre la captura del saldo base y la verificación, que es exactamente lo que se quiere descontar. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…esión - Al confirmar donaciones por diferencia de saldo, se actualiza LAST_BALANCE_KEY al saldo verificado y se alinea confirmedAtBaselineSave para evitar que el mismo saldo base se reutilice en reinicios o sesiones futuras y ofrezca confirmar donaciones ya registradas. - Se resetea confirmedAtBaselineSave junto con confirmedThisSession en resetCounters() y clearEverything(), para que el snapshot no quede desalineado respecto al saldo base persistido cuando se limpian los contadores de sesión. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
confirmedAtBaselineSave era solo en memoria: si la actividad se recreaba (rotación, etc.) o se limpiaban los contadores de sesión, quedaba desincronizado respecto al LAST_BALANCE_KEY persistido, permitiendo que una verificación posterior contara donaciones ya registradas. - Se agrega CONFIRMED_AT_BASELINE_KEY en SharedPreferences para persistir confirmedAtBaselineSave junto a LAST_BALANCE_KEY. - Se carga en onCreate para sobrevivir recreaciones de actividad. - Se actualiza en todos los puntos donde ya se actualiza LAST_BALANCE_KEY (guardar baseline, confirmar verificación). - Se elimina junto a LAST_BALANCE_KEY en resetCounters() y clearEverything(), para que la verificación no pueda comparar contra un saldo anterior al reset. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- clearEverything() declaraba 'val sharedPreferences' local que pisaba el campo de la actividad, haciendo que remove(LAST_BALANCE_KEY) operara sobre 'donation_prefs' en vez de 'SmsSenderPrefs'. Se renombra la variable local a 'donationPrefs'. - El handler del botón 'Confirmar' en el diálogo de verificación usaba pendingToConfirm capturado al mostrar el diálogo. Si llegaban confirmaciones automáticas antes de que el usuario tocara el botón, el valor quedaba viejo y se podían registrar donaciones dobles. Se recalcula actualPending dentro del handler con los valores actuales de confirmedThisSession y confirmedAtBaselineSave. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- confirmedThisSession era solo en memoria: al recrear la actividad volvía a 0 mientras confirmedAtBaselineSave se restauraba desde SharedPreferences, haciendo que confirmedSinceBaseline fuera negativo o incorrecto. Se agrega CONFIRMED_THIS_SESSION_KEY para persistirlo y se restaura en onCreate junto a confirmedAtBaselineSave. Se actualiza en addDonation() y manuallyConfirmPending(), y se limpia en los paths de reset. - Cuando la verificación por saldo no detecta diferencia positiva (diff <= 0, por ejemplo después de una recarga), no se actualizaba el baseline. Una donación posterior podía compararse contra el saldo viejo y reportar nada. Ahora se avanza LAST_BALANCE_KEY y CONFIRMED_AT_BASELINE_KEY al saldo actual en el caso diff <= 0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Al restaurar confirmedThisSession desde SharedPreferences pero dejar sentThisSession en 0 tras recrear la actividad, los cálculos de pendientes (sentThisSession - confirmedThisSession) quedaban negativos y refreshDonationStats() mostraba valores incorrectos. Se agrega SENT_THIS_SESSION_KEY para persistir sentThisSession: se carga en onCreate junto a confirmedThisSession, se actualiza en sendSMSImmediate() y sendSMSWithDelay() cada vez que se incrementa, y se resetea a 0 en resetCounters() y clearEverything(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…anuallyConfirmPending confirmedThisSession se incrementaba dentro del withContext(Main) del bloque async, dejando una ventana donde una confirmación automática (addDonation) podía llegar, incrementar el contador y escribir en la DB, y luego el bloque manual también escribía su conteo completo → doble conteo. Se mueve el incremento de confirmedThisSession y su persistencia al comienzo de manuallyConfirmPending(), en el Main thread, antes de lanzar la coroutine. Así el conteo queda 'reservado' de forma atómica respecto a cualquier confirmación automática que llegue mientras se hace el DB write. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
addDonation() escribía en la DB incondicionalmente. Un SMS tardío de 'Gracias por colaborar' podía llegar después de que manuallyConfirmPending() ya había reclamado todas las pendientes (confirmedThisSession == sentThisSession), causando doble conteo. Se agrega un guard al inicio de addDonation(): si confirmedThisSession >= sentThisSession no hay mensajes sin confirmar que consumir, y se descarta el SMS. Esto aprovecha que manuallyConfirmPending() ya reserva el conteo síncronamente antes del write async. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
El guard anterior (confirmedThisSession >= sentThisSession) fallaba si se enviaba un segundo batch antes de que llegaran SMS tardíos del primero: sentThisSession volvía a ser mayor y los SMS tardíos se registraban igual. Se introduce manuallyClaimedCount: cuando manuallyConfirmPending() reclama N confirmaciones, incrementa este contador. En addDonation(), si manuallyClaimedCount > 0 se consume un claim, se actualiza la UI y se descarta el write en DB (confirmedThisSession ya fue pre-incrementado). Esto asocia los SMS tardíos al batch que los generó en lugar de comparar totales agregados. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ación manuallyClaimedCount trataba cualquier SMS de confirmación como tardío mientras el contador era positivo, lo que hacía que confirmaciones de un nuevo batch fueran descartadas si el usuario enviaba un segundo batch antes de que llegaran los SMS tardíos del primero. El problema raíz es que sin IDs únicos por SMS (que el operador no provee) no es posible distinguir un SMS tardío de un batch anterior de una confirmación genuina de un nuevo batch. Se elimina el mecanismo defectuoso y se documenta esta limitación en addDonation(). El fix principal de este PR (confirmedSinceBaseline) sigue siendo correcto y previene el doble conteo en el flujo de verificación por saldo, que es el bug original reportado. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
733
to
+742
| private fun addDonation(amount: Int) { | ||
| // NOTE: Late auto-confirmation SMSs that arrive after a manual confirmation of the same | ||
| // donation will be recorded again here. Preventing this reliably requires per-message | ||
| // tracking (unique IDs per SMS), which the carrier does not provide. This is a known | ||
| // limitation; the balance-verification fix (confirmedSinceBaseline) already prevents the | ||
| // primary double-count from the verification flow itself. | ||
| val today = dateFormat.format(Calendar.getInstance().time) | ||
| // Increment session confirmation counter | ||
| // Increment session confirmation counter and persist so it survives activity recreation | ||
| confirmedThisSession++ | ||
| sharedPreferences.edit().putInt(CONFIRMED_THIS_SESSION_KEY, confirmedThisSession).apply() |
Comment on lines
+820
to
+827
| // Save the current balance for future verification comparisons (only when establishing a new baseline, not during verification) | ||
| val previousBalance = sharedPreferences.getFloat(LAST_BALANCE_KEY, -1f) | ||
| sharedPreferences.edit().putFloat(LAST_BALANCE_KEY, balance.toFloat()).apply() | ||
| if (!isVerifyingBalance) { | ||
| sharedPreferences.edit() | ||
| .putFloat(LAST_BALANCE_KEY, balance.toFloat()) | ||
| .putInt(CONFIRMED_AT_BASELINE_KEY, confirmedThisSession) | ||
| .apply() | ||
| confirmedAtBaselineSave = confirmedThisSession |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problema
Al usar Verificar por saldo para confirmar donaciones pendientes, el cálculo era incorrecto: la diferencia de crédito detectada representa el total de mensajes enviados, pero parte de ellos ya habían sido confirmados automáticamente vía SMS ("Gracias por colaborar").
Ejemplo del bug
sentThisSession = 10confirmedThisSession = 8, DB tiene 80$manuallyConfirmPending(10)→ agrega 100$ al historialSolución
Se resta
confirmedThisSessional total detectado por diferencia de saldo antes de llamar amanuallyConfirmPending():También se actualiza el mensaje del diálogo para mostrar el desglose completo cuando hay confirmaciones previas (total detectado / ya confirmados / a agregar ahora).