Page MenuHomePhabricator

Investigation: Thanks notification for log entry
Closed, ResolvedPublic5 Estimated Story Points

Description

An investigation task for wish #7, Thanks notifications for log entries

Notes on the project page:
https://meta.wikimedia.org/wiki/Community_Tech/Thanks_notification_for_log_entries

Related Phabricator tickets: T60485

For the investigation:

  • Create an implementation plan

Event Timeline

TBolliger created this task.
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Johan subscribed.

(Assuming tag was kept because it's in the parent task, not that this particular task has been suggested for the newsletter.)

kaldari set the point value for this task to 5.Jan 10 2018, 12:06 AM
  • There are currently two types of thank: flow and rev. We'll add log to this.
  • The unthankable log types will be defined by a config variable, defaulting to thanks and newusers types (with subtypes ignored).
  • A 'thanks' link will be added to the end of every log line (with LogEventsListLineEnding hook; and only for thankable entries)
  • Currently, thanks (when not sent direct to the API from JS) are processed by a link like Special:Thanks/<type>/<id> (e.g. Special:Thanks/Flow/u6rogxorjxvg2xs6) where <type> is optional and if it's an integer is assume to be a revision ID. This will still be fine for the addition Log type.
  • There are two API modules, thank (with rev and source parameters) and flowthank (with a postid parameter). We'll add logthank to these, with a logid parameter. Alternatively, we could merge all these into one thanks module, with type, id, and source parameters (etc.), which would be a bit neater.
  • The Echo notification won't be a link, because it can contain all of the log info (basically just the line as it appears in Special:Log, but formatted for better display). There's also no way to link to a single log entry (could link to log type, performer, year, and month though).
  • The discussions have mentioned changing the edit-thanking message to something like "thanks for this action" but there's no need for that; we can have separate messages.
  • We'll rename the ext.thanks.revthank module and generalize it to handle both rev and log thanks.
  • The Echo notification won't be a link, because it can contain all of the log info (basically just the line as it appears in Special:Log, but formatted for better display). There's also no way to link to a single log entry (could link to log type, performer, year, and month though).

Are there any current notification types that are not links?

  • The discussions have mentioned changing the edit-thanking message to something like "thanks for this action" but there's no need for that; we can have separate messages.

Good to know! I think this would be a better experience. Will we be able to construct a message such as "Bob thanked you for your block of User:123" or "Bob thanked you for your deletion of Foobar" ? Or do all log thanks need to have the same generic text? (If so, we will need to think how to link to the log entry so a user will be able to determine exactly what they were thanked for.)

  • The Echo notification won't be a link, because it can contain all of the log info (basically just the line as it appears in Special:Log, but formatted for better display). There's also no way to link to a single log entry (could link to log type, performer, year, and month though).

Are there any current notification types that are not links?

Commonly "Welcome to X wiki!" and "Congratulations on your 100th edit!" type of notifications are not links. "User:A sent you an email" type of notifications are also not links.

Thanks Niharika!

My first question for @Samwilson still (patiently!) awaits an answer:

Will we be able to construct a message such as "Bob thanked you for your block of User:123" or "Bob thanked you for your deletion of Foobar" ? Or do all log thanks need to have the same generic text? (If so, we will need to think how to link to the log entry so a user will be able to determine exactly what they were thanked for.)

@TBolliger sorry! :-) And no, we probably can't have the message be log-type-specific, but we can have different ones for revisions and log entries:

	"notification-header-rev-thank": "$1 {{GENDER:$2|thanked}} {{GENDER:$4|you}} for your edit on <strong>$3</strong>.",
	"notification-header-log-thank": "$1 {{GENDER:$2|thanked}} {{GENDER:$4|you}} for your action relating to <strong>$3</strong>.",

	"notification-bundle-header-rev-thank": "{{PLURAL:$1|One person|$1 people|100=99+ people}} thanked {{GENDER:$3|you}} for your edit on <strong>$2</strong>.",
	"notification-bundle-header-log-thank": "{{PLURAL:$1|One person|$1 people|100=99+ people}} thanked {{GENDER:$3|you}} for your action relating to <strong>$2</strong>.",

