Skip to content

Improved logging around preApproval screens - #243

Open
Hopsaheysa wants to merge 10 commits into
developfrom
issues/232-preaproval-logs
Open

Improved logging around preApproval screens#243
Hopsaheysa wants to merge 10 commits into
developfrom
issues/232-preaproval-logs

Conversation

@Hopsaheysa

@Hopsaheysa Hopsaheysa commented Jun 24, 2026

Copy link
Copy Markdown
Member

#232

heading and message are required fields but the decoder silently substituted empty strings (?: "") when they were missing.
fail decoding of that screen and add log about it

this PR depends on changes of the changelog + mig. guide in #240

Copilot AI review requested due to automatic review settings June 24, 2026 12:32

Copilot AI left a comment

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.

Pull request overview

Addresses issue #232 by tightening PreApprovalScreen decoding so missing required fields no longer get silently substituted, and by adding additional logging to help diagnose backend payload problems around pre-approval UI screens.

Changes:

  • Treat missing heading / message as a decoding failure for PreApprovalScreen (instead of defaulting to empty strings), with explicit error logs.
  • Update OperationUIDataDeserializer pre-approval parsing to better handle plural preApprovalScreens arrays and skip invalid entries.
  • Add unit tests covering missing required fields and mixed-validity screen arrays.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
library/src/test/java/JsonDeserializationTests.kt Adds regression tests for required-field enforcement and mixed-validity pre-approval screen arrays.
library/src/main/java/com/wultra/android/mtokensdk/api/operation/PreApprovalScreenDeserializer.kt Enforces required heading/message during deserialization and adds error/warn logs for invalid/legacy payloads.
library/src/main/java/com/wultra/android/mtokensdk/api/operation/OperationUIDataDeserializer.kt Adjusts decoding logic and logging around legacy vs plural pre-approval screen payload formats.

@Hopsaheysa
Hopsaheysa changed the base branch from develop to issues/240-proximity-improvements June 24, 2026 15:08
Base automatically changed from issues/240-proximity-improvements to develop June 25, 2026 14:57
# Conflicts:
#	docs/Migration-3.0.md
#	docs/Using-Operations-Service.md
#	library/src/main/java/com/wultra/android/mtokensdk/api/operation/model/UserOperation.kt
#	library/src/main/java/com/wultra/android/mtokensdk/operation/OperationsService.kt

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

library/src/main/java/com/wultra/android/mtokensdk/api/operation/OperationUIDataDeserializer.kt:69

  • When both legacy preApprovalScreen and new preApprovalScreens are present, a malformed preApprovalScreens value (non-array) returns early and prevents falling back to the legacy object. Previously (via safeDeserializeArray), legacy decoding could still succeed if the plural field was invalid. Consider logging the error but continuing to try legacy parsing when available, and avoid catching Throwable when parsing individual screens.
            val el = obj.get("preApprovalScreens")
            if (!el.isJsonArray) {
                WMTLogger.e("'preApprovalScreens' is present but is not a JSON array — ignoring.")
                return null
            }

docs/Changelog.md:14

  • Changelog entry references issue #226 in the link text but links to issue #232 in the URL. This looks like a typo and makes the changelog inconsistent.
- Improved `PreApprovalScreen` decoding & logging. [(#226)](https://github.com/wultra/mtoken-sdk-android/issues/232).

docs/Using-Operations-Service.md:839

  • The ProximityCheck snippet now shows var timestampReceived: ZonedDateTime without an initializer, but the implementation defaults it to ZonedDateTime.now() and the setter is internal. Updating the snippet keeps the docs aligned with the actual API and the surrounding comment (“Captured automatically at creation”).
     * Timestamp when the operation was scanned (QR Code) or delivered to the device (Deeplink).
     * Captured automatically at creation. The SDK adjusts this to server time during authorization.
     */
    var timestampReceived: ZonedDateTime
}

library/src/test/java/JsonDeserializationTests.kt:607

  • This test can pass even if ui fails to deserialize entirely (since op!!.ui?.preApprovalScreens would be null). Adding an explicit assertion that ui is not null ensures the test only validates the intended behavior: skipping the invalid pre-approval screen.
        val op = response.responseObject.firstOrNull()
        Assert.assertNotNull(op)
        Assert.assertNull("Screen should fail to decode when heading/message are missing", op!!.ui?.preApprovalScreens)
    }

@Hopsaheysa
Hopsaheysa requested review from kober32 and marek-ch July 28, 2026 16:09
@Hopsaheysa

Copy link
Copy Markdown
Member Author

forgotten but not lost

@kober32 kober32 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants