Page MenuHomePhabricator

Track mentions on an individual field-by-field basis
Open, WishlistPublic

Description

See PHI1496. See PHI1602.

Currently, if you mention an object on another object (like mentioning T123 in a comment T456), we permanently write a timeline story on the mentioned object ("alice mentioned this on Txxx.") and create a permanent "mentioned" relationship. This relationship is append-only, so if you later edit your comment to remove the mention in the comment text, that does not unpublish the story or revoke the relationship.

In both PHI1496 and PHI1602, users mentioned some objects by mistake and wished to undo these relationships later.

There are two general concerns with this: a product concern that undoing things is generally confusing, and a technical concern that it's currently hard/expensive/impossible to undo them anyway.

On the product concern:

"Mentioned" currently means "was ever mentioned", not "is currently mentioned". [P]art of it is for consistency and so that mentions don't vanish after users make edits. For example, if you're looking at some other object and see "alice mentioned this on Txxx." in the timeline for that object, it's potentially either confusing or inconsistent if a later edit to the mentioning task can remove the mention ("confusing" if the timeline story vanishes and you aren't sure why; "inconsistent" if the timeline story sticks around and still shows the mention, but the mention list updates and removes the mention). There are no other cases today where a timeline story can vanish (outside of explicit object destruction with bin/remove destroy).

On the technical concern:

[T]he technical change to make this mean "is currently mentioned" isn't straightforward, either. Consider and object with a large number of comments (say, 5,000 -- maybe this is a Conpherence chatroom, or a task with a lot of bot activity). If I add another comment which mentions T123, then edit my comment to remove the mention of T123, we can't just remove the mention relationship: it's possible one or more other comments also mention T123. We'd have to load all 5,000 comments and reprocess them to see if any of them mention T123, or store the relationship in a reference-counted way, or change the relationship to be a field-level relationship between particular blocks of text and remote objects. All of these approaches involve additional complexity -- for example, blocks of text do not have PHIDs, and a given object may have multiple blocks of text which can trigger mentions (Revisions have both "Summary" and "Test Plan").

PHI1602 raises the difficulty further: the mentions were in a custom field.

