Page MenuHomePhabricator

Clean up lego message in user-rights notification
Closed, ResolvedPublic

Description

Currently, UserrightsPresentationModel uses lego messages:

"notification-header-user-rights": "Your user rights were {{GENDER:$2|changed}} by $1. $3.",
"notification-user-rights-add": "You are now a member of {{PLURAL:$2|this group|these groups}}: $1",
"notification-user-rights-remove": "You are no longer a member of {{PLURAL:$2|this group|these groups}}: $1",

where the $3 parameter is either the notification-user-rights-add message, or the notification-user-rights-remove message, or a semicolon-separated "list" of both. The $1 parameter to each of the submessages is a comma-separated list of groups. This makes the result look fairly ugly:

  • "Your user rights were changed by Roan Kattouw. You are now a member of this group: Administrators."
  • "Your user rights were changed by Roan Kattouw. You are no longer a member: Administrators, Bureaucrats, Oversighters."
  • "Your user rights were changed by Roan Kattouw. You are now a member of this group: Arbcom; You are no longer a member of these groups: Oversighters, Checkusers."

I suggest we split this into three messages along different lines: one message for when groups were added but not removed, one for when groups were removed but not added, and one for when both happened. That way we won't have to inject the contents of one message into another ("lego").

While we're at it, I suggest we fix the following annoyances:

  • In the "both" case, we currently have a semicolon followed by a capital letter. We could lowercase the letter after the semicolon, or use different punctuation (e.g. a period)
  • In the multiple groups case, we should use a comma-separated list that ends with and (Language::listToText()) rather than a simple comma-separated list (Language::commaList())
  • In single group case, we could use "You are now a member of the $1 group" instead of "You are now a member of this group: $1". We could even use this style in all cases if we wanted to ("You are now a member of the Administrators, Checkusers and Oversighters groups")

See also

Event Timeline

Catrope raised the priority of this task from to Needs Triage.
Catrope updated the task description. (Show Details)

Assigning to Joe to finalize wording.

Quiddity set Security to None.

Below is my suggested language. Note that I'm recommending a colon followed by a capital letter after "changed." This reads well in English and conforms with AP style (though not Chicago style in group #3 -- do we have to go there?). HOWEVER, if it makes things simpler to program, maintain or internationalize, it would be perfectly acceptable to use periods instead of the colons.

There's some question as to whether it is strictly correct to use "and" when removing multiple items (#2 and #3). "Or" sounds more idiomatic in English, but Roan says it would be more complicated to implement. "And" is perfectly logical and I don't think it is incorrect. So I think it is OK unless someone knows differently.

#1: Groups added but not removed

SINGLE "Your user rights were changed: You are now a member of the Administrators group."
PLURAL "Your user rights were changed: You are now a member of the Administrators, Checkusers and Oversighters groups.”

#2: Groups removed but not added

SINGLE "Your user rights were changed: You are no longer a member of the Administrators group."
PLURAL "Your user rights were changed: You are no longer a member of the Administrators, Checkusers and Oversighters groups.”

#3: Both added and removed.

SINGLE "Your user rights were changed: You are now a member of the Administrators group. You are no longer a member of the Checkusers group."
PLURAL "Your user rights were changed: You are now a member of the Administrators, Checkusers and Oversighters groups. You are no longer a member of the X, Y and Z groups.”

Catrope triaged this task as Medium priority.

Thanks for that copy, Joe. That also nicely takes care of removing the user name from the message, which we already wanted to do in light of T121737: Add secondary link for the agent to some notification types. However, that does mean that we should do that (add the secondary link) before implementing this language change.

Change 267044 had a related patch set uploaded (by Sbisson):
Fix lego message in 'user-rights' notification

https://gerrit.wikimedia.org/r/267044

"You are now a member of the X, Y and Z group" implies that the groups mentioned are *all* those that you belong to, including those you were member of already (which is not the case, it only lists those that you have just been added to)

Change 267044 merged by jenkins-bot:
Fix lego messages in 'user-rights' notification

https://gerrit.wikimedia.org/r/267044

#1: Groups added but not removed
SINGLE

Screen Shot 2016-01-29 at 5.04.33 PM.png (172×696 px, 36 KB)

PLURAL
Screen Shot 2016-01-29 at 4.47.23 PM.png (174×758 px, 40 KB)

#2: Groups removed but not added - there is a bug T125279: Removed group rights notification messages do not display group names.

#3: Both added and removed - the same bug for removing group rights T125279: Removed group rights notification messages do not display group names.

Screen Shot 2016-01-29 at 5.07.46 PM.png (180×736 px, 42 KB)

