From f9ad2235f20be25ae53036dda7b7ad6c1e343940 Mon Sep 17 00:00:00 2001 From: Rick van den Bergh Date: Tue, 3 Nov 2026 00:38:12 +0100 Subject: [PATCH] fix(sync): name the locked note and explain how to unlock it When the server answers 423 Locked for a note, the sync message only said that "a note is open in another app". In practice the lock is often a stale Files Lock entry left behind by the Text app, the note is not open anywhere, and the user cannot tell which note is affected or what to do. The sync error now names the locked note (or counts them), tells the user to close it in the other app or unlock it in the web interface, and the error dialog gets two tips with the concrete steps. Relates to #3387, #2332, #3420 AI-assistant: Claude Code 2.1.309 (Claude Fable 5.1) Signed-off-by: Rick van den Bergh --- .../notes/exception/tips/TipsAdapter.java | 5 ++ .../owncloud/notes/main/MainActivity.java | 6 +- .../owncloud/notes/main/SyncErrorMessage.kt | 37 +++++++-- .../notes/persistence/NoteSyncException.kt | 2 +- .../notes/persistence/SyncFailureReason.kt | 2 +- app/src/main/res/values/strings.xml | 9 +- .../notes/main/SyncErrorMessageTest.kt | 83 +++++++++++++++++++ 7 files changed, 130 insertions(+), 14 deletions(-) create mode 100644 app/src/test/java/it/niedermann/owncloud/notes/main/SyncErrorMessageTest.kt diff --git a/app/src/main/java/it/niedermann/owncloud/notes/exception/tips/TipsAdapter.java b/app/src/main/java/it/niedermann/owncloud/notes/exception/tips/TipsAdapter.java index 8806122f8..6561f62aa 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/exception/tips/TipsAdapter.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/exception/tips/TipsAdapter.java @@ -36,6 +36,8 @@ import it.niedermann.owncloud.notes.BuildConfig; import it.niedermann.owncloud.notes.R; +import it.niedermann.owncloud.notes.persistence.NoteSyncException; +import it.niedermann.owncloud.notes.persistence.SyncFailureReason; public class TipsAdapter extends RecyclerView.Adapter { @@ -89,6 +91,9 @@ public void setThrowables(@NonNull List throwables) { add(R.string.error_dialog_timeout_toggle, new Intent(Settings.ACTION_WIFI_SETTINGS).putExtra(INTENT_EXTRA_BUTTON_TEXT, R.string.error_action_open_network)); } else if (throwable instanceof JSONException || throwable instanceof NullPointerException) { add(R.string.error_dialog_check_server); + } else if (throwable instanceof NoteSyncException syncException && syncException.getReason() == SyncFailureReason.LOCKED) { + add(R.string.error_dialog_tip_note_locked_close); + add(R.string.error_dialog_tip_note_locked_unlock); } else if (throwable instanceof NextcloudHttpRequestFailedException) { final int statusCode = ((NextcloudHttpRequestFailedException) throwable).getStatusCode(); switch (statusCode) { diff --git a/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java b/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java index 2f8fe3855..d3e891361 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java @@ -216,7 +216,7 @@ protected void onCreate(Bundle savedInstanceState) { mainViewModel.getSyncStatus().observe(this, syncStatus -> swipeRefreshLayout.setRefreshing(syncStatus)); mainViewModel.getSyncErrors().observe(this, exceptions -> { if (mainViewModel.containsNonInfrastructureRelatedItems(exceptions)) { - BrandedSnackbar.make(coordinatorLayout, SyncErrorMessage.of(exceptions), Snackbar.LENGTH_LONG) + BrandedSnackbar.make(coordinatorLayout, SyncErrorMessage.of(MainActivity.this, exceptions), Snackbar.LENGTH_LONG) .setAnchorView(binding.activityNotesListView.fabCreate) .setAction(R.string.simple_more, v -> ExceptionDialogFragment.newInstance(exceptions) .show(getSupportFragmentManager(), ExceptionDialogFragment.class.getSimpleName())) @@ -325,7 +325,7 @@ public void onError(@NonNull Throwable t) { .setAnchorView(binding.activityNotesListView.fabCreate) .show(); } else { - BrandedSnackbar.make(coordinatorLayout, SyncErrorMessage.of(t), Snackbar.LENGTH_LONG) + BrandedSnackbar.make(coordinatorLayout, SyncErrorMessage.of(MainActivity.this, t), Snackbar.LENGTH_LONG) .setAction(R.string.simple_more, v -> ExceptionDialogFragment.newInstance(t) .show(getSupportFragmentManager(), ExceptionDialogFragment.class.getSimpleName())) .setAnchorView(binding.activityNotesListView.fabCreate) @@ -609,7 +609,7 @@ public void onError(@NonNull Throwable t) { .setAnchorView(binding.activityNotesListView.fabCreate) .show(); } else { - BrandedSnackbar.make(coordinatorLayout, SyncErrorMessage.of(t), Snackbar.LENGTH_LONG) + BrandedSnackbar.make(coordinatorLayout, SyncErrorMessage.of(MainActivity.this, t), Snackbar.LENGTH_LONG) .setAction(R.string.simple_more, v -> ExceptionDialogFragment.newInstance(t) .show(getSupportFragmentManager(), ExceptionDialogFragment.class.getSimpleName())) .setAnchorView(binding.activityNotesListView.fabCreate) diff --git a/app/src/main/java/it/niedermann/owncloud/notes/main/SyncErrorMessage.kt b/app/src/main/java/it/niedermann/owncloud/notes/main/SyncErrorMessage.kt index 3585d4b15..46e995fb1 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/main/SyncErrorMessage.kt +++ b/app/src/main/java/it/niedermann/owncloud/notes/main/SyncErrorMessage.kt @@ -6,27 +6,48 @@ */ package it.niedermann.owncloud.notes.main -import androidx.annotation.StringRes +import android.content.Context import com.nextcloud.android.sso.exceptions.NextcloudApiNotRespondingException import com.nextcloud.android.sso.exceptions.NextcloudHttpRequestFailedException import com.nextcloud.android.sso.exceptions.NextcloudNetworkException import com.nextcloud.android.sso.exceptions.TokenMismatchException +import it.niedermann.owncloud.notes.R import it.niedermann.owncloud.notes.persistence.NoteSyncException import it.niedermann.owncloud.notes.persistence.SyncFailureReason object SyncErrorMessage { @JvmStatic - @StringRes - fun of(exceptions: Collection): Int = - (exceptions.map { it.syncFailureReason() }.distinct().singleOrNull() ?: SyncFailureReason.UNKNOWN).message + fun of(context: Context, exceptions: Collection): String { + val reasons = exceptions.map { it.syncFailureReason() }.distinct() + val reason = reasons.singleOrNull() ?: SyncFailureReason.UNKNOWN + if (reason != SyncFailureReason.LOCKED) { + return context.getString(reason.message) + } + return lockedMessage(context, exceptions.mapNotNull { it.lockedNoteTitle() }.distinct()) + } @JvmStatic - @StringRes - fun of(exception: Throwable): Int = exception.syncFailureReason().message + fun of(context: Context, exception: Throwable): String = of(context, listOf(exception)) + + private fun lockedMessage(context: Context, lockedNoteTitles: List): String = when (lockedNoteTitles.size) { + 0 -> context.getString(SyncFailureReason.LOCKED.message) + 1 -> context.getString(R.string.error_sync_note_locked, lockedNoteTitles.single()) + else -> context.resources.getQuantityString( + R.plurals.error_sync_notes_locked, + lockedNoteTitles.size, + lockedNoteTitles.size, + ) + } + + private fun Throwable.lockedNoteTitle(): String? = causeChain() + .filterIsInstance() + .firstOrNull { it.reason == SyncFailureReason.LOCKED } + ?.noteTitle private fun Throwable.syncFailureReason(): SyncFailureReason = - generateSequence(this) { it.cause }.firstNotNullOfOrNull { it.directSyncFailureReason() } - ?: SyncFailureReason.UNKNOWN + causeChain().firstNotNullOfOrNull { it.directSyncFailureReason() } ?: SyncFailureReason.UNKNOWN + + private fun Throwable.causeChain(): Sequence = generateSequence(this) { it.cause } private fun Throwable.directSyncFailureReason(): SyncFailureReason? = when (this) { is NoteSyncException -> reason diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NoteSyncException.kt b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NoteSyncException.kt index 4a666292a..0ec51b96f 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NoteSyncException.kt +++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NoteSyncException.kt @@ -8,7 +8,7 @@ package it.niedermann.owncloud.notes.persistence import retrofit2.Response -class NoteSyncException(noteTitle: String, response: Response<*>) : +class NoteSyncException(val noteTitle: String, response: Response<*>) : Exception("Could not sync \"$noteTitle\" (HTTP ${response.code()}): ${response.message()}") { val reason = SyncFailureReason.fromStatusCode(response.code()) } diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/SyncFailureReason.kt b/app/src/main/java/it/niedermann/owncloud/notes/persistence/SyncFailureReason.kt index 34dbed04d..7276cfd7c 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/SyncFailureReason.kt +++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/SyncFailureReason.kt @@ -18,7 +18,7 @@ enum class SyncFailureReason(@StringRes val message: Int) { UNAUTHORIZED(R.string.error_sync_unauthorized), PERMISSION(R.string.error_sync_permission), NOTES_APP_UNAVAILABLE(R.string.error_sync_notes_app_unavailable), - LOCKED(R.string.error_sync_locked), + LOCKED(R.string.error_sync_unknown_note_locked), SERVER_ERROR(R.string.error_sync_server_error), MAINTENANCE(R.string.error_sync_maintenance), INSUFFICIENT_STORAGE(R.string.error_sync_insufficient_storage), diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 9dd008cac..3b803caaa 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -195,7 +195,12 @@ Synchronization failed: %1$s Synchronization failed - A note is open in another app. Close it there to sync your changes. + A note is locked on the server. Close it in the other app, or unlock it in the web interface. + “%1$s” is locked on the server. Close it in the other app, or unlock it in the web interface. + + %d note is locked on the server. Close it in the other app, or unlock it in the web interface. + %d notes are locked on the server. Close them in the other app, or unlock them in the web interface. + Your login is no longer valid. Log in again in the Nextcloud Files app. You don’t have permission to save this note. Please check your access and try again. The Notes app is not available on your server. @@ -354,6 +359,8 @@ Your server did respond with a HTTP 302 status code, which implies, that you do not have installed the Notes app on your server or something is misconfigured. This can be caused by custom overrides in a .htaccess-file or by Nextcloud apps like OID Client. Please disable all battery optimizations for Nextcloud and the Notes app. The Notes Android app requires the Nextcloud Android app to be at least version 3.18. + The note is locked on the server. This usually means it is open in the Text editor or in another app. Close it there and sync again. + If the note is not open anywhere, the lock is stale. In the Nextcloud web interface, open Files, find the note in the Notes folder, open its menu and choose Unlock. If that is not offered, ask your administrator to remove the lock. Added "%1$s" Shared text was empty Append to note diff --git a/app/src/test/java/it/niedermann/owncloud/notes/main/SyncErrorMessageTest.kt b/app/src/test/java/it/niedermann/owncloud/notes/main/SyncErrorMessageTest.kt new file mode 100644 index 000000000..440608425 --- /dev/null +++ b/app/src/test/java/it/niedermann/owncloud/notes/main/SyncErrorMessageTest.kt @@ -0,0 +1,83 @@ +/* + * Nextcloud Notes - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package it.niedermann.owncloud.notes.main + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import it.niedermann.owncloud.notes.R +import it.niedermann.owncloud.notes.persistence.NoteSyncException +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import retrofit2.Response + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [36]) +class SyncErrorMessageTest { + + private val context: Context = ApplicationProvider.getApplicationContext() + + @Test + fun singleLockedNoteIsNamed() { + val message = SyncErrorMessage.of(context, listOf(noteSyncException(GROCERIES, HTTP_LOCKED))) + + assertEquals(context.getString(R.string.error_sync_note_locked, GROCERIES), message) + } + + @Test + fun severalLockedNotesAreCounted() { + val exceptions = listOf( + noteSyncException(GROCERIES, HTTP_LOCKED), + noteSyncException(INVOICES, HTTP_LOCKED), + ) + + val message = SyncErrorMessage.of(context, exceptions) + + assertEquals(context.resources.getQuantityString(R.plurals.error_sync_notes_locked, 2, 2), message) + } + + @Test + fun lockedNoteIsFoundInCauseChain() { + val wrapped = RuntimeException(IllegalStateException(noteSyncException(GROCERIES, HTTP_LOCKED))) + + val message = SyncErrorMessage.of(context, wrapped) + + assertEquals(context.getString(R.string.error_sync_note_locked, GROCERIES), message) + } + + @Test + fun lockedTogetherWithOtherFailuresFallsBackToGenericMessage() { + val exceptions = listOf( + noteSyncException(GROCERIES, HTTP_LOCKED), + noteSyncException(INVOICES, HTTP_INSUFFICIENT_STORAGE), + ) + + val message = SyncErrorMessage.of(context, exceptions) + + assertEquals(context.getString(R.string.error_synchronization), message) + } + + @Test + fun otherFailureReasonsKeepTheirMessage() { + val message = SyncErrorMessage.of(context, noteSyncException(GROCERIES, HTTP_INSUFFICIENT_STORAGE)) + + assertEquals(context.getString(R.string.error_sync_insufficient_storage), message) + } + + private fun noteSyncException(title: String, statusCode: Int) = + NoteSyncException(title, Response.error(statusCode, "".toResponseBody())) + + companion object { + private const val GROCERIES = "Groceries" + private const val INVOICES = "Invoices" + private const val HTTP_LOCKED = 423 + private const val HTTP_INSUFFICIENT_STORAGE = 507 + } +}