Skip to content

Commit

Permalink
remove duplicate identifiers from IGListBindingSectionController objects
Browse files Browse the repository at this point in the history
Summary:
I was building a new `IGListBindingSectionController` subclass and accidentally used two view models with the same ID. Was seeing strange results and realized we're not removing dups or asserting here.

Adding a call to `objectsWithDuplicateIdentifiersRemoved` when the view models are first requested.

Reviewed By: rnystrom

Differential Revision: D8303601

fbshipit-source-id: 42c62adc401feaec2c7dce2a83cfc6533599752b
  • Loading branch information
Adam Stern authored and facebook-github-bot committed Jun 6, 2018
1 parent 0796e92 commit a1ee4c1
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag

- `[IGListAdapterUpdater performBatchUpdatesWithCollectionViewBlock:]` and `[IGListAdapterUpdater performReloadDataWithCollectionViewBlock:]` clean state and run completion blocks if their `UICollectionView` is nil. [Brandon Darin](https://github.com/jbd1030) (tbd)

- Ensuring view models with duplicate diff identifiers are removed when view models are first requested by `IGListBindingSectionController` [Adam Stern](https://github.com/adamastern) (tbd)

3.4.0
-----

Expand Down
3 changes: 2 additions & 1 deletion Source/IGListBindingSectionController.m
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ - (void)didUpdateToObject:(id)object {
self.object = object;

if (oldObject == nil) {
self.viewModels = [[self.dataSource sectionController:self viewModelsForObject:object] copy];
NSArray *viewModels = [self.dataSource sectionController:self viewModelsForObject:object];
self.viewModels = objectsWithDuplicateIdentifiersRemoved(viewModels);
} else {
IGAssert([oldObject isEqualToDiffableObject:object],
@"Unequal objects %@ and %@ will cause IGListBindingSectionController to reload the entire section",
Expand Down
30 changes: 23 additions & 7 deletions Tests/IGListBindingSectionControllerTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,19 @@ - (void)test_whenInitialLoad_withMultipleViewModels_thatCellsMappedAndConfigured
XCTAssertEqualObjects(cell12.textField.text, @"42");
}

- (void)test_withDuplicateDiffIdentifiers_thatDuplicatesAreRemoved {
[self setupWithObjects:@[
[[IGTestDiffingObject alloc] initWithKey:@1 objects:@[@7, @7]],
]];

XCTAssertEqual([self.collectionView numberOfSections], 1);
XCTAssertEqual([self.collectionView numberOfItemsInSection:0], 1);

IGTestNumberBindableCell *cell00 = [self cellAtSection:0 item:0];

XCTAssertEqualObjects(cell00.textField.text, @"7");
}

- (void)test_whenUpdating_withAddedModels_thatCellsCorrectAndConfigured {
[self setupWithObjects:@[
[[IGTestDiffingObject alloc] initWithKey:@1 objects:@[@7, @"seven"]],
Expand Down Expand Up @@ -362,21 +375,24 @@ - (void)test_whenUpdatingManually_with2Updates_thatBothCompletionBlocksCalled {

- (void)test_whenUpdating_withMutableArrayObject_thatViewModelsDontMutate {
NSArray *objects = @[
@"foo",
@"bar"
];
@"foo",
@"bar"
];

NSMutableArray *initObjects = [NSMutableArray arrayWithArray:objects];

[self setupWithObjects:@[
[[IGTestDiffingObject alloc] initWithKey:@1 objects:initObjects]
]];

IGTestDiffingSectionController *section = [self.adapter sectionControllerForObject:self.dataSource.objects.firstObject];

NSArray *oldModels = [section.viewModels copy];

XCTAssertNotEqual(initObjects, section.viewModels);
XCTAssertEqualObjects(initObjects, section.viewModels);

[initObjects removeAllObjects];

XCTAssertEqual(oldModels, section.viewModels);
XCTAssertNotEqualObjects(initObjects, section.viewModels);
}

@end
Expand Down

0 comments on commit a1ee4c1

Please sign in to comment.