Improve item detail fields (#1051)

* Fix item detail editing for attachment type

* Update Item date modified with each saved change

* Improve code

* Add isEditable property to ItemDetailState.Field struct

* Improve code

* Improve code

* Use OrderedDictionary for ItemDetailState creators

* Improve code

* 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

* Make ItemDetailState.Data isAttachment a computed property

* Remove unused code

* Remove ItemDetailState.Data attributedTitle property

* Improve BackgroundTimer code

* Cache Item Detail State attributed title upon change

* Improve comment

* Improve item fields edit delay timer logic

* Simplify BackgroundTimer

* Simplify ItemDetailActionHandler logic for endEditing ItemAction

* Improve code

* End pending item creations on app launch

Fix app delegate migration

Zotero.xcodeproj/project.pbxproj

@@ -824,7 +824,7 @@
 		B3830CF725545EE400910FE0 /* TagPickerCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = B3830CF525545EE400910FE0 /* TagPickerCell.xib */; };
 		B385B70E25C03E7E0073CA6F /* PDFExportState.swift in Sources */ = {isa = PBXBuildFile; fileRef = B385B70D25C03E7E0073CA6F /* PDFExportState.swift */; };
 		B386328626C5499900183062 /* TranslatorsAndStylesController.swift in Sources */ = {isa = PBXBuildFile; fileRef = B36A988C2428E059005D5790 /* TranslatorsAndStylesController.swift */; };
-		B3863FC82AD819AB005082F0 /* EndItemDetailEditingDbRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3863FC72AD819AB005082F0 /* EndItemDetailEditingDbRequest.swift */; };
+		B3863FC82AD819AB005082F0 /* EndItemCreationDbRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3863FC72AD819AB005082F0 /* EndItemCreationDbRequest.swift */; };
 		B3863FCA2AD830DE005082F0 /* DeleteCreatorItemDetailDbRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3863FC92AD830DE005082F0 /* DeleteCreatorItemDetailDbRequest.swift */; };
 		B3863FCC2AD830F0005082F0 /* EditCreatorItemDetailDbRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3863FCB2AD830F0005082F0 /* EditCreatorItemDetailDbRequest.swift */; };
 		B3863FCE2AD830FE005082F0 /* ReorderCreatorsItemDetailDbRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3863FCD2AD830FE005082F0 /* ReorderCreatorsItemDetailDbRequest.swift */; };
@@ -1858,7 +1858,7 @@
 		B3830CF425545EE400910FE0 /* TagPickerCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TagPickerCell.swift; sourceTree = "<group>"; };
 		B3830CF525545EE400910FE0 /* TagPickerCell.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = TagPickerCell.xib; sourceTree = "<group>"; };
 		B385B70D25C03E7E0073CA6F /* PDFExportState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PDFExportState.swift; sourceTree = "<group>"; };
-		B3863FC72AD819AB005082F0 /* EndItemDetailEditingDbRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EndItemDetailEditingDbRequest.swift; sourceTree = "<group>"; };
+		B3863FC72AD819AB005082F0 /* EndItemCreationDbRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EndItemCreationDbRequest.swift; sourceTree = "<group>"; };
 		B3863FC92AD830DE005082F0 /* DeleteCreatorItemDetailDbRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DeleteCreatorItemDetailDbRequest.swift; sourceTree = "<group>"; };
 		B3863FCB2AD830F0005082F0 /* EditCreatorItemDetailDbRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EditCreatorItemDetailDbRequest.swift; sourceTree = "<group>"; };
 		B3863FCD2AD830FE005082F0 /* ReorderCreatorsItemDetailDbRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReorderCreatorsItemDetailDbRequest.swift; sourceTree = "<group>"; };
@@ -2430,7 +2430,7 @@
 				B3BF7EE828A51EDA00A5A659 /* EditTagsForItemDbRequest.swift */,
 				B3863FCF2AD83698005082F0 /* EditTypeItemDetailDbRequest.swift */,
 				B3B613F6260B844B00B92017 /* EmptyTrashDbRequest.swift */,
-				B3863FC72AD819AB005082F0 /* EndItemDetailEditingDbRequest.swift */,
+				B3863FC72AD819AB005082F0 /* EndItemCreationDbRequest.swift */,
 				B37DD575272AAF500038537D /* FilterAttachmentsDbRequest.swift */,
 				B302DC53293A22A6003497D9 /* FixChildItemsWithCollectionsDbRequest.swift */,
 				B3D4159D2948B3DA004ABB3E /* FixNotesWithEmptyTitlesDbRequest.swift */,
