Prompt user when inviting users with uncached identities by kaylendog · Pull Request #5331 · element-hq/element-x-ios

kaylendog · 2026-03-31T14:15:14Z

If the user attempts to invite someone (to a room or creating a DM) who's identity is not cached, we prompt them to make sure this was their intention.

Part of element-hq/element-meta#3163

Pull Request Checklist

I read the contributing guide.
Pull request contains a changelog label.

UI changes have been tested with:

iPhone and iPad simulators in portrait and landscape orientations.
Dark mode enabled and disabled.
Various sizes of dynamic type.
Voiceover enabled.

codecov · 2026-04-01T15:26:47Z

Codecov Report

❌ Patch coverage is 86.92308% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.46%. Comparing base (2b8c21a) to head (11c98cb).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...ens/InviteUsersScreen/View/InviteUsersScreen.swift	50.00%	3 Missing ⚠️
...ens/StartChatScreen/StartChatScreenViewModel.swift	78.57%	3 Missing ⚠️
...Screens/StartChatScreen/View/StartChatScreen.swift	0.00%	3 Missing ⚠️
...InviteUsersScreen/InviteUsersScreenViewModel.swift	92.59%	2 Missing ⚠️
...viteUsersScreen/View/ConfirmInviteUsersSheet.swift	94.73%	2 Missing ⚠️
...tailsScreen/RoomMemberDetailsScreenViewModel.swift	90.90%	1 Missing ⚠️
...erDetailsScreen/View/RoomMemberDetailsScreen.swift	66.66%	1 Missing ⚠️
...UserProfileScreen/UserProfileScreenViewModel.swift	50.00%	1 Missing ⚠️
...ens/UserProfileScreen/View/UserProfileScreen.swift	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #5331      +/-   ##
===========================================
+ Coverage    74.38%   74.46%   +0.07%     
===========================================
  Files          800      801       +1     
  Lines        53519    53639     +120     
===========================================
+ Hits         39811    39940     +129     
+ Misses       13708    13699       -9

