[CIS-807] Add controllerWillChangeChannels delegate callback

[VojtaStavik]

This is mainly to avoid the crashes when UICollectionView is updated before it's fully laid out. We already try other (simpler) fixes but none of those worked properly. This is hopefully the last fix of this issue.

• I added willChange callback to ListDatabaseObsever
• I added controllerWillChangeChannels to ChatChannelListController delegate
• ChatChannelListVC now forces its UICollectionView to lay out before the update form the controller comes

---

If we see this pattern works and helps with the issue, we can extend other list controllers, too.

[codecov]

Codecov Report

Merging #1024 (042acfc) into main (2052e67) will increase coverage by 0.03%.
The diff coverage is 93.75%.

@@            Coverage Diff             @@
##             main    #1024      +/-   ##
==========================================
+ Coverage   89.95%   89.99%   +0.03%     
==========================================
  Files         213      213              
  Lines        8962     8986      +24     
==========================================
+ Hits         8062     8087      +25     
+ Misses        900      899       -1     

Flag	Coverage Δ
llc-tests	89.80% <93.75%> (+0.01%)	⬆️
llc-tests-ios12	86.62% <93.75%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
.../ChannelListController/ChannelListController.swift	93.18% <80.00%> (-1.69%)	⬇️
Sources/StreamChat/Controllers/Controller.swift	100.00% <100.00%> (ø)
...treamChat/Controllers/EntityDatabaseObserver.swift	100.00% <100.00%> (ø)
.../StreamChat/Controllers/ListDatabaseObserver.swift	93.16% <100.00%> (+0.31%)	⬆️
Sources/StreamChat/Utils/Logger/Logger.swift	82.27% <0.00%> (+2.53%)	⬆️
...ources/StreamChat/Controllers/DataController.swift	83.33% <0.00%> (+8.33%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2052e67...042acfc. Read the comment docs.

[olejnjak]

LGTM!

[olejnjak]

Just for a consideration, I usually prefer to create a closure that allows to define what will be executed in function so for this case I'd have something like this:

var controllerWillChangeChannelsBody: (ChatChannelListController) -> Void = { _ in }

func controllerWillChangeChannels(_ controller: ChatChannelListController) {
    controllerWillChangeChannelsBody(controller)
    validateQueue() // not sure if it would come here or to the default body
}

This approach is a bit more verbose but generally gives you flexibility in tests. But as I mentioned, just for future considerations.

Generally it is annoying to switch to this approach, especially with 1100 tests, but it is quite simple and fast to use it in new tests.

[VojtaStavik]

@olejnjak I see the benefits of this approach in terms of better flexibility, on the other hand, it also requires bigger setup in the test:

controller.simulateSomething()
AssertAsync.willBeTrue(mock.willChangeChannels_called)

// vs.

var wasCalled = false
mock.controllerWillChangeChannelsBody = { wasCalled = true  }

controller.simulateSomething()
AssertAsync.willBeTrue(wasCalled)

It's quite common for our current test setup to have multiple "mocks" for the same object but with different functionalities. So instead of having one almighty global mock, we have multiple local mocks. This basically replicates what you suggest but with concrete types rather than closures.

In this comparison, I don't see any clear winner there. However, thank you for pointing this pattern out. I'm sure there are cases where it works better than the "hardcoded" mock style.