Skip to content

Show distinct message when Thread sync requires admin account#6816

Draft
agners wants to merge 2 commits intohome-assistant:mainfrom
agners:fix/thread-sync-non-admin-message
Draft

Show distinct message when Thread sync requires admin account#6816
agners wants to merge 2 commits intohome-assistant:mainfrom
agners:fix/thread-sync-non-admin-message

Conversation

@agners
Copy link
Copy Markdown
Member

@agners agners commented May 7, 2026

Summary

When syncing Thread credentials as a non-admin user on the Home Assistant server, the app showed "The Home Assistant server does not support Thread", which is misleading.

7049b449-c5c5-471c-95a4-a511efaafc12

This change splits the admin check out of coreSupportsThread so syncPreferredDataset can return a new ServerUserNotAdmin result distinct from ServerUnsupported, and surfaces a dedicated message:

Managing Thread credentials requires an administrator account on the Home Assistant server

Checklist

  • New or updated tests have been added to cover the changes following the testing guidelines.
  • The code follows the project's code style and best_practices.
  • The changes have been thoroughly tested, and edge cases have been considered.
  • Changes are backward compatible whenever feasible. Any breaking changes are documented in the changelog for users and/or in the code for developers depending on the relevance.

🤖 Generated with Claude Code

When a non-admin account tried to sync Thread credentials, the app
showed "The Home Assistant server does not support Thread", which is
misleading. Surface a dedicated result and message indicating that
managing Thread credentials requires an administrator account.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 7, 2026 09:57
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves Thread credential sync feedback when the Home Assistant server supports Thread but the signed-in user lacks admin privileges, avoiding misleading “server unsupported” messaging.

Changes:

  • Add a new ThreadManager.SyncResult.ServerUserNotAdmin result to distinguish permission failures from true server incompatibility
  • Split the Thread “component present” check into a separate helper and use it in syncPreferredDataset
  • Surface a dedicated developer settings message when the user is not an admin

Reviewed changes

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

File Description
common/src/main/res/values/strings.xml Adds a new string for the “admin required” Thread sync result
app/src/main/kotlin/io/homeassistant/companion/android/thread/ThreadManager.kt Extends the sync result sealed class with ServerUserNotAdmin
app/src/main/kotlin/io/homeassistant/companion/android/settings/developer/DeveloperSettingsPresenterImpl.kt Displays the new “admin required” message for Thread debug sync
app/src/full/kotlin/io/homeassistant/companion/android/thread/ThreadManagerImpl.kt Separates server capability check from user/admin gating inside sync logic

- Make coreSupportsThread strictly a server-capability check; gate admin
  separately at call sites so the contract matches the function name and
  KDoc.
- Surface ServerUserNotAdmin in the WebView/frontend Thread export flow
  via a new MatterThreadStep so users see a dedicated admin-required
  dialog instead of the generic Thread error.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HomeAssistantVersion.fromString(config.version)?.isAtLeast(2023, 3, 0) == true
}

private fun userIsAdmin(serverId: Int): Boolean =
HomeAssistantVersion.fromString(config.version)?.isAtLeast(2023, 3, 0) == true
}

private fun userIsAdmin(serverId: Int): Boolean =
@TimoPtr TimoPtr marked this pull request as draft May 7, 2026 11:49
@jpelgrom
Copy link
Copy Markdown
Member

jpelgrom commented May 7, 2026

No objections to merge from me once ktlint is addressed

(Note in #3417 I simply used the existing 'server doesn't support' state out of convenience, not assuming this would be around for so long and the APIs would improve but unfortunately not)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants