[trello.com/c/pwKrXiiC] Update the logic of reading messages. #703
Conversation
vovameister
requested review from
art-divin,
ChrisBenua,
Lainaaa and
PingPongEzZ
| @@ -39,4 +40,7 @@ extension MessageTransaction { | |||
| reactionsData = data | |||
| } | |||
| } | |||
| func addToRichMessageTransactions(_ transaction: RichMessageTransaction) { | |||
| self.mutableSetValue(forKey: "richMessageTransactions").add(transaction) | |||
There was a problem hiding this comment.
Oh, I see why you had to do like this. If you mutate richMessageTransactions, nothing will be saved to database.
The same thing would happen if you've used raw NSMutableSet
Comment is for other reviewers)
| @@ -215,13 +224,22 @@ final class ChatViewController: MessagesViewController { | |||
|
|
|||
| override func scrollViewDidScroll(_ scrollView: UIScrollView) { | |||
| super.scrollViewDidScroll(scrollView) | |||
| let visibleIndexPaths = messagesCollectionView.indexPathsForVisibleItems | |||
There was a problem hiding this comment.
same code is repeated twice. Please extract it to function
| @@ -70,6 +70,11 @@ final class ChatListViewController: KeyboardObservingViewController { | |||
| var searchController: UISearchController? | |||
|
|
|||
| private var transactionsRequiringBalanceUpdate: [String] = [] | |||
| private lazy var chatsManuallyMarkedAsUnread: Set<Int> = Set() { | |||
There was a problem hiding this comment.
do we really need lazy var here?
There was a problem hiding this comment.
yes because NSFetchedResultsController(setBadgeValue(unreadController?.fetchedObjects?.count)) will be available later
There was a problem hiding this comment.
I don't get it. You set lazy var to empty set, for what?
There was a problem hiding this comment.
oh sorry I might use another value before, and it crushed the app on init. I will remove lazy
| @@ -20,6 +20,8 @@ final class InMemoryCoreDataStack: CoreDataStack { | |||
|
|
|||
| let description = NSPersistentStoreDescription() | |||
| description.type = NSInMemoryStoreType | |||
| description.shouldMigrateStoreAutomatically = true | |||
There was a problem hiding this comment.
Why? I don't think that NSInMemoryStoreType requires migrations at all
There was a problem hiding this comment.
@vovameister take a look, please!
| @@ -541,6 +542,21 @@ private extension ChatMessageFactory { | |||
| ] | |||
| )) | |||
| } | |||
| func checkTransactionForUnreadReaction(transaction: ChatTransaction) -> Bool { | |||
| if let messageTransaction = transaction as? MessageTransaction, | |||
There was a problem hiding this comment.
Maybe as part of this тask we can take richMessageTransactions out to ChatTransaction and avoid code duplication?
the following changes have been added: