Add an action to adding Panels from ApplicationSearch
ClosedPublic
Actions

Authored by chad on Mar 20 2017, 7:39 PM.

Details

Reviewers

epriestley

Maniphest Tasks

T5307: UX for taking actions with search results

Commits

rP2921bad1ff08: Add an action to adding Panels from ApplicationSearch

Summary

Ref T5307. This adds an additional action to Use Results for creating a panel from the query.

Test Plan

Navigate to Maniphest, select dropdown for Use Results. Try any of the following:

Try to set a panel without a name (fail)
Muck up query or engine (fail)
Set a fake Dashboard ID (fail)

Give panel a name and select a dashboard I have edit permissions to, get taken to dashboard.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

chad created this revision.Mar 20 2017, 7:39 PM

Herald added a subscriber: Korvin. · View Herald TranscriptMar 20 2017, 7:39 PM

does not work

Harbormaster completed remote builds in B16040: Diff 42125.Mar 20 2017, 7:40 PM

chad added inline comments.Mar 20 2017, 7:40 PM

src/applications/dashboard/controller/PhabricatorDashboardQueryPanelInstallController.php
63–70	I don't know how to save these custom fields since the user isn't setting them and I want them hidden.

For the moment, you can just hard-code transactions of type PhabricatorTransactions::TYPE_CUSTOMFIELD:

$xactions[] = id(new PhabricatorDashboardPanelTransaction())
  ->setTransactionType(PhabricatorTransactions::TYPE_CUSTOMFIELD)
  ->setMetadataValue('customfield:key', $field_key)
  ->setNewValue($field_value);

I believe the $field_key is std:dashboard:core:class for the SearchEngine class, and std:dashboard:core:key for the query key.

chad planned changes to this revision.Mar 20 2017, 7:57 PM

validate errors, set fields correctly

Harbormaster completed remote builds in B16042: Diff 42127.Mar 20 2017, 8:54 PM

chad edited the test plan for this revision. (Show Details)Mar 20 2017, 8:57 PM

Closed by commit rP2921bad1ff08: Add an action to adding Panels from ApplicationSearch (authored by chad, committed by chad). · Explain WhyMar 20 2017, 9:15 PM

This revision was automatically updated to reflect the committed changes.

ROFL

I meant to type arc diff I swear

tarnation

Ideally, use $e_name = true; and $e_name = pht('Required'); to provide grey "Required" text before submission and red "Required" text after submission if the name is missing.
Extra-ideally, maybe prefill the name with the query name? Not sure if this gets reasonable results or is easy to do. I can point you in the right direction if you want to try.
Maybe put dashboard IDs in the <select />, or use the existing PhabricatorDashboardDatasource + typeahead? I get like 20 copies of "New Simple Dashboard" on my local install.

src/applications/dashboard/controller/PhabricatorDashboardQueryPanelInstallController.php
30	I think this check is too strict: if I run an ad hoc query (just type some random stuff into fields) I should still be able to turn it into a dashboard panel, but it won't be a "named" query. I think the right check is this (basically: we're OK if the query key is a builtin query, or the query key is a saved query). It doesn't have to be a named query: if ($engine->isBuiltinQuery($query_key)) { $ok = true; } else { $saved_query = id(new PhabricatorSavedQueryQuery()) ->setViewer($viewer) ->withEngineClassNames(array($engine_class)) ->withQueryKeys(array($query_key)) ->executeOne(); if ($saved_query) { $ok = true; } } if (!$ok) { // 404 }
131	I think you can use `$engine->getQueryResultsPageURI($query_key)` for this, should get you back to the results page.
src/applications/search/controller/PhabricatorApplicationSearchController.php
558–560	nothin' says "this is a menu" like `fa-bars`