Flag	Coverage Δ
unittests	`64.62% <86.92%> (+0.09%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kaylendog · 2026-04-01T18:17:55Z

ElementX/Sources/Screens/InviteUsersScreen/View/ConfirmInviteUsersSheet.swift

+import Compound
+import SwiftUI
+
+struct ConfirmInviteUsersSheetView: View {


I am wondering if it would be better to repurpose the existing SendInviteConfirmationView to support arbitrary numbers of members, but this seems to suffice for now.

I think it's fine too keep them separate but can we maybe name it InviteUsersConfirmationView so it somewhat matches the other one?

kaylendog · 2026-04-01T18:19:04Z

ElementX/Sources/Screens/InviteUsersScreen/InviteUsersScreenViewModel.swift

+                Task { @MainActor in
+                    // If we do not, we will prompt the user to confirm they meant to invite them.
+                    self.state.usersToConfirm.append(user)
+                }


There is a potential race condition here, but I'm not sure how to fix it and since we're only loading from the store, these tasks should be very short-lived.

these tasks should be very short-lived

I agree so why not run them directly on the current actor?

kaylendog · 2026-04-01T18:21:27Z

ElementX/Sources/Screens/InviteUsersScreen/InviteUsersScreenViewModel.swift

+                    MXLog.error("Failed to get cached user identity for \(user.userID)")
+                    return


It might be worth appending here even in the case of failure, like the changes to the other view models do.

Makes sense to me

kaylendog · 2026-04-01T18:23:11Z

I can't see why the Apple CI is failing, but otherwise I believe this is ready for review.

stefanceriu

Looking good, nice start 👍

stefanceriu · 2026-04-02T11:14:17Z

ElementX/Resources/Localizations/en.lproj/Untranslated.strings

+"crypto_history_sharing_confirm_invite_dialog_title" = "Invite new contacts to this room?";
+"crypto_history_sharing_confirm_invite_dialog_content" = "You currently don’t have any chats with these contacts. Confirm inviting them to this room before continuing.";
+"crypto_history_sharing_confirm_start_chat_dialog_title" = "Start a chat with this new contact?";
+"crypto_history_sharing_confirm_start_chat_dialog_content" = "You currently don’t have any chats with this person. Confirm inviting them before continuing.";


Should these have made it to localazy already?

It's on my list of things to do!

stefanceriu · 2026-04-02T11:26:11Z

ElementX/Sources/Screens/InviteUsersScreen/View/ConfirmInviteUsersSheet.swift

+import Compound
+import SwiftUI
+
+struct ConfirmInviteUsersSheetView: View {


I think it's fine too keep them separate but can we maybe name it InviteUsersConfirmationView so it somewhat matches the other one?

stefanceriu · 2026-04-02T11:34:35Z

ElementX/Sources/Screens/InviteUsersScreen/InviteUsersScreenModels.swift

+    /// Whether we are showing the confirmation dialog.
+    var presentConfirmationDialog = false


Can we infer this from usersToConfirm and have it be computed instead or at least set when that changes? 🤔

stefanceriu · 2026-04-02T11:41:14Z

ElementX/Sources/Screens/InviteUsersScreen/InviteUsersScreenViewModel.swift

+                Task { @MainActor in
+                    // If we do not, we will prompt the user to confirm they meant to invite them.
+                    self.state.usersToConfirm.append(user)
+                }


these tasks should be very short-lived

I agree so why not run them directly on the current actor?

stefanceriu · 2026-04-02T11:44:02Z

ElementX/Sources/Screens/InviteUsersScreen/InviteUsersScreenViewModel.swift

+                    MXLog.error("Failed to get cached user identity for \(user.userID)")
+                    return


Makes sense to me

stefanceriu · 2026-04-02T12:29:39Z

ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenViewModel.swift

+                        self.state.bindings.inviteConfirmationUserIdentityKnown = switch identity {
+                        case .success(let identity): identity != nil
+                        default: false
+                        }


Can also be written as

self.state.bindings.inviteConfirmationUserIdentityKnown = if case .success(let identity) = await self.userSession.clientProxy.userIdentity(for: roomMemberProxy.userID, fallBackToServer: false) { identity != nil } else { false }

stefanceriu · 2026-04-02T12:30:29Z

ElementX/Sources/Screens/StartChatScreen/View/SendInviteConfirmationView.swift


 struct SendInviteConfirmationView: View {
    let userToInvite: UserProfileProxy
+    let userIdentityKnown: Bool


I would rename this to isUserIdentityKnown

stefanceriu · 2026-04-02T12:31:28Z

ElementX/Sources/Screens/StartChatScreen/View/SendInviteConfirmationView.swift

        SendInviteConfirmationView(userToInvite: .mockBob,
+                                   userIdentityKnown: true,
+                                   mediaProvider: nil) { }
+            .previewDisplayName("With Identity")


Technically With identity known and unknown as you're actually passing the identity in.

stefanceriu · 2026-04-02T12:32:25Z

ElementX/Sources/Screens/StartChatScreen/StartChatScreenModels.swift

    var alertInfo: AlertInfo<StartChatScreenErrorType>?

    var selectedUserToInvite: UserProfileProxy?
+    var selectedUserIdentityKnown = false


Same question here, why is this a binding?

stefanceriu · 2026-04-02T12:33:38Z

ElementX/Sources/Screens/StartChatScreen/StartChatScreenViewModel.swift

+                if appSettings.enableKeyShareOnInvite {
+                    Task.detached {
+                        let identity = await self.userSession.clientProxy.userIdentity(for: user.userID, fallBackToServer: false)
+                        Task { @MainActor in


kaylendog self-assigned this Mar 31, 2026

kaylendog force-pushed the kaylendog/history-sharing/prompt branch 2 times, most recently from 6a44d49 to edeaec7 Compare March 31, 2026 16:28

kaylendog added 3 commits April 1, 2026 18:16

feat: Prompt user when inviting users with uncached identities

8a1ef11

tests: Add unit tests for inviting users with no cached identity

b625db7

tests: Add preview tests for confirmation sheet

23b8da1

kaylendog force-pushed the kaylendog/history-sharing/prompt branch from d6c1c97 to 2761e63 Compare April 1, 2026 17:18

kaylendog added 3 commits April 1, 2026 19:12

feat: Make SendInviteConfirmationView aware of user identity

a93b5bc

tests: Update snapshots for SendInviteConfirmationView

d59f106

tests: Add unit tests for identity fetching in invite confirmation

11c98cb

kaylendog force-pushed the kaylendog/history-sharing/prompt branch from 2761e63 to 11c98cb Compare April 1, 2026 18:13

kaylendog added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Apr 1, 2026

kaylendog commented Apr 1, 2026

View reviewed changes

kaylendog marked this pull request as ready for review April 1, 2026 18:23

kaylendog requested a review from a team as a code owner April 1, 2026 18:23

kaylendog requested review from stefanceriu and removed request for a team April 1, 2026 18:23

kaylendog mentioned this pull request Apr 1, 2026

History sharing: should we warn the user if they are inviting someone they haven't previously TOFUed? element-hq/element-meta#3163

Open

3 tasks

stefanceriu reviewed Apr 2, 2026

View reviewed changes

		MXLog.error("Failed to get cached user identity for \(user.userID)")
		return

		/// Whether we are showing the confirmation dialog.
		var presentConfirmationDialog = false

Conversation

kaylendog commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Checklist

Uh oh!

codecov bot commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaylendog commented Apr 1, 2026

Uh oh!

stefanceriu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kaylendog commented Mar 31, 2026 •

edited

Loading

codecov bot commented Apr 1, 2026 •

edited

Loading