Page MenuHomePhabricator

Allow Herald rules to add comments
ClosedPublic

Authored by epriestley on Dec 11 2017, 7:49 PM.
Tags
None
Referenced Files
F14077047: D18823.diff
Thu, Nov 21, 9:05 PM
Unknown Object (File)
Mon, Nov 18, 2:48 PM
Unknown Object (File)
Thu, Nov 14, 9:01 PM
Unknown Object (File)
Sun, Nov 10, 8:42 AM
Unknown Object (File)
Wed, Nov 6, 10:01 AM
Unknown Object (File)
Wed, Oct 30, 6:41 AM
Unknown Object (File)
Oct 14 2024, 10:05 PM
Unknown Object (File)
Oct 9 2024, 9:19 AM
Subscribers
None

Details

Summary

See PHI242. All use cases for this that I know of are pretty hacky, but they don't seem perilous, and it's easier than webhooks.

See P1895, T10183, and T9853 for me previously refusing to implement this since all those use cases were also pretty bad.

Test Plan
  • Wrote a rule to add comments, saw it add comments.
  • Reviewed summary, re-edited rule, reviewed transcript to check that all the strings worked OK.
  • Wrote a new rule for a non-commentable object (a blog) to make sure I wasn't offered the "Add a comment" action.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

What do these herald-generated comments look like? Are they attributed to the user who created the Herald rule? Looking towards the future, should we (perhaps insanely) add an option that lets you specify the author as any bot user visible to the user creating the Herald rule?

This revision is now accepted and ready to land.Dec 13 2017, 3:16 PM

They currently say "Herald added a comment. [ View Transcript ]". You can click the transcript link to puzzle out exactly why the comment was added, although it may take a bit of work.

Global and Project rules (which are the only rule types which can use this action -- for now, at least) don't currently have an owner, so we couldn't directly attribute this stuff to a first-class owner. (They did in the past, sort of by accident, but that ran into a bunch of murky stuff when users were on vacation or left the company and their global rules were misbehaving.)

We could let you attribute this action to a user explicitly instead, although all users can always see all other users today and I'm a little hesitant to conflate "see" with "edit on behalf of, if the target is a bot" -- I don't think there's any real peril there, but it's a bit of a mess if we, e.g., eventually lock bots down to specific owners and have to try to migrate rules to be consistent.

If there's ambiguity/confusion, some thoughts on ways to approach it are:

  • tell installs to toughen up and use hooks/extensions/anything but this to solve these problems in a real way, using an actual bot with a real API key to get proper attribution and full edit capability; or
  • maybe do something dumb like automatically prefix all comments with "This comment was added by {H123}: ..." to more explicitly attribute comments.

But I think you can "solve" this "problem" by just adding more text to the comment which explains what's going on, and this isn't materially more bad than using "Add comment..." in the first place.

Philosophically, I think this isn't the best solution to any problem we've seen users report, it just feels excessively paternalistic to refuse to implement it since I don't think there's much real support/technical/product risk, and the limitations should (hopefully?) be pretty self evident.

(My biggest fear on this is that we get requests for variable replacement/templating.)

This revision was automatically updated to reflect the committed changes.