@matthiasmullie
It's seems that it's smart enough to mention only the recent group right granted - not to list all groups you are in. In the screenshot there is a missing group name before a comma, but it's another bug.

Screen Shot 2016-01-29 at 5.39.27 PM.png (257×736 px, 61 KB)

Yeah it indeed only mentions those that you were just added to, but I thought the text "You are now a member of ..." seems to imply that those are the *only* groups you're member of.

If I'm pre-existing member of group A & B, and just got added to groups C & D, my notification would read: "You are now a member of C & D".
To me, it seems to imply that I'm only a member of C & D. I think rephrasing it like "You were just added to the C & D groups" would be clearer.
But it may just be me who thought it's a bit confusing :)

@matthiasmullie asks

To me, it seems to imply that I'm only a member of C & D....But it may just be me who thought it's a bit confusing :)

I have to say, I don't hear the implication that we're listing the totality of groups you're in. For me, the first part of the message -- "Your user rights have changed:" -- clearly set up that in the second part of the message, we're going to be talking about (only) the things that have changed. That's what the colon does, in English anyway: it sets up a kind of equivalency where statements on the right side of the colon are understood to explain or define the statements on the left side of the colon. Also the use of the word "now" in the second half -- "You are now a member..." -- for me indicates that we're talking about what's changed.

But maybe that's just me, as you say. Do others hear this the way Matthias does?

I agree with everything you said about the use of the colon in there, but I had still read the message in a different way.

I just read it as "Some things have changes, so here's the current complete list" instead of "Some things have changed and those are the ones that were added".

But I'm not going to block this (I already merged the patch), just wanted to point out how I read it. Unless others read it like I did, there's not need to change the wording ;)

I agree with everything you said about the use of the colon in there, but I had still read the message in a different way.

I just read it as "Some things have changes, so here's the current complete list" instead of "Some things have changed and those are the ones that were added".

I agree with @matthiasmullie. In my opinion, it's ambiguous. Both are plausible readings (/You are now a member of these and only these exact groups/, /You are now a member of these additional groups/). Someone is more likely to read it wrong (/You are now a member of these and only these exact groups/) when there are multiple new groups.

I think we could make it clearer, e.g. by using the word "added".

Thanks for adding your perspective @Mattflaschen. It's easy enough to clarify the messaging along the lines you suggest.

CURRENT MESSAGING VARIATIONS

  • Your user rights were changed. You are now a member of: Oversighters.
  • Your user rights were changed. You are no longer a member of: Oversighters, Administrators.
  • Your user rights were changed. You are now a member of Oversighters. You are no longer a member of: X, Y, Z.

PROPOSED IMPROVED MESSAGING

  • Your user rights were changed. You have been added to: Oversighters.
  • Your user rights were changed. You are no longer a member of: Oversighters, Administrators. [NO CHANGE]
  • Your user rights were changed. You have been added to: Oversighters. You are no longer a member of: X, Y, Z.

If that works for everyone, I'll make the changes in the Notifications spreadsheet.

Change 267842 had a related patch set uploaded (by Matthias Mullie):
Rephrase user rights change messages

https://gerrit.wikimedia.org/r/267842

Works for me - patch is up for review.

I was not concerned about the previous one, but the new one looks good too.
I updated the Notification Catalog mockup (M132) to reflect the change.

Thanks for adding your perspective @Mattflaschen. It's easy enough to clarify the messaging along the lines you suggest.

CURRENT MESSAGING VARIATIONS

  • Your user rights were changed. You are now a member of: Oversighters.
  • Your user rights were changed. You are no longer a member of: Oversighters, Administrators.
  • Your user rights were changed. You are now a member of Oversighters. You are no longer a member of: X, Y, Z.

Current messages use the form "member of the ... groups" and not "member of: ..." Do we really want to change that part?

Current messages use the form "member of the ... groups" and not "member of: ..." Do we really want to change that part?

If I recall correctly, it was discussed (probably over spreadsheet comments) that by just saying "members of" the notion of "group" was implicitly understood. Based on that, the "members of:" approach was preferred for brevity reasons.

Not sure if there were additional considerations that made us reconsider that.

Current messages use the form "member of the ... groups" and not "member of: ..." Do we really want to change that part?

If I recall correctly, it was discussed (probably over spreadsheet comments) that by just saying "members of" the notion of "group" was implicitly understood. Based on that, the "members of:" approach was preferred for brevity reasons.

Not sure if there were additional considerations that made us reconsider that.

