Revert Navigation Changes

[jumaallan]

Story: https://app.shortcut.com/smileid/story/xxx

Summary

A few sentences/bullet points about the changes

Known Issues

Any shortcomings in your work. This may include corner cases not correctly handled or issues related
to but not within the scope of your PR. Design compromises should be discussed here if they were not
already discussed above.

Test Instructions

Concise test instructions on how to verify that your feature works as intended. This should include
changes to the development environment (if applicable) and all commands needed to run your work.

Screenshot

If applicable (e.g. UI changes), add screenshots to help explain your work.

[prfectionist]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Bug The removal of skipApiSubmission parameter could break existing functionality that relied on skipping API submission. Need to verify this change doesn't impact offline/testing scenarios. Code Smell The new implementation directly handles UI state and navigation without using a navigation framework. This could make the code harder to test and maintain. Code Review Needed There's a TODO comment about useStrictMode and skipApiSubmission that needs to be addressed before merging.

[prfectionist]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Enhancement	Improve error handling and cancellation support using coroutine scope for asynchronous operations Consider using a coroutine scope with error handling for the job submission process to properly handle exceptions and cancellation. lib/src/main/java/com/smileidentity/viewmodel/document/OrchestratedDocumentViewModel.kt [125-131] -private fun submitJob() { - val documentFrontFile = documentFrontFile - ?: throw IllegalStateException("documentFrontFile is null") - _uiState.update { - it.copy(currentStep = DocumentCaptureFlow.ProcessingScreen(ProcessingState.InProgress)) +private fun submitJob() = viewModelScope.launch { + try { + val documentFrontFile = documentFrontFile ?: run { + _uiState.update { it.copy(errorMessage = StringResource.ResId(R.string.error_missing_document)) } + return@launch + } + _uiState.update { + it.copy(currentStep = DocumentCaptureFlow.ProcessingScreen(ProcessingState.InProgress)) + } + } catch (e: Exception) { + onError(e) } Suggestion importance[1-10]: 8 Why: Adding proper coroutine scope with error handling would improve reliability and maintainability of asynchronous operations, while providing better error recovery.	8
Possible bug	Make when expression exhaustive to handle all possible states and prevent runtime errors Consider using a sealed class or interface for DocumentCaptureFlow to ensure type safety and handle all possible states in the when expression. Add an else branch or make the when expression exhaustive. lib/src/main/java/com/smileidentity/compose/document/OrchestratedDocumentVerificationScreen.kt [54-131] when (val currentStep = uiState.currentStep) { DocumentCaptureFlow.FrontDocumentCapture -> DocumentCaptureScreen( DocumentCaptureFlow.BackDocumentCapture -> DocumentCaptureScreen( DocumentCaptureFlow.SelfieCapture -> OrchestratedSelfieCaptureScreen( is DocumentCaptureFlow.ProcessingScreen -> ProcessingScreen( + else -> error("Unexpected state: $currentStep") } Suggestion importance[1-10]: 7 Why: Adding an else branch to handle unexpected states would improve type safety and prevent potential runtime errors if new states are added to DocumentCaptureFlow in the future.	7
Performance	Add debouncing to image analysis to prevent excessive processing The imageAnalyzer callback is being invoked frequently. Consider adding a debounce mechanism to prevent excessive processing. lib/src/main/java/com/smileidentity/compose/document/DocumentCaptureScreen.kt [242-246] imageAnalyzer = cameraState.rememberImageAnalyzer( - analyze = { imageAnalyzer(it, cameraState) }, + analyze = { image -> + withContext(Dispatchers.Default) { + debounce(300) { imageAnalyzer(image, cameraState) } + } + }, imageAnalysisBackpressureStrategy = KeepOnlyLatest, ) Suggestion importance[1-10]: 7 Why: The suggestion offers a valid performance optimization by adding debouncing to prevent excessive image analysis processing, which could improve app responsiveness and battery life.	7
Best practice	Handle null cases more gracefully using Kotlin's null safety operators instead of throwing exceptions Consider using null safety checks with let() or run() instead of throwing IllegalStateException for null checks on documentFrontFile and selfieFile. lib/src/main/java/com/smileidentity/viewmodel/document/OrchestratedDocumentViewModel.kt [126-128] -val documentFrontFile = documentFrontFile - ?: throw IllegalStateException("documentFrontFile is null") +val documentFrontFile = documentFrontFile.let { file -> + file ?: return@let null +} ?: run { + _uiState.update { it.copy(errorMessage = StringResource.ResId(R.string.error_missing_document)) } + return +} Suggestion importance[1-10]: 6 Why: Using Kotlin's null safety operators would provide more graceful error handling and better user experience than throwing exceptions.	6
Maintainability	Simplify nested conditional logic using early returns The photoPickerLauncher callback contains nested if conditions that could be simplified using early returns for better readability. lib/src/main/java/com/smileidentity/compose/document/DocumentCaptureScreen.kt [119-127] if (uri == null) { Timber.e("selectedUri is null") context.toast(R.string.si_doc_v_capture_error_subtitle) return@rememberLauncherForActivityResult } -if (isValidDocumentImage(context, uri)) { - val documentFile = createDocumentFile(jobId, (side == DocumentCaptureSide.Front)) - writeUriToFile(documentFile, uri, context) +if (!isValidDocumentImage(context, uri)) { + SmileIDCrashReporting.hub.addBreadcrumb("Gallery upload document image too small") + context.toast(R.string.si_doc_v_validation_image_too_small) + return@rememberLauncherForActivityResult +} +val documentFile = createDocumentFile(jobId, (side == DocumentCaptureSide.Front)) +writeUriToFile(documentFile, uri, context) Suggestion importance[1-10]: 5 Why: The suggestion improves code readability by restructuring the conditional logic with early returns, making the flow easier to follow and maintain.	5

[robin-ankele]

I've looked through the code changes and generally look fine, but I haven't spend too much time on getting a comprehensive overview given the large PR. I've run some tests and done the following enrolments which all worked fine:

• Selfie Enrolment
• Selfie Authentication
• Enhanced Selfie Enrolment
• Enhanced Selfie Authentication
• Document Verification
• Biometric KYC

[robin-ankele]

Do all those shuffling around of params cause some breaking changes on someone who integrated it already?

[jumaallan]

Yes, we will need to rework the integrations later - esp on Flutter and RN

[JNdhlovu]

@jumaallan

• I think there is elements we need to take from the previous work like back button click events we've said we won't do nav on ios as well but we need the back/up click event to work as expected
• On cancel we also need the results for cancelled events which builds up on top of the above point not sure if it's the sample app only but on back click there is no toast/event to show this

[jumaallan]

@jumaallan

• I think there is elements we need to take from the previous work like back button click events we've said we won't do nav on ios as well but we need the back/up click event to work as expected
• On cancel we also need the results for cancelled events which builds up on top of the above point not sure if it's the sample app only but on back click there is no toast/event to show this

Addressed on Slack