@@ -5303,7 +5303,7 @@
 				B398A915270C6A4300968EE8 /* WebDavController.swift in Sources */,
 				B3A17D1927FC33B800322CAD /* LowPowerModeController.swift in Sources */,
 				B31CC57B286468780055C114 /* ManualLookupViewController.swift in Sources */,
-				B3863FC82AD819AB005082F0 /* EndItemDetailEditingDbRequest.swift in Sources */,
+				B3863FC82AD819AB005082F0 /* EndItemCreationDbRequest.swift in Sources */,
 				B3DDDAAC24CAD6810014DF99 /* InsetLabel.swift in Sources */,
 				B3329A552B738C4E00F17636 /* CitationAuthorCell.swift in Sources */,
 				B3E8FE8927143BDD00F51458 /* SyncSettingsView.swift in Sources */,

Zotero/AppDelegate.swift

@@ -145,6 +145,16 @@ final class AppDelegate: UIResponder {
         }
     }
 
+    private func endPendingItemCreations(queue: DispatchQueue) {
+        guard let dbStorage = controllers.userControllers?.dbStorage else { return }
+        try? dbStorage.perform(request: EndPendingItemCreationsDbRequest(), on: queue)
+        do {
+            try dbStorage.perform(request: EndPendingItemCreationsDbRequest(), on: queue)
+        } catch let error {
+            DDLogError("AppDelegate: can't ending creation for pending items - \(error)")
+        }
+    }
+
     // MARK: - Setups
 
     private func setupLogs() {
@@ -234,9 +244,12 @@ extension AppDelegate: UIApplicationDelegate {
         self.migrateItemsSortType()
 
         let queue = DispatchQueue(label: "org.zotero.AppDelegateMigration", qos: .userInitiated)
-        queue.async {
-            self.removeFinishedUploadFiles(queue: queue)
-            self.updateCreatorSummaryFormat(queue: queue)
+        DispatchQueue.main.async {
+            queue.async {
+                self.removeFinishedUploadFiles(queue: queue)
+                self.updateCreatorSummaryFormat(queue: queue)
+                self.endPendingItemCreations(queue: queue)
+            }
         }
 
         return true

Zotero/Controllers/BackgroundTimer.swift

@@ -11,51 +11,65 @@
 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) {
+    init(timeInterval: DispatchTimeInterval, queue: DispatchQueue = .main) {
         self.timeInterval = timeInterval
         self.queue = queue
     }
 
     deinit {
-        self.timer.setEventHandler {}
-        self.timer.cancel()
+        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
          */
-        self.resume()
-        self.eventHandler = nil
+        if state == .suspended {
+            state = .resumed
+            timer.resume()
+        }
+        eventHandler = nil
     }
 
     func resume() {
-        guard self.state != .resumed else { return }
-        self.state = .resumed
-        self.timer.resume()
+        guard state != .resumed else { return }
+        if let startTime, startTime + timeInterval <= .now() {
+            eventHandler?()
+        } else {
+            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()
+    }
+
+    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
     }
 }

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

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)
     }
 }

Zotero/Controllers/Database/Requests/EditItemFromDetailDbRequest.swift

@@ -8,7 +8,6 @@
 
 import Foundation
 
-import CocoaLumberjackSwift
 import RealmSwift
 
 struct EditItemFromDetailDbRequest: DbRequest {
@@ -48,9 +47,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

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)
         }

Zotero/Controllers/Database/Requests/EndItemCreationDbRequest.swift

@@ -0,0 +1,40 @@
+//
+//  EndItemCreationDbRequest.swift
+//  Zotero
+//
+//  Created by Michal Rentka on 12.10.2023.
+//  Copyright © 2023 Corporation for Digital Scholarship. All rights reserved.
+//
+
+import Foundation
+
+import CocoaLumberjackSwift
+import RealmSwift
+
+struct EndItemCreationDbRequest: DbRequest {
+    var needsWrite: Bool { return true }
+
+    let libraryId: LibraryIdentifier
+    let itemKey: String
+
+    func process(in database: Realm) throws {
+        guard let item = database.objects(RItem.self).uniqueObject(key: itemKey, libraryId: libraryId) else { return }
+        item.changesSyncPaused = false
+        item.changeType = .user
+    }
+}
+
+struct EndPendingItemCreationsDbRequest: DbRequest {
+    var needsWrite: Bool { return true }
+
+    func process(in database: Realm) throws {
+        let pendingItems = database.objects(RItem.self).filter("changesSyncPaused == true")
+        guard !pendingItems.isEmpty else { return }
+        DDLogInfo("EndPendingItemCreationsDbRequest: ending creation for \(pendingItems.count) pending items")
+        for item in pendingItems {
+            guard !item.isInvalidated else { continue }
+            item.changesSyncPaused = false
+            item.changeType = .user
+        }
+    }
+}