Setting aside the product concern, we can address the technical concern by making each mention store the field where it appears (for example: the summary, or comment X, or custom field Q, and tracking mentions on a field-by-field basis. When a particular field is edited, we can safely remove mentions attributed to that field which are no longer present.

I suspect we shouldn't actually undo the mention or story, but the UI could change to look more like this:

Mentioned in Summary:

  • T123
  • T456

Mentioned in Comments:

  • T789

(9 other tasks were previously mentioned on this object. Show Outdated Mentions)

Mentions are currently stored as edges, and we can't reasonably fit the additional data required by field-level tracking into the edge schema. T13419 and/or T13056 may make some aspects of this easier. I suspect the path forward is just a custom table, but perhaps that custom table can be the same as the table probably implied by T3577.

Event Timeline

epriestley triaged this task as Wishlist priority.Jan 14 2020, 6:32 PM
epriestley created this task.

In PHI1602, I proposed a script to "reset" mentions on an object. The use case this serves is:

  • You have a task with a large number of accidental, obsolete, or mistaken mentions.
  • You want to reset the "Mentioned Here" list to show only objects mentioned in current field and comment text on the task.
  • You don't care about the feed/timeline stories.

This script has significant limitations:

  • It can only reset mentions on tasks (not other types of objects).
  • It will remove the mention edges, but will not unpublish feed stories on mentioned objects.
  • It is flimsy, and likely to break in future versions of Phabricator.
  • I've tested it only casually.

Invoke it as:

$ phabricator/ php -f reset-mentions.php T123

Run it at your own risk.

reset-mentions.php
<?php

require_once 'scripts/init/init-script.php';

$args = id(new PhutilArgumentParser($argv))
  ->parseStandardArguments()
  ->parse(
    array(
      array(
        'name' => 'object',
        'wildcard' => true,
      )
    ));

$object_names = $args->getArg('object');
if (count($object_names) !== 1) {
  throw new PhutilArgumentUsageException(
    pht(
      'Specify exactly one object to reset mentions on.'));
}

$viewer = PhabricatorUser::getOmnipotentUser();
$query = id(new PhabricatorObjectQuery())
  ->setViewer($viewer)
  ->withNames($object_names);
$query->execute();

$objects = $query->getNamedResults();
foreach ($object_names as $object_name) {
  if (!isset($objects[$object_name])) {
    throw new PhutilArgumentUsageException(
      pht(
        'Unable to load object "%s".',
        $object_name));
  }
}

$mention_type = PhabricatorObjectMentionsObjectEdgeType::EDGECONST;

foreach ($objects as $object) {
  $okay = false;
  if ($object instanceof ManiphestTask) {
    $okay = true;
  }

  if (!$okay) {
    throw new PhutilArgumentUsageException(
      pht(
        'This script can only reset mentions on tasks.'));
  }

  $old_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
    $object->getPHID(),
    $mention_type);

  $remarkup_blocks = array();
  $remarkup_blocks[] = $object->getDescription();

  $custom_fields = PhabricatorCustomField::getObjectFields(
    $object,
    PhabricatorCustomField::ROLE_APPLICATIONTRANSACTIONS);

  $custom_fields
    ->setViewer($viewer)
    ->readFieldsFromStorage($object);

  foreach ($custom_fields->getFields() as $custom_field) {
    if ($custom_field instanceof ManiphestConfiguredCustomField) {
      $proxy = $custom_field->getProxy();
      if ($proxy instanceof PhabricatorStandardCustomFieldRemarkup) {
        $remarkup_blocks[] = $custom_field->getValueForStorage();
      }
    }
  }

  $xactions = id(new ManiphestTransactionQuery())
    ->setViewer($viewer)
    ->withObjectPHIDs(array($object->getPHID()))
    ->needComments(true)
    ->execute();

  foreach ($xactions as $xaction) {
    if ($xaction->hasComment()) {
      $remarkup_blocks[] = $xaction->getComment()->getContent();
    }
  }

  $engine = PhabricatorMarkupEngine::getEngine('extract');
  $engine->setConfig('viewer', $viewer);

  $new_phids = array();
  foreach ($remarkup_blocks as $remarkup_block) {
    $engine->markupText($remarkup_block);

    $new_phids += $engine->getTextMetadata(
      PhabricatorMentionRemarkupRule::KEY_MENTIONED,
      array());

    $new_phids += $engine->getTextMetadata(
      PhabricatorObjectRemarkupRule::KEY_MENTIONED_OBJECTS,
      array());
  }

  $old_phids = array_fuse($old_phids);
  $new_phids = array_fuse($new_phids);

  $all_phids = $old_phids + $new_phids;
  $add_phids = array_diff_key($new_phids, $old_phids);
  $rem_phids = array_diff_key($old_phids, $new_phids);
  $hold_phids = array_intersect_key($old_phids, $new_phids);

  $handles = $viewer->loadHandles($all_phids);
  $handles = iterator_to_array($handles);

  if ($rem_phids) {
    echo tsprintf(
      "%s\n\n%B\n",
      pht('These mentions will be removed:'),
      reset_mentions_format_handle_list(
        array_select_keys($handles, $rem_phids)));
  } else {
    echo tsprintf(
      "%s\n",
      pht('No mentions will be removed.'));
  }

  if ($add_phids) {
    echo tsprintf(
      "%s\n\n%B\n",
      pht('These mentions will be added:'),
      reset_mentions_format_handle_list(
        array_select_keys($handles, $add_phids)));
  } else {
    echo tsprintf(
      "%s\n",
      pht('No mentions will be added.'));
  }

  if ($hold_phids) {
    echo tsprintf(
      "%s\n\n%B\n",
      pht('These mentions will not be affected:'),
      reset_mentions_format_handle_list(
        array_select_keys($handles, $hold_phids)));
  }

  if (!$add_phids && !$rem_phids) {
    echo tsprintf(
      "%s\n",
      pht('Nothing to do.'));
    return 0;
  }

  $prompt = pht('Apply these changes?');
  if (!phutil_console_confirm($prompt)) {
    throw new PhutilArgumentUsageException(
      pht(
        'User aborted the workflow.'));
  }

  $editor = new PhabricatorEdgeEditor();

  foreach ($add_phids as $add_phid) {
    $editor->addEdge(
      $object->getPHID(),
      $mention_type,
      $add_phid);
  }

  foreach ($rem_phids as $rem_phid) {
    $editor->removeEdge(
      $object->getPHID(),
      $mention_type,
      $rem_phid);
  }

  $editor->save();

  echo tsprintf(
    "%s\n",
    pht('Saved changes.'));
}


function reset_mentions_format_handle_list(array $handles) {
  $result = array();

  foreach ($handles as $phid => $handle) {
    $result[] = tsprintf(
      "    - [%s] %s\n",
      $phid,
      $handle->getFullName());
  }

  return implode('', $result);
}

(Since a full implementation here will imply that removing a custom field from configuration may "unmention" an arbitrarily large number of relationships, and we can't easily do that inline or at display time, the ultimately implementation would likely include a more robust version of this script.)