modo-studio / SugarRecord

CoreData/Realm sweet wrapper written in Swift
MIT License
2.11k stars 223 forks source link

CoreDataObservable always returning index=0 on changes #335

Closed ctotheameron closed 7 years ago

ctotheameron commented 7 years ago

I recently upgraded from an older version of SugarRecord since migrating to Swift 3. I'm noticing some seemingly incorrect behavior when using CoreDataObservables.

Whenever a data change is reported, the index for the updates are always coming back as 0.

I took a look at the CoreDataObservable class, and think the problem is the way that indexPaths are being transformed into the index reported to the callback from the controller(_:didChange:at:for:newIndexPath:) delegate method

(https://github.com/carambalabs/SugarRecord/blob/master/SugarRecord/Source/CoreData/Entities/CoreDataObservable.swift#L54).

self.batchChanges.append(.delete(indexPath![0], anObject as! T))

After fiddling around with this, I was curious as the the subscripting of indexPath. I assume you're trying to pull out the "row" or "item" fields. I tried changing the subscript to be 1, and the behavior was as I originally expected (indexes matched the ones provided from the .initial).

Adding some print statements

// MARK: - NSFetchedResultsControllerDelegate

public func controller(_ controller: NSFetchedResultsController<NSFetchRequestResult>, didChange anObject: Any, at indexPath: IndexPath?, for type: NSFetchedResultsChangeType, newIndexPath: IndexPath?) {
    print("\(indexPath?.row)")      // 3
    print("\(indexPath?.section)")  // 0
    print("\(indexPath?.item)")     // 3
    print("\(indexPath?[0])")       // 0
    print("\(indexPath?[1])")       // 3

    switch type {
    case .delete:
        self.batchChanges.append(.delete(indexPath![0], anObject as! T))
    case .insert:
        self.batchChanges.append(.insert(newIndexPath![0], anObject as! T))
    case .update:
        self.batchChanges.append(.update(indexPath![0], anObject as! T))
    default: break
    }
}
ctotheameron commented 7 years ago

Also as a sidenote, any plans to support the NSFetchedResultsChangeType.move? As it currently stands, I'm forced to always sort by a non-changing field like id because this library doesn't correctly handle changes to fields that were used to sort.

Thanks in advance! This is a great library.

pepicrft commented 7 years ago

Thank you very much @ctotheameron, great catch. I didn't have any plans to support move but contributions are welcome. If you need help with it, let me know. Glad that you find the library useful