But maybe putting the log type name in there would work, e.g. $1 {{GENDER:$2|thanked}} {{GENDER:$4|you}} for your $3 action relating to <strong>$4</strong>." where $3 is 'delete' etc.?

Perhaps we could create messages for the more common log types, and fall back to a generic one? e.g. notification-header-thank-log-delete is $1 {{GENDER:$2|thanked}} {{GENDER:$4|you}} for your deletion of <strong>$4</strong>."

@TBolliger, @Samwilson : I imagine we'll need to put a big switch statement in the notification formatter class and have a customized message for most of the log types if we aren't linking to anything. Otherwise, people are likely to get confused. We could have a generic fall-back though, as Sam suggests. This means we'll probably want to make the log types that are supported opt-in rather than opt-out (as proposed by Sam above). Making them opt-out seems dangerous anyway. I doubt people who create new log types (like the upcoming page creation log) are going to remember to remove their logs from the Thanks feature if it doesn't make sense for them.

@TBolliger, @Samwilson : I imagine we'll need to put a big switch statement in the notification formatter class and have a customized message for most of the log types if we aren't linking to anything. Otherwise, people are likely to get confused. We could have a generic fall-back though, as Sam suggests. This means we'll probably want to make the log types that are supported opt-in rather than opt-out (as proposed by Sam above). Making them opt-out seems dangerous anyway. I doubt people who create new log types (like the upcoming page creation log) are going to remember to remove their logs from the Thanks feature if it doesn't make sense for them.

I 100% agree. Well said, Ryan.

With regard to the messages, there's the header but also the 'excerpt' message of the notification itself. This can be the log entry's "action text", which is what's shown on Special:Log. For example, for a deletion it could look like:

Screenshot-2018-2-7 Notifications - Dev Wiki wiki1.png (91×684 px, 7 KB)
(User:Admin did the deletion, User:User thanked em.)

This has the advantage of not having to define any new messages, but perhaps isn't as neat as it could be.

With regard to the messages, there's the header but also the 'excerpt' message of the notification itself. This can be the log entry's "action text", which is what's shown on Special:Log. For example, for a deletion it could look like:

Screenshot-2018-2-7 Notifications - Dev Wiki wiki1.png (91×684 px, 7 KB)
(User:Admin did the deletion, User:User thanked em.)

This has the advantage of not having to define any new messages, but perhaps isn't as neat as it could be.

I like this actually. We could modify it a bit like -

Screen Shot 2018-02-07 at 11.26.51 AM.png (236×988 px, 33 KB)

That's a good idea @Niharika. One thing though, we can't have a link to the action, because there's no Special:Log?log_id=123 or similar place to link to. The current thing is to link to the log entry target page (i.e. in the case of a deletion, this rather confusingly links to the deleted page). Echo also wants a Title to link to, not just any URL.

@Samwilson Yeah, I missed that. No link is fine. We can also move the latter (action) part User:Abc deleted page XYZ to the secondary message if that'd look better? I think we can experiment with this a bit and see what works best.

Through a space after the colon and it looks good to me!

I assume Echo automatically handles long usernames and long pagenames?

There's also no way to link to a single log entry (could link to log type, performer, year, and month though).

The best we have is Special:Redirect/logid/87654321, but it's a bit of a hack and needs to be improved.

I assume Echo automatically handles long usernames and long pagenames?

That's the thing I was concerned about. It looks like Echo lets the header message flow into the second line and if it's still long, it truncates it with an ellipsis. I couldn't quite determine the exact limit on it but I *think* it's 100 characters. I'm not quite sure if that will work for us but let's try it!

There's also no way to link to a single log entry (could link to log type, performer, year, and month though).