In this ticket, the accepted version was "member of ... groups". {T121661#1886603}

In this ticket, the accepted version was "member of ... groups". {T121661#1886603}

Which is inconsistent with the information on the spreadsheet. So one of those must be wrong.

In this ticket, the accepted version was "member of ... groups". {T121661#1886603}

Which is inconsistent with the information on the spreadsheet. So one of those must be wrong.

The joy of duplication ;)

I'm ok one way or another. I just wanted to make sure we don't change it by accident after thinking about it carefully.

Also, the spreadsheet was excellent to with in bulk and get an overview of everything but ultimately, the tickets are used for dev, QA, and are public.

On the narrow issue, yes, in the back and forth on the spreadsheet we decided for brevity to omit the talk of "groups." Which means the ticket is out of date and the messaging should be:

Your user rights were changed. You have been added to: Oversighters.
Your user rights were changed. You are no longer a member of: Oversighters, Administrators.
Your user rights were changed. You have been added to: Oversighters. You are no longer a member of: X, Y, Z.

Re. @SBisson's comment: "ultimately, the tickets are used for dev, QA, and are public," I'm sorry if I created confusion on this. My thought was that the efficient thing would be for QA and developers to have one, authoritative place to check -- the spreadsheet. With all the back and forth and changes and the many many tickets that mention language, I'm concerned it would be virtually impossible to keep all those multiple sources in synch. (I very much doubt that all the tickets that mention language are up to date right now...)

The spreadsheet is public, in the sense that its rights allow anyone with the link to edit and view it. James suggests that the way to handle this is to document in a ticket that the spreadsheet is the authoritative source for all language, secondary links and icons -- all three of which are up to date as far as I know. I can create such a ticket, with the proposed title: "Review all Notifications and create a master list of approved language, secondary links and icons." (I also updated Pau's Notifications catalog, M132, with a link to the Spreadsheet.)

Work for you?

The spreadsheet is public, in the sense that its rights allow anyone with the link to edit and view it. James suggests that the way to handle this is to document in a ticket that the spreadsheet is the authoritative source for all language, secondary links and icons -- all three of which are up to date as far as I know. I can create such a ticket, with the proposed title: "Review all Notifications and create a master list of approved language, secondary links and icons." (I also updated Pau's Notifications catalog, M132, with a link to the Spreadsheet.)

Concerning the spreadsheet, it should be public on "Comment/View mode" only. It is a complex document, where a vandalism may remain unseen.

<s>Concerning the task, I'm pretty sure it already exists, but I can't find it again. @Quiddity?</s>
Edit Get confused. I was thinking about this table here: T123018, which can be used.

Finding the right balance between having an overview of all the items and the individual detail of each one is not easy. Some thoughts that may help:

  • If we cannot avoid duplicity, let's facilitate navigation. Adding links from the spreadsheet to the phabricator tickets aimed at implementing the proposed adjustments will make it easy to check them and verify the text is consistent. On the other hand, having related phabricator tickets as sub-tasks of a common parent task can also help to avoid items fall through the cracks.
  • The spreadsheet. One of the main purposes of the spreadsheet is comparing the previous status with the new one for notifications that we plan to adjust. Once we do the adjustment, we no longer need such comparison with the previous state. The remaining purpose of the spreadsheet is to reflect the current state to make sure new notifications added are consistent with the existing ones. For the later we may no need a spreadsheet and we can do with a wiki page.

Change 267842 merged by jenkins-bot:
Rephrase user rights change messages

https://gerrit.wikimedia.org/r/267842

[...] With all the back and forth and changes and the many many tickets that mention language, I'm concerned it would be virtually impossible to keep all those multiple sources in synch. (I very much doubt that all the tickets that mention language are up to date right now...)

Agreed, and it directly affects the quality of our work. This is what worries me the most.

The spreadsheet is public, in the sense that its rights allow anyone with the link to edit and view it. James suggests that the way to handle this is to document in a ticket that the spreadsheet is the authoritative source for all language, secondary links and icons -- all three of which are up to date as far as I know. I can create such a ticket, with the proposed title: "Review all Notifications and create a master list of approved language, secondary links and icons." (I also updated Pau's Notifications catalog, M132, with a link to the Spreadsheet.)

What if we could easily generate a spreadsheet that contains the truth (from the code) as of now instead of relying on a file that we think or hope is up to date? It could be used to get a complete picture of the current state whenever we need it.

About how to feed changes to the spreadsheet into the development process, I'm not sure. We use tasks, assignments, statuses, discussions, etc to keep track of what's going on and google sheets doesn't really shine there. Would it be possible to use the spreadsheet as a supporting document, and link to it from the ticket, but do the actual authoring and discussion in the ticket? Would it work for small enough changes, like one notification type at a time? We could come up with a more complex system if we change everything at once again.

Checked in betalabs and compared to specs in V 2.0 Notifications -- Showing Updated Text and Links

@jmatazzoni
Notice - there is extra colon in Notifications (see the screenshot) after "Your user rights were changed."

Should be fixed or it's ok?
Colon is typically used after a complete sentence to clarify on what comes before colon.

My correction would be: a colon after complete sentence and no capitalization after colon, e.g.
"Your user rights were changed: you have been added to Reviewers and File movers."

  1. Added rights only

Your user rights were changed. You have been added to: Oversighters.

Screen Shot 2016-02-03 at 3.56.43 PM.png (236×609 px, 43 KB)

2, Removed rights only

Your user rights were changed. You are no longer a member of: Oversighters, Administrators.

Screen Shot 2016-02-03 at 3.57.24 PM.png (181×571 px, 32 KB)

  1. Removed and added rights.

Your user rights were changed. You have been added to: Oversighters. You are no longer a member of: X, Y, Z.

Screen Shot 2016-02-03 at 3.56.00 PM.png (173×701 px, 39 KB)

The fix has been merged and deployed to beta last night. I can't see when exactly your message was posted and when the fix was deployed. Would you mind testing again? :)

The language in the screenshots above, with the double colons, is definitely wrong and should be fixed.

However, @Etonkovidova is proposing a change to the approved language to make the messages more elegant. So, instead of (to pick the most complex example):

Your user rights were changed. You have been added to: Oversighters, File movers. You are no longer a member of: X, Y, Z.

She proposes:

Your user rights were changed: You have been added to Reviewers and File movers. You are no longer a member of X, Y, and Z.

Her proposed language is more idiomatic and isn't appreciably longer, but it requires some logic that wasn't part of previous versions of these messages to manage the insertion of the "and"s for the series when appropriate.

@SBisson, I think this is your call. If you have to go back into this anyway to fix the double colon issue cited above AND if adding the logic to handle managing the "and"s is trivial, then Elena's language is better and should be used. But if you've already fixed the double colons or there's anything tricky about adding the "and" logic, then I think we can call it a day and move on. How do you prefer to proceed?

@jmatazzoni, @Etonkovidova
I'm removing the colons in added to: Group1 and member of: Group1.

Notice - there is extra colon in Notifications (see the screenshot) after "Your user rights were changed."

Elena was also saying that Your user rights were changed: You have ... doesn't make sense because there's a colon followed by a capital letter. I don't know what to think about it. I've seen both being used. I generally avoid the problem and use a period when both parts are complete sentences.

A capital after the colon does appear to be Wikipedia style when the bit after the colon is a complete sentence, according to the WP Manual of Style.

However, I don't think that's exactly what Elena was suggesting. She was saying, I think, that we could eliminate the need for that colon before the series of groups by inserting commas and an "and," enabling the use of the colon elsewhere int he formula. (I.e., Your user rights were changed: You have been added to X, Y and Z.)

But that's beside the point. I am fine with sticking with the previous approved language, which, to be clear one last time, I hope, is this:

  • Your user rights were changed. You have been added to: Oversighters.
  • Your user rights were changed. You are no longer a member of: Oversighters, Administrators.
  • Your user rights were changed. You have been added to: Oversighters. You are no longer a member of: X, Y, Z.

(the capitalization shown in these examples for the group namess assumes that the group names are properly capitalized. I.e., it has nothing todo with the colons.) Whew!

A capital after the colon does appear to be Wikipedia style when the bit after the colon is a complete sentence, according to the WP Manual of Style.

However, I don't think that's exactly what Elena was suggesting. She was saying, I think, that we could eliminate the need for that colon before the series of groups by inserting commas and an "and," enabling the use of the colon elsewhere in the formula. (I.e., Your user rights were changed: You have been added to X, Y and Z.)

But that's beside the point. I am fine with sticking with the previous approved language, which, to be clear one last time, I hope, is this:

  • Your user rights were changed. You have been added to: Oversighters.
  • Your user rights were changed. You are no longer a member of: Oversighters, Administrators.
  • Your user rights were changed. You have been added to: Oversighters. You are no longer a member of: X, Y, Z.

(the capitalization shown in these examples for the group namess assumes that the group names are properly capitalized. I.e., it has nothing todo with the colons.) Whew!

Change 269117 had a related patch set uploaded (by Sbisson):
Fix 'user-rights' notification messages

https://gerrit.wikimedia.org/r/269117

Change 269117 merged by jenkins-bot:
Fix 'user-rights' notification messages

https://gerrit.wikimedia.org/r/269117

Notifications have been corrected.

Screen Shot 2016-02-08 at 1.27.30 PM.png (282×695 px, 61 KB)

Screen Shot 2016-02-08 at 1.41.05 PM.png (167×608 px, 30 KB)

Screen Shot 2016-02-08 at 2.20.31 PM.png (266×702 px, 54 KB)