Fix an issue where inlines could be duplicated in the client list
ClosedPublic
Actions

Authored by epriestley on Mar 18 2021, 6:33 PM.

Details

Reviewers: None
Maniphest Tasks: T13559: Include "Moved/Copied" gutter in 1-up diffs, and add aural cues, coverage, and fix inline counting
Commits: rP87c6c270b435: Fix an issue where inlines could be duplicated in the client list

Summary

Ref T13559. D21261 added caching here, but the logic in rebuilding inlines wasn't quite correct, and could lead to us double-appending.

Instead, when rebuilding, unconditionally discard the old list.

Test Plan

Added inline comments to a file in Differential.
Marked some done.
Scrolled so the inline comment header was visible, saw "X / Y Comments" button in header.
Clicked "Show 20 more lines" on the changeset with inlines (or toggle "View Unified" / "View Side-by-Side", or other interactions likely work too).
- Before: saw "X / Y" change improperly (because inlines in that file were double-counted).
- After: saw stable count.
Grepped for "differential-inline-comment-refresh", got no hits, concluded this event has no listeners.

Repository

rP Phabricator

Branch

inline1

Lint

Lint Passed

Unit

No Test Coverage

Build Status

	Time	Test
	205 ms	PhabricatorCelerityTestCase::testCelerityMaps Assertion failed, expected 'true' (at PhabricatorCelerityTestCase.php:32): When this test fails, it means the Celerity resource map is out of date. Run `bin/celerity map` to rebuild it. ACTUAL VALUE
	0 ms	AlmanacNamesTestCase::testServiceOrDeviceNames 30 assertions passed.
	0 ms	AlmanacServiceTypeTestCase::testGetAllServiceTypes 1 assertion passed.
	0 ms	AphrontHTTPHeaderParserTestCase::testHeaderParser 18 assertions passed.
	0 ms	AphrontHTTPSinkTestCase::testHTTPHeaderNames 2 assertions passed.
		View Full Test Results (1 Failed · 424 Passed · 2 Skipped)