Page MenuHomePhabricator

[betalabs] Mentorship pages issues
Open, MediumPublicBUG REPORT

Description

DONE (1) Special:MentorDashboard error message on updating "Number of mentees assigned to me".

  • On enwiki betalabs or cswiki betalabs go to Special:MentorDashboard and change "Number of mentees assigned to me"
  • The confirmation message will display the raw message -
    Screen Shot 2022-08-05 at 2.47.28 PM.png (336×686 px, 34 KB)

(2) enwiki betalabs and testwiki wmf.25: Special:EnrollAsMentor displays "Introduction message is too long." for message of any length:

incorrect_length2.gif (431×1 px, 94 KB)

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Thanks @Etonkovidova. Both en.wikipedia issues are caused by the same underlying bug. Dev console's Networking tab shows the API response:

{
  "error": {
    "code": "growthexperiments-mentor-writer-error-message-too-long",
    "info": "Introduction message is too long. Maximum length is 240.",
    "*": "See https://en.wikipedia.beta.wmflabs.org/w/api.php for API usage. Subscribe to the mediawiki-api-announce mailing list at <https://lists.wikimedia.org/postorius/lists/mediawiki-api-announce.lists.wikimedia.org/> for notice of API deprecations and breaking changes."
  },
  "servedby": "deployment-mediawiki11"
}

I cannot reproduce this at beta cs.wikipedia. @Etonkovidova Can you please have a look? Perhaps it was only a temporary issue.


In addition to what I said above, the growthexperiments-mentor-dashboard-mentor-tools-mentor-weight-error-unknown message is not declared. That's definitely an issue: I'll upload a patch to fix that.

Change 822338 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] Declare missing i18n message

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

Urbanecm_WMF triaged this task as Medium priority.
Urbanecm_WMF changed the task status from Open to In Progress.Thu, Aug 11, 11:05 AM

Thanks @Etonkovidova. Both en.wikipedia issues are caused by the same underlying bug. Dev console's Networking tab shows the API response:

{
  "error": {
    "code": "growthexperiments-mentor-writer-error-message-too-long",
    "info": "Introduction message is too long. Maximum length is 240.",
    "*": "See https://en.wikipedia.beta.wmflabs.org/w/api.php for API usage. Subscribe to the mediawiki-api-announce mailing list at <https://lists.wikimedia.org/postorius/lists/mediawiki-api-announce.lists.wikimedia.org/> for notice of API deprecations and breaking changes."
  },
  "servedby": "deployment-mediawiki11"
}

At beta en.wikipedia, this happens because the current validation messages looks for any messages that are too long, not only the message relevant to the current user. That's an unfortunate disadvantage of putting the validation logic into the validator :/. @Tgr Do you have any opinion here? Should we pass the user identity all the way down to StructuredMentorListValidator? Or ignore the problem, as it has no way to happen assuming T312576: Structured mentor list should validate the maximum length of a mentor message is properly fixed? Personally, I vote for the let's ignore it option, as this issue should be pretty much nonexistent after the validation.

Optionally, we can also fail on warnings in ConfigHooks::onEditFilterMergedContent, which would make it impossiblehard for an admin to sneak in a message that's too long.

I manually truncated the messages at en.wikipedia beta by editing MediaWiki:GrowthMentors.json.

I think it would be nice to mention the affected user name in the error; if the problem does occur somehow, that will make it easy for the wiki admins to understand and fix what is wrong. Other than that, I would ignore it.

And yeah, EditFilterMergedContent should be strict about warnings - they basically mean "don't allow introducing this but don't fail if it's already present".

I think it would be nice to mention the affected user name in the error; if the problem does occur somehow, that will make it easy for the wiki admins to understand and fix what is wrong. Other than that, I would ignore it.

This was originally my plan, but then I figured the message is usually displayed in Special:EnrollAsMentor (or the Mentor dashboard), rather than when someone edits the list manually. Should we make the validator aware of who the current user is, so it can switch "User X's message is too long" and "Your message is too long" as appropriate? Or is there a better way of doing that?

And yeah, EditFilterMergedContent should be strict about warnings - they basically mean "don't allow introducing this but don't fail if it's already present".

Thanks. I filled T315097: Community configuration: Warnings should prevent saving the configuration page for that (and uploaded a patch while I'm on it).

Thanks @Etonkovidova. Both en.wikipedia issues are caused by the same underlying bug. Dev console's Networking tab shows the API response:

{
  "error": {
    "code": "growthexperiments-mentor-writer-error-message-too-long",
    "info": "Introduction message is too long. Maximum length is 240.",
    "*": "See https://en.wikipedia.beta.wmflabs.org/w/api.php for API usage. Subscribe to the mediawiki-api-announce mailing list at <https://lists.wikimedia.org/postorius/lists/mediawiki-api-announce.lists.wikimedia.org/> for notice of API deprecations and breaking changes."
  },
  "servedby": "deployment-mediawiki11"
}

I cannot reproduce this at beta cs.wikipedia. @Etonkovidova Can you please have a look? Perhaps it was only a temporary issue.

The issue was only on enwiki betalabs.

I checked betalabs - looks that both issues are fixed.

Thanks for reviewing, @Etonkovidova. I checked beta cswiki as well, because
the description says "On enwiki betalabs or cswiki betalabs go to
Special:MentorDashboard and change "Number of mentees assigned to me"". If
it was never present at cswiki, then I think this should be all-solved once
the pending patch
is merged.

Moving to code review, assuming there is no other bug at cswiki to fix.

This was originally my plan, but then I figured the message is usually displayed in Special:EnrollAsMentor (or the Mentor dashboard), rather than when someone edits the list manually. Should we make the validator aware of who the current user is, so it can switch "User X's message is too long" and "Your message is too long" as appropriate? Or is there a better way of doing that?

I would pass the user's name as a message parameter (which doesn't require changes to the validator other than keeping track of the array key), and discard "Your message is too long" and just use errors from the validator as they are. They will be a bit awkwardly phrased for EnrollAsMentor, but users can only see them if they work around the frontend length validation somehow, so that's fine; and they are still specific enough to allow figuring out what's wrong.

Change 822338 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Declare missing i18n message

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

Urbanecm_WMF changed the task status from In Progress to Open.Tue, Aug 16, 6:08 AM

Change 824151 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] StructuredMentorListValidator: Include mentor's name in too long message

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

This was originally my plan, but then I figured the message is usually displayed in Special:EnrollAsMentor (or the Mentor dashboard), rather than when someone edits the list manually. Should we make the validator aware of who the current user is, so it can switch "User X's message is too long" and "Your message is too long" as appropriate? Or is there a better way of doing that?

I would pass the user's name as a message parameter (which doesn't require changes to the validator other than keeping track of the array key), and discard "Your message is too long" and just use errors from the validator as they are. They will be a bit awkwardly phrased for EnrollAsMentor, but users can only see them if they work around the frontend length validation somehow, so that's fine; and they are still specific enough to allow figuring out what's wrong.

Done. Moving back to code review.