Page MenuHomePhabricator

Fix an issue with mention transactions in Calendar
ClosedPublic

Authored by epriestley on Jun 8 2015, 5:42 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 21, 7:22 AM
Unknown Object (File)
Wed, Dec 18, 12:21 AM
Unknown Object (File)
Sat, Dec 14, 4:05 PM
Unknown Object (File)
Thu, Dec 12, 2:49 AM
Unknown Object (File)
Sat, Dec 7, 3:55 AM
Unknown Object (File)
Thu, Dec 5, 6:19 PM
Unknown Object (File)
Nov 20 2024, 8:36 AM
Unknown Object (File)
Nov 15 2024, 4:49 PM

Details

Summary

Fixes an issue where mentioning an event name (like E999) in a revision summary (and probably elsewhere) would produce an error like this:

2015/06/08 17:30:22 [error] 27702#0: *307450 FastCGI sent in stderr: "PHP message: [2015-06-08 17:30:22] EXCEPTION: (Exception) Transaction ("PHID-XACT-CEVT-zglgzy36aote5ja", of type "core:edge") requires a handle ("PHID-DREV-4m6vorimvg4bm3ltskca") that it did not load. at [<phabricator>/src/applications/transactions/storage/PhabricatorApplicationTransaction.php:285]
PHP message: arcanist(head=master, ref.master=8c589f1f759f), phabricator(head=master, ref.master=b4de79741ceb), phutil(head=master, ref.master=4a0e1b47a584)
PHP message:   #0 <#2> PhabricatorApplicationTransaction::getHandle(string) called at [<phabricator>/src/applications/transactions/storage/PhabricatorApplicationTransaction.php:474]
PHP message:   #1 <#2> PhabricatorApplicationTransaction::shouldHide() called at [<phabricator>/src/applications/calendar/storage/PhabricatorCalendarEventTransaction.php:82]
PHP message:   #2 <#2> PhabricatorCalendarEventTransaction::shouldHide() called at [<phutil>/src/utils/utils.php:428]
PHP message:   #3 <#2> mfilter(array, string, boolean) called at [<phabricator>/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php:350]
PHP message:   #4 <#2> PhabricatorCalendarEventEditor::shouldSendMail(PhabricatorCalendarEvent, array) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:990]
PHP message:   #5 <#2> PhabricatorApplicationTransactionEditor::applyTransactions(PhabricatorCalendarEvent, array) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:2937]
PHP message:   #6 <#2> PhabricatorApplicationTransactionEditor::applyInverseEdgeTransactions(DifferentialRevision, DifferentialTransaction, integer) called at [<phabricator>/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:602]
...

This is similar to rP98aae51c / rP0fc0af64, but for Calendar. (Likely, I copy/pasted the Editor from Maniphest a while ago.)

We don't need to do this filtering here because we do it later before sending mail. Additionally, because some transactions may hide or show depending on the viewer, it's strictly incorrect to do it here.

Test Plan
  • Created a revision which mentioned a bunch of events.
  • Grepped for other implementations of shouldSendMail() and verified that none try to perform similar filtering.

Diff Detail

Repository
rP Phabricator
Branch
event1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 6626
Build 6648: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley retitled this revision from to Fix an issue with mention transactions in Calendar.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
epriestley added a subscriber: lpriestley.
btrahan edited edge metadata.

That mfilter() pattern is (was?) in many an editor me thinks...

This revision is now accepted and ready to land.Jun 8 2015, 5:46 PM

It looks like we're OK at HEAD:

$ git grep -i mfilter
src/applications/dashboard/customfield/PhabricatorDashboardPanelSearchApplicationCustomField.php:    $engines = mfilter($engines, 'canUseInPanelContext');
src/applications/dashboard/customfield/PhabricatorDashboardPanelSearchQueryCustomField.php:    $engines = mfilter($engines, 'canUseInPanelContext');
src/applications/pholio/query/PholioMockQuery.php:      $active_images = mfilter($mock_images, 'getIsObsolete', true);
src/applications/releeph/editor/ReleephRequestTransactionalEditor.php:      if (!mfilter($xactions, 'isBoringPickStatus', true /* negate */)) {
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php:    $xactions = mfilter($xactions, 'shouldHideForFeed', true);
This revision was automatically updated to reflect the committed changes.