Page MenuHomePhabricator
Create Task
Maniphest T12787

Creating a subtask 'reopens' the new task
Closed, ResolvedPublic
Actions

Description

Creating a new subtask creates a feed story for the parent task about the subtask being reopened, although it's a brand new task.

  1. Go to an existing task.
  2. Click Edit Related Tasks → Create Subtask.
  3. Save new subtask.
  4. Look at feed.

image.png (196×448 px, 29 KB)

phabricator d3b7a0f37c975397455d633254e9b4a1246023f9 (Sat, May 27) (branched from 9d37ad3022bfff8a2900b9d728e6abbb6a1e27f0 on origin)
arcanist 21fe07925b07cab0f73ec824fe8ce02f53b24fe0 (Sat, May 20) (branched from 129d51fa0936c9bae48fadf3a3f39e26d69d24da on origin)
phutil d02cc05931b02c684d4c729510090591ca45f951 (Apr 29 2017) (branched from a900d7b63e954e221efe140f0f33d3d701524aae on origin)

Revisions and Commits

rP Phabricator
D19179rPe57dbcda336a Hide "abraham landed Dxyz irresponsibly" stories from feed
D18059rPb69174a4c8f7 Remove non-operational `shouldHideFromFeed()` from…
D18058rP995c1503e7ef Hide "X created Y, a subtask of P." feed stories again

Related Objects
Search...

StatusAssignedTask
 ResolvedepriestleyT12787 Creating a subtask 'reopens' the new task
 InvalidNoneT12788 Test for T12787

Event Timeline

ivo created this task.Jun 1 2017, 12:54 PM
ivo added a project: Feed.
epriestley claimed this task.Jun 1 2017, 12:55 PM
epriestley created subtask T12788: Test for T12787.
epriestley added a subscriber: epriestley.

Thanks, this reproduces here for me.

Screen Shot 2017-06-01 at 5.55.51 AM.png (248×787 px, 37 KB)

epriestley closed subtask T12788: Test for T12787 as Invalid.Jun 1 2017, 12:56 PM
epriestley added a revision: D18058: Hide "X created Y, a subtask of P." feed stories again.Jun 1 2017, 2:59 PM
epriestley closed this task as Resolved by committing rP995c1503e7ef: Hide "X created Y, a subtask of P." feed stories again.Jun 1 2017, 3:08 PM
epriestley added a commit: rP995c1503e7ef: Hide "X created Y, a subtask of P." feed stories again.
epriestley added a comment.Jun 1 2017, 3:14 PM

This transaction uses shouldHideForFeed(), which isn't currently supported by Modular Transactions. It got swapped over in D17844 or some related change and we didn't catch that the logic wasn't firing anymore since this is a bit of an obscure case.

D18058 basically reverts the change with a "TODO: fix this properly".

The shouldHideForFeed() mechanism is very rarely used (only this, "points", and a couple of Phriction transactions use it). In fact, I think the implementation in "Points" no longer works, for reasons similar to this. (I'll add a note on D18018.)

It's also a publish-time mechanism, not a display-time mechanism. That is, it really means something more like shouldPublishAFeedStoryAboutThis().

We may want to move away from publish-time hooks in the future, as we pursue tasks like T10363. I'm not sure if that mechanism will be totally independent of feed or not, but hooks should be able to get a fairly raw event stream if they want, including events which we hide from feed like this one. This might motivate a display-time hook instead.

epriestley mentioned this in D18018: Restore missing feed rendering for Maniphest points transactions.Jun 1 2017, 3:17 PM
epriestley added a revision: D18059: Remove non-operational `shouldHideFromFeed()` from ManiphestTaskPointsTransaction.Jun 1 2017, 3:20 PM
epriestley mentioned this in rPf2fcafb40dde: Help PROFESSIONAL SOFTWARE ENGINEERS copy text to their clipboard.Jun 1 2017, 4:12 PM
epriestley added a commit: rPb69174a4c8f7: Remove non-operational `shouldHideFromFeed()` from….Jun 1 2017, 4:14 PM
epriestley added a revision: D19179: Hide "abraham landed Dxyz irresponsibly" stories from feed.Mar 7 2018, 1:41 AM
epriestley added a commit: rPe57dbcda336a: Hide "abraham landed Dxyz irresponsibly" stories from feed.Mar 7 2018, 1:48 AM
Phabricator · Built by Phacility