From ed3b934483c9bedcdb1d08f6415f32e5bae4b0ce Mon Sep 17 00:00:00 2001
From: Miltiadis Vasilakis <mvasilak@gmail.com>
Date: Wed, 8 Jan 2025 19:39:38 +0200
Subject: [PATCH 01/12] Fix item detail editing for attachment type

---
 .../ItemDetail/ItemDetailDataCreator.swift    | 79 +++++++----------
 .../ItemDetail/Models/ItemDetailState.swift   |  7 +-
 .../ViewModels/ItemDetailActionHandler.swift  | 84 +++++++++++--------
 .../ItemDetailCollectionViewHandler.swift     | 29 ++++---
 ZoteroTests/SyncActionsSpec.swift             |  4 -
 5 files changed, 101 insertions(+), 102 deletions(-)

diff --git a/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift b/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
index 49e68802b..e035b03cd 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
@@ -7,6 +7,7 @@
 //
 
 import Foundation
+import OrderedCollections
 
 import CocoaLumberjackSwift
 
@@ -67,7 +68,7 @@ struct ItemDetailDataCreator {
             throw ItemDetailError.cantCreateData
         }
 
-        let (fieldIds, fields, hasAbstract) = try fieldData(for: itemType, schemaController: schemaController, dateParser: dateParser, urlDetector: urlDetector, doiDetector: doiDetector)
+        let (fields, hasAbstract) = try fieldData(for: itemType, schemaController: schemaController, dateParser: dateParser, urlDetector: urlDetector, doiDetector: doiDetector)
         let date = Date()
         let attachments: [Attachment] = child.flatMap({ [$0] }) ?? []
         let data = ItemDetailState.Data(
@@ -79,7 +80,6 @@ struct ItemDetailDataCreator {
             creators: [:],
             creatorIds: [],
             fields: fields,
-            fieldIds: fieldIds,
             abstract: (hasAbstract ? "" : nil),
             dateModified: date,
             dateAdded: date
@@ -125,10 +125,9 @@ struct ItemDetailDataCreator {
             }
         }
 
-        let (fieldIds, fields, _) = try fieldData(for: item.rawType, schemaController: schemaController, dateParser: dateParser,
-                                                  urlDetector: urlDetector, doiDetector: doiDetector, getExistingData: { key, _ in
+        let (fields, _) = try fieldData(for: item.rawType, schemaController: schemaController, dateParser: dateParser, urlDetector: urlDetector, doiDetector: doiDetector) { key, _ in
             return (nil, values[key])
-        })
+        }
 
         var creatorIds: [String] = []
         var creators: [String: ItemDetailState.Creator] = [:]
@@ -185,7 +184,6 @@ struct ItemDetailDataCreator {
             creators: creators,
             creatorIds: creatorIds,
             fields: fields,
-            fieldIds: fieldIds,
             abstract: abstract,
             dateModified: item.dateModified,
             dateAdded: item.dateAdded
@@ -209,27 +207,27 @@ struct ItemDetailDataCreator {
         urlDetector: UrlDetector,
         doiDetector: (String) -> Bool,
         getExistingData: ((String, String?) -> (String?, String?))? = nil
-    ) throws -> ([String], [String: ItemDetailState.Field], Bool) {
+    ) throws -> (OrderedDictionary<String, ItemDetailState.Field>, Bool) {
         guard var fieldSchemas = schemaController.fields(for: itemType) else {
             throw ItemDetailError.typeNotSupported(itemType)
         }
 
-        var fieldKeys = fieldSchemas.map({ $0.field })
-        let abstractIndex = fieldKeys.firstIndex(of: FieldKeys.Item.abstract)
+        var hasAbstract: Bool = false
+        let titleKey = schemaController.titleKey(for: itemType)
+        var fields: OrderedDictionary<String, ItemDetailState.Field> = [:]
+        for schema in fieldSchemas {
+            let key = schema.field
+            // Remove title and abstract keys, those 2 are used separately in Data struct.
+            if key == FieldKeys.Item.abstract {
+                hasAbstract = true
+                continue
+            }
 
-        // Remove title and abstract keys, those 2 are used separately in Data struct
-        if let index = abstractIndex {
-            fieldKeys.remove(at: index)
-            fieldSchemas.remove(at: index)
-        }
-        if let key = schemaController.titleKey(for: itemType), let index = fieldKeys.firstIndex(of: key) {
-            fieldKeys.remove(at: index)
-            fieldSchemas.remove(at: index)
-        }
+            if key == titleKey {
+                continue
+            }
 
-        var fields: [String: ItemDetailState.Field] = [:]
-        for (offset, key) in fieldKeys.enumerated() {
-            let baseField = fieldSchemas[offset].baseField
+            let baseField = schema.baseField
             let (existingName, existingValue) = (getExistingData?(key, baseField) ?? (nil, nil))
 
             let name = existingName ?? schemaController.localized(field: key) ?? ""
@@ -241,45 +239,30 @@ struct ItemDetailDataCreator {
                 additionalInfo = [.dateOrder: order]
             }
             if key == FieldKeys.Item.accessDate, let date = Formatter.iso8601.date(from: value) {
-                additionalInfo = [.formattedDate: Formatter.dateAndTime.string(from: date),
-                                  .formattedEditDate: Formatter.sqlFormat.string(from: date)]
+                additionalInfo = [.formattedDate: Formatter.dateAndTime.string(from: date), .formattedEditDate: Formatter.sqlFormat.string(from: date)]
             }
 
-            fields[key] = ItemDetailState.Field(key: key,
-                                                baseField: baseField,
-                                                name: name,
-                                                value: value,
-                                                isTitle: false,
-                                                isTappable: isTappable,
-                                                additionalInfo: additionalInfo)
+            fields[key] = ItemDetailState.Field(key: key, baseField: baseField, name: name, value: value, isTitle: false, isTappable: isTappable, additionalInfo: additionalInfo)
         }
 
-        return (fieldKeys, fields, (abstractIndex != nil))
+        return (fields, hasAbstract)
     }
 
     /// Returns all field keys for given item type, except those that should not appear as fields in item detail.
-    static func allFieldKeys(for itemType: String, schemaController: SchemaController) -> [String] {
+    static func allFieldKeys(for itemType: String, schemaController: SchemaController) -> OrderedSet<String> {
         guard let fieldSchemas = schemaController.fields(for: itemType) else { return [] }
-        var fieldKeys = fieldSchemas.map({ $0.field })
-        // Remove title and abstract keys, those 2 are used separately in Data struct
-        if let index = fieldKeys.firstIndex(of: FieldKeys.Item.abstract) {
-            fieldKeys.remove(at: index)
-        }
-        if let key = schemaController.titleKey(for: itemType), let index = fieldKeys.firstIndex(of: key) {
-            fieldKeys.remove(at: index)
+        var fieldKeys: OrderedSet<String> = OrderedSet(fieldSchemas.map({ $0.field }))
+        // Remove title and abstract keys, those 2 are used separately in Data struct.
+        fieldKeys.remove(FieldKeys.Item.abstract)
+        if let titleKey = schemaController.titleKey(for: itemType) {
+            fieldKeys.remove(titleKey)
         }
         return fieldKeys
     }
 
-    /// Returns filtered, sorted array of keys for fields that have non-empty values.
-    static func filteredFieldKeys(from fieldKeys: [String], fields: [String: ItemDetailState.Field]) -> [String] {
-        var newFieldKeys: [String] = []
-        fieldKeys.forEach { key in
-            if !(fields[key]?.value ?? "").isEmpty {
-                newFieldKeys.append(key)
-            }
-        }
-        return newFieldKeys
+    /// Returns filtered, ordered set of keys for fields that have non-empty values.
+    static func filteredFieldKeys(from fields: OrderedDictionary<String, ItemDetailState.Field>) -> OrderedSet<String> {
+        return fields.filter({ !$0.value.value.isEmpty }).keys
     }
 
     /// Checks whether field is tappable based on its key and value.
diff --git a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
index 1c0c75aad..c7c05fde7 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
@@ -7,6 +7,7 @@
 //
 
 import UIKit
+import OrderedCollections
 
 import CocoaLumberjackSwift
 import RealmSwift
@@ -177,8 +178,7 @@ struct ItemDetailState: ViewModelState {
         var localizedType: String
         var creators: [String: Creator]
         var creatorIds: [String]
-        var fields: [String: Field]
-        var fieldIds: [String]
+        var fields: OrderedDictionary<String, Field>
         var abstract: String?
 
         var dateModified: Date
@@ -222,7 +222,6 @@ struct ItemDetailState: ViewModelState {
                 creators: [:],
                 creatorIds: [],
                 fields: [:],
-                fieldIds: [],
                 abstract: nil,
                 dateModified: date,
                 dateAdded: date,
@@ -248,6 +247,7 @@ struct ItemDetailState: ViewModelState {
     var data: Data
     var snapshot: Data?
     var promptSnapshot: Data?
+    var presentedFieldIds: OrderedSet<String>
     var notes: [Note]
     var attachments: [Attachment]
     var tags: [Tag]
@@ -291,6 +291,7 @@ struct ItemDetailState: ViewModelState {
         self.userId = userId
         self.changes = []
         self.data = .empty
+        self.presentedFieldIds = []
         self.attachments = []
         self.notes = []
         self.tags = []
diff --git a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
index d3138e12f..81c11ad43 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
@@ -249,7 +249,7 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
                 self.itemChanged(change, in: viewModel)
             }
 
-            var (data, attachments, notes, tags) = try ItemDetailDataCreator.createData(
+            let (data, attachments, notes, tags) = try ItemDetailDataCreator.createData(
                 from: .existing(item: item, ignoreChildren: false),
                 schemaController: self.schemaController,
                 dateParser: self.dateParser,
@@ -260,43 +260,43 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
                 doiDetector: FieldKeys.Item.isDoi
             )
 
-            if !canEdit {
-                data.fieldIds = ItemDetailDataCreator.filteredFieldKeys(from: data.fieldIds, fields: data.fields)
-            }
-
-            self.saveReloaded(data: data, attachments: attachments, notes: notes, tags: tags, isEditing: canEdit, library: library, token: token, in: viewModel)
+            saveReloaded(data: data, attachments: attachments, notes: notes, tags: tags, isEditing: canEdit, library: library, token: token, in: viewModel)
         } catch let error {
             DDLogError("ItemDetailActionHandler: can't load data - \(error)")
             self.update(viewModel: viewModel) { state in
                 state.error = .cantCreateData
             }
         }
-    }
 
-    private func saveReloaded(
-        data: ItemDetailState.Data,
-        attachments: [Attachment],
-        notes: [Note],
-        tags: [Tag],
-        isEditing: Bool,
-        library: Library,
-        token: NotificationToken,
-        in viewModel: ViewModel<ItemDetailActionHandler>
-    ) {
-        self.update(viewModel: viewModel) { state in
-            state.data = data
-            if state.snapshot != nil || isEditing {
-                state.snapshot = data
-                state.snapshot?.fieldIds = ItemDetailDataCreator.filteredFieldKeys(from: data.fieldIds, fields: data.fields)
+        func saveReloaded(
+            data: ItemDetailState.Data,
+            attachments: [Attachment],
+            notes: [Note],
+            tags: [Tag],
+            isEditing: Bool,
+            library: Library,
+            token: NotificationToken,
+            in viewModel: ViewModel<ItemDetailActionHandler>
+        ) {
+            update(viewModel: viewModel) { state in
+                state.data = data
+                if state.snapshot != nil || isEditing {
+                    state.snapshot = data
+                }
+                if isEditing && !data.isAttachment {
+                    state.presentedFieldIds = data.fields.keys
+                } else {
+                    state.presentedFieldIds = ItemDetailDataCreator.filteredFieldKeys(from: data.fields)
+                }
+                state.attachments = attachments
+                state.notes = notes
+                state.tags = tags
+                state.library = library
+                state.isLoadingData = false
+                state.isEditing = isEditing
+                state.observationToken = token
+                state.changes.insert(.reloadedData)
             }
-            state.attachments = attachments
-            state.notes = notes
-            state.tags = tags
-            state.library = library
-            state.isLoadingData = false
-            state.isEditing = isEditing
-            state.observationToken = token
-            state.changes.insert(.reloadedData)
         }
     }
 
@@ -648,16 +648,28 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
     private func startEditing(in viewModel: ViewModel<ItemDetailActionHandler>) {
         self.update(viewModel: viewModel) { state in
             state.snapshot = state.data
-            state.data.fieldIds = ItemDetailDataCreator.allFieldKeys(for: state.data.type, schemaController: self.schemaController)
+            if !state.data.isAttachment {
+                state.presentedFieldIds = ItemDetailDataCreator.allFieldKeys(for: state.data.type, schemaController: schemaController)
+            }
             state.isEditing = true
             state.changes.insert(.editing)
         }
     }
 
     private func endEditing(in viewModel: ViewModel<ItemDetailActionHandler>) {
-        guard viewModel.state.snapshot != viewModel.state.data else { return }
+        guard viewModel.state.snapshot != viewModel.state.data else {
+            update(viewModel: viewModel) { state in
+                state.snapshot = nil
+                state.presentedFieldIds = ItemDetailDataCreator.filteredFieldKeys(from: state.data.fields)
+                state.isEditing = false
+                state.type = .preview(key: state.key)
+                state.isSaving = false
+                state.changes.insert(.editing)
+            }
+            return
+        }
 
-        self.update(viewModel: viewModel) { state in
+        update(viewModel: viewModel) { state in
             state.isSaving = true
         }
 
@@ -708,7 +720,7 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
 
                     newState.data.dateModified = Date()
                     newState.snapshot = nil
-                    newState.data.fieldIds = ItemDetailDataCreator.filteredFieldKeys(from: newState.data.fieldIds, fields: newState.data.fields)
+                    newState.presentedFieldIds = ItemDetailDataCreator.filteredFieldKeys(from: newState.data.fields)
                     newState.isEditing = false
                     newState.type = .preview(key: newState.key)
                     newState.changes.insert(.editing)
@@ -835,6 +847,7 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
         self.update(viewModel: viewModel) { state in
             if droppedFields.isEmpty {
                 state.data = itemData
+                state.presentedFieldIds = ItemDetailDataCreator.allFieldKeys(for: state.data.type, schemaController: schemaController)
                 state.changes.insert(.type)
             } else {
                 // Notify the user, that some fields with values will be dropped
@@ -862,7 +875,7 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
                 throw ItemDetailError.typeNotSupported(type)
             }
 
-            let (fieldIds, fields, hasAbstract) = try ItemDetailDataCreator.fieldData(
+            let (fields, hasAbstract) = try ItemDetailDataCreator.fieldData(
                 for: type,
                 schemaController: self.schemaController,
                 dateParser: self.dateParser,
@@ -884,7 +897,6 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
             data.isAttachment = type == ItemTypes.attachment
             data.localizedType = localizedType
             data.fields = fields
-            data.fieldIds = fieldIds
             data.abstract = hasAbstract ? (originalData.abstract ?? "") : nil
             data.creators = try creators(for: type, from: originalData.creators)
             data.creatorIds = originalData.creatorIds
diff --git a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
index cc7fcdba0..2772cc633 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
@@ -457,7 +457,8 @@ final class ItemDetailCollectionViewHandler: NSObject {
             guard let self else { return }
             // Assign new id to all sections, just reload everything
             let id = UUID().uuidString
-            let sections = sections(for: state.data, isEditing: state.isEditing, library: state.library).map({ SectionType(identifier: id, section: $0) })
+            let sections = sections(for: state.data, hasPresentedFields: !state.presentedFieldIds.isEmpty, isEditing: state.isEditing, library: state.library)
+                .map({ SectionType(identifier: id, section: $0) })
             var snapshot = NSDiffableDataSourceSnapshot<SectionType, Row>()
             snapshot.appendSections(sections)
             for section in sections {
@@ -473,23 +474,29 @@ final class ItemDetailCollectionViewHandler: NSObject {
         /// - parameter data: New data.
         /// - parameter isEditing: Current editing table view state.
         /// - returns: Array of visible sections.
-        func sections(for data: ItemDetailState.Data, isEditing: Bool, library: Library) -> [Section] {
+        func sections(for data: ItemDetailState.Data, hasPresentedFields: Bool, isEditing: Bool, library: Library) -> [Section] {
+            // Title and item type are always visible.
+            var sections: [Section] = [.title, .type]
+
             if isEditing {
                 // Only "metadata" sections are visible during editing.
-                if data.isAttachment {
-                    return [.title, .type, .fields, .dates]
-                } else {
-                    return [.title, .type, .creators, .fields, .dates, .abstract]
+                if !data.isAttachment {
+                    sections.append(.creators)
+                }
+                if hasPresentedFields {
+                    sections.append(.fields)
+                }
+                sections.append(.dates)
+                if !data.isAttachment {
+                    sections.append(.abstract)
                 }
+                return sections
             }
 
-            var sections: [Section] = [.title]
-            // Item type is always visible
-            sections.append(.type)
             if !data.creators.isEmpty {
                 sections.append(.creators)
             }
-            if !data.fieldIds.isEmpty {
+            if hasPresentedFields {
                 sections.append(.fields)
             }
             sections.append(.dates)
@@ -667,7 +674,7 @@ final class ItemDetailCollectionViewHandler: NSObject {
             return [.dateAdded(state.data.dateAdded), .dateModified(state.data.dateModified)]
 
         case .fields:
-            return state.data.fieldIds.compactMap({ fieldId in
+            return state.presentedFieldIds.compactMap({ fieldId in
                 return .field(key: fieldId, multiline: (fieldId == FieldKeys.Item.extra))
             })
 
diff --git a/ZoteroTests/SyncActionsSpec.swift b/ZoteroTests/SyncActionsSpec.swift
index 7bdaebd32..df9ba43f5 100644
--- a/ZoteroTests/SyncActionsSpec.swift
+++ b/ZoteroTests/SyncActionsSpec.swift
@@ -103,7 +103,6 @@ final class SyncActionsSpec: QuickSpec {
                         creators: [:],
                         creatorIds: [],
                         fields: [:],
-                        fieldIds: [],
                         abstract: "New abstract",
                         dateModified: Date(),
                         dateAdded: Date()
@@ -117,7 +116,6 @@ final class SyncActionsSpec: QuickSpec {
                         creators: [:],
                         creatorIds: [],
                         fields: [:],
-                        fieldIds: [],
                         abstract: "Some note",
                         dateModified: Date(),
                         dateAdded: Date()
@@ -368,7 +366,6 @@ final class SyncActionsSpec: QuickSpec {
                         creators: [:],
                         creatorIds: [],
                         fields: [:],
-                        fieldIds: [],
                         abstract: "New abstract",
                         dateModified: Date(),
                         dateAdded: Date()
@@ -382,7 +379,6 @@ final class SyncActionsSpec: QuickSpec {
                         creators: [:],
                         creatorIds: [],
                         fields: [:],
-                        fieldIds: [],
                         abstract: "Some note",
                         dateModified: Date(),
                         dateAdded: Date()

From 4c30c26d1cb5ab944e66945e2a6e683480e6cbd6 Mon Sep 17 00:00:00 2001
From: Miltiadis Vasilakis <mvasilak@gmail.com>
Date: Thu, 9 Jan 2025 15:53:32 +0200
Subject: [PATCH 02/12] Update Item date modified with each saved change

---
 .../Requests/EditItemFieldsDbRequest.swift    | 49 +++++++++++---
 .../EndItemDetailEditingDbRequest.swift       |  1 -
 .../ViewModels/ItemDetailActionHandler.swift  | 67 ++++++++++++++-----
 3 files changed, 91 insertions(+), 26 deletions(-)

diff --git a/Zotero/Controllers/Database/Requests/EditItemFieldsDbRequest.swift b/Zotero/Controllers/Database/Requests/EditItemFieldsDbRequest.swift
index 0267e1579..9d293079b 100644
--- a/Zotero/Controllers/Database/Requests/EditItemFieldsDbRequest.swift
+++ b/Zotero/Controllers/Database/Requests/EditItemFieldsDbRequest.swift
@@ -10,16 +10,18 @@ import Foundation
 
 import RealmSwift
 
-struct EditItemFieldsDbRequest: DbRequest {
-    let key: String
-    let libraryId: LibraryIdentifier
-    let fieldValues: [KeyBaseKeyPair: String]
-    let dateParser: DateParser
+protocol EditItemFieldsBaseRequest {
+    var key: String { get }
+    var libraryId: LibraryIdentifier { get }
+    var fieldValues: [KeyBaseKeyPair: String] { get }
+    var dateParser: DateParser { get }
 
-    var needsWrite: Bool { return true }
+    func processAndReturnResponse(in database: Realm) throws -> Date?
+}
 
-    func process(in database: Realm) throws {
-        guard !fieldValues.isEmpty, let item = database.objects(RItem.self).uniqueObject(key: key, libraryId: libraryId) else { return }
+extension EditItemFieldsBaseRequest {
+    func processAndReturnResponse(in database: Realm) throws -> Date? {
+        guard !fieldValues.isEmpty, let item = database.objects(RItem.self).uniqueObject(key: key, libraryId: libraryId) else { return nil }
 
         var didChange = false
 
@@ -60,6 +62,37 @@ struct EditItemFieldsDbRequest: DbRequest {
             item.changes.append(RObjectChange.create(changes: RItemChanges.fields))
             item.changeType = .user
             item.dateModified = Date()
+            return item.dateModified
         }
+
+        return nil
+    }
+}
+
+struct EditItemFieldsDbRequest: EditItemFieldsBaseRequest, DbRequest {
+    let key: String
+    let libraryId: LibraryIdentifier
+    let fieldValues: [KeyBaseKeyPair: String]
+    let dateParser: DateParser
+
+    var needsWrite: Bool { return true }
+
+    func process(in database: Realm) throws {
+        _ = try processAndReturnResponse(in: database)
+    }
+}
+
+struct EditItemFieldsDbResponseRequest: EditItemFieldsBaseRequest, DbResponseRequest {
+    typealias Response = Date?
+
+    let key: String
+    let libraryId: LibraryIdentifier
+    let fieldValues: [KeyBaseKeyPair: String]
+    let dateParser: DateParser
+
+    var needsWrite: Bool { return true }
+
+    func process(in database: Realm) throws -> Date? {
+        return try processAndReturnResponse(in: database)
     }
 }
diff --git a/Zotero/Controllers/Database/Requests/EndItemDetailEditingDbRequest.swift b/Zotero/Controllers/Database/Requests/EndItemDetailEditingDbRequest.swift
index ba8869162..f9e61a454 100644
--- a/Zotero/Controllers/Database/Requests/EndItemDetailEditingDbRequest.swift
+++ b/Zotero/Controllers/Database/Requests/EndItemDetailEditingDbRequest.swift
@@ -19,7 +19,6 @@ struct EndItemDetailEditingDbRequest: DbRequest {
 
     func process(in database: Realm) throws {
         guard let item = database.objects(RItem.self).uniqueObject(key: itemKey, libraryId: libraryId) else { return }
-        item.dateModified = Date()
         item.changesSyncPaused = false
         item.changeType = .user
     }
diff --git a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
index 81c11ad43..4d5590bd4 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
@@ -712,13 +712,19 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
                         newState.data.fields[updated.key] = updated
                     }
 
-                    var requests: [DbRequest] = [EndItemDetailEditingDbRequest(libraryId: state.library.identifier, itemKey: state.key)]
-                    if !updatedFields.isEmpty {
-                        requests.insert(EditItemFieldsDbRequest(key: state.key, libraryId: state.library.identifier, fieldValues: updatedFields, dateParser: dateParser), at: 0)
+                    let endEditingRequest = EndItemDetailEditingDbRequest(libraryId: state.library.identifier, itemKey: state.key)
+                    var dateModified: Date?
+                    try dbStorage.perform(on: queue) { coordinator in
+                        if !updatedFields.isEmpty {
+                            let request = EditItemFieldsDbResponseRequest(key: state.key, libraryId: state.library.identifier, fieldValues: updatedFields, dateParser: dateParser)
+                            dateModified = try coordinator.perform(request: request)
+                        }
+                        try coordinator.perform(request: endEditingRequest)
                     }
-                    try self.dbStorage.perform(writeRequests: requests, on: queue)
 
-                    newState.data.dateModified = Date()
+                    if let dateModified {
+                        newState.data.dateModified = dateModified
+                    }
                     newState.snapshot = nil
                     newState.presentedFieldIds = ItemDetailDataCreator.filteredFieldKeys(from: newState.data.fields)
                     newState.isEditing = false
@@ -967,10 +973,19 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
         }
 
         let keyPair = KeyBaseKeyPair(key: key, baseKey: (key != FieldKeys.Item.title ? FieldKeys.Item.title : nil))
-        let request = EditItemFieldsDbRequest(key: viewModel.state.key, libraryId: viewModel.state.library.identifier, fieldValues: [keyPair: viewModel.state.data.title], dateParser: dateParser)
-        self.perform(request: request) { error in
-            guard let error else { return }
-            DDLogError("ItemDetailActionHandler: can't store title - \(error)")
+        let request = EditItemFieldsDbResponseRequest(key: viewModel.state.key, libraryId: viewModel.state.library.identifier, fieldValues: [keyPair: viewModel.state.data.title], dateParser: dateParser)
+        perform(request: request, invalidateRealm: false) { [weak viewModel] result in
+            switch result {
+            case .success(let dateModified):
+                guard let viewModel, let dateModified else { return }
+                update(viewModel: viewModel) { state in
+                    state.data.dateModified = dateModified
+                    state.reload = .section(.dates)
+                }
+
+            case .failure(let error):
+                DDLogError("ItemDetailActionHandler: can't store title - \(error)")
+            }
         }
     }
 
@@ -980,15 +995,24 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
             state.reload = .row(.abstract)
         }
 
-        let request = EditItemFieldsDbRequest(
+        let request = EditItemFieldsDbResponseRequest(
             key: viewModel.state.key,
             libraryId: viewModel.state.library.identifier,
             fieldValues: [KeyBaseKeyPair(key: FieldKeys.Item.abstract, baseKey: nil): abstract],
             dateParser: dateParser
         )
-        self.perform(request: request) { error in
-            guard let error else { return }
-            DDLogError("ItemDetailActionHandler: can't store abstract - \(error)")
+        perform(request: request, invalidateRealm: false) { [weak viewModel] result in
+            switch result {
+            case .success(let dateModified):
+                guard let viewModel, let dateModified else { return }
+                update(viewModel: viewModel) { state in
+                    state.data.dateModified = dateModified
+                    state.reload = .section(.dates)
+                }
+
+            case .failure(let error):
+                DDLogError("ItemDetailActionHandler: can't store abstract - \(error)")
+            }
         }
     }
 
@@ -1011,15 +1035,24 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
             state.reload = .row(.field(key: field.key, multiline: (field.id == FieldKeys.Item.extra)))
         }
 
-        let request = EditItemFieldsDbRequest(
+        let request = EditItemFieldsDbResponseRequest(
             key: viewModel.state.key,
             libraryId: viewModel.state.library.identifier,
             fieldValues: [KeyBaseKeyPair(key: field.key, baseKey: field.baseField): field.value],
             dateParser: dateParser
         )
-        self.perform(request: request) { error in
-            guard let error else { return }
-            DDLogError("ItemDetailActionHandler: can't store field \(error)")
+        perform(request: request, invalidateRealm: false) { [weak viewModel] result in
+            switch result {
+            case .success(let dateModified):
+                guard let viewModel, let dateModified else { return }
+                update(viewModel: viewModel) { state in
+                    state.data.dateModified = dateModified
+                    state.reload = .section(.dates)
+                }
+
+            case .failure(let error):
+                DDLogError("ItemDetailActionHandler: can't store field - \(error)")
+            }
         }
     }
 

From 5cd90874c080d5ba55f5658ae04d3feea38999a1 Mon Sep 17 00:00:00 2001
From: Miltiadis Vasilakis <mvasilak@gmail.com>
Date: Fri, 10 Jan 2025 18:17:58 +0200
Subject: [PATCH 03/12] Improve code

---
 Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift | 4 ++--
 .../ItemDetail/ViewModels/ItemDetailActionHandler.swift     | 6 +++---
 .../ItemDetail/Views/ItemDetailCollectionViewHandler.swift  | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift b/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
index e035b03cd..4d21e3ff2 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
@@ -260,8 +260,8 @@ struct ItemDetailDataCreator {
         return fieldKeys
     }
 
-    /// Returns filtered, ordered set of keys for fields that have non-empty values.
-    static func filteredFieldKeys(from fields: OrderedDictionary<String, ItemDetailState.Field>) -> OrderedSet<String> {
+    /// Returns ordered set of keys for fields that have non-empty values.
+    static func nonEmptyFieldKeys(from fields: OrderedDictionary<String, ItemDetailState.Field>) -> OrderedSet<String> {
         return fields.filter({ !$0.value.value.isEmpty }).keys
     }
 
diff --git a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
index 4d5590bd4..ef4d3e6e0 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
@@ -286,7 +286,7 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
                 if isEditing && !data.isAttachment {
                     state.presentedFieldIds = data.fields.keys
                 } else {
-                    state.presentedFieldIds = ItemDetailDataCreator.filteredFieldKeys(from: data.fields)
+                    state.presentedFieldIds = ItemDetailDataCreator.nonEmptyFieldKeys(from: data.fields)
                 }
                 state.attachments = attachments
                 state.notes = notes
@@ -660,7 +660,7 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
         guard viewModel.state.snapshot != viewModel.state.data else {
             update(viewModel: viewModel) { state in
                 state.snapshot = nil
-                state.presentedFieldIds = ItemDetailDataCreator.filteredFieldKeys(from: state.data.fields)
+                state.presentedFieldIds = ItemDetailDataCreator.nonEmptyFieldKeys(from: state.data.fields)
                 state.isEditing = false
                 state.type = .preview(key: state.key)
                 state.isSaving = false
@@ -726,7 +726,7 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
                         newState.data.dateModified = dateModified
                     }
                     newState.snapshot = nil
-                    newState.presentedFieldIds = ItemDetailDataCreator.filteredFieldKeys(from: newState.data.fields)
+                    newState.presentedFieldIds = ItemDetailDataCreator.nonEmptyFieldKeys(from: newState.data.fields)
                     newState.isEditing = false
                     newState.type = .preview(key: newState.key)
                     newState.changes.insert(.editing)
diff --git a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
index 2772cc633..81d641441 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
@@ -908,7 +908,7 @@ extension ItemDetailCollectionViewHandler: UICollectionViewDelegate {
             guard viewModel.state.isEditing else { return }
             observer.on(.next(.openCreatorEditor(creator)))
 
-        case .note(let key, let title, let isProcessing):
+        case .note(let key, _, let isProcessing):
             guard !isProcessing else { return }
             observer.on(.next(.openNoteEditor(key: key)))
 

From e8d16522655cdccb0d80802e1c3e68ab8c6ec8fb Mon Sep 17 00:00:00 2001
From: Miltiadis Vasilakis <mvasilak@gmail.com>
Date: Sun, 12 Jan 2025 14:16:42 +0200
Subject: [PATCH 04/12] Add isEditable property to ItemDetailState.Field struct

---
 .../ItemDetail/ItemDetailDataCreator.swift    | 33 +++++++++-------
 .../ItemDetail/Models/ItemDetailState.swift   | 39 +++++++++++--------
 .../ViewModels/ItemDetailActionHandler.swift  | 16 +++++---
 .../ItemDetailCollectionViewHandler.swift     |  6 +--
 4 files changed, 54 insertions(+), 40 deletions(-)

diff --git a/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift b/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
index 4d21e3ff2..f7a4b33db 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
@@ -199,7 +199,7 @@ struct ItemDetailDataCreator {
     /// - parameter doiDetector: DOI detector.
     /// - parameter getExistingData: Closure for getting available data for given field. It passes the field key and baseField and receives existing
     ///                              field name and value if available.
-    /// - returns: Tuple with 3 values: field keys of new fields, actual fields, `Bool` indicating whether this item type contains an abstract.
+    /// - returns: Tuple with 2 values: orderered dictionary of fields by field key, `Bool` indicating whether this item type contains an abstract.
     static func fieldData(
         for itemType: String,
         schemaController: SchemaController,
@@ -208,12 +208,13 @@ struct ItemDetailDataCreator {
         doiDetector: (String) -> Bool,
         getExistingData: ((String, String?) -> (String?, String?))? = nil
     ) throws -> (OrderedDictionary<String, ItemDetailState.Field>, Bool) {
-        guard var fieldSchemas = schemaController.fields(for: itemType) else {
+        guard let fieldSchemas = schemaController.fields(for: itemType) else {
             throw ItemDetailError.typeNotSupported(itemType)
         }
 
         var hasAbstract: Bool = false
         let titleKey = schemaController.titleKey(for: itemType)
+        let isEditable = itemType != ItemTypes.attachment
         var fields: OrderedDictionary<String, ItemDetailState.Field> = [:]
         for schema in fieldSchemas {
             let key = schema.field
@@ -242,29 +243,31 @@ struct ItemDetailDataCreator {
                 additionalInfo = [.formattedDate: Formatter.dateAndTime.string(from: date), .formattedEditDate: Formatter.sqlFormat.string(from: date)]
             }
 
-            fields[key] = ItemDetailState.Field(key: key, baseField: baseField, name: name, value: value, isTitle: false, isTappable: isTappable, additionalInfo: additionalInfo)
+            fields[key] = ItemDetailState.Field(
+                key: key,
+                baseField: baseField,
+                name: name,
+                value: value,
+                isTitle: false,
+                isEditable: isEditable,
+                isTappable: isTappable,
+                additionalInfo: additionalInfo
+            )
         }
 
         return (fields, hasAbstract)
     }
 
-    /// Returns all field keys for given item type, except those that should not appear as fields in item detail.
-    static func allFieldKeys(for itemType: String, schemaController: SchemaController) -> OrderedSet<String> {
-        guard let fieldSchemas = schemaController.fields(for: itemType) else { return [] }
-        var fieldKeys: OrderedSet<String> = OrderedSet(fieldSchemas.map({ $0.field }))
-        // Remove title and abstract keys, those 2 are used separately in Data struct.
-        fieldKeys.remove(FieldKeys.Item.abstract)
-        if let titleKey = schemaController.titleKey(for: itemType) {
-            fieldKeys.remove(titleKey)
-        }
-        return fieldKeys
-    }
-
     /// Returns ordered set of keys for fields that have non-empty values.
     static func nonEmptyFieldKeys(from fields: OrderedDictionary<String, ItemDetailState.Field>) -> OrderedSet<String> {
         return fields.filter({ !$0.value.value.isEmpty }).keys
     }
 
+    /// Returns ordered set of keys for fields that are either editable or have non-empty values.
+    static func editableOrNonEmptyFieldKeys(from fields: OrderedDictionary<String, ItemDetailState.Field>) -> OrderedSet<String> {
+        return fields.filter({ $0.value.isEditable || !$0.value.value.isEmpty }).keys
+    }
+
     /// Checks whether field is tappable based on its key and value.
     /// - parameter key: Key of field.
     /// - parameter value: Value of field.
diff --git a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
index c7c05fde7..2d4e53445 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
@@ -47,6 +47,7 @@ struct ItemDetailState: ViewModelState {
         var name: String
         var value: String
         let isTitle: Bool
+        let isEditable: Bool
         var isTappable: Bool
         var additionalInfo: [AdditionalInfoKey: String]?
 
@@ -188,24 +189,30 @@ struct ItemDetailState: ViewModelState {
         var maxNonemptyFieldTitleWidth: CGFloat = 0
 
         func databaseFields(schemaController: SchemaController) -> [Field] {
-            var allFields = Array(self.fields.values)
-
-            if let titleKey = schemaController.titleKey(for: self.type) {
-                allFields.append(Field(key: titleKey,
-                                       baseField: (titleKey != FieldKeys.Item.title ? FieldKeys.Item.title : nil),
-                                       name: "",
-                                       value: self.title,
-                                       isTitle: true,
-                                       isTappable: false))
+            var allFields = Array(fields.values)
+
+            if let titleKey = schemaController.titleKey(for: type) {
+                allFields.append(Field(
+                    key: titleKey,
+                    baseField: (titleKey != FieldKeys.Item.title ? FieldKeys.Item.title : nil),
+                    name: "",
+                    value: title,
+                    isTitle: true,
+                    isEditable: !isAttachment,
+                    isTappable: false
+                ))
             }
 
-            if let abstract = self.abstract {
-                allFields.append(Field(key: FieldKeys.Item.abstract,
-                                       baseField: nil,
-                                       name: "",
-                                       value: abstract,
-                                       isTitle: false,
-                                       isTappable: false))
+            if let abstract {
+                allFields.append(Field(
+                    key: FieldKeys.Item.abstract,
+                    baseField: nil,
+                    name: "",
+                    value: abstract,
+                    isTitle: false,
+                    isEditable: isAttachment,
+                    isTappable: false
+                ))
             }
 
             return allFields
diff --git a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
index ef4d3e6e0..8451d4364 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
@@ -283,9 +283,11 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
                 if state.snapshot != nil || isEditing {
                     state.snapshot = data
                 }
-                if isEditing && !data.isAttachment {
-                    state.presentedFieldIds = data.fields.keys
+                if isEditing {
+                    // During editing present only editable fields or non-empty, non-editable ones.
+                    state.presentedFieldIds = ItemDetailDataCreator.editableOrNonEmptyFieldKeys(from: data.fields)
                 } else {
+                    // Otherwise present only non-empty fields.
                     state.presentedFieldIds = ItemDetailDataCreator.nonEmptyFieldKeys(from: data.fields)
                 }
                 state.attachments = attachments
@@ -648,9 +650,9 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
     private func startEditing(in viewModel: ViewModel<ItemDetailActionHandler>) {
         self.update(viewModel: viewModel) { state in
             state.snapshot = state.data
-            if !state.data.isAttachment {
-                state.presentedFieldIds = ItemDetailDataCreator.allFieldKeys(for: state.data.type, schemaController: schemaController)
-            }
+            // state.data.fields has all available fields for this state.data.type,
+            // so we present only those that are editable or non-empty.
+            state.presentedFieldIds = ItemDetailDataCreator.editableOrNonEmptyFieldKeys(from: state.data.fields)
             state.isEditing = true
             state.changes.insert(.editing)
         }
@@ -853,7 +855,9 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
         self.update(viewModel: viewModel) { state in
             if droppedFields.isEmpty {
                 state.data = itemData
-                state.presentedFieldIds = ItemDetailDataCreator.allFieldKeys(for: state.data.type, schemaController: schemaController)
+                // state.data.fields has all available fields for the changed state.data.type,
+                // so we present only those that are editable or non-empty.
+                state.presentedFieldIds = ItemDetailDataCreator.editableOrNonEmptyFieldKeys(from: state.data.fields)
                 state.changes.insert(.type)
             } else {
                 // Notify the user, that some fields with values will be dropped
diff --git a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
index 81d641441..2e617f370 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
@@ -219,7 +219,7 @@ final class ItemDetailCollectionViewHandler: NSObject {
 
                 case .field(let key, let multiline):
                     guard let field = viewModel.state.data.fields[key] else { return collectionView.dequeueConfiguredReusableCell(using: emptyRegistration, for: indexPath, item: ()) }
-                    if !isEditing || viewModel.state.data.isAttachment {
+                    if !isEditing || !field.isEditable {
                         return collectionView.dequeueConfiguredReusableCell(using: fieldRegistration, for: indexPath, item: (.field(field), titleWidth))
                     }
                     if multiline {
@@ -917,8 +917,8 @@ extension ItemDetailCollectionViewHandler: UICollectionViewDelegate {
             observer.on(.next(.openTypePicker))
 
         case .field(let fieldId, _):
-            // Tappable fields should be only tappable when not in editing mode, in case of attachment, URL is not editable, so keep it tappable even while editing.
-            guard !viewModel.state.isEditing || (viewModel.state.data.type == ItemTypes.attachment), let field = viewModel.state.data.fields[fieldId], field.isTappable else { return }
+            // Tappable fields should be only tappable when not in editing mode, or field is not editable. E.g. in case of attachment, URL is not editable, so keep it tappable even while editing.
+            guard let field = viewModel.state.data.fields[fieldId], field.isTappable, !viewModel.state.isEditing || !field.isEditable else { return }
             switch field.key {
             case FieldKeys.Item.Attachment.url:
                 observer.on(.next(.openUrl(field.value)))

From 59990c49f7e2dd79354f506ce76dc6f0edca9796 Mon Sep 17 00:00:00 2001
From: Miltiadis Vasilakis <mvasilak@gmail.com>
Date: Mon, 13 Jan 2025 12:43:33 +0200
Subject: [PATCH 05/12] Improve code

---
 .../ItemDetail/ItemDetailDataCreator.swift     | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift b/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
index f7a4b33db..76ef96b01 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
@@ -236,11 +236,19 @@ struct ItemDetailDataCreator {
             let isTappable = ItemDetailDataCreator.isTappable(key: key, value: value, urlDetector: urlDetector, doiDetector: doiDetector)
             var additionalInfo: [ItemDetailState.Field.AdditionalInfoKey: String]?
 
-            if key == FieldKeys.Item.date || baseField == FieldKeys.Item.date, let order = dateParser.parse(string: value)?.orderWithSpaces {
-                additionalInfo = [.dateOrder: order]
-            }
-            if key == FieldKeys.Item.accessDate, let date = Formatter.iso8601.date(from: value) {
-                additionalInfo = [.formattedDate: Formatter.dateAndTime.string(from: date), .formattedEditDate: Formatter.sqlFormat.string(from: date)]
+            switch (key, baseField) {
+            case (FieldKeys.Item.date, _), (_, FieldKeys.Item.date):
+                if let order = dateParser.parse(string: value)?.orderWithSpaces {
+                    additionalInfo = [.dateOrder: order]
+                }
+
+            case (FieldKeys.Item.accessDate, _):
+                if let date = Formatter.iso8601.date(from: value) {
+                    additionalInfo = [.formattedDate: Formatter.dateAndTime.string(from: date), .formattedEditDate: Formatter.sqlFormat.string(from: date)]
+                }
+
+            default:
+                break
             }
 
             fields[key] = ItemDetailState.Field(

From b7a2f26f820d18d14c248a0c788df425587c9379 Mon Sep 17 00:00:00 2001
From: Miltiadis Vasilakis <mvasilak@gmail.com>
Date: Mon, 13 Jan 2025 12:48:42 +0200
Subject: [PATCH 06/12] Improve code

---
 .../ItemDetail/Models/ItemDetailState.swift   |  4 ++--
 .../ViewModels/ItemDetailActionHandler.swift  | 20 +++++++++----------
 .../ItemDetailCollectionViewHandler.swift     | 10 +++++-----
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
index 2d4e53445..ae7a993bd 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
@@ -254,7 +254,7 @@ struct ItemDetailState: ViewModelState {
     var data: Data
     var snapshot: Data?
     var promptSnapshot: Data?
-    var presentedFieldIds: OrderedSet<String>
+    var visibleFieldIds: OrderedSet<String>
     var notes: [Note]
     var attachments: [Attachment]
     var tags: [Tag]
@@ -298,7 +298,7 @@ struct ItemDetailState: ViewModelState {
         self.userId = userId
         self.changes = []
         self.data = .empty
-        self.presentedFieldIds = []
+        self.visibleFieldIds = []
         self.attachments = []
         self.notes = []
         self.tags = []
diff --git a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
index 8451d4364..ac57b8a65 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
@@ -284,11 +284,11 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
                     state.snapshot = data
                 }
                 if isEditing {
-                    // During editing present only editable fields or non-empty, non-editable ones.
-                    state.presentedFieldIds = ItemDetailDataCreator.editableOrNonEmptyFieldKeys(from: data.fields)
+                    // During editing show only editable fields or non-empty, non-editable ones.
+                    state.visibleFieldIds = ItemDetailDataCreator.editableOrNonEmptyFieldKeys(from: data.fields)
                 } else {
-                    // Otherwise present only non-empty fields.
-                    state.presentedFieldIds = ItemDetailDataCreator.nonEmptyFieldKeys(from: data.fields)
+                    // Otherwise show only non-empty fields.
+                    state.visibleFieldIds = ItemDetailDataCreator.nonEmptyFieldKeys(from: data.fields)
                 }
                 state.attachments = attachments
                 state.notes = notes
@@ -651,8 +651,8 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
         self.update(viewModel: viewModel) { state in
             state.snapshot = state.data
             // state.data.fields has all available fields for this state.data.type,
-            // so we present only those that are editable or non-empty.
-            state.presentedFieldIds = ItemDetailDataCreator.editableOrNonEmptyFieldKeys(from: state.data.fields)
+            // so we show only those that are editable or non-empty.
+            state.visibleFieldIds = ItemDetailDataCreator.editableOrNonEmptyFieldKeys(from: state.data.fields)
             state.isEditing = true
             state.changes.insert(.editing)
         }
@@ -662,7 +662,7 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
         guard viewModel.state.snapshot != viewModel.state.data else {
             update(viewModel: viewModel) { state in
                 state.snapshot = nil
-                state.presentedFieldIds = ItemDetailDataCreator.nonEmptyFieldKeys(from: state.data.fields)
+                state.visibleFieldIds = ItemDetailDataCreator.nonEmptyFieldKeys(from: state.data.fields)
                 state.isEditing = false
                 state.type = .preview(key: state.key)
                 state.isSaving = false
@@ -728,7 +728,7 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
                         newState.data.dateModified = dateModified
                     }
                     newState.snapshot = nil
-                    newState.presentedFieldIds = ItemDetailDataCreator.nonEmptyFieldKeys(from: newState.data.fields)
+                    newState.visibleFieldIds = ItemDetailDataCreator.nonEmptyFieldKeys(from: newState.data.fields)
                     newState.isEditing = false
                     newState.type = .preview(key: newState.key)
                     newState.changes.insert(.editing)
@@ -856,8 +856,8 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
             if droppedFields.isEmpty {
                 state.data = itemData
                 // state.data.fields has all available fields for the changed state.data.type,
-                // so we present only those that are editable or non-empty.
-                state.presentedFieldIds = ItemDetailDataCreator.editableOrNonEmptyFieldKeys(from: state.data.fields)
+                // so we show only those that are editable or non-empty.
+                state.visibleFieldIds = ItemDetailDataCreator.editableOrNonEmptyFieldKeys(from: state.data.fields)
                 state.changes.insert(.type)
             } else {
                 // Notify the user, that some fields with values will be dropped
diff --git a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
index 2e617f370..bb12cfae9 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
@@ -457,7 +457,7 @@ final class ItemDetailCollectionViewHandler: NSObject {
             guard let self else { return }
             // Assign new id to all sections, just reload everything
             let id = UUID().uuidString
-            let sections = sections(for: state.data, hasPresentedFields: !state.presentedFieldIds.isEmpty, isEditing: state.isEditing, library: state.library)
+            let sections = sections(for: state.data, hasVisibleFields: !state.visibleFieldIds.isEmpty, isEditing: state.isEditing, library: state.library)
                 .map({ SectionType(identifier: id, section: $0) })
             var snapshot = NSDiffableDataSourceSnapshot<SectionType, Row>()
             snapshot.appendSections(sections)
@@ -474,7 +474,7 @@ final class ItemDetailCollectionViewHandler: NSObject {
         /// - parameter data: New data.
         /// - parameter isEditing: Current editing table view state.
         /// - returns: Array of visible sections.
-        func sections(for data: ItemDetailState.Data, hasPresentedFields: Bool, isEditing: Bool, library: Library) -> [Section] {
+        func sections(for data: ItemDetailState.Data, hasVisibleFields: Bool, isEditing: Bool, library: Library) -> [Section] {
             // Title and item type are always visible.
             var sections: [Section] = [.title, .type]
 
@@ -483,7 +483,7 @@ final class ItemDetailCollectionViewHandler: NSObject {
                 if !data.isAttachment {
                     sections.append(.creators)
                 }
-                if hasPresentedFields {
+                if hasVisibleFields {
                     sections.append(.fields)
                 }
                 sections.append(.dates)
@@ -496,7 +496,7 @@ final class ItemDetailCollectionViewHandler: NSObject {
             if !data.creators.isEmpty {
                 sections.append(.creators)
             }
-            if hasPresentedFields {
+            if hasVisibleFields {
                 sections.append(.fields)
             }
             sections.append(.dates)
@@ -674,7 +674,7 @@ final class ItemDetailCollectionViewHandler: NSObject {
             return [.dateAdded(state.data.dateAdded), .dateModified(state.data.dateModified)]
 
         case .fields:
-            return state.presentedFieldIds.compactMap({ fieldId in
+            return state.visibleFieldIds.compactMap({ fieldId in
                 return .field(key: fieldId, multiline: (fieldId == FieldKeys.Item.extra))
             })
 

From 0ad0e2d594a2a6e07c7a67df23eea954b95e47f9 Mon Sep 17 00:00:00 2001
From: Miltiadis Vasilakis <mvasilak@gmail.com>
Date: Mon, 13 Jan 2025 15:00:38 +0200
Subject: [PATCH 07/12] Use OrderedDictionary for ItemDetailState creators

---
 .../CreateItemFromDetailDbRequest.swift       |  4 +--
 .../EditItemFromDetailDbRequest.swift         |  4 +--
 .../EditTypeItemDetailDbRequest.swift         | 14 ++++-----
 .../ItemDetail/ItemDetailDataCreator.swift    |  6 +---
 .../ItemDetail/Models/ItemDetailState.swift   |  4 +--
 .../ViewModels/ItemDetailActionHandler.swift  | 29 +++++++++----------
 .../ItemDetailCollectionViewHandler.swift     |  5 +---
 ZoteroTests/SyncActionsSpec.swift             |  4 ---
 8 files changed, 26 insertions(+), 44 deletions(-)

diff --git a/Zotero/Controllers/Database/Requests/CreateItemFromDetailDbRequest.swift b/Zotero/Controllers/Database/Requests/CreateItemFromDetailDbRequest.swift
index f17bf91b0..9ad89464f 100644
--- a/Zotero/Controllers/Database/Requests/CreateItemFromDetailDbRequest.swift
+++ b/Zotero/Controllers/Database/Requests/CreateItemFromDetailDbRequest.swift
@@ -58,9 +58,7 @@ struct CreateItemFromDetailDbRequest: DbResponseRequest {
 
         // Create creators
 
-        for (offset, creatorId) in self.data.creatorIds.enumerated() {
-            guard let creator = self.data.creators[creatorId] else { continue }
-
+        for (offset, (_, creator)) in data.creators.enumerated() {
             let rCreator = RCreator()
             rCreator.uuid = UUID().uuidString
             rCreator.rawType = creator.type
diff --git a/Zotero/Controllers/Database/Requests/EditItemFromDetailDbRequest.swift b/Zotero/Controllers/Database/Requests/EditItemFromDetailDbRequest.swift
index eb0f2f21c..4beee8d91 100644
--- a/Zotero/Controllers/Database/Requests/EditItemFromDetailDbRequest.swift
+++ b/Zotero/Controllers/Database/Requests/EditItemFromDetailDbRequest.swift
@@ -48,9 +48,7 @@ struct EditItemFromDetailDbRequest: DbRequest {
     private func updateCreators(with data: ItemDetailState.Data, snapshot: ItemDetailState.Data, item: RItem, changes: inout RItemChanges, database: Realm) {
         guard data.creators != snapshot.creators else { return }
         database.delete(item.creators)
-        for (offset, creatorId) in data.creatorIds.enumerated() {
-            guard let creator = data.creators[creatorId] else { continue }
-
+        for (offset, (_, creator)) in data.creators.enumerated() {
             let rCreator = RCreator()
             rCreator.uuid = UUID().uuidString
             rCreator.rawType = creator.type
diff --git a/Zotero/Controllers/Database/Requests/EditTypeItemDetailDbRequest.swift b/Zotero/Controllers/Database/Requests/EditTypeItemDetailDbRequest.swift
index f1df666fa..22ad8184a 100644
--- a/Zotero/Controllers/Database/Requests/EditTypeItemDetailDbRequest.swift
+++ b/Zotero/Controllers/Database/Requests/EditTypeItemDetailDbRequest.swift
@@ -7,6 +7,7 @@
 //
 
 import Foundation
+import OrderedCollections
 
 import RealmSwift
 
@@ -15,8 +16,7 @@ struct EditTypeItemDetailDbRequest: DbRequest {
     let libraryId: LibraryIdentifier
     let type: String
     var fields: [ItemDetailState.Field]
-    let creatorIds: [String]
-    let creators: [String: ItemDetailState.Creator]
+    let creators: OrderedDictionary<String, ItemDetailState.Creator>
     let dateParser: DateParser
 
     var needsWrite: Bool { return true }
@@ -28,7 +28,7 @@ struct EditTypeItemDetailDbRequest: DbRequest {
 
         var changes: RItemChanges = [.type]
         update(fields: fields, item: item, changes: &changes, database: database)
-        update(creatorIds: creatorIds, creators: creators, item: item, changes: &changes, database: database)
+        update(creators: creators, item: item, changes: &changes, database: database)
         item.changes.append(RObjectChange.create(changes: changes))
     }
 
@@ -92,17 +92,17 @@ struct EditTypeItemDetailDbRequest: DbRequest {
         }
     }
 
-    private func update(creatorIds: [String], creators: [String: ItemDetailState.Creator], item: RItem, changes: inout RItemChanges, database: Realm) {
+    private func update(creators: OrderedDictionary<String, ItemDetailState.Creator>, item: RItem, changes: inout RItemChanges, database: Realm) {
         // Remove creator types which don't exist for this item type
-        let toRemove = item.creators.filter("not uuid in %@", creatorIds)
+        let toRemove = item.creators.filter("not uuid in %@", creators.keys)
         if !toRemove.isEmpty {
             changes.insert(.creators)
         }
         database.delete(toRemove)
 
-        for creatorId in creatorIds {
+        for (creatorId, creator) in creators {
             // When changing item type, only thing that can change for creator is it's type
-            guard let creator = creators[creatorId], let rCreator = item.creators.filter("uuid == %@", creatorId).first, rCreator.rawType != creator.type else { continue }
+            guard let rCreator = item.creators.filter("uuid == %@", creatorId).first, rCreator.rawType != creator.type else { continue }
             rCreator.rawType = creator.type
             changes.insert(.creators)
         }
diff --git a/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift b/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
index 76ef96b01..eb9936c2a 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
@@ -78,7 +78,6 @@ struct ItemDetailDataCreator {
             isAttachment: (itemType == ItemTypes.attachment),
             localizedType: localizedType,
             creators: [:],
-            creatorIds: [],
             fields: fields,
             abstract: (hasAbstract ? "" : nil),
             dateModified: date,
@@ -129,8 +128,7 @@ struct ItemDetailDataCreator {
             return (nil, values[key])
         }
 
-        var creatorIds: [String] = []
-        var creators: [String: ItemDetailState.Creator] = [:]
+        var creators: OrderedDictionary<String, ItemDetailState.Creator> = [:]
         for creator in item.creators.sorted(byKeyPath: "orderId") {
             guard let localizedType = schemaController.localized(creator: creator.rawType) else { continue }
 
@@ -143,7 +141,6 @@ struct ItemDetailDataCreator {
                 primary: schemaController.creatorIsPrimary(creator.rawType, itemType: item.rawType),
                 localizedType: localizedType
             )
-            creatorIds.append(creator.id)
             creators[creator.id] = creator
         }
 
@@ -182,7 +179,6 @@ struct ItemDetailDataCreator {
             isAttachment: (item.rawType == ItemTypes.attachment),
             localizedType: localizedType,
             creators: creators,
-            creatorIds: creatorIds,
             fields: fields,
             abstract: abstract,
             dateModified: item.dateModified,
diff --git a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
index ae7a993bd..7c8b3f59f 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
@@ -177,8 +177,7 @@ struct ItemDetailState: ViewModelState {
         var type: String
         var isAttachment: Bool
         var localizedType: String
-        var creators: [String: Creator]
-        var creatorIds: [String]
+        var creators: OrderedDictionary<String, Creator>
         var fields: OrderedDictionary<String, Field>
         var abstract: String?
 
@@ -227,7 +226,6 @@ struct ItemDetailState: ViewModelState {
                 isAttachment: false,
                 localizedType: "",
                 creators: [:],
-                creatorIds: [],
                 fields: [:],
                 abstract: nil,
                 dateModified: date,
diff --git a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
index ac57b8a65..1dfa63f25 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
@@ -7,6 +7,7 @@
 //
 
 import Foundation
+import OrderedCollections
 
 import Alamofire
 import CocoaLumberjackSwift
@@ -909,12 +910,11 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
             data.fields = fields
             data.abstract = hasAbstract ? (originalData.abstract ?? "") : nil
             data.creators = try creators(for: type, from: originalData.creators)
-            data.creatorIds = originalData.creatorIds
             return data
         }
 
-        func creators(for type: String, from originalData: [String: ItemDetailState.Creator]) throws -> [String: ItemDetailState.Creator] {
-            guard let schemas = self.schemaController.creators(for: type), let primary = schemas.first(where: { $0.primary }) else { throw ItemDetailError.typeNotSupported(type) }
+        func creators(for type: String, from originalData: OrderedDictionary<String, ItemDetailState.Creator>) throws -> OrderedDictionary<String, ItemDetailState.Creator> {
+            guard let schemas = schemaController.creators(for: type), let primary = schemas.first(where: { $0.primary }) else { throw ItemDetailError.typeNotSupported(type) }
 
             var creators = originalData
             for (key, originalCreator) in originalData {
@@ -927,7 +927,7 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
                 } else {
                     creator.type = "contributor"
                 }
-                creator.localizedType = self.schemaController.localized(creator: creator.type) ?? ""
+                creator.localizedType = schemaController.localized(creator: creator.type) ?? ""
 
                 creators[key] = creator
             }
@@ -953,7 +953,6 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
             libraryId: viewModel.state.library.identifier,
             type: viewModel.state.data.type,
             fields: viewModel.state.data.databaseFields(schemaController: schemaController),
-            creatorIds: viewModel.state.data.creatorIds,
             creators: viewModel.state.data.creators,
             dateParser: dateParser
         )
@@ -1061,10 +1060,9 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
     }
 
     private func deleteCreator(with id: String, in viewModel: ViewModel<ItemDetailActionHandler>) {
-        guard let index = viewModel.state.data.creatorIds.firstIndex(of: id) else { return }
-        
+        guard viewModel.state.data.creators[id] != nil else { return }
+
         self.update(viewModel: viewModel) { state in
-            state.data.creatorIds.remove(at: index)
             state.data.creators[id] = nil
             state.reload = .section(.creators)
         }
@@ -1078,14 +1076,11 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
 
     private func save(creator: State.Creator, in viewModel: ViewModel<ItemDetailActionHandler>) {
         self.update(viewModel: viewModel) { state in
-            if !state.data.creatorIds.contains(creator.id) {
-                state.data.creatorIds.append(creator.id)
-            }
             state.data.creators[creator.id] = creator
             state.reload = .section(.creators)
         }
 
-        guard let orderId = viewModel.state.data.creatorIds.firstIndex(of: creator.id) else { return }
+        guard let orderId = viewModel.state.data.creators.index(forKey: creator.id) else { return }
         let request = EditCreatorItemDetailDbRequest(key: viewModel.state.key, libraryId: viewModel.state.library.identifier, creator: creator, orderId: orderId)
         self.perform(request: request) { error in
             guard let error else { return }
@@ -1094,11 +1089,15 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
     }
 
     private func moveCreators(diff: CollectionDifference<String>, in viewModel: ViewModel<ItemDetailActionHandler>) {
-        self.update(viewModel: viewModel) { state in
-            state.data.creatorIds = state.data.creatorIds.applying(diff) ?? []
+        update(viewModel: viewModel) { state in
+            var movedCreators: OrderedDictionary<String, ItemDetailState.Creator> = [:]
+            (state.data.creators.keys.applying(diff) ?? []).forEach {
+                movedCreators[$0] = state.data.creators[$0]
+            }
+            state.data.creators = movedCreators
         }
         
-        let request = ReorderCreatorsItemDetailDbRequest(key: viewModel.state.key, libraryId: viewModel.state.library.identifier, ids: viewModel.state.data.creatorIds)
+        let request = ReorderCreatorsItemDetailDbRequest(key: viewModel.state.key, libraryId: viewModel.state.library.identifier, ids: Array(viewModel.state.data.creators.keys))
         self.perform(request: request) { error in
             guard let error else { return }
             DDLogError("ItemDetailActionHandler: can't reorder creators \(error)")
diff --git a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
index bb12cfae9..2dbc13f76 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
@@ -661,10 +661,7 @@ final class ItemDetailCollectionViewHandler: NSObject {
             return attachments
 
         case .creators:
-            let creators: [Row] = state.data.creatorIds.compactMap({ creatorId in
-                guard let creator = state.data.creators[creatorId] else { return nil }
-                return .creator(creator)
-            })
+            let creators: [Row] = state.data.creators.values.map({ .creator($0) })
             if state.isEditing {
                 return creators + [.addCreator]
             }
diff --git a/ZoteroTests/SyncActionsSpec.swift b/ZoteroTests/SyncActionsSpec.swift
index df9ba43f5..55113c2ba 100644
--- a/ZoteroTests/SyncActionsSpec.swift
+++ b/ZoteroTests/SyncActionsSpec.swift
@@ -101,7 +101,6 @@ final class SyncActionsSpec: QuickSpec {
                         isAttachment: false,
                         localizedType: "Magazine Article",
                         creators: [:],
-                        creatorIds: [],
                         fields: [:],
                         abstract: "New abstract",
                         dateModified: Date(),
@@ -114,7 +113,6 @@ final class SyncActionsSpec: QuickSpec {
                         isAttachment: false,
                         localizedType: "Thesis",
                         creators: [:],
-                        creatorIds: [],
                         fields: [:],
                         abstract: "Some note",
                         dateModified: Date(),
@@ -364,7 +362,6 @@ final class SyncActionsSpec: QuickSpec {
                         isAttachment: false,
                         localizedType: "Magazine Article",
                         creators: [:],
-                        creatorIds: [],
                         fields: [:],
                         abstract: "New abstract",
                         dateModified: Date(),
@@ -377,7 +374,6 @@ final class SyncActionsSpec: QuickSpec {
                         isAttachment: false,
                         localizedType: "Thesis",
                         creators: [:],
-                        creatorIds: [],
                         fields: [:],
                         abstract: "Some note",
                         dateModified: Date(),

From 704152c9daece42605b76571656b104e438be039 Mon Sep 17 00:00:00 2001
From: Miltiadis Vasilakis <mvasilak@gmail.com>
Date: Mon, 13 Jan 2025 15:15:39 +0200
Subject: [PATCH 08/12] Improve code

---
 .../ItemDetail/ViewModels/ItemDetailActionHandler.swift       | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
index 1dfa63f25..46dadfebb 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
@@ -281,10 +281,8 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
         ) {
             update(viewModel: viewModel) { state in
                 state.data = data
-                if state.snapshot != nil || isEditing {
-                    state.snapshot = data
-                }
                 if isEditing {
+                    state.snapshot = data
                     // During editing show only editable fields or non-empty, non-editable ones.
                     state.visibleFieldIds = ItemDetailDataCreator.editableOrNonEmptyFieldKeys(from: data.fields)
                 } else {

From 20126a82df43ac98ce28e8e0b5a35ad3bb21589a Mon Sep 17 00:00:00 2001
From: Miltiadis Vasilakis <mvasilak@gmail.com>
Date: Wed, 15 Jan 2025 20:27:28 +0200
Subject: [PATCH 09/12] Improve item fields editing

Delay item fields editing in action handler instead of debouncing row
text fields
Update collection view snapshot when individual rows change
---
 Zotero/Controllers/BackgroundTimer.swift      |  62 ++--
 .../ItemDetail/Models/ItemDetailState.swift   |   1 +
 .../ViewModels/ItemDetailActionHandler.swift  | 316 ++++++++++--------
 .../ItemDetailCollectionViewHandler.swift     |  17 +-
 .../Views/ItemDetailViewController.swift      |   3 +
 5 files changed, 223 insertions(+), 176 deletions(-)

diff --git a/Zotero/Controllers/BackgroundTimer.swift b/Zotero/Controllers/BackgroundTimer.swift
index 627645e36..04cd3ca0a 100644
--- a/Zotero/Controllers/BackgroundTimer.swift
+++ b/Zotero/Controllers/BackgroundTimer.swift
@@ -11,25 +11,18 @@
 import Foundation
 
 final class BackgroundTimer {
-    private enum State {
+    enum State {
         case suspended
         case resumed
     }
 
     private let timeInterval: DispatchTimeInterval
     private let queue: DispatchQueue
+    private var timer: DispatchSourceTimer?
+    private(set) var startTime: DispatchTime?
 
     var eventHandler: (() -> Void)?
-    private var state: State = .suspended
-    private lazy var timer: DispatchSourceTimer = {
-        let t = DispatchSource.makeTimerSource(flags: [], queue: self.queue)
-        t.schedule(deadline: .now() + self.timeInterval, repeating: 0)
-        t.setEventHandler(handler: { [weak self] in
-            self?.eventHandler?()
-            self?.suspend()
-        })
-        return t
-    }()
+    private(set) var state: State = .suspended
 
     init(timeInterval: DispatchTimeInterval, queue: DispatchQueue) {
         self.timeInterval = timeInterval
@@ -37,25 +30,44 @@ final class BackgroundTimer {
     }
 
     deinit {
-        self.timer.setEventHandler {}
-        self.timer.cancel()
-        /*
-         If the timer is suspended, calling cancel without resuming
-         triggers a crash. This is documented here https://forums.developer.apple.com/thread/15902
-         */
-        self.resume()
-        self.eventHandler = nil
+        invalidate()
     }
 
     func resume() {
-        guard self.state != .resumed else { return }
-        self.state = .resumed
-        self.timer.resume()
+        guard state != .resumed else { return }
+        state = .resumed
+        timer = timer ?? createTimer()
+        
+        timer?.resume()
     }
 
     func suspend() {
-        guard self.state != .suspended else { return }
-        self.state = .suspended
-        self.timer.suspend()
+        guard let timer, state != .suspended else { return }
+        state = .suspended
+        timer.suspend()
+    }
+
+    func invalidate() {
+        guard let timer else { return }
+        timer.setEventHandler {}
+        timer.cancel()
+        /*
+         If the timer is suspended, calling cancel without resuming
+         triggers a crash. This is documented here https://forums.developer.apple.com/thread/15902
+         */
+        resume()
+        eventHandler = nil
+    }
+
+    private func createTimer() -> DispatchSourceTimer {
+        let timer = DispatchSource.makeTimerSource(flags: [], queue: queue)
+        let now = DispatchTime.now()
+        startTime = now
+        timer.schedule(deadline: now + timeInterval, repeating: 0)
+        timer.setEventHandler(handler: { [weak self] in
+            self?.eventHandler?()
+            self?.suspend()
+        })
+        return timer
     }
 }
diff --git a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
index 7c8b3f59f..3ca3c9346 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
@@ -238,6 +238,7 @@ struct ItemDetailState: ViewModelState {
 
     enum TableViewReloadType {
         case row(ItemDetailCollectionViewHandler.Row)
+        case rows([ItemDetailCollectionViewHandler.Row])
         case section(ItemDetailCollectionViewHandler.Section)
     }
 
diff --git a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
index 46dadfebb..a2f4ffd3e 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
@@ -15,7 +15,7 @@ import RealmSwift
 import RxSwift
 import ZIPFoundation
 
-struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingActionHandler {
+final class ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingActionHandler {
     typealias State = ItemDetailState
     typealias Action = ItemDetailAction
 
@@ -163,8 +163,8 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
 
         do {
             let libraryToken: NotificationToken?
-            (library, libraryToken) = try viewModel.state.library.identifier.observe(in: dbStorage, changes: { [weak viewModel] library in
-                guard let viewModel else { return }
+            (library, libraryToken) = try viewModel.state.library.identifier.observe(in: dbStorage, changes: { [weak self, weak viewModel] library in
+                guard let self, let viewModel else { return }
                 reloadData(isEditing: viewModel.state.isEditing, library: library, in: viewModel)
             })
 
@@ -694,38 +694,13 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
             .disposed(by: self.disposeBag)
 
         func endEditing(state: ItemDetailState, queue: DispatchQueue) -> Single<ItemDetailState> {
-            return Single.create { subscriber -> Disposable in
+            return Single.create { [weak self] subscriber -> Disposable in
                 do {
                     var newState = state
 
-                    var updatedFields: [KeyBaseKeyPair: String] = [:]
-                    if let field = state.data.fields[FieldKeys.Item.accessDate] {
-                        let updated = updated(accessDateField: field, originalField: state.snapshot?.fields[FieldKeys.Item.accessDate])
-                        if updated.value != field.value {
-                            updatedFields[KeyBaseKeyPair(key: updated.key, baseKey: updated.baseField)] = updated.value
-                            newState.data.fields[updated.key] = updated
-                        }
-                    }
-                    if let field = state.data.fields.values.first(where: { $0.baseField == FieldKeys.Item.date || $0.key == FieldKeys.Item.date }),
-                        let updated = updated(dateField: field),
-                        updated.value != field.value {
-                        updatedFields[KeyBaseKeyPair(key: updated.key, baseKey: updated.baseField)] = updated.value
-                        newState.data.fields[updated.key] = updated
-                    }
-
                     let endEditingRequest = EndItemDetailEditingDbRequest(libraryId: state.library.identifier, itemKey: state.key)
-                    var dateModified: Date?
-                    try dbStorage.perform(on: queue) { coordinator in
-                        if !updatedFields.isEmpty {
-                            let request = EditItemFieldsDbResponseRequest(key: state.key, libraryId: state.library.identifier, fieldValues: updatedFields, dateParser: dateParser)
-                            dateModified = try coordinator.perform(request: request)
-                        }
-                        try coordinator.perform(request: endEditingRequest)
-                    }
+                    try self?.dbStorage.perform(request: endEditingRequest, on: queue)
 
-                    if let dateModified {
-                        newState.data.dateModified = dateModified
-                    }
                     newState.snapshot = nil
                     newState.visibleFieldIds = ItemDetailDataCreator.nonEmptyFieldKeys(from: newState.data.fields)
                     newState.isEditing = false
@@ -739,58 +714,6 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
                 return Disposables.create()
             }
         }
-
-        func updated(accessDateField: ItemDetailState.Field, originalField: ItemDetailState.Field?) -> ItemDetailState.Field {
-            var field = accessDateField
-
-            var date: Date?
-            if let _date = parseDateSpecialValue(from: field.value) {
-                date = _date
-            } else if let _date = Formatter.sqlFormat.date(from: field.value) {
-                date = _date
-            }
-
-            if let date = date {
-                field.value = Formatter.iso8601.string(from: date)
-                field.additionalInfo = [.formattedDate: Formatter.dateAndTime.string(from: date), .formattedEditDate: Formatter.sqlFormat.string(from: date)]
-            } else {
-                if let originalField {
-                    field = originalField
-                } else {
-                    field.value = ""
-                    field.additionalInfo = [:]
-                }
-            }
-
-            return field
-        }
-
-        func updated(dateField: ItemDetailState.Field) -> ItemDetailState.Field? {
-            guard let date = parseDateSpecialValue(from: dateField.value) else { return nil }
-            var field = dateField
-            field.value = Formatter.dateWithDashes.string(from: date)
-            if let order = self.dateParser.parse(string: field.value)?.orderWithSpaces {
-                field.additionalInfo?[.dateOrder] = order
-            }
-            return field
-        }
-
-        func parseDateSpecialValue(from value: String) -> Date? {
-            // TODO: - check for current localization
-            switch value.lowercased() {
-            case "tomorrow":
-                return Calendar.current.date(byAdding: .day, value: 1, to: Date())
-
-            case "today":
-                return Date()
-                
-            case "yesterday":
-                return Calendar.current.date(byAdding: .day, value: -1, to: Date())
-
-            default:
-                return nil
-            }
-        }
     }
 
     private func cancelChanges(in viewModel: ViewModel<ItemDetailActionHandler>) {
@@ -961,100 +884,61 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
     }
 
     private func set(title: NSAttributedString, in viewModel: ViewModel<ItemDetailActionHandler>) {
-        guard let key = self.schemaController.titleKey(for: viewModel.state.data.type) else {
+        guard let key = schemaController.titleKey(for: viewModel.state.data.type) else {
             DDLogError("ItemDetailActionHandler: schema controller doesn't contain title key for item type \(viewModel.state.data.type)")
             return
         }
         guard title != viewModel.state.data.attributedTitle else { return }
 
-        self.update(viewModel: viewModel) { state in
+        update(viewModel: viewModel) { state in
             state.data.attributedTitle = title
             state.data.title = htmlAttributedStringConverter.convert(attributedString: title)
             state.reload = .row(.title)
         }
 
         let keyPair = KeyBaseKeyPair(key: key, baseKey: (key != FieldKeys.Item.title ? FieldKeys.Item.title : nil))
-        let request = EditItemFieldsDbResponseRequest(key: viewModel.state.key, libraryId: viewModel.state.library.identifier, fieldValues: [keyPair: viewModel.state.data.title], dateParser: dateParser)
-        perform(request: request, invalidateRealm: false) { [weak viewModel] result in
-            switch result {
-            case .success(let dateModified):
-                guard let viewModel, let dateModified else { return }
-                update(viewModel: viewModel) { state in
-                    state.data.dateModified = dateModified
-                    state.reload = .section(.dates)
-                }
-
-            case .failure(let error):
-                DDLogError("ItemDetailActionHandler: can't store title - \(error)")
-            }
-        }
+        delayItemFieldsEdit(fieldValues: [keyPair: viewModel.state.data.title], in: viewModel)
     }
 
     private func set(abstract: String, in viewModel: ViewModel<ItemDetailActionHandler>) {
-        self.update(viewModel: viewModel) { state in
+        update(viewModel: viewModel) { state in
             state.data.abstract = abstract
             state.reload = .row(.abstract)
         }
 
-        let request = EditItemFieldsDbResponseRequest(
-            key: viewModel.state.key,
-            libraryId: viewModel.state.library.identifier,
-            fieldValues: [KeyBaseKeyPair(key: FieldKeys.Item.abstract, baseKey: nil): abstract],
-            dateParser: dateParser
-        )
-        perform(request: request, invalidateRealm: false) { [weak viewModel] result in
-            switch result {
-            case .success(let dateModified):
-                guard let viewModel, let dateModified else { return }
-                update(viewModel: viewModel) { state in
-                    state.data.dateModified = dateModified
-                    state.reload = .section(.dates)
-                }
-
-            case .failure(let error):
-                DDLogError("ItemDetailActionHandler: can't store abstract - \(error)")
-            }
-        }
+        delayItemFieldsEdit(fieldValues: [KeyBaseKeyPair(key: FieldKeys.Item.abstract, baseKey: nil): abstract], in: viewModel)
     }
 
     private func setField(value: String, for id: String, in viewModel: ViewModel<ItemDetailActionHandler>) {
-        guard var field = viewModel.state.data.fields[id] else { return }
-
+        guard let previousField = viewModel.state.data.fields[id] else { return }
+        var field = previousField
         field.value = value
-        field.isTappable = ItemDetailDataCreator.isTappable(key: field.key, value: field.value, urlDetector: self.urlDetector, doiDetector: FieldKeys.Item.isDoi)
-
-        if field.key == FieldKeys.Item.date || field.baseField == FieldKeys.Item.date, let order = self.dateParser.parse(string: value)?.orderWithSpaces {
-            var info = field.additionalInfo ?? [:]
-            info[.dateOrder] = order
-            field.additionalInfo = info
-        } else if field.additionalInfo != nil {
-            field.additionalInfo = nil
+        field.isTappable = ItemDetailDataCreator.isTappable(key: field.key, value: field.value, urlDetector: urlDetector, doiDetector: FieldKeys.Item.isDoi)
+
+        // If a date field has it's value edited, update only additional info here, as the user may still be typing.
+        // Modify data & accessed date field values just before storing.
+        switch (field.key, field.baseField) {
+        case (FieldKeys.Item.date, _), (_, FieldKeys.Item.date):
+            if let order = dateParser.parse(string: field.value)?.orderWithSpaces {
+                var info = field.additionalInfo ?? [:]
+                info[.dateOrder] = order
+                field.additionalInfo = info
+            } else {
+                field.additionalInfo = nil
+            }
+
+        default:
+            break
         }
 
-        self.update(viewModel: viewModel) { state in
+        guard previousField != field else { return }
+        
+        update(viewModel: viewModel) { state in
             state.data.fields[id] = field
             state.reload = .row(.field(key: field.key, multiline: (field.id == FieldKeys.Item.extra)))
         }
 
-        let request = EditItemFieldsDbResponseRequest(
-            key: viewModel.state.key,
-            libraryId: viewModel.state.library.identifier,
-            fieldValues: [KeyBaseKeyPair(key: field.key, baseKey: field.baseField): field.value],
-            dateParser: dateParser
-        )
-        perform(request: request, invalidateRealm: false) { [weak viewModel] result in
-            switch result {
-            case .success(let dateModified):
-                guard let viewModel, let dateModified else { return }
-                update(viewModel: viewModel) { state in
-                    state.data.dateModified = dateModified
-                    state.reload = .section(.dates)
-                }
-
-            case .failure(let error):
-                DDLogError("ItemDetailActionHandler: can't store field - \(error)")
-            }
-        }
+        delayItemFieldsEdit(fieldValues: [KeyBaseKeyPair(key: field.key, baseKey: field.baseField): field.value], in: viewModel)
     }
 
     private func deleteCreator(with id: String, in viewModel: ViewModel<ItemDetailActionHandler>) {
@@ -1101,4 +985,140 @@ struct ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcessingAc
             DDLogError("ItemDetailActionHandler: can't reorder creators \(error)")
         }
     }
+
+    private var pendingFieldValues: [KeyBaseKeyPair: String] = [:]
+    private var delayTimer: BackgroundTimer?
+    static private let delay: DispatchTimeInterval = .milliseconds(500)
+    private func delayItemFieldsEdit(fieldValues: [KeyBaseKeyPair: String], in viewModel: ViewModel<ItemDetailActionHandler>) {
+        // First suspend delayTimer in case it's running.
+        delayTimer?.suspend()
+        if let delayTimer {
+            // If there is a delay timer, it is now suspended.
+            // Add or replace pending field values.
+            for (key, value) in fieldValues {
+                pendingFieldValues[key] = value
+            }
+            if let startTime = delayTimer.startTime, startTime + Self.delay > .now() {
+                // If deadline wasn't reached, resume timer.
+                delayTimer.resume()
+            } else {
+                // Deadline has been reached, process pending field values, and free the timer.
+                // TODO: implement processing
+                storeItemFieldsEdit(fieldValues: pendingFieldValues, in: viewModel)
+                pendingFieldValues = [:]
+                self.delayTimer = nil
+            }
+        } else {
+            // Create new timer, and delay processing of pending field values.
+            pendingFieldValues = fieldValues
+            delayTimer = BackgroundTimer(timeInterval: Self.delay, queue: .main)
+            delayTimer?.eventHandler = { [weak self] in
+                guard let self else { return }
+                // Deadline has been reached, process pending field values, and free the timer.
+                // TODO: move to nested function call?
+                storeItemFieldsEdit(fieldValues: pendingFieldValues, in: viewModel)
+                pendingFieldValues = [:]
+                delayTimer = nil
+            }
+            delayTimer?.resume()
+        }
+
+        func storeItemFieldsEdit(fieldValues: [KeyBaseKeyPair: String], in viewModel: ViewModel<ItemDetailActionHandler>) {
+            guard !fieldValues.isEmpty else { return }
+            var updatedFieldValues = fieldValues
+
+            var updatedState = viewModel.state
+            var updatedRows: [ItemDetailCollectionViewHandler.Row] = []
+            // Just before storing, modify date & access date values if needed. If so update state.
+            if let (key, _) = updatedFieldValues.first(where: { $0.key.key == FieldKeys.Item.accessDate }), let field = updatedState.data.fields[key.key] {
+                let updated = updated(accessDateField: field, originalField: viewModel.state.snapshot?.fields[FieldKeys.Item.accessDate])
+                if updated.value != field.value {
+                    updatedFieldValues[key] = updated.value
+                    updatedState.data.fields[updated.key] = updated
+                    updatedRows.append(.field(key: updated.key, multiline: false))
+                }
+            }
+            if let (key, _) = updatedFieldValues.first(where: { $0.key.baseKey == FieldKeys.Item.date || $0.key.key == FieldKeys.Item.date }),
+               let field = updatedState.data.fields[key.key],
+               let updated = updated(dateField: field),
+               updated.value != field.value {
+                updatedFieldValues[key] = updated.value
+                updatedState.data.fields[updated.key] = updated
+                updatedRows.append(.field(key: updated.key, multiline: false))
+            }
+            if !updatedRows.isEmpty {
+                update(viewModel: viewModel) { state in
+                    state = updatedState
+                    state.reload = .rows(updatedRows)
+                }
+            }
+
+            let request = EditItemFieldsDbResponseRequest(
+                key: viewModel.state.key,
+                libraryId: viewModel.state.library.identifier,
+                fieldValues: updatedFieldValues,
+                dateParser: dateParser
+            )
+            perform(request: request, invalidateRealm: false) { [weak self, weak viewModel] result in
+                switch result {
+                case .success(let dateModified):
+                    guard let self, let viewModel, let dateModified else { return }
+                    update(viewModel: viewModel) { state in
+                        state.data.dateModified = dateModified
+                        state.reload = .section(.dates)
+                    }
+
+                case .failure(let error):
+                    DDLogError("ItemDetailActionHandler: can't store item fields edit - \(error)")
+                }
+            }
+
+            func updated(accessDateField: ItemDetailState.Field, originalField: ItemDetailState.Field?) -> ItemDetailState.Field {
+                var field = accessDateField
+
+                if let date = parseDateSpecialValue(from: field.value) ?? Formatter.sqlFormat.date(from: field.value) {
+                    field.value = Formatter.iso8601.string(from: date)
+                    field.additionalInfo = [.formattedDate: Formatter.dateAndTime.string(from: date), .formattedEditDate: Formatter.sqlFormat.string(from: date)]
+                } else {
+                    if let originalField {
+                        field = originalField
+                    } else {
+                        field.value = ""
+                        field.additionalInfo = [:]
+                    }
+                }
+
+                return field
+            }
+
+            func updated(dateField: ItemDetailState.Field) -> ItemDetailState.Field? {
+                guard let date = parseDateSpecialValue(from: dateField.value) else { return nil }
+                var field = dateField
+                field.value = Formatter.dateWithDashes.string(from: date)
+                if let order = dateParser.parse(string: field.value)?.orderWithSpaces {
+                    var info = field.additionalInfo ?? [:]
+                    info[.dateOrder] = order
+                    field.additionalInfo = info
+                }
+                return field
+            }
+
+            func parseDateSpecialValue(from value: String) -> Date? {
+                // TODO: - check for current localization
+                switch value.lowercased().trimmingCharacters(in: .whitespacesAndNewlines) {
+                case "tomorrow":
+                    return Calendar.current.date(byAdding: .day, value: 1, to: Date())
+
+                case "today":
+                    return Date()
+
+                case "yesterday":
+                    return Calendar.current.date(byAdding: .day, value: -1, to: Date())
+
+                default:
+                    return nil
+                }
+            }
+        }
+    }
 }
diff --git a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
index 2dbc13f76..4bd621d23 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
@@ -586,7 +586,11 @@ final class ItemDetailCollectionViewHandler: NSObject {
         guard let indexPath = dataSource.indexPath(for: row), let cellFrame = collectionView.cellForItem(at: indexPath)?.frame else { return }
         updateQueue.async { [weak self] in
             guard let self else { return }
-            let snapshot = dataSource.snapshot()
+            var snapshot = dataSource.snapshot()
+            // Reconfigure the item, otherwise the collection view will use the previously cached cells, if it needs to layout again.
+            // E.g. if you press the command key in an external keyboard, while editing, you'll see edited fields revert to their initial value,
+            // but only visually, view model hasn't changed!
+            snapshot.reconfigureItems([row])
             dataSource.apply(snapshot, animatingDifferences: false) { [weak self] in
                 guard let self else { return }
                 let cellBottom = cellFrame.maxY - collectionView.contentOffset.y
@@ -603,6 +607,15 @@ final class ItemDetailCollectionViewHandler: NSObject {
         }
     }
 
+    func updateRows(rows: [Row], state: ItemDetailState) {
+        updateQueue.async { [weak self] in
+            guard let self else { return }
+            var snapshot = dataSource.snapshot()
+            snapshot.reconfigureItems(rows)
+            dataSource.apply(snapshot, animatingDifferences: false)
+        }
+    }
+
     func updateAttachment(with attachment: Attachment, isProcessing: Bool) {
         updateQueue.async { [weak self] in
             guard let self else { return }
@@ -793,7 +806,6 @@ final class ItemDetailCollectionViewHandler: NSObject {
             guard let self else { return }
             let configuration = ItemDetailFieldEditCell.ContentConfiguration(field: data.0, titleWidth: data.1, layoutMargins: layoutMargins(for: indexPath, self: self))
             let disposable = configuration.textObservable
-                .debounce(.milliseconds(500), scheduler: MainScheduler.instance)
                 .subscribe(onNext: { [weak self] text in
                     self?.viewModel.process(action: .setFieldValue(id: data.0.key, value: text))
                 })
@@ -807,7 +819,6 @@ final class ItemDetailCollectionViewHandler: NSObject {
             guard let self else { return }
             let configuration = ItemDetailFieldMultilineEditCell.ContentConfiguration(field: data.0, titleWidth: data.1, layoutMargins: layoutMargins(for: indexPath, self: self))
             let disposable = configuration.textObservable
-                .debounce(.milliseconds(500), scheduler: MainScheduler.instance)
                 .subscribe(onNext: { [weak self] text in
                     self?.viewModel.process(action: .setFieldValue(id: data.0.key, value: text))
                 })
diff --git a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailViewController.swift b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailViewController.swift
index d6d90bace..91c13aad1 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailViewController.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailViewController.swift
@@ -272,6 +272,9 @@ final class ItemDetailViewController: UIViewController {
             case .row(let row):
                 collectionViewHandler.updateHeightAndScrollToUpdated(row: row, state: state)
 
+            case .rows(let rows):
+                collectionViewHandler.updateRows(rows: rows, state: state)
+
             case .section(let section):
                 collectionViewHandler.reload(section: section, state: state, animated: true)
             }

From e7309d4fe097acf50a891aa51c3ad66eb0ae2d8b Mon Sep 17 00:00:00 2001
From: Miltiadis Vasilakis <mvasilak@gmail.com>
Date: Sun, 19 Jan 2025 19:44:37 +0200
Subject: [PATCH 10/12] Make ItemDetailState.Data isAttachment a computed
 property

---
 Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift | 2 --
 .../Scenes/Detail/ItemDetail/Models/ItemDetailState.swift   | 6 ++++--
 .../ItemDetail/ViewModels/ItemDetailActionHandler.swift     | 3 ---
 ZoteroTests/SyncActionsSpec.swift                           | 4 ----
 4 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift b/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
index eb9936c2a..92eb46967 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
@@ -75,7 +75,6 @@ struct ItemDetailDataCreator {
             title: "",
             attributedTitle: .init(string: ""),
             type: itemType,
-            isAttachment: (itemType == ItemTypes.attachment),
             localizedType: localizedType,
             creators: [:],
             fields: fields,
@@ -176,7 +175,6 @@ struct ItemDetailDataCreator {
             title: item.baseTitle,
             attributedTitle: htmlAttributedStringConverter.convert(text: item.baseTitle, baseAttributes: [.font: titleFont]),
             type: item.rawType,
-            isAttachment: (item.rawType == ItemTypes.attachment),
             localizedType: localizedType,
             creators: creators,
             fields: fields,
diff --git a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
index 3ca3c9346..c3c27f04f 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
@@ -175,7 +175,6 @@ struct ItemDetailState: ViewModelState {
         var title: String
         var attributedTitle: NSAttributedString
         var type: String
-        var isAttachment: Bool
         var localizedType: String
         var creators: OrderedDictionary<String, Creator>
         var fields: OrderedDictionary<String, Field>
@@ -187,6 +186,10 @@ struct ItemDetailState: ViewModelState {
         var maxFieldTitleWidth: CGFloat = 0
         var maxNonemptyFieldTitleWidth: CGFloat = 0
 
+        var isAttachment: Bool {
+            return type == ItemTypes.attachment
+        }
+
         func databaseFields(schemaController: SchemaController) -> [Field] {
             var allFields = Array(fields.values)
 
@@ -223,7 +226,6 @@ struct ItemDetailState: ViewModelState {
                 title: "",
                 attributedTitle: .init(string: ""),
                 type: "",
-                isAttachment: false,
                 localizedType: "",
                 creators: [:],
                 fields: [:],
diff --git a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
index a2f4ffd3e..82c64b330 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
@@ -826,7 +826,6 @@ final class ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcess
 
             var data = originalData
             data.type = type
-            data.isAttachment = type == ItemTypes.attachment
             data.localizedType = localizedType
             data.fields = fields
             data.abstract = hasAbstract ? (originalData.abstract ?? "") : nil
@@ -1003,7 +1002,6 @@ final class ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcess
                 delayTimer.resume()
             } else {
                 // Deadline has been reached, process pending field values, and free the timer.
-                // TODO: implement processing
                 storeItemFieldsEdit(fieldValues: pendingFieldValues, in: viewModel)
                 pendingFieldValues = [:]
                 self.delayTimer = nil
@@ -1015,7 +1013,6 @@ final class ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcess
             delayTimer?.eventHandler = { [weak self] in
                 guard let self else { return }
                 // Deadline has been reached, process pending field values, and free the timer.
-                // TODO: move to nested function call?
                 storeItemFieldsEdit(fieldValues: pendingFieldValues, in: viewModel)
                 pendingFieldValues = [:]
                 delayTimer = nil
diff --git a/ZoteroTests/SyncActionsSpec.swift b/ZoteroTests/SyncActionsSpec.swift
index 55113c2ba..c5d1d1c8b 100644
--- a/ZoteroTests/SyncActionsSpec.swift
+++ b/ZoteroTests/SyncActionsSpec.swift
@@ -98,7 +98,6 @@ final class SyncActionsSpec: QuickSpec {
                         title: "New title",
                         attributedTitle: .init(string: "New title"),
                         type: "magazineArticle",
-                        isAttachment: false,
                         localizedType: "Magazine Article",
                         creators: [:],
                         fields: [:],
@@ -110,7 +109,6 @@ final class SyncActionsSpec: QuickSpec {
                         title: "Bachelor thesis",
                         attributedTitle: .init(string: "Bachelor thesis"),
                         type: "thesis",
-                        isAttachment: false,
                         localizedType: "Thesis",
                         creators: [:],
                         fields: [:],
@@ -359,7 +357,6 @@ final class SyncActionsSpec: QuickSpec {
                         title: "New title",
                         attributedTitle: .init(string: "New title"),
                         type: "magazineArticle",
-                        isAttachment: false,
                         localizedType: "Magazine Article",
                         creators: [:],
                         fields: [:],
@@ -371,7 +368,6 @@ final class SyncActionsSpec: QuickSpec {
                         title: "Bachelor thesis",
                         attributedTitle: .init(string: "Bachelor thesis"),
                         type: "thesis",
-                        isAttachment: false,
                         localizedType: "Thesis",
                         creators: [:],
                         fields: [:],

From d0bfd3c06a45d50abab2f1bb90e501a33895addc Mon Sep 17 00:00:00 2001
From: Miltiadis Vasilakis <mvasilak@gmail.com>
Date: Sun, 19 Jan 2025 19:46:38 +0200
Subject: [PATCH 11/12] Remove unused code

---
 .../Scenes/Detail/ItemDetail/Models/ItemDetailState.swift  | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
index c3c27f04f..ba0b60d0b 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
@@ -183,9 +183,6 @@ struct ItemDetailState: ViewModelState {
         var dateModified: Date
         let dateAdded: Date
 
-        var maxFieldTitleWidth: CGFloat = 0
-        var maxNonemptyFieldTitleWidth: CGFloat = 0
-
         var isAttachment: Bool {
             return type == ItemTypes.attachment
         }
@@ -231,9 +228,7 @@ struct ItemDetailState: ViewModelState {
                 fields: [:],
                 abstract: nil,
                 dateModified: date,
-                dateAdded: date,
-                maxFieldTitleWidth: 0,
-                maxNonemptyFieldTitleWidth: 0
+                dateAdded: date
             )
         }
     }

From 5bad313e77dca6d7526fa54d07e64b81f1cefd0c Mon Sep 17 00:00:00 2001
From: Miltiadis Vasilakis <mvasilak@gmail.com>
Date: Mon, 20 Jan 2025 09:13:34 +0200
Subject: [PATCH 12/12] Remove ItemDetailState.Data attributedTitle property

---
 .../Detail/ItemDetail/ItemDetailDataCreator.swift  |  5 -----
 .../Detail/ItemDetail/Models/ItemDetailState.swift |  2 --
 .../ViewModels/ItemDetailActionHandler.swift       |  9 +++------
 .../Views/ItemDetailCollectionViewHandler.swift    | 14 +++++++++++---
 .../Views/ItemDetailViewController.swift           |  1 +
 ZoteroTests/SyncActionsSpec.swift                  |  4 ----
 6 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift b/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
index 92eb46967..45b5fb796 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ItemDetailDataCreator.swift
@@ -32,7 +32,6 @@ struct ItemDetailDataCreator {
         fileStorage: FileStorage,
         urlDetector: UrlDetector,
         htmlAttributedStringConverter: HtmlAttributedStringConverter,
-        titleFont: UIFont,
         doiDetector: (String) -> Bool
     ) throws -> (ItemDetailState.Data, [Attachment], [Note], [Tag]) {
         switch type {
@@ -48,7 +47,6 @@ struct ItemDetailDataCreator {
                 fileStorage: fileStorage,
                 urlDetector: urlDetector,
                 htmlAttributedStringConverter: htmlAttributedStringConverter,
-                titleFont: titleFont,
                 doiDetector: doiDetector
             )
         }
@@ -73,7 +71,6 @@ struct ItemDetailDataCreator {
         let attachments: [Attachment] = child.flatMap({ [$0] }) ?? []
         let data = ItemDetailState.Data(
             title: "",
-            attributedTitle: .init(string: ""),
             type: itemType,
             localizedType: localizedType,
             creators: [:],
@@ -103,7 +100,6 @@ struct ItemDetailDataCreator {
         fileStorage: FileStorage,
         urlDetector: UrlDetector,
         htmlAttributedStringConverter: HtmlAttributedStringConverter,
-        titleFont: UIFont,
         doiDetector: (String) -> Bool
     ) throws -> (ItemDetailState.Data, [Attachment], [Note], [Tag]) {
         guard let localizedType = schemaController.localized(itemType: item.rawType) else {
@@ -173,7 +169,6 @@ struct ItemDetailDataCreator {
         let tags = item.tags.sorted(byKeyPath: "tag.name").map(Tag.init)
         let data = ItemDetailState.Data(
             title: item.baseTitle,
-            attributedTitle: htmlAttributedStringConverter.convert(text: item.baseTitle, baseAttributes: [.font: titleFont]),
             type: item.rawType,
             localizedType: localizedType,
             creators: creators,
diff --git a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
index ba0b60d0b..04e8cd6f7 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Models/ItemDetailState.swift
@@ -173,7 +173,6 @@ struct ItemDetailState: ViewModelState {
 
     struct Data: Equatable {
         var title: String
-        var attributedTitle: NSAttributedString
         var type: String
         var localizedType: String
         var creators: OrderedDictionary<String, Creator>
@@ -221,7 +220,6 @@ struct ItemDetailState: ViewModelState {
             let date = Date()
             return Data(
                 title: "",
-                attributedTitle: .init(string: ""),
                 type: "",
                 localizedType: "",
                 creators: [:],
diff --git a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
index 82c64b330..215a1b0e1 100644
--- a/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/ViewModels/ItemDetailActionHandler.swift
@@ -182,7 +182,6 @@ final class ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcess
                     fileStorage: self.fileStorage,
                     urlDetector: self.urlDetector,
                     htmlAttributedStringConverter: htmlAttributedStringConverter,
-                    titleFont: viewModel.state.titleFont,
                     doiDetector: FieldKeys.Item.isDoi
                 )
 
@@ -196,7 +195,6 @@ final class ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcess
                     fileStorage: self.fileStorage,
                     urlDetector: self.urlDetector,
                     htmlAttributedStringConverter: htmlAttributedStringConverter,
-                    titleFont: viewModel.state.titleFont,
                     doiDetector: FieldKeys.Item.isDoi
                 )
 
@@ -257,7 +255,6 @@ final class ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcess
                 fileStorage: self.fileStorage,
                 urlDetector: self.urlDetector,
                 htmlAttributedStringConverter: htmlAttributedStringConverter,
-                titleFont: viewModel.state.titleFont,
                 doiDetector: FieldKeys.Item.isDoi
             )
 
@@ -887,11 +884,11 @@ final class ItemDetailActionHandler: ViewModelActionHandler, BackgroundDbProcess
             DDLogError("ItemDetailActionHandler: schema controller doesn't contain title key for item type \(viewModel.state.data.type)")
             return
         }
-        guard title != viewModel.state.data.attributedTitle else { return }
+        let htmlTitle = htmlAttributedStringConverter.convert(attributedString: title)
+        guard htmlTitle != viewModel.state.data.title else { return }
 
         update(viewModel: viewModel) { state in
-            state.data.attributedTitle = title
-            state.data.title = htmlAttributedStringConverter.convert(attributedString: title)
+            state.data.title = htmlTitle
             state.reload = .row(.title)
         }
 
diff --git a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
index 4bd621d23..77ddc2190 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailCollectionViewHandler.swift
@@ -98,6 +98,7 @@ final class ItemDetailCollectionViewHandler: NSObject {
     private var maxNonemptyTitleWidth: CGFloat = 0
     private var dataSource: UICollectionViewDiffableDataSource<SectionType, Row>!
     private let updateQueue: DispatchQueue
+    private unowned let htmlAttributedStringConverter: HtmlAttributedStringConverter
     private weak var fileDownloader: AttachmentDownloader?
     weak var delegate: ItemDetailCollectionViewHandlerDelegate?
 
@@ -111,9 +112,16 @@ final class ItemDetailCollectionViewHandler: NSObject {
 
     // MARK: - Lifecycle
 
-    init(collectionView: UICollectionView, containerWidth: CGFloat, viewModel: ViewModel<ItemDetailActionHandler>, fileDownloader: AttachmentDownloader?) {
+    init(
+        collectionView: UICollectionView,
+        containerWidth: CGFloat,
+        viewModel: ViewModel<ItemDetailActionHandler>,
+        htmlAttributedStringConverter: HtmlAttributedStringConverter,
+        fileDownloader: AttachmentDownloader?
+    ) {
         self.collectionView = collectionView
         self.viewModel = viewModel
+        self.htmlAttributedStringConverter = htmlAttributedStringConverter
         self.fileDownloader = fileDownloader
         observer = PublishSubject()
         disposeBag = DisposeBag()
@@ -180,8 +188,8 @@ final class ItemDetailCollectionViewHandler: NSObject {
 
                 switch row {
                 case .title:
-                    let title = viewModel.state.data.attributedTitle
-                    return collectionView.dequeueConfiguredReusableCell(using: titleRegistration, for: indexPath, item: (title, isEditing))
+                    let attributedTitle = htmlAttributedStringConverter.convert(text: viewModel.state.data.title, baseAttributes: [.font: viewModel.state.titleFont])
+                    return collectionView.dequeueConfiguredReusableCell(using: titleRegistration, for: indexPath, item: (attributedTitle, isEditing))
 
                 case .creator(let creator):
                     return collectionView.dequeueConfiguredReusableCell(using: fieldRegistration, for: indexPath, item: (.creator(creator), titleWidth))
diff --git a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailViewController.swift b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailViewController.swift
index 91c13aad1..5d4517c45 100644
--- a/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailViewController.swift
+++ b/Zotero/Scenes/Detail/ItemDetail/Views/ItemDetailViewController.swift
@@ -35,6 +35,7 @@ final class ItemDetailViewController: UIViewController {
             collectionView: collectionView,
             containerWidth: width,
             viewModel: viewModel,
+            htmlAttributedStringConverter: controllers.htmlAttributedStringConverter,
             fileDownloader: controllers.userControllers?.fileDownloader
         )
         collectionViewHandler.delegate = self
diff --git a/ZoteroTests/SyncActionsSpec.swift b/ZoteroTests/SyncActionsSpec.swift
index c5d1d1c8b..feee8b28a 100644
--- a/ZoteroTests/SyncActionsSpec.swift
+++ b/ZoteroTests/SyncActionsSpec.swift
@@ -96,7 +96,6 @@ final class SyncActionsSpec: QuickSpec {
                     try! dbStorage.perform(request: EditCollectionDbRequest(libraryId: .group(1234123), key: "BBBBBBBB", name: "New name", parentKey: nil), on: .main)
                     let data = ItemDetailState.Data(
                         title: "New title",
-                        attributedTitle: .init(string: "New title"),
                         type: "magazineArticle",
                         localizedType: "Magazine Article",
                         creators: [:],
@@ -107,7 +106,6 @@ final class SyncActionsSpec: QuickSpec {
                     )
                     let snapshot = ItemDetailState.Data(
                         title: "Bachelor thesis",
-                        attributedTitle: .init(string: "Bachelor thesis"),
                         type: "thesis",
                         localizedType: "Thesis",
                         creators: [:],
@@ -355,7 +353,6 @@ final class SyncActionsSpec: QuickSpec {
                     try! dbStorage.perform(request: EditCollectionDbRequest(libraryId: .group(1234123), key: "BBBBBBBB", name: "New name", parentKey: nil), on: .main)
                     let data = ItemDetailState.Data(
                         title: "New title",
-                        attributedTitle: .init(string: "New title"),
                         type: "magazineArticle",
                         localizedType: "Magazine Article",
                         creators: [:],
@@ -366,7 +363,6 @@ final class SyncActionsSpec: QuickSpec {
                     )
                     let snapshot = ItemDetailState.Data(
                         title: "Bachelor thesis",
-                        attributedTitle: .init(string: "Bachelor thesis"),
                         type: "thesis",
                         localizedType: "Thesis",
                         creators: [:],