GetStream / swift-activity-feed

Stream Swift iOS Activity Feed Components
https://getstream.io/
BSD 3-Clause "New" or "Revised" License
30 stars 17 forks source link

Fix comments being duplicated on refresh #8

Closed cardoso closed 4 years ago

cardoso commented 4 years ago

This bug can be easily replicated by running the sample and refreshing an activity with comments

cardoso commented 4 years ago

The bug happens because it's not reaching this path in ReactionPaginator's load function due to the Pagination being .limit(100):

if case .none = pagination {
    self.items = []
    self.next = .none
}

So I just changed it to .none (default) like it's done for the FlatFeed .

b-onc commented 4 years ago

@cardoso Your observation is right, paginator is not removing items when load is called with a pagination option. IMHO this is a bug, load should not append items, it should load from start and loadNext should append the items. So

if case .none = pagination {
  self.items = []
  self.next = .none
}

should be

if pagination != next {
  self.items = []
  self.next = .none
}

or a similar logic to make sure load and loadNext behaves differently. Since this changes a lot of things (Pagination is not Equatable), I suggest introducing reset function to PaginatorProtocol to reset items (items = []) and putting these calls before load(.limit(100))s in DetailViewController, which will make sure we won't get replicated items. This bug is probably affecting other places but we don't know yet, this is the easiest and safest solution.

cardoso commented 4 years ago

@b-onc changes made

cardoso commented 4 years ago

@b-onc nice catch. Since it's in so many places, I also introduced separate function for it.

cardoso commented 4 years ago

Should be fine now @b-onc