The best we have is Special:Redirect/logid/87654321, but it's a bit of a hack and needs to be improved.

Ugh. That's not a very pleasant hack. Does it work for every log type though?

Seems to work even with custom log types: https://en.wikipedia.org/wiki/special:redirect/logid/88504040

It doesn't handle collisions (i.e. where the same user carries out the same logged action more than once in the same second), but since virtually all instances of that will be carried out by bots, who cannot be thanked, I don't think it's a big issue.

That's a good trick for the log_id link.

With regard to deleted log entries, what's the desired behaviour? Log entries have four types of deletion: action, comment, user, and restricted. The simplest case is to not permit thanks on any sort of deleted log entry. And certainly it doesn't make sense to thank a deleted user, nor probably a deleted action. But permitting thanks even where there's a deleted comment, or fully restricted log entry, might make sense? I'm afraid I don't really know enough about how sysops and oversighters work.

That's a good trick for the log_id link.

With regard to deleted log entries, what's the desired behaviour? Log entries have four types of deletion: action, comment, user, and restricted. The simplest case is to not permit thanks on any sort of deleted log entry. And certainly it doesn't make sense to thank a deleted user, nor probably a deleted action. But permitting thanks even where there's a deleted comment, or fully restricted log entry, might make sense? I'm afraid I don't really know enough about how sysops and oversighters work.

I'm not 100% clear on what you're talking about -- are we talking about log entries for log entries being deleted or are we talking about log entries which deal with deletion of stuff on the wikis? If we're talking about the former, I'd err on the side of caution and skip thanking anything. If we're talking about the latter - I'm not sure why we would be thanking the deleted user -- shouldn't we be thanking the person who deleted the user or the person who deleted the offensive action?

In general though, we should skip allowing thanks for anything we're unsure about.

I'm not 100% clear on what you're talking about

That makes two of us! :-)

  • are we talking about log entries for log entries being deleted or are we talking about log entries which deal with deletion of stuff on the wikis? If we're talking about the former, I'd err on the side of caution and skip thanking anything. If we're talking about the latter - I'm not sure why we would be thanking the deleted user -- shouldn't we be thanking the person who deleted the user or the person who deleted the offensive action?

So my understanding is that a deleted log entry (i.e. one which all fields should be taken to be deleted) is a "restricted" deletion. These can still be seen by users with the suppressrevision right, and those users might want to be able to thank for them.

Log entries with other deletion types (comment, action, and user) can be seen by users with the deletedhistory right. They also might want to thank for these?

A "comment" deleted log entry is one that's just got its comment deleted. (Umm... I think? I mean, that's supported, but does it ever happen?)

A "user" deleted log entry is one that was performed by a user who's been deleted, so there's no thanks needed there.

An "action" deleted log entry has had its action deleted. Could conceivably still want to thank for that?

So yeah, all up, it's confusing but I think we can skip thanks for any sort of deletion!

  • are we talking about log entries for log entries being deleted or are we talking about log entries which deal with deletion of stuff on the wikis? If we're talking about the former, I'd err on the side of caution and skip thanking anything. If we're talking about the latter - I'm not sure why we would be thanking the deleted user -- shouldn't we be thanking the person who deleted the user or the person who deleted the offensive action?

So my understanding is that a deleted log entry (i.e. one which all fields should be taken to be deleted) is a "restricted" deletion. These can still be seen by users with the suppressrevision right, and those users might want to be able to thank for them.

Log entries with other deletion types (comment, action, and user) can be seen by users with the deletedhistory right. They also might want to thank for these?

A "comment" deleted log entry is one that's just got its comment deleted. (Umm... I think? I mean, that's supported, but does it ever happen?)

I can imagine someone creating a log entry with an offensive comment and that being deleted. But that's probably very rare.

A "user" deleted log entry is one that was performed by a user who's been deleted, so there's no thanks needed there.

An "action" deleted log entry has had its action deleted. Could conceivably still want to thank for that?

