fix(feature:client): fix silent creation failure and state/snackbar issues for client identifier

[biplab1]

Fixes - Jira-#462

Summary of changes:

1. Fixed silent failure of Client Identifier creation by adding @Serializable to the relevant data classes.
2. Fixed the state and snackbar message issues that were listed below in the first comment.

Before Fix:

before-fix-client-identifier-creation.webm

After Fix:

client-identifiers-create-delete.webm

Please make sure these boxes are checked before submitting your pull request - thanks!

• Run the static analysis check ./gradlew check or ci-prepush.sh to make sure you didn't break anything

• If you have multiple commits please combine them into one commit by squashing them.

[biplab1]

Note:

• The snackbar messages for both identifier creation and deletion are not fully reliable yet.
• For creation, the message shows after a successful operation but reappears when clicking the "+" icon again because the success state isn’t cleared properly.
• For deletion, the snackbar never showed as expected because the success state handling isn’t correctly triggered and gets immediately overwritten by the list reload.
• Fixes were tried for both cases, which led to these observations and edge cases.
• Handling this properly would require managing the ViewModel state inside the screen, which breaks Compose previews.
• Also, clicking the "+" icon right after creating an identifier reloads the identifier screen. Clicking it again opens the dialog screen.
• The dialog screen persists briefly after a successful submission — a fix to close it immediately worked but caused other issues, like the "+" icon not reopening the dialog after the first creation.
• Due to these problems not being fully resolved yet, they have not been included in this PR. Suggestions to handle this cleanly are welcome.

[HekmatullahAmin]

Both of them achieve the same thing you are not doing anything new to do it this way, so useasDataStateFlow that extension was created for this purpose

[biplab1]

Got it, will use asDataStateFlow. Thanks!

[HekmatullahAmin]

@biplab1 ,

I was reviewing the current implementation of ClientIdentifiersDialogScreen, and I noticed that the snackbar you're trying to show after a successful identifier creation isn't actually being displayed.

The issue is that the SnackbarHostState is defined inside the dialog, but it's not connected to any Scaffold. Since Snackbars only render when attached to a Scaffold with a SnackbarHostState, defining it locally inside the dialog won't work — there's no host to display it.

How to fix it:
Instead of trying to show the snackbar inside the dialog itself, it’s better to:

