Page MenuHomePhorge

Fix exception for anonymous viewers on dashboard query panels with user-specific data
ClosedPublic

Authored by aklapper on Apr 19 2024, 11:52.

Details

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
Branch
T15792DashboardPanelException (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 1190
Build 1190: arc lint + arc unit

Event Timeline

src/applications/dashboard/paneltype/PhabricatorDashboardQueryPanelType.php
85

Maybe the above condition can be moved below, like:

if (!$saved) {
  if (!$viewer->isLoggedIn()) {
    return
  }
  ..
}
src/applications/dashboard/paneltype/PhabricatorDashboardQueryPanelType.php
83

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 '.

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

Oh liberate me from my previous flawed attempts, almighty arc

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

Tested before and after, I love this! Thanks!

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