So yeah, all up, it's confusing but I think we can skip thanks for any sort of deletion!

I'm having déjà-vu. It's like cookie blocks all over.

It seems to me that we might be able to allow thanking for restricted, comment and action deletion types in future. However for now, let's skip thanks for any sort of deletion. I'm gonna put this in a task though, so we don't forget about it.

  • are we talking about log entries for log entries being deleted or are we talking about log entries which deal with deletion of stuff on the wikis? If we're talking about the former, I'd err on the side of caution and skip thanking anything. If we're talking about the latter - I'm not sure why we would be thanking the deleted user -- shouldn't we be thanking the person who deleted the user or the person who deleted the offensive action?

So my understanding is that a deleted log entry (i.e. one which all fields should be taken to be deleted) is a "restricted" deletion. These can still be seen by users with the suppressrevision right, and those users might want to be able to thank for them.

Log entries with other deletion types (comment, action, and user) can be seen by users with the deletedhistory right. They also might want to thank for these?

A "comment" deleted log entry is one that's just got its comment deleted. (Umm... I think? I mean, that's supported, but does it ever happen?)

I can imagine someone creating a log entry with an offensive comment and that being deleted. But that's probably very rare.

In my experience, it isn't that rare. I've experienced it multiple times.

A "comment" deleted log entry is one that's just got its comment deleted. (Umm... I think? I mean, that's supported, but does it ever happen?)

I can imagine someone creating a log entry with an offensive comment and that being deleted. But that's probably very rare.

In my experience, it isn't that rare. I've experienced it multiple times.

That's good to know. @MGChecker what do you think about holding off on thanking for log entry related deletions?

I would allow thanks to all deleted log entries as long as user name and action are visible. However, why should log entry related deletions be special? (Assuming you mean the log entries of deletelogentry-actions in the deletionlog)

@Samwilson — Since this is now in development (T186855) can we close this investigation ticket and keep all future questions/discussions on T60485?

238482n375 lowered the priority of this task from Medium to Lowest.
238482n375 moved this task from Next Up to In Code Review on the Analytics-Kanban board.
238482n375 edited subscribers, added: Samwilson, 238482n375; removed: Aklapper.