Trigger onIdentifierCreated() from the dialog (as you're already doing).
change accordingly

 is ClientIdentifierDialogUiState.IdentifierCreatedSuccessfully -> {
                            // Notify parent to close dialog and show snackbar
                            LaunchedEffect(Unit) {
                                onIdentifierCreated()
                            }
                        }

Then in the parent composable (ClientIdentifiersScreen), call snackbarHostState.showSnackbar(...) there — since that's where the Scaffold and SnackbarHostState actually exist.

// Show Dialog if requested
    if (showCreateIdentifierDialog) {
        ClientIdentifiersDialogScreen(
            clientId = clientId,
            onDismiss = { showCreateIdentifierDialog = false },
            onIdentifierCreated = {
                showCreateIdentifierDialog = false
                viewModel.loadIdentifiers(clientId)
                scope.launch {
                    snackbarHostState.showSnackbar(
                        getString(Res.string.feature_client_identifier_created_successfully)
                    )
                }
            },
        )
    }

[biplab1]

Thanks for sharing this! This is quite similar to what I’ve implemented too.

[HekmatullahAmin]

Move loadIdentifiers to ViewModel init. this causes re-fetching when recomposed unnecessarily.

[biplab1]

Thanks for the suggestion!

In this screen, the identifiers list needs to re-fetch whenever the user navigates back here (for example, after adding or deleting an identifier from ClientDetailsScreen). Using LaunchedEffect(Unit) ties the fetch to the screen’s composition lifecycle, so it reloads fresh data each time the screen is re-entered, not on every recomposition.

Refs: LaunchedEffect docs — it only re-executes if the key changes or the composable leaves and re-enters composition.

[biplab1]

Update:
I looked at this again — my main thought was that every user action (create, delete, refresh, retry) already reloads the data anyway, so the initial loadIdentifiers only really needs to happen once when the ViewModel is created. Since the clientId comes from SavedStateHandle, that works fine too and there’s no risk of stale data.

I’ll clean this up and drop the LaunchedEffect — thanks for pointing it out, this does simplify it.

[HekmatullahAmin]

@biplab1 i see in your commit message that the snackbar issue has been resolved. And i played the video you attached there is not snackbar i believe the videos are not updated right?

[biplab1]

@biplab1 i see in your commit message that the snackbar issue has been resolved. And i played the video you attached there is not snackbar i believe the videos are not updated right?

Thanks for pointing it out. I’ve fixed the issue and updated the video in the PR description.

[HekmatullahAmin]

also move it to top composable so you wouldn't needed to call the return:

// Correct and clean place to react to success state
    LaunchedEffect(state) {
        if (state is ClientIdentifierDialogUiState.IdentifierCreatedSuccessfully) {
            onIdentifierCreated()
        }
    }

[biplab1]

I moved it to the top. However, without the return statement, the dialog screen briefly persists even after a successful submission. I had tried removing the return statement earlier, but keeping it in place resolved the issue. I’ve attached a screen recording for reference.

Without Return:

without-return.webm

With Return:

with-return.webm

[HekmatullahAmin]

• first there is no need to reset the state. your resetUitState logic is to show the loading bar as i see which causes two time showing the loading bar. if a identifier is created, the dialog gets closed and the ClientIdentifiersScreen itself will do a loading and load all identifiers.
  Secondly if in case u did it there is no need to inititalize another viewmodel here in the screen just for the purpose of reseting the state you can do it in dialog screen itself.

[biplab1]

Regarding your first comment: If I remove clientIdentifiersDialogViewModel.resetUiState(), clicking the "+" icon right after creating an identifier reloads the identifier screen instead of opening the dialog. Clicking it again does open the dialog, but only on the second attempt. I had tried removing it earlier but kept it as is because of this issue. I’ve attached a screen recording showing what happens without resetting the state.

without-resetUi-dialogVM.webm

[HekmatullahAmin]

Hey @biplab1 , thanks for the explanation and the screen recording — that definitely helps clarify the behavior you're addressing.

If this issue (needing to click "+" twice) only happens when resetUiState() is removed, that likely indicates some state or logic isn’t being properly managed inside the ViewModel. Ideally, the screen itself should just reflect the ViewModel state — otherwise, this kind of logic in the UI layer can make the flow harder to follow and maintain, especially for others coming into the codebase later.

That said, no worries for now — I’ll approve the PR. But could you please add a short comment above clientIdentifiersDialogViewModel.resetUiState() explaining why it’s needed (to prevent confusion for future readers)?

Also, regarding the second point — did you try moving that reset logic into the dialog screen itself? That way, we could avoid initializing the ViewModel again just for this purpose in the parent screen. If that doesn’t work or breaks the flow, feel free to keep it as-is — just curious if it was tried.

[biplab1]

I did consider your second point, but the question is: where should the reset logic actually go?
If I put it inside DataState.Success in ClientIdentifiersDialogViewModel, we run into another problem — if we reset the state right after setting ClientIdentifierDialogUiState.IdentifierCreatedSuccessfully, there’s no chance for that success state to exist long enough for the snackbar to work. Setting it back to loading immediately cancels out the success state altogether.

Using something like delay(200) just to keep the success state alive feels like a bad workaround and not something we should rely on. If we set it back to loading too soon, anything depending on that success signal — like showing the snackbar — won’t work, and we still end up with two loading states visible.

The root issue is that after a successful submission, the dialog screen’s state remains stuck on IdentifierCreatedSuccessfully. So if the user clicks the + icon again immediately, it re-triggers onCreatedIdentifier() with the old state still active, which causes the main screen to reload.

Given all that, the only reasonable approach I could find was to directly call onCreatedIdentifier() together with resetUiState() on the ClientIdentifiersDialogViewModel after a successful submission. This makes sure the dialog closes cleanly and the main screen reloads fresh data without getting tripped up by stale state. The downside is that this still results in two back-to-back loading states — first in the dialog, then again in the main screen — but that seems unavoidable with the current flow.

[biplab1]

@niyajali Although this PR has received two approvals, I’m still exploring a better solution for managing the dialog state after a successful submission and for passing information back to the main screen to display snackbar messages. Also, clicking the ‘+’ icon right after a submission reloads the main screen due to the stale state of the dialog, and the dialog only opens on the second click because it’s then reset to a loading state. I suspect this might be a recurring issue in any screen that involves both dialog handling and snackbar communication. I could be wrong, so I’d appreciate your guidance here.

Currently, I’ve implemented a workaround that resets the dialog state in the main screen after a successful submission. The downside is that this causes two consecutive loading states — one in the dialog and another in the main screen after a successful submission.

[biplab1]

@niyajali If you’re okay with it for now, feel free to go ahead and merge it — we can revisit and improve it later if needed.

[niyajali]

@biplab1 you're adding a new item I don't see that on screen the last deleted item is just populating when state change so delete is also not working

[biplab1]

@niyajali If you are referring to the after-fix video in the PR description, here is the flow:

• There are two items initially.
• I delete one item — the screen refreshes and shows only one item.
• I add one more item — it appears as expected, watch for the ID change to see the new item.
• I click the "+" icon immediately after submission — the dialog screen opens.
• I do not add anything — no new item is added.

[biplab1]

@niyajali I have already clarified the flow of the video in the PR description, but to avoid any confusion, I am uploading another video that clearly shows the add and delete actions along with the web app reflecting the changes.

client-identifier-add-delete-web-app.mp4