Reduce the amount of weird "static" and "cache" behavior in Pholio query classes
ClosedPublic
Actions

Authored by epriestley on Dec 20 2018, 8:18 PM.

Details

Reviewers

amckinley

Maniphest Tasks

T11351: Move Pholio to EditEngine and modern APIs

Commits

rPb1e7e3a10e8c: Reduce the amount of weird "static" and "cache" behavior in Pholio query classes

Summary

Depends on D19922. Ref T11351. These query classes have some slightly weird behavior, including public static function loadImages(...). Convert all this stuff into more standard query patterns.

Test Plan

Grepped for callsites, browsed around in Pholio.

Diff Detail

Repository

rP Phabricator

Branch

pholio11

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 21401
Build 29146: Run Core Tests
Build 29145: arc lint + arc unit

Event Timeline

epriestley created this revision.Dec 20 2018, 8:18 PM

Harbormaster completed remote builds in B21401: Diff 47548.Dec 20 2018, 8:20 PM

epriestley requested review of this revision.Dec 20 2018, 8:20 PM

epriestley added inline comments.Dec 20 2018, 8:21 PM

src/applications/pholio/query/PholioImageQuery.php
23	`withMocks(...)` now has the effect of working like a cache and implying `withMockPHIDs(...)`, similar to `withRepositories(...)` elsewhere.
78	There's a behavioral change here: we'll now load Images if they have a `null` value for `mockPHID` (i.e., an image not yet attached to a mock). Previously, we would always reject them.
84	(Not actually that scary: it does some extra fetches, but hits the cache the second time through.)
src/applications/pholio/view/PholioMockImagesView.php
113	This fixes a sort-of-bug. The `mpull()` intends to rekey the array (the lines below care about the array keys) but changes the values instead. The code worked before because the array happened to always be keyed by `ID` anyway.