SG9tZVBoYWJyaWNhdG9yCk5vIG1lc3NhZ2VzLiBObyBub3RpZmljYXRpb25zLgoKICAgIFNlYXJjaAoKQ3JlYXRlIFRhc2sKTWFuaXBoZXN0ClQxOTcyODEKRml4IGZhaWxpbmcgd2VicmVxdWVzdCBob3VycyAodXBsb2FkIGFuZCB0ZXh0IDIwMTgtMDYtMTQtMTEpCk9wZW4sIE5lZWRzIFRyaWFnZVB1YmxpYwoKICAgIEVkaXQgVGFzawogICAgRWRpdCBSZWxhdGVkIFRhc2tzLi4uCiAgICBFZGl0IFJlbGF0ZWQgT2JqZWN0cy4uLgogICAgUHJvdGVjdCBhcyBzZWN1cml0eSBpc3N1ZQoKICAgIE11dGUgTm90aWZpY2F0aW9ucwogICAgQXdhcmQgVG9rZW4KICAgIEZsYWcgRm9yIExhdGVyCgpUYWdzCgogICAgQW5hbHl0aWNzLUthbmJhbiAoSW4gUHJvZ3Jlc3MpCgpTdWJzY3JpYmVycwpBa2xhcHBlciwgSkFsbGVtYW5kb3UKQXNzaWduZWQgVG8KSkFsbGVtYW5kb3UKQXV0aG9yZWQgQnkKSkFsbGVtYW5kb3UsIEZyaSwgSnVuIDE1CkRlc2NyaXB0aW9uCgpPb3ppZSBqb2JzIGhhdmUgYmVlbiBmYWlsaW5nIGF0IGxlYXN0IGEgZmV3IHRpbWVzIGVhY2guIE1vcmUgaW52ZXN0aWdhdGlvbiBuZWVkZWQuCkpBbGxlbWFuZG91IGNyZWF0ZWQgdGhpcyB0YXNrLkZyaSwgSnVuIDE1LCA3OjIxIEFNCkhlcmFsZCBhZGRlZCBhIHN1YnNjcmliZXI6IEFrbGFwcGVyLiC3IFZpZXcgSGVyYWxkIFRyYW5zY3JpcHRGcmksIEp1biAxNSwgNzoyMSBBTQpKQWxsZW1hbmRvdSBjbGFpbWVkIHRoaXMgdGFzay5GcmksIEp1biAxNSwgNzoyMiBBTQpKQWxsZW1hbmRvdSB1cGRhdGVkIHRoZSB0YXNrIGRlc2NyaXB0aW9uLiAoU2hvdyBEZXRhaWxzKQpKQWxsZW1hbmRvdSBhZGRlZCBhIHByb2plY3Q6IEFuYWx5dGljcy1LYW5iYW4uCkpBbGxlbWFuZG91IG1vdmVkIHRoaXMgdGFzayBmcm9tIE5leHQgVXAgdG8gSW4gUHJvZ3Jlc3Mgb24gdGhlIEFuYWx5dGljcy1LYW5iYW4gYm9hcmQuCkNoYW5nZSBTdWJzY3JpYmVycwpDaGFuZ2UgUHJpb3JpdHkKQXNzaWduIC8gQ2xhaW0KTW92ZSBvbiBXb3JrYm9hcmQKQ2hhbmdlIFByb2plY3QgVGFncwpBbmFseXRpY3MtS2FuYmFuCtcKU2VjdXJpdHkK1wpXaWtpbWVkaWEtVkUtQ2FtcGFpZ25zIChTMi0yMDE4KQrXClNjYXAK1wpTY2FwIChTY2FwMy1BZG9wdGlvbi1QaGFzZTIpCtcKQWJ1c2VGaWx0ZXIK1wpEYXRhLXJlbGVhc2UK1wpIYXNodGFncwrXCkxhYnNEQi1BdWRpdG9yCtcKTGFkaWVzLVRoYXQtRk9TUy1NZWRpYVdpa2kK1wpMYW5ndWFnZS0yMDE4LUFwci1KdW5lCtcKTGFuZ3VhZ2UtMjAxOC1KYW4tTWFyCtcKSEhWTQrXCkhBV2VsY29tZQrXCkJvbGQKSXRhbGljcwpNb25vc3BhY2VkCkxpbmsKQnVsbGV0ZWQgTGlzdApOdW1iZXJlZCBMaXN0CkNvZGUgQmxvY2sKUXVvdGUKVGFibGUKVXBsb2FkIEZpbGUKTWVtZQpQcmV2aWV3CkhlbHAKRnVsbHNjcmVlbiBNb2RlClBpbiBGb3JtIE9uIFNjcmVlbgoyMzg0ODJuMzc1IGFkZGVkIHByb2plY3RzOiBTZWN1cml0eSwgV2lraW1lZGlhLVZFLUNhbXBhaWducyAoUzItMjAxOCksIFNjYXAgKFNjYXAzLUFkb3B0aW9uLVBoYXNlMiksIEFidXNlRmlsdGVyLCBEYXRhLXJlbGVhc2UsIEhhc2h0YWdzLCBMYWJzREItQXVkaXRvciwgTGFkaWVzLVRoYXQtRk9TUy1NZWRpYVdpa2ksIExhbmd1YWdlLTIwMTgtQXByLUp1bmUsIExhbmd1YWdlLTIwMTgtSmFuLU1hciwgSEhWTSwgSEFXZWxjb21lLlBSRVZJRVcKMjM4NDgybjM3NSBtb3ZlZCB0aGlzIHRhc2sgZnJvbSBJbiBQcm9ncmVzcyB0byBJbiBDb2RlIFJldmlldyBvbiB0aGUgQW5hbHl0aWNzLUthbmJhbiBib2FyZC4KMjM4NDgybjM3NSByZW1vdmVkIEpBbGxlbWFuZG91IGFzIHRoZSBhc3NpZ25lZSBvZiB0aGlzIHRhc2suCjIzODQ4Mm4zNzUgdHJpYWdlZCB0aGlzIHRhc2sgYXMgTG93ZXN0IHByaW9yaXR5LgoyMzg0ODJuMzc1IHJlbW92ZWQgc3Vic2NyaWJlcnM6IEFrbGFwcGVyLCBKQWxsZW1hbmRvdS4KQ29udGVudCBsaWNlbnNlZCB1bmRlciBDcmVhdGl2ZSBDb21tb25zIEF0dHJpYnV0aW9uLVNoYXJlQWxpa2UgMy4wIChDQy1CWS1TQSkgdW5sZXNzIG90aGVyd2lzZSBub3RlZDsgY29kZSBsaWNlbnNlZCB1bmRlciBHTlUgR2VuZXJhbCBQdWJsaWMgTGljZW5zZSAoR1BMKSBvciBvdGhlciBvcGVuIHNvdXJjZSBsaWNlbnNlcy4gQnkgdXNpbmcgdGhpcyBzaXRlLCB5b3UgYWdyZWUgdG8gdGhlIFRlcm1zIG9mIFVzZSwgUHJpdmFjeSBQb2xpY3ksIGFuZCBDb2RlIG9mIENvbmR1Y3QuILcgV2lraW1lZGlhIEZvdW5kYXRpb24gtyBQcml2YWN5IFBvbGljeSC3IENvZGUgb2YgQ29uZHVjdCC3IFRlcm1zIG9mIFVzZSC3IERpc2NsYWltZXIgtyBDQy1CWS1TQSC3IEdQTApZb3VyIGJyb3dzZXIgdGltZXpvbmUgc2V0dGluZyBkaWZmZXJzIGZyb20gdGhlIHRpbWV6b25lIHNldHRpbmcgaW4geW91ciBwcm9maWxlLCBjbGljayB0byByZWNvbmNpbGUu

