Differential D25331 Diff 1090 src/applications/project/engineextension/PhabricatorProjectHovercardEngineExtension.php

Changeset View

Standalone View

src/applications/project/engineextension/PhabricatorProjectHovercardEngineExtension.php

Show All 22 Lines	public function willRenderHovercards(array $objects) {

$projects = id(new PhabricatorProjectQuery())		$projects = id(new PhabricatorProjectQuery())
->setViewer($viewer)		->setViewer($viewer)
->withPHIDs($phids)		->withPHIDs($phids)
->needImages(true)		->needImages(true)
->execute();		->execute();
$projects = mpull($projects, null, 'getPHID');		$projects = mpull($projects, null, 'getPHID');

		$custom_fields = array();
		foreach ($projects as $project) {
		$field = PhabricatorCustomField::getObjectField(
		$project,
		PhabricatorCustomField::ROLE_VIEW,
		'std:project:internal:description');
		if ($field === null) {
		// This means the field is disabled, it would always be null.
		break;
		}
		valerio.bozzolanUnsubmitted Not Done Inline Actions I thought the intention here was to skip just the current disabled one, so I'm surprised about the break instead of continue. Maybe a better comment to clarify? // The field is disabled, and also the others would always be null. valerio.bozzolan: I thought the intention here was to skip just the current disabled one, so I'm surprised about…
		aviveyAuthorUnsubmitted Done Inline Actions It's always the same field... we only have one... Would it be cleared if it was `continue`? It's more-or-less meaningless from the POV of performance (I'm not sure there's any non-test way to get into this loop with more then one object...) avivey: It's always the same field... we only have one... Would it be cleared if it was `continue`?
		valerio.bozzolanUnsubmitted Not Done Inline Actions Maybe we can use `$project = head($projects)` to avoid to work inside a loop and just operate on the first element valerio.bozzolan: Maybe we can use `$project = head($projects)` to avoid to work inside a loop and just operate…
		aviveyAuthorUnsubmitted Done Inline Actions no, the projects are different, the field instances are different, fields definitions are the same. Each project instance needs its own field instance. avivey: no, the projects are different, the field instances are different, fields definitions are the…
		valerio.bozzolanUnsubmitted Done Inline Actions So in theory we should also reset `$custom_fields` if we find a NULL `$field` but in that case the `$custom_fields` is already empty so a break is enough since it always will happen in the first cycle. OK. valerio.bozzolan: So in theory we should also reset `$custom_fields` if we find a NULL `$field` but in that case…
		valerio.bozzolanUnsubmitted Not Done Inline Actions Again maybe a comment like // The description is disabled for this Project, but also for any other. valerio.bozzolan: Again maybe a comment like ``` // The description is disabled for this Project, but also for…
		$field
		->setViewer($viewer)
		->readValueFromObject($project);
		$custom_fields[] = $field;
		}

		id(new PhabricatorCustomFieldStorageQuery())
		->addFields($custom_fields)
		->execute();

return array(		return array(
'projects' => $projects,		'projects' => $projects,
);		);
}		}

public function renderHovercard(		public function renderHovercard(
PHUIHovercardView $hovercard,		PHUIHovercardView $hovercard,
PhabricatorObjectHandle $handle,		PhabricatorObjectHandle $handle,
Show All 17 Lines