Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15792: Dashboard query panel requiring a Current Viewer throws exception when not logged in

Commits

rPb32b84b64515: Fix exception for anonymous viewers on dashboard query panels with user…

Summary

When a viewer is not logged in and opens a public dashboard which embeds a query panel whose data requires a Current Viewer context (e.g. assigned or authored tasks, or joined projects), do not throw an exception but show an explanatory placeholder sentence (similar to already existing 'No tasks found.' or 'No documents found.' strings).

EXCEPTION: (Exception) Query "assigned" is unknown to application search engine "ManiphestTaskSearchEngine"! at [<phorge>/src/applications/dashboard/paneltype/PhabricatorDashboardQueryPanelType.php:80]
EXCEPTION: (Exception) Query "joined" is unknown to application search engine "PhabricatorProjectSearchEngine"! at [<phorge>/src/applications/dashboard/paneltype/PhabricatorDashboardQueryPanelType.php:80]

Closes T15792

Test Plan

While logged in, set up a dashboard query panel with "Search For: Maniphest Tasks" and "Query: Assigned" and add it to a public Dashboard. While logged out, access the Dashboard and see explanation message instead of Exception: Query "assigned" is unknown to application search engine "ManiphestTaskSearchEngine"! on the dashboard.

Diff Detail

Repository

rP Phorge

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

aklapper created this revision.Fri, Apr 19, 11:52

Owners added a reviewer: O1: Blessed Committers.Fri, Apr 19, 11:52

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptFri, Apr 19, 11:52

aklapper requested review of this revision.Fri, Apr 19, 11:52

Harbormaster completed remote builds in B1190: Diff 1887.Fri, Apr 19, 11:52

valerio.bozzolan added inline comments.Thu, Apr 25, 15:16

src/applications/dashboard/paneltype/PhabricatorDashboardQueryPanelType.php
85	Maybe the above condition can be moved below, like: if (!$saved) { if (!$viewer->isLoggedIn()) { return } .. }

valerio.bozzolan added inline comments.Thu, Apr 25, 15:19

src/applications/dashboard/paneltype/PhabricatorDashboardQueryPanelType.php

Minor suggestion. Instead of "see" → "access". Instead of "this data" → "panel".

$ grep -R 'You must log in to' .
./applications/diffusion/controller/DiffusionServeController.php:            pht('You must log in to access repositories.'));
./applications/diffusion/controller/DiffusionServeController.php:            pht('You must log in to access this repository.'));
./applications/diffusion/controller/DiffusionServeController.php:              pht('You must log in to push to this repository.'));
./applications/auth/controller/PhabricatorAuthStartController.php:      ->appendParagraph(pht('You must log in to take this action.'))
./applications/legalpad/controller/LegalpadDocumentSignController.php:            'This document requires a corporate signatory. You must log in to '.

aklapper marked 2 inline comments as done.Thu, Apr 25, 17:54

Faster logic, higher performance, and better looking strings! Now all included in this brand new version!

Harbormaster completed remote builds in B1194: Diff 1892.Thu, Apr 25, 17:55

Oh liberate me from my previous flawed attempts, almighty arc

Harbormaster completed remote builds in B1195: Diff 1893.Thu, Apr 25, 17:58

Run arc liberate from my clean working copy that has not ManiphestDefaultPriorityEditCapability (?)
Minor fix indentation of a comment in PhabricatorDashboardQueryPanelType.php:81

Harbormaster completed remote builds in B1196: Diff 1894.Fri, Apr 26, 09:27

Tested before and after, I love this! Thanks!

This revision is now accepted and ready to land.Fri, Apr 26, 09:41

Closed by commit rPb32b84b64515: Fix exception for anonymous viewers on dashboard query panels with user…. · Explain WhyFri, Apr 26, 10:54

This revision was automatically updated to reflect the committed changes.

aklapper added a commit: rPb32b84b64515: Fix exception for anonymous viewers on dashboard query panels with user….

avivey mentioned this in D25603: arc liberate.Sun, Apr 28, 15:48

avivey mentioned this in rPf75c7ce7664b: arc liberate.Sun, Apr 28, 16:54