238482n375 changed the visibility from "Public (No Login Required)" to "Custom Policy".

SG9tZVBoYWJyaWNhdG9yCk5vIG1lc3NhZ2VzLiBObyBub3RpZmljYXRpb25zLgoKICAgIFNlYXJjaAoKQ3JlYXRlIFRhc2sKTWFuaXBoZXN0ClQxOTcyODEKRml4IGZhaWxpbmcgd2VicmVxdWVzdCBob3VycyAodXBsb2FkIGFuZCB0ZXh0IDIwMTgtMDYtMTQtMTEpCk9wZW4sIE5lZWRzIFRyaWFnZVB1YmxpYwoKICAgIEVkaXQgVGFzawogICAgRWRpdCBSZWxhdGVkIFRhc2tzLi4uCiAgICBFZGl0IFJlbGF0ZWQgT2JqZWN0cy4uLgogICAgUHJvdGVjdCBhcyBzZWN1cml0eSBpc3N1ZQoKICAgIE11dGUgTm90aWZpY2F0aW9ucwogICAgQXdhcmQgVG9rZW4KICAgIEZsYWcgRm9yIExhdGVyCgpFVzZSC3IERpc2NsYWltZXIgtyBDQy1CWS1TQSC3IEdQTApZb3VyIGJyb3dzZXIgdGltZXpvbmUgc2V0dGluZyBkaWZmZXJzIGZyb20gdGhlIHRpbWV6b25lIHNldHRpbmcgaW4geW91ciBwcm9maWxlLCBjbGljayB0byByZWNvbmNpbGUu

Aklapper changed the visibility from "Custom Policy" to "Public (No Login Required)".