From 217be197077f4290e436238c1a6cc205890e0d38 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 22 Sep 2026 10:45:56 -0400 Subject: [PATCH 1/4] feat(chat): edit a group's name and icon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Puts an overflow menu on the chat profile screen with a single Edit item, gated on the server-computed `canEdit` permission and nothing else, and pushes a list with two rows behind it: Icon and Name. The design (node 10187:110373) draws four rows. Only Icon has contract support — `EditChatRequest` carries a title and a picture, so Membership Card, Description and Social Links would be rows that cannot save. Name is the counterpart the design omits, styled to match Icon. Both rows are partial updates. `EditChat` treats an unset field as unchanged, so each screen submits only its own field, which is what keeps a rename from clearing the picture. The title goes through the existing `GroupTitleValidator` for the contract's 1...64 before it is sent, and the picture waits for the blob to reach READY — a pending blob is answered `PICTURE_BLOB_NOT_ACCEPTED`. On OK the response metadata is applied through the same store mutators the `titleChanged` and `pictureChanged` stream events use, rather than through `.metadataRefresh`. Both are last-write-wins assignments, so the server's echo re-applies identical values instead of landing the edit twice, and the held membership and viewer state survive — a wholesale upsert would drop them. Error copy mirrors the StartChat mapping case for case, so the same server answer reads the same way wherever a group name is submitted. A moderated title keeps the user on the edit screen with the field as they typed it. --- .../Controllers/ConversationController.swift | 22 ++ .../Navigation/AppRouter+Destination.swift | 18 +- .../AppRouter+DestinationView.swift | 18 + .../Screens/Profile/ChatProfileScreen.swift | 29 ++ .../Edit Group/EditGroupIconScreen.swift | 233 +++++++++++++ .../Profile/Edit Group/EditGroupModel.swift | 154 +++++++++ .../Edit Group/EditGroupNameScreen.swift | 180 ++++++++++ .../Profile/Edit Group/EditGroupScreen.swift | 70 ++++ .../Profile/Edit Group/GroupChatEditing.swift | 63 ++++ .../Conversation/ConversationRules.swift | 14 + FlipcashTests/Chat/EditGroupModelTests.swift | 315 ++++++++++++++++++ .../FlipcashUI/Theme/Image+Symbols.swift | 6 + 12 files changed, 1120 insertions(+), 2 deletions(-) create mode 100644 Flipcash/Core/Screens/Profile/Edit Group/EditGroupIconScreen.swift create mode 100644 Flipcash/Core/Screens/Profile/Edit Group/EditGroupModel.swift create mode 100644 Flipcash/Core/Screens/Profile/Edit Group/EditGroupNameScreen.swift create mode 100644 Flipcash/Core/Screens/Profile/Edit Group/EditGroupScreen.swift create mode 100644 Flipcash/Core/Screens/Profile/Edit Group/GroupChatEditing.swift create mode 100644 FlipcashTests/Chat/EditGroupModelTests.swift diff --git a/Flipcash/Core/Controllers/ConversationController.swift b/Flipcash/Core/Controllers/ConversationController.swift index 803764a4f..c445e99c3 100644 --- a/Flipcash/Core/Controllers/ConversationController.swift +++ b/Flipcash/Core/Controllers/ConversationController.swift @@ -666,6 +666,28 @@ final class ConversationController { refreshFeedPreview(for: conversation.id) } + /// Seats the metadata `EditChat` returns for a chat the signed-in user just edited, so its + /// profile and the Chats list show the new title or picture without waiting for anything else. + /// + /// Applies the two fields through the very mutators the `titleChanged` / `pictureChanged` stream + /// events apply — deliberately, rather than `.metadataRefresh`. Two reasons, both load-bearing: + /// + /// - `.metadataRefresh` upserts the whole conversation, replacing the held row outright. The + /// edit response carries chat metadata, not the locally-held membership flag, viewer state or + /// roster, so a wholesale replace would drop them. + /// - Both mutators are last-write-wins assignments, so when the server's own echo of this edit + /// arrives on the stream it re-applies identical values. The local apply and the echo are the + /// same write, which is what keeps an edit from landing twice. + func applyEdit(_ conversation: Conversation) { + if let title = conversation.title { + store.applyTitleChanged(title, in: conversation.id) + } + if let picture = conversation.picture { + store.applyPictureChanged(picture, in: conversation.id) + } + persistConversation(conversation.id) + } + /// Leaves a group. The chat stays in the store — the screen the user left from is still on top and /// needs it to render the gate — but drops out of ``joinedGroups``, so it leaves the Chats list. /// diff --git a/Flipcash/Core/Navigation/AppRouter+Destination.swift b/Flipcash/Core/Navigation/AppRouter+Destination.swift index 0196b0d4f..71a6277b1 100644 --- a/Flipcash/Core/Navigation/AppRouter+Destination.swift +++ b/Flipcash/Core/Navigation/AppRouter+Destination.swift @@ -109,6 +109,13 @@ extension AppRouter { case userProfile(UserID) /// A group chat's own profile, pushed from its head card or its navigation title. case chatProfile(ConversationID) + /// What an editor can change about a group, pushed from the chat profile's overflow menu. + /// Only reachable while ``Conversation/canEdit`` holds. + case editGroup(ConversationID) + /// Renaming a group, pushed from the Name row of `editGroup`. + case editGroupName(ConversationID) + /// Replacing a group's picture, pushed from the Icon row of `editGroup`. + case editGroupIcon(ConversationID) /// The stack this destination naturally belongs in. Cross-stack /// navigation uses this to know which sheet to present, or which tab @@ -132,7 +139,8 @@ extension AppRouter { return .you case .profileName, .profilePhoto, .tipcard, .usernameLookup, .newChat, .newPublicGroup, .tipConversation, .tipConversationWithKeyboard, .tipConversationForUser, - .userProfile, .chatProfile: + .userProfile, .chatProfile, + .editGroup, .editGroupName, .editGroupIcon: return .tips } } @@ -182,6 +190,9 @@ extension AppRouter { case .tipConversationForUser: "tipConversationForUser" case .userProfile: "userProfile" case .chatProfile: "chatProfile" + case .editGroup: "editGroup" + case .editGroupName: "editGroupName" + case .editGroupIcon: "editGroupIcon" } } @@ -203,7 +214,10 @@ extension AppRouter { return activity.id.base58 case .tipConversation(let conversationID), .tipConversationWithKeyboard(let conversationID), - .chatProfile(let conversationID): + .chatProfile(let conversationID), + .editGroup(let conversationID), + .editGroupName(let conversationID), + .editGroupIcon(let conversationID): return conversationID.description case .userProfile(let userID), .tipConversationForUser(let userID): diff --git a/Flipcash/Core/Navigation/AppRouter+DestinationView.swift b/Flipcash/Core/Navigation/AppRouter+DestinationView.swift index 94bc87726..cf37f817b 100644 --- a/Flipcash/Core/Navigation/AppRouter+DestinationView.swift +++ b/Flipcash/Core/Navigation/AppRouter+DestinationView.swift @@ -15,6 +15,7 @@ struct DestinationView: View { @Environment(Container.self) private var container @Environment(SessionContainer.self) private var sessionContainer + @Environment(ConversationController.self) private var conversationController let destination: AppRouter.Destination @@ -196,6 +197,23 @@ struct DestinationView: View { case .chatProfile(let conversationID): ChatProfileScreen(conversationID: conversationID) .id(conversationID) + + case .editGroup(let conversationID): + EditGroupScreen(conversationID: conversationID) + .id(conversationID) + + case .editGroupName(let conversationID): + // Seeded here, like `.changeDisplayName` above, so the field opens on the name it is + // about to replace instead of animating it in over the push. + EditGroupNameScreen( + conversationID: conversationID, + currentTitle: conversationController.conversation(withID: conversationID)?.title ?? "" + ) + .id(conversationID) + + case .editGroupIcon(let conversationID): + EditGroupIconScreen(conversationID: conversationID) + .id(conversationID) } } } diff --git a/Flipcash/Core/Screens/Profile/ChatProfileScreen.swift b/Flipcash/Core/Screens/Profile/ChatProfileScreen.swift index 7859281f3..4c3623a8d 100644 --- a/Flipcash/Core/Screens/Profile/ChatProfileScreen.swift +++ b/Flipcash/Core/Screens/Profile/ChatProfileScreen.swift @@ -50,6 +50,15 @@ struct ChatProfileScreen: View { return conversationController.isMember(of: conversation) } + /// Whether to offer the overflow menu at all: it holds Edit and nothing else, so with the edit + /// permission withheld there is no menu to draw. + /// + /// Reads ``Conversation/canEdit`` — the server's answer — and nothing beside it. Membership and + /// creator identity are deliberately not consulted; see that property. + private var canEdit: Bool { + conversation?.canEdit ?? false + } + var body: some View { Background(color: .backgroundMain) { VStack(spacing: 16) { @@ -125,6 +134,26 @@ struct ChatProfileScreen: View { } .navigationTitle("") .toolbarTitleDisplayMode(.inline) + .toolbar { + if canEdit { + ToolbarItem(placement: .topBarTrailing) { + Menu { + Button { + router.push(.editGroup(conversationID)) + } label: { + Label { + Text("Edit") + } icon: { + Image.system(.pencil) + } + } + } label: { + Image.system(.ellipsis) + } + .accessibilityIdentifier("chat-profile-overflow") + } + } + } .dialog(item: $dialogItem) .sheet(isPresented: $isInviting) { GroupInviteSheet(conversationID: conversationID, isPresented: $isInviting) diff --git a/Flipcash/Core/Screens/Profile/Edit Group/EditGroupIconScreen.swift b/Flipcash/Core/Screens/Profile/Edit Group/EditGroupIconScreen.swift new file mode 100644 index 000000000..ca935c8db --- /dev/null +++ b/Flipcash/Core/Screens/Profile/Edit Group/EditGroupIconScreen.swift @@ -0,0 +1,233 @@ +// +// EditGroupIconScreen.swift +// Flipcash +// + +import SwiftUI +import UniformTypeIdentifiers +import FlipcashCore +import FlipcashUI + +private let logger = Logger(label: "flipcash.edit-group-icon") + +/// Replaces a group's picture. The Icon row's destination (node 10187:110373), drawn the way +/// ``ProfilePhotoScreen`` draws the profile photo: the picture itself is the picker. +struct EditGroupIconScreen: View { + + let conversationID: ConversationID + + @Environment(Container.self) private var container + @Environment(SessionContainer.self) private var sessionContainer + @Environment(ConversationController.self) private var conversationController + @Environment(AppRouter.self) private var router + + @State private var model = EditGroupModel() + @State private var isShowingPhotoPicker = false + @State private var isShowingFilePicker = false + @State private var submitTask: Task? + @State private var dialog: DialogItem? + + /// Drives Save: spinner while uploading, then the checkmark the rest of the app shows on a + /// completed action. + @State private var buttonState: ButtonState = .normal + + /// The picture already on the group, so the screen opens on what it is about to replace rather + /// than on an empty circle. Drawn but never submitted — Save stays shut until a new one is + /// picked. + @State private var currentIcon: UIImage? + + private static let avatarSize: CGFloat = 158 + private static let plusSize: CGFloat = 64 + + private var isSubmitting: Bool { submitTask != nil } + + private var conversation: Conversation? { + conversationController.conversation(withID: conversationID) + } + + var body: some View { + Background(color: .backgroundMain) { + VStack(spacing: 0) { + Spacer() + + Menu { + Button("Photo Library", systemImage: "photo.on.rectangle") { isShowingPhotoPicker = true } + Button("Choose File", systemImage: "folder") { isShowingFilePicker = true } + } label: { + CircleImage( + image: model.picture ?? currentIcon, + size: Self.avatarSize, + plusSize: Self.plusSize + ) + } + .menuIndicator(.hidden) + .disabled(isSubmitting) + .accessibilityIdentifier("edit-group-icon-picker") + + if let title = conversation?.title { + Text(title) + .font(.appDisplayCompact) + .foregroundStyle(Color.textMain) + .lineLimit(1) + .padding(.top, 21) + } + + Spacer() + + Button(action: submit) { + ButtonStateLabel("Save", state: buttonState) + } + .buttonStyle(.filled) + // The checkmark hold keeps the picture selected, so the button needs the state to + // stay shut against a second submission. + .disabled(!model.canSavePicture || !buttonState.isNormal || isSubmitting) + .accessibilityIdentifier("edit-group-icon-save-button") + .padding(.bottom, 20) + } + .padding(.horizontal, 20) + .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .top) + } + .navigationTitle("Icon") + .toolbarTitleDisplayMode(.inline) + .dialog(item: $dialog) + .fullScreenCover(isPresented: $isShowingPhotoPicker) { + ImagePickerWithEditor( + onImagePicked: model.select(picture:), + onDismiss: { isShowingPhotoPicker = false } + ) + .ignoresSafeArea() + } + .fileImporter( + isPresented: $isShowingFilePicker, + allowedContentTypes: [.image], + allowsMultipleSelection: false, + onCompletion: handleFileImport + ) + // Keyed on the blob so a picture that changes underneath — including the one this screen + // just saved — is what the circle ends up drawing. + .task(id: conversation?.picture?.thumbnailBlobID) { + currentIcon = await ProfilePictureLoader.thumbnail( + for: conversation?.picture, + using: container.flipClient, + owner: sessionContainer.session.ownerKeyPair + ) + } + .onDisappear { submitTask?.cancel() } + } + + private func submit() { + guard model.canSavePicture, !isSubmitting else { return } + + buttonState = .loading + + submitTask = Task { + defer { submitTask = nil } + + do { + let conversation = try await model.savePicture( + for: conversationID, + using: SessionGroupChatEditor( + session: sessionContainer.session, + flipClient: container.flipClient + ) + ) + + // The response carries the post-edit metadata, so the store is seated from it + // rather than from a refetch. + conversationController.applyEdit(conversation) + + buttonState = .success + // Same beat the rest of the app holds its checkmark for. + try? await Task.delay(milliseconds: 500) + + guard !Task.isCancelled else { return } + router.popTopmost() + + } catch { + buttonState = .normal + handle(error) + } + } + } + + /// Mirrors ``NewPublicGroupScreen``'s mapping so the same server answer reads the same way + /// wherever a group picture is submitted. + private func handle(_ error: Error) { + guard !Task.isCancelled else { return } + + switch error { + case ErrorEditChat.pictureBlobNotAccepted: + logger.info("Group picture not accepted") + ErrorReporting.captureError(error, reason: "Group picture not accepted") + dialog = .error( + title: "This Photo Isn't Allowed", + subtitle: "Try a different photo" + ) + + case ErrorEditChat.denied: + logger.info("Group edit denied") + ErrorReporting.captureError(error, reason: "Group edit denied") + dialog = .error( + title: "You Can't Edit This Group", + subtitle: "Try again later" + ) + + case ErrorEditChat.notFound: + logger.info("Group edit target not found") + ErrorReporting.captureError(error, reason: "Group edit target not found") + dialog = .error( + title: "This Group No Longer Exists", + subtitle: "It may have been deleted" + ) + + case let blobError as ErrorBlob: + logger.info("Group picture upload failed", metadata: ["error": "\(blobError)"]) + ErrorReporting.captureError(blobError, reason: "Group picture upload failed", userFacing: true) + dialog = .profilePictureFailed(blobError) + + case let encoderError as ImageEncoderError: + logger.error("Failed to encode the group picture", metadata: ["error": "\(encoderError)"]) + ErrorReporting.captureError(encoderError, reason: "Failed to encode the group picture") + dialog = .imageProcessingFailed + + default: + logger.error("Failed to edit group icon", metadata: ["error": "\(error)"]) + ErrorReporting.captureError(error, reason: "Failed to edit group icon") + dialog = .error( + title: "Couldn't Save This Photo", + subtitle: "Try again" + ) + } + } + + private func handleFileImport(_ result: Result<[URL], Error>) { + switch result { + case .failure(let error): + logger.info("Group icon file import failed", metadata: ["error": "\(error)"]) + return + + case .success(let urls): + guard let url = urls.first else { return } + importImage(at: url) + } + } + + private func importImage(at url: URL) { + Task { + guard url.startAccessingSecurityScopedResource() else { return } + defer { url.stopAccessingSecurityScopedResource() } + + let image = await Task.detached(priority: .userInitiated) { () -> UIImage? in + guard let data = try? Data(contentsOf: url) else { return nil } + return UIImage(data: data) + }.value + + guard let image else { + dialog = .imageProcessingFailed + return + } + + model.select(picture: image) + } + } +} diff --git a/Flipcash/Core/Screens/Profile/Edit Group/EditGroupModel.swift b/Flipcash/Core/Screens/Profile/Edit Group/EditGroupModel.swift new file mode 100644 index 000000000..f33bf540c --- /dev/null +++ b/Flipcash/Core/Screens/Profile/Edit Group/EditGroupModel.swift @@ -0,0 +1,154 @@ +// +// EditGroupModel.swift +// Flipcash +// + +import UIKit +import FlipcashCore + +/// Thrown when a save runs against a form that has nothing the server would accept. Both screens +/// disable their button in that state, so this is a programming error rather than something either +/// puts in front of the user. +struct EditGroupIncomplete: Error {} + +/// What a single group edit holds while it is being made, and the one call that submits it (node +/// 10187:110373). +/// +/// One instance per edit rather than one shared across the Edit list's rows: `EditChat` treats +/// every field as *unset means unchanged*, so a name edit and a picture edit are two independent +/// partial updates. ``saveTitle(for:using:)`` and ``savePicture(for:using:)`` each send only their +/// own field, which is what keeps editing the name from clearing the picture. +@MainActor +@Observable +final class EditGroupModel { + + /// The title as typed. Submitted through ``GroupTitleValidator``, never raw — the contract caps + /// `Title.value` at 1...64 and the server rejects anything outside it. + var title: String + + /// The picture the user picked, before it is encoded or uploaded. Nil until they pick one; the + /// group's existing picture is drawn by the screen and never resubmitted. + private(set) var picture: UIImage? + + /// Whether a save is in flight, so the button stops taking taps. + private(set) var isSaving = false + + /// The blob the picture landed in, held across retries: the reservation signs the byte count, + /// so a retry that re-encoded would be storing a second copy of a picture the server already + /// has. Mirrors ``NewPublicGroupModel/reservedBlobID``. + private(set) var reservedBlobID: BlobID? + + /// Cap on the encoded picture, matching the profile photo's and the new-group form's. + private static let maxUploadBytes = 2 * 1_024 * 1_024 + + private let validator = GroupTitleValidator() + + /// Seeded with the title the group carries, so the name field opens on what it is about to + /// replace rather than on a blank. + init(title: String = "") { + self.title = title + } + + // MARK: - Form state - + + func select(picture: UIImage) { + self.picture = picture + // A different picture is a different upload: the held reservation signed the previous + // image's byte count and would upload that one instead. + reservedBlobID = nil + } + + /// The title as it would be submitted, or nil while the field holds nothing the server accepts. + var validatedTitle: String? { + validator.validate(title) + } + + /// How many more Unicode scalars the title field accepts. + var remainingTitleScalars: Int { + validator.remaining(in: title) + } + + /// Whether Save is enabled for the name: a title the server will take, actually different from + /// the one the group carries, and nothing already in flight. + /// + /// Re-sending the current title spends a moderation round trip to change nothing, the same + /// reason ``ProfileNameScreen`` holds its Save shut on an unchanged name. + func canSaveTitle(currentTitle: String?) -> Bool { + guard !isSaving, let validatedTitle else { return false } + return validatedTitle != currentTitle + } + + /// Whether Save is enabled for the picture: one has been picked and nothing is in flight. The + /// group's existing picture is not a submission, so this stays shut until the user picks. + var canSavePicture: Bool { + !isSaving && picture != nil + } + + // MARK: - Save - + + /// Sends the title alone and returns the chat's post-edit metadata. + /// + /// `pictureBlobID` is nil by construction: unset means unchanged, so a name edit leaves the + /// group's picture exactly where it was. + func saveTitle(for conversationID: ConversationID, using editor: some GroupChatEditing) async throws -> Conversation { + guard let title = validatedTitle else { + throw EditGroupIncomplete() + } + + isSaving = true + defer { isSaving = false } + + return try await editor.editChat( + conversationID: conversationID, + title: title, + pictureBlobID: nil + ) + } + + /// Uploads the picked picture, waits for the server to finalize it, then sends it alone and + /// returns the chat's post-edit metadata. + /// + /// The wait is not optional: `EditChat` answers `PICTURE_BLOB_NOT_ACCEPTED` for a blob that is + /// still pending or processing, so the finalization has to land before the edit is sent. + /// `title` is nil by construction, leaving the group's name untouched. + func savePicture(for conversationID: ConversationID, using editor: some GroupChatEditing) async throws -> Conversation { + guard picture != nil else { + throw EditGroupIncomplete() + } + + isSaving = true + defer { isSaving = false } + + let blobID = try await uploadPicture(using: editor) + + return try await editor.editChat( + conversationID: conversationID, + title: nil, + pictureBlobID: blobID + ) + } + + /// Stores and finalizes the picture, returning the blob `EditChat` should carry. A rejected + /// blob clears the reservation so a different picture starts clean. + private func uploadPicture(using editor: some GroupChatEditing) async throws -> BlobID { + if reservedBlobID == nil, let picture { + // Encode before storing: the reservation signs the byte count, so the bytes may not + // change afterwards. + let data = try await ImageEncoder.encodeForUpload(picture, maxBytes: Self.maxUploadBytes) + reservedBlobID = try await editor.storeBlob(data, mimeType: "image/jpeg") + } + + guard let blobID = reservedBlobID else { + throw EditGroupIncomplete() + } + + do { + try await editor.awaitBlobFinalization(blobID: blobID) + } catch ErrorBlob.rejected(let reason) { + reservedBlobID = nil + throw ErrorBlob.rejected(reason) + } + + return blobID + } +} diff --git a/Flipcash/Core/Screens/Profile/Edit Group/EditGroupNameScreen.swift b/Flipcash/Core/Screens/Profile/Edit Group/EditGroupNameScreen.swift new file mode 100644 index 000000000..ffaafe9e6 --- /dev/null +++ b/Flipcash/Core/Screens/Profile/Edit Group/EditGroupNameScreen.swift @@ -0,0 +1,180 @@ +// +// EditGroupNameScreen.swift +// Flipcash +// + +import SwiftUI +import FlipcashCore +import FlipcashUI + +private let logger = Logger(label: "flipcash.edit-group-name") + +/// Renames a group. The Name row's destination (node 10187:110373), drawn the way +/// ``ProfileNameScreen`` draws the display name: one field, a countdown near the cap, and a Save +/// that wears its own progress. +struct EditGroupNameScreen: View { + + let conversationID: ConversationID + + @Environment(Container.self) private var container + @Environment(SessionContainer.self) private var sessionContainer + @Environment(ConversationController.self) private var conversationController + @Environment(AppRouter.self) private var router + + @State private var model: EditGroupModel + @State private var submitTask: Task? + @State private var dialog: DialogItem? + + /// Drives Save: spinner while saving, then the checkmark the rest of the app shows on a + /// completed action. + @State private var buttonState: ButtonState = .normal + + @FocusState private var isNameFocused: Bool + + /// Shown only once the limit is close enough to explain a disabled Save. Matches the + /// display-name field's countdown and the new-group form's. + private static let countdownThreshold = 10 + + init(conversationID: ConversationID, currentTitle: String) { + self.conversationID = conversationID + // Seeded here rather than in `.onAppear` so the field opens on the name it is about to + // replace instead of animating it in over the push. + _model = State(initialValue: EditGroupModel(title: currentTitle)) + } + + /// True while the submission is in flight, including the checkmark hold at the end of it, so + /// the field stays locked through the confirmation. + private var isSubmitting: Bool { submitTask != nil } + + private var conversation: Conversation? { + conversationController.conversation(withID: conversationID) + } + + var body: some View { + @Bindable var model = model + + Background(color: .backgroundMain) { + VStack(alignment: .leading, spacing: 0) { + TextField("Group Name", text: $model.title) + .font(.appDisplayMedium) + .foregroundStyle(Color.textMain) + .focused($isNameFocused) + .submitLabel(.done) + .onSubmit(submit) + .padding(.top, 32) + .disabled(isSubmitting) + + Text("This is how the group appears to everyone in it") + .font(.appTextSmall) + .foregroundStyle(Color.textSecondary) + .padding(.top, 8) + + Spacer() + + if model.remainingTitleScalars < Self.countdownThreshold { + Text("\(model.remainingTitleScalars) characters") + .font(.appTextSmall) + .foregroundStyle(Color.textSecondary) + .frame(maxWidth: .infinity, alignment: .leading) + .padding(.bottom, 12) + } + + Button(action: submit) { + ButtonStateLabel("Save", state: buttonState) + } + .buttonStyle(.filled) + .disabled(!model.canSaveTitle(currentTitle: conversation?.title) || isSubmitting) + .accessibilityIdentifier("edit-group-name-save-button") + .padding(.bottom, 20) + } + .padding(.horizontal, 20) + .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) + } + .navigationTitle("Name") + .toolbarTitleDisplayMode(.inline) + .dialog(item: $dialog) + .onAppear { isNameFocused = true } + // Leaving the screen abandons the submission: its only continuation is a pop off a stack + // this screen no longer sits on. + .onDisappear { submitTask?.cancel() } + } + + private func submit() { + guard model.canSaveTitle(currentTitle: conversation?.title), !isSubmitting else { return } + + isNameFocused = false + buttonState = .loading + + submitTask = Task { + defer { submitTask = nil } + + do { + let conversation = try await model.saveTitle( + for: conversationID, + using: SessionGroupChatEditor( + session: sessionContainer.session, + flipClient: container.flipClient + ) + ) + + // The response carries the post-edit metadata, so the store is seated from it + // rather than from a refetch. + conversationController.applyEdit(conversation) + + buttonState = .success + // Same beat the rest of the app holds its checkmark for. + try? await Task.delay(milliseconds: 500) + + guard !Task.isCancelled else { return } + router.popTopmost() + + } catch { + buttonState = .normal + handle(error) + } + } + } + + /// Mirrors ``NewPublicGroupScreen``'s mapping so the same server answer reads the same way + /// wherever a group name is submitted. + /// + /// Every case leaves the user on this screen with the field as they typed it, which is what a + /// moderated name needs: the only way forward is to amend it here. + private func handle(_ error: Error) { + guard !Task.isCancelled else { return } + + switch error { + case ErrorEditChat.titleModerated(let category): + logger.info("Group title moderation denied", metadata: ["category": "\(category)"]) + ErrorReporting.captureError(error, reason: "Group title moderation denied") + dialog = .error( + title: "This Name is Not Allowed", + subtitle: "Try a different group name" + ) + + case ErrorEditChat.denied: + logger.info("Group edit denied") + ErrorReporting.captureError(error, reason: "Group edit denied") + dialog = .error( + title: "You Can't Edit This Group", + subtitle: "Try again later" + ) + + case ErrorEditChat.notFound: + logger.info("Group edit target not found") + ErrorReporting.captureError(error, reason: "Group edit target not found") + dialog = .error( + title: "This Group No Longer Exists", + subtitle: "It may have been deleted" + ) + + default: + logger.error("Failed to edit group name", metadata: ["error": "\(error)"]) + ErrorReporting.captureError(error, reason: "Failed to edit group name") + dialog = .error( + title: "Couldn't Save This Name", + subtitle: "Try again" + ) + } + } +} diff --git a/Flipcash/Core/Screens/Profile/Edit Group/EditGroupScreen.swift b/Flipcash/Core/Screens/Profile/Edit Group/EditGroupScreen.swift new file mode 100644 index 000000000..1b0d78ba7 --- /dev/null +++ b/Flipcash/Core/Screens/Profile/Edit Group/EditGroupScreen.swift @@ -0,0 +1,70 @@ +// +// EditGroupScreen.swift +// Flipcash +// + +import SwiftUI +import FlipcashCore +import FlipcashUI + +/// What a group's editor can change, one row each (node 10187:110373). +/// +/// The design draws four rows — Icon, Membership Card, Description and Social Links — but only the +/// first has contract support: `EditChatRequest` carries a title and a picture and nothing else, so +/// the other three would be rows that cannot save. Name is the counterpart the design omits, styled +/// to match Icon. +struct EditGroupScreen: View { + + let conversationID: ConversationID + + @Environment(ConversationController.self) private var conversationController + @Environment(AppRouter.self) private var router + + private var conversation: Conversation? { + conversationController.conversation(withID: conversationID) + } + + var body: some View { + Background(color: .backgroundMain) { + VStack(spacing: 0) { + Row(insets: rowInsets, accessory: .chevron) { + Image.system(.photoCircle) + .font(.system(size: 26)) + .frame(width: 26, height: 26) + Text("Icon") + } action: { + router.push(.editGroupIcon(conversationID)) + } + .accessibilityIdentifier("edit-group-icon") + + Row(insets: rowInsets, accessory: .chevron) { + Image.system(.textformat) + .font(.system(size: 26)) + .frame(width: 26, height: 26) + Text("Name") + } action: { + router.push(.editGroupName(conversationID)) + } + .accessibilityIdentifier("edit-group-name") + + Spacer() + } + .font(.appDisplayXS) + .padding(.horizontal, 20) + } + .navigationTitle("Edit Group") + .toolbarTitleDisplayMode(.inline) + // An edit permission the server withdraws while this screen is open leaves the user on a + // list of things they can no longer save. Unwinding is the honest answer, and matches the + // gate that put the menu item there in the first place. + .onChange(of: conversation?.canEdit ?? false) { _, canEdit in + if !canEdit { router.popTopmost() } + } + } + + /// The design's ~24pt vertical rhythm. Leading inset is zero because the row's own icon starts + /// the content; the 20pt screen padding is what holds it off the edge. + private var rowInsets: EdgeInsets { + .init(top: 24, leading: 0, bottom: 24, trailing: 0) + } +} diff --git a/Flipcash/Core/Screens/Profile/Edit Group/GroupChatEditing.swift b/Flipcash/Core/Screens/Profile/Edit Group/GroupChatEditing.swift new file mode 100644 index 000000000..aeec4fe35 --- /dev/null +++ b/Flipcash/Core/Screens/Profile/Edit Group/GroupChatEditing.swift @@ -0,0 +1,63 @@ +// +// GroupChatEditing.swift +// Flipcash +// + +import Foundation +import FlipcashCore + +/// The remote half of editing a group chat, in the order the flow calls it. +/// +/// The owner is bound by the conformer, so the flow never handles keys — the same seam +/// ``GroupChatCreating`` draws for creation, for the same reason. The blob calls sit here rather +/// than in a protocol of their own because `EditChat` will only accept a picture the server has +/// already finalized, so the two calls are one ordered sequence rather than two the screen would +/// have to interleave. +protocol GroupChatEditing { + + /// Stores `data` and returns its blob, before the server has finalized it. + func storeBlob(_ data: Data, mimeType: String) async throws -> BlobID + + /// Returns once the blob is servable, throwing `ErrorBlob.rejected` when it is refused and + /// `ErrorBlob.timedOut` when it is still processing. + func awaitBlobFinalization(blobID: BlobID) async throws + + /// Applies a partial edit and returns the chat's post-edit metadata. + /// + /// Both fields are independently optional and *unset means unchanged*, so a caller editing the + /// name passes `pictureBlobID: nil` and leaves the picture alone. Passing neither is a no-op + /// the server answers `OK`. + func editChat( + conversationID: ConversationID, + title: String?, + pictureBlobID: BlobID? + ) async throws -> Conversation +} + +/// Edits on behalf of the signed-in owner. +struct SessionGroupChatEditor: GroupChatEditing { + + let session: Session + let flipClient: FlipClient + + func storeBlob(_ data: Data, mimeType: String) async throws -> BlobID { + try await flipClient.storeBlob(data, mimeType: mimeType, owner: session.ownerKeyPair) + } + + func awaitBlobFinalization(blobID: BlobID) async throws { + try await flipClient.awaitBlobFinalization(blobID: blobID, owner: session.ownerKeyPair) + } + + func editChat( + conversationID: ConversationID, + title: String?, + pictureBlobID: BlobID? + ) async throws -> Conversation { + try await flipClient.editChat( + owner: session.ownerKeyPair, + conversationID: conversationID, + title: title, + pictureBlobID: pictureBlobID + ) + } +} diff --git a/FlipcashCore/Sources/FlipcashCore/Models/Conversation/ConversationRules.swift b/FlipcashCore/Sources/FlipcashCore/Models/Conversation/ConversationRules.swift index c4d2abc43..1dfc455c9 100644 --- a/FlipcashCore/Sources/FlipcashCore/Models/Conversation/ConversationRules.swift +++ b/FlipcashCore/Sources/FlipcashCore/Models/Conversation/ConversationRules.swift @@ -88,6 +88,20 @@ extension ConversationViewerState { } } +extension Conversation { + /// Whether the signed-in viewer may edit this chat's title or picture — the one gate an edit + /// affordance may turn on. + /// + /// Forwards ``ConversationViewerState/canEdit`` and nothing else. Membership, creator identity + /// and chat kind are deliberately *not* consulted: the server computes this from the viewer's + /// standing against the chat's rules, and any client-side conjunct would either grant an + /// affordance the server will deny or hide one it would have allowed. Absent viewer state means + /// not permitted. + public var canEdit: Bool { + viewerState?.canEdit ?? false + } +} + /// Whether, and until when, the signed-in user has muted a chat. See `chat.v1.MuteState`. public enum ConversationMuteState: Hashable, Codable, Sendable { /// Muted until this date, after which the mute lapses client-side. Nothing is sent when it diff --git a/FlipcashTests/Chat/EditGroupModelTests.swift b/FlipcashTests/Chat/EditGroupModelTests.swift new file mode 100644 index 000000000..f533bdfc8 --- /dev/null +++ b/FlipcashTests/Chat/EditGroupModelTests.swift @@ -0,0 +1,315 @@ +// +// EditGroupModelTests.swift +// FlipcashTests +// + +import Foundation +import UIKit +import Testing +@testable import Flipcash +import FlipcashCore + +@MainActor +@Suite("Group edit form") +struct EditGroupModelTests { + + // MARK: - Doubles - + + /// Records what the form asked of the server, and answers with whatever the test set. + private final class SpyEditor: GroupChatEditing { + + private(set) var editCalls: [(title: String?, blobID: BlobID?)] = [] + private(set) var storeBlobCallCount = 0 + private(set) var finalizationCallCount = 0 + + /// The order the seam was called in, so a test can assert the blob was finalized *before* + /// the edit was sent rather than merely that both happened. + private(set) var callOrder: [String] = [] + + /// Thrown by the next `editChat`, then cleared — so a test can fail one attempt and let the + /// retry through. + var nextEditError: Error? + /// Thrown by the next `awaitBlobFinalization`, then cleared. + var nextFinalizationError: Error? + + func storeBlob(_ data: Data, mimeType: String) async throws -> BlobID { + storeBlobCallCount += 1 + callOrder.append("store") + return BlobID(data: Data(repeating: UInt8(storeBlobCallCount), count: 32)) + } + + func awaitBlobFinalization(blobID: BlobID) async throws { + finalizationCallCount += 1 + callOrder.append("finalize") + if let error = nextFinalizationError { + nextFinalizationError = nil + throw error + } + } + + func editChat( + conversationID: ConversationID, + title: String?, + pictureBlobID: BlobID? + ) async throws -> Conversation { + editCalls.append((title, pictureBlobID)) + callOrder.append("edit") + if let error = nextEditError { + nextEditError = nil + throw error + } + return Conversation( + id: conversationID, + members: [], + lastMessage: nil, + lastActivity: Date(), + type: .group, + title: title + ) + } + } + + private let conversationID = ConversationID.test(7) + + private func swatch() -> UIImage { + UIGraphicsImageRenderer(size: CGSize(width: 8, height: 8)).image { context in + UIColor.systemTeal.setFill() + context.fill(CGRect(x: 0, y: 0, width: 8, height: 8)) + } + } + + // MARK: - Title validation (contract: 1...64) - + + @Test("An empty title is not submittable") + func emptyTitleIsRejected() { + let model = EditGroupModel(title: "") + + #expect(model.validatedTitle == nil) + } + + @Test("A whitespace-only title is not submittable") + func whitespaceTitleIsRejected() { + let model = EditGroupModel(title: " ") + + #expect(model.validatedTitle == nil) + } + + @Test("A single character clears the contract's minimum") + func oneCharacterIsAccepted() { + let model = EditGroupModel(title: "a") + + #expect(model.validatedTitle == "a") + } + + @Test("A 64-character title sits exactly on the contract's maximum and is accepted") + func sixtyFourCharactersIsAccepted() { + let model = EditGroupModel(title: String(repeating: "a", count: 64)) + + #expect(model.validatedTitle?.count == 64) + } + + @Test("A 65-character title exceeds the contract's maximum and is refused before sending") + func sixtyFiveCharactersIsRejected() { + let model = EditGroupModel(title: String(repeating: "a", count: 65)) + + #expect(model.validatedTitle == nil) + } + + @Test("An over-long title is never sent to the server") + func overLongTitleNeverReachesTheServer() async { + let model = EditGroupModel(title: String(repeating: "a", count: 65)) + let editor = SpyEditor() + + await #expect(throws: EditGroupIncomplete.self) { + try await model.saveTitle(for: conversationID, using: editor) + } + + #expect(editor.editCalls.isEmpty) + } + + // MARK: - Save gating - + + @Test("Save stays shut on a title matching the one the group already carries") + func unchangedTitleCannotBeSaved() { + let model = EditGroupModel(title: "BadBoys") + + #expect(model.canSaveTitle(currentTitle: "BadBoys") == false) + } + + @Test("Save opens once the title differs from the group's") + func changedTitleCanBeSaved() { + let model = EditGroupModel(title: "GoodBoys") + + #expect(model.canSaveTitle(currentTitle: "BadBoys")) + } + + @Test("Save stays shut for the picture until one is picked") + func pictureCannotBeSavedUntilPicked() { + let model = EditGroupModel() + + #expect(model.canSavePicture == false) + + model.select(picture: swatch()) + + #expect(model.canSavePicture) + } + + // MARK: - Partial update construction - + + @Test("A name edit sends the title and no picture, so the group's picture is left unchanged") + func nameEditSendsNoPicture() async throws { + let model = EditGroupModel(title: "GoodBoys") + let editor = SpyEditor() + + _ = try await model.saveTitle(for: conversationID, using: editor) + + #expect(editor.editCalls.count == 1) + #expect(editor.editCalls[0].title == "GoodBoys") + #expect(editor.editCalls[0].blobID == nil) + } + + @Test("A name edit uploads nothing") + func nameEditTouchesNoBlob() async throws { + let model = EditGroupModel(title: "GoodBoys") + let editor = SpyEditor() + + _ = try await model.saveTitle(for: conversationID, using: editor) + + #expect(editor.storeBlobCallCount == 0) + #expect(editor.finalizationCallCount == 0) + } + + @Test("A picture edit sends the blob and no title, so the group's name is left unchanged") + func pictureEditSendsNoTitle() async throws { + // Seeded with the group's current title the way the screen seeds it, to prove the title is + // omitted by construction rather than merely because the field happened to be empty. + let model = EditGroupModel(title: "BadBoys") + model.select(picture: swatch()) + let editor = SpyEditor() + + _ = try await model.savePicture(for: conversationID, using: editor) + + #expect(editor.editCalls.count == 1) + #expect(editor.editCalls[0].title == nil) + #expect(editor.editCalls[0].blobID != nil) + } + + @Test("A picture edit finalizes the blob before sending it, never after") + func pictureIsFinalizedBeforeTheEdit() async throws { + let model = EditGroupModel() + model.select(picture: swatch()) + let editor = SpyEditor() + + _ = try await model.savePicture(for: conversationID, using: editor) + + #expect(editor.callOrder == ["store", "finalize", "edit"]) + } + + @Test("A blob the server refuses is never sent as an edit") + func refusedBlobNeverReachesTheEdit() async { + let model = EditGroupModel() + model.select(picture: swatch()) + let editor = SpyEditor() + editor.nextFinalizationError = ErrorBlob.rejected(.moderation) + + await #expect(throws: ErrorBlob.self) { + try await model.savePicture(for: conversationID, using: editor) + } + + #expect(editor.editCalls.isEmpty) + } + + @Test("A retry after a failed edit reuses the reservation rather than storing a second copy") + func retryReusesTheReservedBlob() async throws { + let model = EditGroupModel() + model.select(picture: swatch()) + let editor = SpyEditor() + editor.nextEditError = ErrorEditChat.unknown + + await #expect(throws: ErrorEditChat.self) { + try await model.savePicture(for: conversationID, using: editor) + } + + _ = try await model.savePicture(for: conversationID, using: editor) + + #expect(editor.storeBlobCallCount == 1) + #expect(editor.editCalls.count == 2) + } + + @Test("Picking a different picture drops the reservation, so the new bytes are what upload") + func newPictureDropsTheReservation() async throws { + let model = EditGroupModel() + model.select(picture: swatch()) + let editor = SpyEditor() + + _ = try await model.savePicture(for: conversationID, using: editor) + model.select(picture: swatch()) + _ = try await model.savePicture(for: conversationID, using: editor) + + #expect(editor.storeBlobCallCount == 2) + } +} + +@MainActor +@Suite("Group edit permission gate") +struct ConversationCanEditTests { + + private func group(viewerState: ConversationViewerState?) -> Conversation { + Conversation( + id: .test(7), + members: [], + lastMessage: nil, + lastActivity: Date(), + type: .group, + title: "BadBoys", + viewerState: viewerState + ) + } + + @Test("A chat whose viewer state permits editing can be edited") + func permittedViewerCanEdit() { + #expect(group(viewerState: ConversationViewerState(canEdit: true)).canEdit) + } + + @Test("A chat whose viewer state withholds the permission cannot be edited") + func unpermittedViewerCannotEdit() { + #expect(group(viewerState: ConversationViewerState(canEdit: false)).canEdit == false) + } + + @Test("A chat with no viewer state at all cannot be edited") + func absentViewerStateCannotEdit() { + #expect(group(viewerState: nil).canEdit == false) + } + + /// The gate is the server's answer and nothing else. A group the user belongs to, holds a title + /// in, and would plausibly own still cannot be edited without the permission — which is what + /// keeps the affordance from being inferred from membership or chat kind. + @Test("Membership and chat kind do not confer edit rights on their own") + func membershipDoesNotConferEditRights() { + let joined = Conversation( + id: .test(7), + members: [], + lastMessage: nil, + lastActivity: Date(), + type: .group, + title: "BadBoys", + rosterSummary: ConversationRosterSummary(memberCount: 12, version: 1), + viewerState: ConversationViewerState(mute: nil, canEdit: false, version: 3) + ) + + #expect(joined.canEdit == false) + } + + @Test("A DM is not editable") + func dmCannotBeEdited() { + let dm = Conversation( + id: .test(8), + members: [], + lastMessage: nil, + lastActivity: Date(), + type: .tipDm + ) + + #expect(dm.canEdit == false) + } +} diff --git a/FlipcashUI/Sources/FlipcashUI/Theme/Image+Symbols.swift b/FlipcashUI/Sources/FlipcashUI/Theme/Image+Symbols.swift index 296fba368..c7070c4ef 100644 --- a/FlipcashUI/Sources/FlipcashUI/Theme/Image+Symbols.swift +++ b/FlipcashUI/Sources/FlipcashUI/Theme/Image+Symbols.swift @@ -85,6 +85,12 @@ public enum SystemSymbol: String { case pencil = "pencil" case trash = "trash" case flag = "flag" + + /// The two rows of the group edit list. Paired deliberately: the design supplies a glyph for + /// the picture row only, so the name row borrows a symbol of the same family and weight rather + /// than sitting next to an asset it cannot match. + case photoCircle = "photo.circle" + case textformat = "textformat" /// The bare glyph, for a confirm affordance that carries its own backing shape. case checkmark = "checkmark" From 58831677421fe43dec2cfa6c3a60c96bd25c5bf6 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 22 Sep 2026 12:15:20 -0400 Subject: [PATCH 2/4] fix(store): bump schema version so viewer state is refetched #824 added a non-optional `canEdit` to `ConversationViewerState` without bumping the schema version. A blob written by an earlier build has no `canEdit` key, so decoding throws `keyNotFound`; the read site uses `try?`, so the whole viewer state degrades to nil. That loses mute state as well as the new permission, and it persists until something refetches the chat. Observed on a simulator upgraded across #824: every conversation held `{"version":N}` with no `canEdit`, and `Conversation.canEdit` read false for two groups the signed-in user created. After the bump the store rebuilt and the same two rows came back `{"canEdit":true,...}`. A rebuild costs a resync and loses nothing: every column in the affected tables is server-derived, and drafts live in a separate file. That is the project's stated substitute for migrations, so use it rather than teaching the decoder to tolerate the older shape. --- FlipcashCore/Sources/FlipcashStore/Database.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/FlipcashCore/Sources/FlipcashStore/Database.swift b/FlipcashCore/Sources/FlipcashStore/Database.swift index 098a630cd..d32646cf5 100644 --- a/FlipcashCore/Sources/FlipcashStore/Database.swift +++ b/FlipcashCore/Sources/FlipcashStore/Database.swift @@ -194,7 +194,7 @@ nonisolated open class Database: @unchecked Sendable { /// the notification service extension needs the same number to decide whether the store on disk /// is one it understands, and an extension cannot read the app's `Info.plist` — separate bundles. /// Both targets link this module, so they cannot disagree. - public static let schemaVersion = 38 + public static let schemaVersion = 39 /// Removes the store and the write-ahead log files beside it. /// From bf4f61d9adf2e116d40e578fd73dcecae566e8e1 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 22 Sep 2026 12:44:24 -0400 Subject: [PATCH 3/4] feat(chat): confirm group edits, and call the row Picture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Saving a group's name or picture changes it for everyone in the group, so neither leaf screen sends on the first tap any more: Save opens a confirmation alert and the alert commits, the way the My Account fields already work. Both take the red banner — where the profile confirmations weigh whether the user can get their own old value back, this edit lands on everyone at once and no later undo takes back what they have seen. The row is Picture, not Icon, which is what `EditChatRequest.picture` carries and what Android ships. Its glyphs come from Android too — `photo` and `textformat` against `Icons.Outlined.Image` and `Icons.Outlined.TextFields` — so the screen reads the same on both platforms. They need `resizable()` to hold the 26pt column: `textformat` is wider than tall, and a frame alone left it overflowing and the labels misaligned. The name screen loses its bar title. The field it opens on is the name, so a title only repeated it back. --- .../Navigation/AppRouter+Destination.swift | 10 +- .../AppRouter+DestinationView.swift | 4 +- .../Edit Group/DialogItem+GroupChange.swift | 57 ++++++++++ .../Edit Group/EditGroupNameScreen.swift | 21 ++-- ...een.swift => EditGroupPictureScreen.swift} | 32 +++--- .../Profile/Edit Group/EditGroupScreen.swift | 24 ++-- .../Chat/GroupChangeDialogTests.swift | 105 ++++++++++++++++++ .../FlipcashUI/Theme/Image+Symbols.swift | 8 +- 8 files changed, 218 insertions(+), 43 deletions(-) create mode 100644 Flipcash/Core/Screens/Profile/Edit Group/DialogItem+GroupChange.swift rename Flipcash/Core/Screens/Profile/Edit Group/{EditGroupIconScreen.swift => EditGroupPictureScreen.swift} (88%) create mode 100644 FlipcashTests/Chat/GroupChangeDialogTests.swift diff --git a/Flipcash/Core/Navigation/AppRouter+Destination.swift b/Flipcash/Core/Navigation/AppRouter+Destination.swift index 71a6277b1..fb8e32261 100644 --- a/Flipcash/Core/Navigation/AppRouter+Destination.swift +++ b/Flipcash/Core/Navigation/AppRouter+Destination.swift @@ -114,8 +114,8 @@ extension AppRouter { case editGroup(ConversationID) /// Renaming a group, pushed from the Name row of `editGroup`. case editGroupName(ConversationID) - /// Replacing a group's picture, pushed from the Icon row of `editGroup`. - case editGroupIcon(ConversationID) + /// Replacing a group's picture, pushed from the Picture row of `editGroup`. + case editGroupPicture(ConversationID) /// The stack this destination naturally belongs in. Cross-stack /// navigation uses this to know which sheet to present, or which tab @@ -140,7 +140,7 @@ extension AppRouter { case .profileName, .profilePhoto, .tipcard, .usernameLookup, .newChat, .newPublicGroup, .tipConversation, .tipConversationWithKeyboard, .tipConversationForUser, .userProfile, .chatProfile, - .editGroup, .editGroupName, .editGroupIcon: + .editGroup, .editGroupName, .editGroupPicture: return .tips } } @@ -192,7 +192,7 @@ extension AppRouter { case .chatProfile: "chatProfile" case .editGroup: "editGroup" case .editGroupName: "editGroupName" - case .editGroupIcon: "editGroupIcon" + case .editGroupPicture: "editGroupPicture" } } @@ -217,7 +217,7 @@ extension AppRouter { .chatProfile(let conversationID), .editGroup(let conversationID), .editGroupName(let conversationID), - .editGroupIcon(let conversationID): + .editGroupPicture(let conversationID): return conversationID.description case .userProfile(let userID), .tipConversationForUser(let userID): diff --git a/Flipcash/Core/Navigation/AppRouter+DestinationView.swift b/Flipcash/Core/Navigation/AppRouter+DestinationView.swift index cf37f817b..ec27edc9c 100644 --- a/Flipcash/Core/Navigation/AppRouter+DestinationView.swift +++ b/Flipcash/Core/Navigation/AppRouter+DestinationView.swift @@ -211,8 +211,8 @@ struct DestinationView: View { ) .id(conversationID) - case .editGroupIcon(let conversationID): - EditGroupIconScreen(conversationID: conversationID) + case .editGroupPicture(let conversationID): + EditGroupPictureScreen(conversationID: conversationID) .id(conversationID) } } diff --git a/Flipcash/Core/Screens/Profile/Edit Group/DialogItem+GroupChange.swift b/Flipcash/Core/Screens/Profile/Edit Group/DialogItem+GroupChange.swift new file mode 100644 index 000000000..3a5af9504 --- /dev/null +++ b/Flipcash/Core/Screens/Profile/Edit Group/DialogItem+GroupChange.swift @@ -0,0 +1,57 @@ +// +// DialogItem+GroupChange.swift +// Flipcash +// + +import FlipcashUI + +extension DialogItem { + + /// The group fields the Edit Group list can replace. + /// + /// The sibling of ``DialogItem/ProfileField``, and confirmed for a different reason: a profile + /// field is the user's own, while both of these are visible to everyone in the group the moment + /// they save. Neither can be reached before the group exists, so unlike the profile fields + /// there is no first-time-setup case to exempt — every edit here is a replacement. + enum GroupField { + + case name + case picture + + /// Group-qualified rather than borrowing the row's bare "Name" / "Picture", which would + /// read as the user's own once the dialog covers the screen that gave them context. + var title: String { + switch self { + case .name: "Group Name" + case .picture: "Group Picture" + } + } + + var subtitle: String { + switch self { + case .name: "This will change the group name for everyone in it" + case .picture: "This will change the group picture for everyone in it" + } + } + } + + /// Confirms replacing a group field. Raised on Save, after validation, so the user is never + /// asked to confirm a name the screen is about to reject anyway. + /// + /// Alerts rather than informs, which is where this parts company with the profile + /// confirmations: those weigh whether the user can get their own old value back, and only the + /// username can't. Here the edit lands on everyone in the group at once, and no later undo + /// takes back what they have already seen. + /// + /// Untracked, for the same reason the profile confirmations are: a user changing a group they + /// are permitted to edit is not an error worth an analytics event. + static func confirmGroupChange( + _ field: GroupField, + onConfirm: @escaping () -> Void + ) -> DialogItem { + .alert(title: "Change \(field.title)?", subtitle: field.subtitle) { + DialogAction.destructive("Change \(field.title)", action: onConfirm) + DialogAction.cancel() + } + } +} diff --git a/Flipcash/Core/Screens/Profile/Edit Group/EditGroupNameScreen.swift b/Flipcash/Core/Screens/Profile/Edit Group/EditGroupNameScreen.swift index ffaafe9e6..7e5cfa0a5 100644 --- a/Flipcash/Core/Screens/Profile/Edit Group/EditGroupNameScreen.swift +++ b/Flipcash/Core/Screens/Profile/Edit Group/EditGroupNameScreen.swift @@ -55,6 +55,11 @@ struct EditGroupNameScreen: View { Background(color: .backgroundMain) { VStack(alignment: .leading, spacing: 0) { + Text("What's the group name?") + .font(.appTextLarge) + .foregroundStyle(Color.textMain) + .padding(.top, 20) + TextField("Group Name", text: $model.title) .font(.appDisplayMedium) .foregroundStyle(Color.textMain) @@ -64,11 +69,6 @@ struct EditGroupNameScreen: View { .padding(.top, 32) .disabled(isSubmitting) - Text("This is how the group appears to everyone in it") - .font(.appTextSmall) - .foregroundStyle(Color.textSecondary) - .padding(.top, 8) - Spacer() if model.remainingTitleScalars < Self.countdownThreshold { @@ -90,8 +90,9 @@ struct EditGroupNameScreen: View { .padding(.horizontal, 20) .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) } - .navigationTitle("Name") - .toolbarTitleDisplayMode(.inline) + // The heading above the field is the screen's title, the way ``ProfileNameScreen`` asks for + // a display name; a bar title on top of it would ask twice. + .navigationBarTitleDisplayMode(.inline) .dialog(item: $dialog) .onAppear { isNameFocused = true } // Leaving the screen abandons the submission: its only continuation is a pop off a stack @@ -99,10 +100,16 @@ struct EditGroupNameScreen: View { .onDisappear { submitTask?.cancel() } } + /// Save proposes; the dialog commits. Nothing is sent until the user confirms, so the whole + /// group sees a new name only on a second, deliberate tap. private func submit() { guard model.canSaveTitle(currentTitle: conversation?.title), !isSubmitting else { return } isNameFocused = false + dialog = .confirmGroupChange(.name) { save() } + } + + private func save() { buttonState = .loading submitTask = Task { diff --git a/Flipcash/Core/Screens/Profile/Edit Group/EditGroupIconScreen.swift b/Flipcash/Core/Screens/Profile/Edit Group/EditGroupPictureScreen.swift similarity index 88% rename from Flipcash/Core/Screens/Profile/Edit Group/EditGroupIconScreen.swift rename to Flipcash/Core/Screens/Profile/Edit Group/EditGroupPictureScreen.swift index ca935c8db..d46b545dd 100644 --- a/Flipcash/Core/Screens/Profile/Edit Group/EditGroupIconScreen.swift +++ b/Flipcash/Core/Screens/Profile/Edit Group/EditGroupPictureScreen.swift @@ -1,5 +1,5 @@ // -// EditGroupIconScreen.swift +// EditGroupPictureScreen.swift // Flipcash // @@ -8,11 +8,11 @@ import UniformTypeIdentifiers import FlipcashCore import FlipcashUI -private let logger = Logger(label: "flipcash.edit-group-icon") +private let logger = Logger(label: "flipcash.edit-group-picture") -/// Replaces a group's picture. The Icon row's destination (node 10187:110373), drawn the way +/// Replaces a group's picture. The Picture row's destination (node 10187:110373), drawn the way /// ``ProfilePhotoScreen`` draws the profile photo: the picture itself is the picker. -struct EditGroupIconScreen: View { +struct EditGroupPictureScreen: View { let conversationID: ConversationID @@ -34,7 +34,7 @@ struct EditGroupIconScreen: View { /// The picture already on the group, so the screen opens on what it is about to replace rather /// than on an empty circle. Drawn but never submitted — Save stays shut until a new one is /// picked. - @State private var currentIcon: UIImage? + @State private var currentPicture: UIImage? private static let avatarSize: CGFloat = 158 private static let plusSize: CGFloat = 64 @@ -55,14 +55,14 @@ struct EditGroupIconScreen: View { Button("Choose File", systemImage: "folder") { isShowingFilePicker = true } } label: { CircleImage( - image: model.picture ?? currentIcon, + image: model.picture ?? currentPicture, size: Self.avatarSize, plusSize: Self.plusSize ) } .menuIndicator(.hidden) .disabled(isSubmitting) - .accessibilityIdentifier("edit-group-icon-picker") + .accessibilityIdentifier("edit-group-picture-picker") if let title = conversation?.title { Text(title) @@ -81,13 +81,13 @@ struct EditGroupIconScreen: View { // The checkmark hold keeps the picture selected, so the button needs the state to // stay shut against a second submission. .disabled(!model.canSavePicture || !buttonState.isNormal || isSubmitting) - .accessibilityIdentifier("edit-group-icon-save-button") + .accessibilityIdentifier("edit-group-picture-save-button") .padding(.bottom, 20) } .padding(.horizontal, 20) .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .top) } - .navigationTitle("Icon") + .navigationTitle("Picture") .toolbarTitleDisplayMode(.inline) .dialog(item: $dialog) .fullScreenCover(isPresented: $isShowingPhotoPicker) { @@ -106,7 +106,7 @@ struct EditGroupIconScreen: View { // Keyed on the blob so a picture that changes underneath — including the one this screen // just saved — is what the circle ends up drawing. .task(id: conversation?.picture?.thumbnailBlobID) { - currentIcon = await ProfilePictureLoader.thumbnail( + currentPicture = await ProfilePictureLoader.thumbnail( for: conversation?.picture, using: container.flipClient, owner: sessionContainer.session.ownerKeyPair @@ -115,9 +115,15 @@ struct EditGroupIconScreen: View { .onDisappear { submitTask?.cancel() } } + /// Save proposes; the dialog commits. Nothing is uploaded or sent until the user confirms, so + /// the whole group sees a new picture only on a second, deliberate tap. private func submit() { guard model.canSavePicture, !isSubmitting else { return } + dialog = .confirmGroupChange(.picture) { save() } + } + + private func save() { buttonState = .loading submitTask = Task { @@ -191,8 +197,8 @@ struct EditGroupIconScreen: View { dialog = .imageProcessingFailed default: - logger.error("Failed to edit group icon", metadata: ["error": "\(error)"]) - ErrorReporting.captureError(error, reason: "Failed to edit group icon") + logger.error("Failed to edit group picture", metadata: ["error": "\(error)"]) + ErrorReporting.captureError(error, reason: "Failed to edit group picture") dialog = .error( title: "Couldn't Save This Photo", subtitle: "Try again" @@ -203,7 +209,7 @@ struct EditGroupIconScreen: View { private func handleFileImport(_ result: Result<[URL], Error>) { switch result { case .failure(let error): - logger.info("Group icon file import failed", metadata: ["error": "\(error)"]) + logger.info("Group picture file import failed", metadata: ["error": "\(error)"]) return case .success(let urls): diff --git a/Flipcash/Core/Screens/Profile/Edit Group/EditGroupScreen.swift b/Flipcash/Core/Screens/Profile/Edit Group/EditGroupScreen.swift index 1b0d78ba7..47ff48565 100644 --- a/Flipcash/Core/Screens/Profile/Edit Group/EditGroupScreen.swift +++ b/Flipcash/Core/Screens/Profile/Edit Group/EditGroupScreen.swift @@ -12,7 +12,8 @@ import FlipcashUI /// The design draws four rows — Icon, Membership Card, Description and Social Links — but only the /// first has contract support: `EditChatRequest` carries a title and a picture and nothing else, so /// the other three would be rows that cannot save. Name is the counterpart the design omits, styled -/// to match Icon. +/// to match. The design's "Icon" is called Picture here, matching both the contract's +/// `EditChatRequest.picture` and the row Android ships. struct EditGroupScreen: View { let conversationID: ConversationID @@ -28,19 +29,17 @@ struct EditGroupScreen: View { Background(color: .backgroundMain) { VStack(spacing: 0) { Row(insets: rowInsets, accessory: .chevron) { - Image.system(.photoCircle) - .font(.system(size: 26)) - .frame(width: 26, height: 26) - Text("Icon") + Image.system(.photo) + .frame(minWidth: 45) + Text("Picture") } action: { - router.push(.editGroupIcon(conversationID)) + router.push(.editGroupPicture(conversationID)) } - .accessibilityIdentifier("edit-group-icon") + .accessibilityIdentifier("edit-group-picture") Row(insets: rowInsets, accessory: .chevron) { Image.system(.textformat) - .font(.system(size: 26)) - .frame(width: 26, height: 26) + .frame(minWidth: 45) Text("Name") } action: { router.push(.editGroupName(conversationID)) @@ -62,9 +61,10 @@ struct EditGroupScreen: View { } } - /// The design's ~24pt vertical rhythm. Leading inset is zero because the row's own icon starts - /// the content; the 20pt screen padding is what holds it off the edge. + /// The 25pt vertical rhythm every other list in the app uses, near enough to the design's ~24pt + /// that matching the app is the better trade. Leading inset is zero because the row's own icon + /// column starts the content; the 20pt screen padding holds it off the edge. private var rowInsets: EdgeInsets { - .init(top: 24, leading: 0, bottom: 24, trailing: 0) + .init(top: 25, leading: 0, bottom: 25, trailing: 0) } } diff --git a/FlipcashTests/Chat/GroupChangeDialogTests.swift b/FlipcashTests/Chat/GroupChangeDialogTests.swift new file mode 100644 index 000000000..9727744fd --- /dev/null +++ b/FlipcashTests/Chat/GroupChangeDialogTests.swift @@ -0,0 +1,105 @@ +// +// GroupChangeDialogTests.swift +// FlipcashTests +// + +import Foundation +import Testing +import FlipcashUI +@testable import Flipcash + +@MainActor +@Suite("Group change confirmation dialog") +struct GroupChangeDialogTests { + + @Test("Names the field in the title and the confirming button", arguments: [ + (DialogItem.GroupField.name, "Group Name"), + (.picture, "Group Picture"), + ]) + func namesField(field: DialogItem.GroupField, label: String) { + let item = DialogItem.confirmGroupChange(field) {} + + #expect(item.title == "Change \(label)?") + #expect(item.actions.first?.title == "Change \(label)") + } + + /// The rows say "Name" and "Picture"; the dialog says whose. Covering the screen that gave + /// those labels their context is what makes the longer form necessary. + @Test("Says the group's, not the user's own", arguments: [ + DialogItem.GroupField.name, + .picture, + ]) + func qualifiesTheFieldAsTheGroups(field: DialogItem.GroupField) { + let item = DialogItem.confirmGroupChange(field) {} + + #expect(item.title?.contains("Group") == true) + #expect(item.subtitle?.contains("your") == false) + } + + @Test("States the change in the body", arguments: [ + (DialogItem.GroupField.name, "This will change the group name for everyone in it"), + (.picture, "This will change the group picture for everyone in it"), + ]) + func statesTheChange(field: DialogItem.GroupField, body: String) { + let item = DialogItem.confirmGroupChange(field) {} + + #expect(item.subtitle == body) + } + + @Test("Confirms over Cancel", arguments: [ + DialogItem.GroupField.name, + .picture, + ]) + func actions(field: DialogItem.GroupField) { + let item = DialogItem.confirmGroupChange(field) {} + + #expect(item.actions.count == 2) + #expect(item.actions[1].title == "Cancel") + } + + @Test("Runs the caller's work only once the change is confirmed", arguments: [ + DialogItem.GroupField.name, + .picture, + ]) + func confirmAction_runsHandler(field: DialogItem.GroupField) { + var confirmed = false + let item = DialogItem.confirmGroupChange(field) { confirmed = true } + + #expect(confirmed == false) + item.actions[0].action() + #expect(confirmed == true) + } + + @Test("Cancel leaves the caller's work unrun", arguments: [ + DialogItem.GroupField.name, + .picture, + ]) + func cancelAction_doesNotRunHandler(field: DialogItem.GroupField) { + var confirmed = false + let item = DialogItem.confirmGroupChange(field) { confirmed = true } + + item.actions[1].action() + #expect(confirmed == false) + } + + /// Both edits land on everyone in the group at once, so both carry the alert's weight rather + /// than the grey banner the reversible profile fields use. + @Test("Red banner for both", arguments: [ + DialogItem.GroupField.name, + .picture, + ]) + func bothAreDestructive(field: DialogItem.GroupField) { + let item = DialogItem.confirmGroupChange(field) {} + + #expect(item.style == .destructive) + #expect(item.actions[0].kind == .destructive) + } + + @Test("Not an error worth reporting", arguments: [ + DialogItem.GroupField.name, + .picture, + ]) + func untracked(field: DialogItem.GroupField) { + #expect(DialogItem.confirmGroupChange(field) {}.tracked == false) + } +} diff --git a/FlipcashUI/Sources/FlipcashUI/Theme/Image+Symbols.swift b/FlipcashUI/Sources/FlipcashUI/Theme/Image+Symbols.swift index c7070c4ef..6b56ebce3 100644 --- a/FlipcashUI/Sources/FlipcashUI/Theme/Image+Symbols.swift +++ b/FlipcashUI/Sources/FlipcashUI/Theme/Image+Symbols.swift @@ -86,10 +86,10 @@ public enum SystemSymbol: String { case trash = "trash" case flag = "flag" - /// The two rows of the group edit list. Paired deliberately: the design supplies a glyph for - /// the picture row only, so the name row borrows a symbol of the same family and weight rather - /// than sitting next to an asset it cannot match. - case photoCircle = "photo.circle" + /// The two rows of the group edit list, matched to the glyphs Android uses for the same rows + /// (`Icons.Outlined.Image` and `Icons.Outlined.TextFields`) so the screen reads the same on + /// both platforms. + case photo = "photo" case textformat = "textformat" /// The bare glyph, for a confirm affordance that carries its own backing shape. case checkmark = "checkmark" From 685a8bae05cc802c0029489a33f50a367a61b328 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 22 Sep 2026 12:44:32 -0400 Subject: [PATCH 4/4] docs(claude): state the schema-version rule in terms of what the store writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule named `SQLiteVersion` in Info.plist, which moved into code as `Database.schemaVersion` when the notification service extension needed the same number. It also only covered table definitions, so #824 read as exempt: it added a non-optional property to a `Codable` persisted as a JSON blob, leaving the column's SQL type untouched while making every row written by an earlier build fail to decode. Because those reads are `try?`, the whole viewer state silently became nil — losing mute state, not just the new field. Both facts are now in the rule, along with why a tolerant decoder is the wrong fix: it keeps a stale value where the bump refetches the correct one, and it is a migration under another name. --- .claude/docs/hard-rules.md | 18 +++++++++++++++++- CLAUDE.md | 2 +- .../Sources/FlipcashStore/Database.swift | 5 ++++- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/.claude/docs/hard-rules.md b/.claude/docs/hard-rules.md index d98863349..cd9584819 100644 --- a/.claude/docs/hard-rules.md +++ b/.claude/docs/hard-rules.md @@ -69,12 +69,28 @@ app can fix itself belongs in the service files that wrap the generated code. ## Database Schema Changes -**Bump `SQLiteVersion` in Info.plist on every schema change.** The app does not run migrations — when the version number increases, the database is deleted and rebuilt from server data on next login (`SessionAuthenticator.initializeDatabase`). This means: +**Bump `Database.schemaVersion` on every change to what the store writes.** The app does not run +migrations — when the version number increases, the database is deleted and rebuilt from server data +on next login (`SessionAuthenticator.initializeDatabase`). This means: - Adding/removing tables or columns → bump version - Changing which table a query reads from → bump version if the old schema can't satisfy the new query +- **Changing the encoding of a value already stored in a column → bump version.** The column's SQL + type is unchanged, so nothing about `Schema.swift` looks different, but rows written by an earlier + build no longer decode. Adding a non-optional property to a `Codable` struct persisted as a JSON + blob is the usual way in: the synthesized decoder throws `keyNotFound` on an older row, and because + these reads are `try?`, the whole value silently becomes `nil` — losing every *other* field in the + blob too, not just the new one. - No migration code needed, but all data must be recoverable from server +The version lives in code, not `Info.plist`. It moved because the notification service extension needs +the same number to decide whether the store on disk is one it understands, and an extension cannot read +the app's `Info.plist`. Both targets link `FlipcashStore`, so they cannot disagree. + +Resist making a decoder tolerate the older shape (`decodeIfPresent`, a default value, a custom +`init(from:)`). It preserves a stale value where the bump would fetch the correct one, and it is a +migration in all but name — the thing this design exists to avoid. + ## Logging: Variables Go in Metadata **All variable data must go in structured `metadata`. The message string is a constant, free-form description.** Two reasons, in order of importance: diff --git a/CLAUDE.md b/CLAUDE.md index a21572697..bad7be21d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -94,7 +94,7 @@ it before touching the relevant area. - **Exhaustive switches** — Prefer `switch` over `if case` for enums so the compiler flags new cases. - **Modernize incrementally** — Use modern Swift/SwiftUI APIs in net-new/isolated code; don't refactor working code just to modernize. One observation system per class. - **Generated protos** — `FlipcashAPI` only re-exports the published contract packages; there is no generated code to edit here. Change the wrapping service files instead. -- **Database schema** — Bump `SQLiteVersion` in Info.plist on every schema change (no migrations; DB is rebuilt from server). +- **Database schema** — Bump `Database.schemaVersion` on every change to what the store *writes*, including the encoding of a persisted blob (no migrations; DB is rebuilt from server). - **Logging** — Message string is a constant; every variable goes in structured `metadata`. Never log proto blobs whole. - **Error reporting** — Call `ErrorReporting.captureError(...)` unconditionally; classify via `ServerError.reportingLevel`, never gate at the call site. Best-effort chatter never reports. - **Form validation** — Validate free-form input through the `Validator` family and submit the validator's `Output`, never inline regex/trim; keypad amounts parse only via `AmountValidator`. diff --git a/FlipcashCore/Sources/FlipcashStore/Database.swift b/FlipcashCore/Sources/FlipcashStore/Database.swift index d32646cf5..b7e1b9815 100644 --- a/FlipcashCore/Sources/FlipcashStore/Database.swift +++ b/FlipcashCore/Sources/FlipcashStore/Database.swift @@ -188,7 +188,10 @@ nonisolated open class Database: @unchecked Sendable { /// /// A launch that finds a lower version recorded beside the store deletes the store and rebuilds /// it from sync, which is the project's substitute for schema migrations. Bump this whenever a - /// table definition in `Schema.swift` changes. + /// table definition in `Schema.swift` changes, and equally whenever the *encoding* of a value + /// already stored in a column changes — a new non-optional property on a `Codable` persisted as + /// a JSON blob leaves the column's SQL type untouched, but rows written by an earlier build stop + /// decoding, and the `try?` at the read sites turns that into a silent nil for the whole value. /// /// This used to be the `SQLiteVersion` key in the app's `Info.plist`. It moved into code because /// the notification service extension needs the same number to decide whether the store on disk