Rename article_ and _(prefixed)text variables to page_ and _(prefixed)title
Open, NormalPublic

Description

This variable has a very confusing name given that context in which this variable is used (content filtering, editing, article_), the word "text" should refer to the page content, but alas, this variable refers to the page title.

Especially given the existence of wikitext, Revision::getText, text database table etc,.

So why is this variable named that way? It's originally named that way to distinguish itself from article_prefixedtext, which is named after the MediaWiki PHP code method Title::getPrefixedText at which point the word "title" unfortunately got lost, despite being the most important word in that signature.

Title::getPrefixedText itself is named that way to distinguish itself from Title::getText (which is the page name without the namespace prefix), and Title::getDBKey/Title::getPrefixedDBKey, which is a variant of the page title where spaces are replaced with underscores instead.

So all this where the word "text" means (textual form, as opposed to database form).

Proposing to rename these to page_title and page_prefixedtitle.

See also: https://www.mediawiki.org/wiki/Extension:AbuseFilter/Rules_format

We should consider renaming the ones used by move as well - and for all, we'll keep the old ones as aliases for back-compat.

CurrentNew
article_textpage_title
article_prefixedtextpage_prefixedtitle
article_articleidpage_id
article_restrictions_editpage_restrictions_edit
article_restrictions_movepage_restrictions_move
article_restrictions_createpage_restrictions_create
article_restrictions_uploadpage_restrictions_upload
article_recent_contributorspage_recent_contributors
article_first_contributorpage_first_contributor
moved_to_textmoved_to_title
moved_to_prefixedtextmoved_to_prefixedtitle
moved_to_articleidmoved_to_id
moved_from_textmoved_from_title
moved_from_prefixedtextmoved_from_prefixedtitle
moved_from_articleidmoved_from_id

Workflow

  1. Add aliases for AbuseFilter and extra logic needed (https://gerrit.wikimedia.org/r/#/c/412487/)
    1. Move all involved messages to the new ones on translatewiki https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/454658/
  2. Add tests for old and new variables to keep the situation under control (https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/454294/)
  3. Update docs on MWwiki for AF variables
  4. Make the changes in other extensions
    1. Flow: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Flow/+/454274/
    2. HitCounters: https://gerrit.wikimedia.org/r/#/c/454277/
    3. Move messages from both extensions on translatewiki
      1. HitCounters done with https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/HitCounters/+/456714/
      2. Flow done with https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Flow/+/458671/
  5. Update docs on MWwiki for Flow and HitCounters variables
  6. Remove old prefixes from other extensions
    1. ArticleFeedbackv5: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/ArticleFeedbackv5/+/456564/
    2. ContentTranslation: https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/ContentTranslation/+/456566/
  7. Remove extra logic from AbuseFilter (https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/454279/)
  8. Remove extra logic from Flow (https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Flow/+/454292/)
  9. Clearly show deprecated variables with Ace highlight (https://gerrit.wikimedia.org/r/#/c/442361/)
  10. Plans for further changes: T173889#4522869
There are a very large number of changes, so older changes are hidden. Show Older Changes

I'd be fine with that, let's wait for some other thoughts. I'll leave the patch as it is for some days, in order to exactly know what to do (and because I won't be able to work on it).

Huji added a comment.Feb 22 2018, 2:03 PM

That'd be a great idea, actually. And coming to think of it, it is best if we do all of these changes in one go.

@matej_suchanek I added you as a reviewer to the patch; can you please opine on how best to handle the Condition dropdown now that we have two of some variables.

Krinkle added a comment.EditedFeb 22 2018, 10:37 PM

For the record, yes: Renaming article_ to page_ should also be done. Thanks for spotting that.

I did notice the old article_ prefix, but didn't propose to change it because it would create an inconsistent interface for AbuseFilter editors, where some page-related fields start with page_ while other are still only named article_.

However, if we are okay with renaming the other article_ fields at the same time, then that's even better!

some page-related fields start with page_ while other are still only named article_.

Searching for "page_" yields nothing but page database columns for me. What exactly do you mean with that?

Huji renamed this task from Rename AbuseFilter var article_text to article_title to Rename AbuseFilter var article_text to page_title.Mar 9 2018, 2:00 PM
Huji updated the task description. (Show Details)
Daimona updated the task description. (Show Details)Mar 9 2018, 2:09 PM
Krinkle added a comment.EditedMar 13 2018, 12:57 AM

[..] some page-related fields start with page_ while other are still only named article_.

Searching for "page_" yields nothing but page database columns for me. What exactly do you mean with that?

The quoted sentence is incomplete, the original quote had a conditional context.

Rephrased:

If we are only changing "article_text", then we should name it "article_title", not "page_title". If we only change "article_text" to "page_title", then we would have some page-related fields as "article_", like "article_first_contributor", and some as "page_", like "page_title". That is not good.

My original proposal was small, and so I recommended changing to "article_title" to keep the migration smaller but still keep the prefix consistent.

But, if we can do both changes in one step, then there is no problem with using "page_title". All is good :)

@Krinkle the current patch replaces every single use of "article" and "text". Not exactly the small patch which was proposed at the beginning, but probably better than switching two times.

Huji added a comment.Mar 13 2018, 4:55 PM

@Krinkle in fact I encourage you to review https://gerrit.wikimedia.org/r/#/c/412487/ where you will see all article_ prefixes are replaced with page_ and some of the poorly chosen suffixes are also being fixed, all while maintaining backward comparability through aliases.

Together with the patch above, we need two other patches: one in Extension:HitCounter for the article_views variable, and one in Extension:Flow for a couple of variables. However, the transition won't be as smooth because we don't have a nice way to emit warning and/or add aliases without duplicating menus ecc. We could add dedicated logic for it, or just check the amount of filters using such variables and switching silently without transition if there are really few of them.

Change 454274 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/Flow@master] Use new syntax for AbuseFilter variables and deprecate the old ones

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

Another thing: we'll need to move old messages to the new ones on translatewiki. @Nikerabbit, @Raymond could someone of you please do that as the above patches will be merged (once for each patch)?

Change 454277 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/HitCounters@master] Use new syntax for AbuseFilter variables and deprecate the old ones

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

Change 454279 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Remove workaround to complete phase 1 of variables migration

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

Change 454292 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/Flow@master] Remove superfluos parameter from AbuseFilter call

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

Change 454294 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add deprecated variables to PHPUnit tests

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

Daimona renamed this task from Rename AbuseFilter var article_text to page_title to Rename article_ and _(prefixed)text variables to page_ and _(prefixed)title.Aug 22 2018, 8:48 AM
Daimona updated the task description. (Show Details)
Huji added a comment.Aug 22 2018, 12:10 PM

I merged the first two patches. We are now in Phase I. Next step is to identify all filters that use the deprecated variable and ask the wikis to update the filters, right Daimona?

Well, we are in phase I only for AF's own variables: other extensions' ones still use the old pref/suffix. To complete this phase, we should first merge everything in the "workflow" section of the task description. At that point, we may ask for a replacement. This should be done in Tech/News, encouraging filter editors to make the change (and the Ace highlighting will make it easy to spot deprecated stuff). Then, we should probably wait a few weeks to see how many filters are still using the old syntax, and decide what to do with them. We may either wait some more time, decide to never remove old variables, or finish the job ourselves.

Daimona updated the task description. (Show Details)Aug 22 2018, 12:32 PM

Change 412487 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add aliases for "_text" and "article_" variables

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

Change 454294 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add deprecated variables to PHPUnit tests

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

Now AbuseFilter messages changed here can be moved for every language.

Now AbuseFilter messages changed here can be moved for every language.

Standard process for changed message keys: Moving will be done by me this night on translatewiki and the new messages will be committed back to Gerrit this night too.

Huji added a comment.EditedAug 22 2018, 3:14 PM

Thanks @Raymond

@Daimona can I ask you to work on the first user notice about this? And also to update the documentation the MW wiki?

Huji updated the task description. (Show Details)Aug 22 2018, 3:18 PM
Huji updated the task description. (Show Details)Aug 22 2018, 3:24 PM

Thanks @Raymond

@Daimona can I ask you to work on the first user notice about this? And also to update the documentation the MW wiki?

Sure! First I'll update the docs, then I'll write the user notice. However, the docs should be updated for external variables as well (adding a note to task description for it)

Daimona updated the task description. (Show Details)Aug 22 2018, 3:28 PM

I updated the docs. Could someone please doublecheck? All those lines confused me so there might be some copy/pasting issue there :-)

And stubbed an announcement for tech/news; again, could someone please check it?

Raymond updated the task description. (Show Details)Aug 22 2018, 8:43 PM
Daimona added a comment.EditedAug 22 2018, 8:50 PM

@Raymond it seems that the article-prefixedtext message got deleted for all languages instesd of being moved to page-prefixedtitle. Could you please check?

Huji updated the task description. (Show Details)Aug 22 2018, 9:41 PM

@Raymond it seems that the article-prefixedtext message got deleted for all languages instesd of being moved to page-prefixedtitle. Could you please check?

Ups, I made a mistake: Text replacement - "Abusefilter-edit-builder-vars-article-prefixedtext" to "busefilter-edit-builder-vars-page-prefixedtitle"

Fixed today with Text replacement - "Busefilter-edit-builder-vars-page-prefixedtitle" to "Abusefilter-edit-builder-vars-page-prefixedtitle". Commit to Gerrit follows this night with the next export.

Many thanks :-) I'll let you know when other messages will be ready to be moved.

Daimona updated the task description. (Show Details)Aug 23 2018, 10:25 AM

Change 455356 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Generate upload variables using new prefixes

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

Change 455356 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Generate upload variables using new prefixes

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

While it looks like it was declined(?) before in T54053, none of the moved_to and moved_from protection values are actually being populated, are these deprecated and should just be removed? Example: https://test.wikipedia.org/wiki/Special:AbuseFilter/examine/398011

@Xaosflux I can't reproduce this. See for instance this entry for moved_from_restrictions_edit. Am I missing something?
Also, just a note: don't get confused by the empty field. Those values are actually false and there's already a patch to make them show as what they really are.

@Daimona thank you for the update, I'll keep an eye on T190653 for resolution of the "false" --> value fix

Change 456089 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add full tests for deprecated variables

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

May we please get a review for Flow and HitCounters patches? We'd better keep this transition period short and remove unnecessary parts from AbuseFilter code.

Restricted Application added a project: Growth-Team. · View Herald TranscriptAug 30 2018, 2:54 PM
Daimona raised the priority of this task from Low to Normal.Aug 30 2018, 2:54 PM

Change 454277 merged by jenkins-bot:
[mediawiki/extensions/HitCounters@master] Use new syntax for AbuseFilter variables and deprecate the old ones

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

Daimona updated the task description. (Show Details)Aug 31 2018, 7:10 AM

@Raymond the abusefilter-edit-builder-vars-article-views message from HitCounters can now be moved

Change 456564 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/ArticleFeedbackv5@master] Use the right prefix for AbuseFilter variables!

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

Change 456566 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/ContentTranslation@master] Use the right prefix for AbuseFilter variables!

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

Daimona updated the task description. (Show Details)Aug 31 2018, 7:50 AM

Change 456566 merged by jenkins-bot:
[mediawiki/extensions/ContentTranslation@master] Use the right prefix for AbuseFilter variables!

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

Daimona updated the task description. (Show Details)Sep 1 2018, 9:52 AM
Raymond updated the task description. (Show Details)Sep 1 2018, 9:57 AM
Daimona raised the priority of this task from Normal to High.Sep 5 2018, 4:31 PM

This transition phase was envisioned to take only a wmf.xx branch. It's already taking two, and we cannot make it slip to 1.33. It's true that we still have ~50 days for it, but there may be unforeseen outcomes for which we may need time, and backporting should be the last resort.

Huji updated the task description. (Show Details)Sep 5 2018, 5:53 PM

I agree with elevating priority here. I am personally busy with RL matters, or else would have spent more time on this. I hope I will become more available in about two weeks; but it should not wait until then.

That said, are we only waiting for patches related to items 4A, 7 and 8 in the task definition? (With 4A being the most urgent one?) Are there any maintenance scripts to be run at any point? Or any "double-check" queries to be done before merging steps 7 and 8?

Change 456564 merged by jenkins-bot:
[mediawiki/extensions/ArticleFeedbackv5@master] Use the right prefix for AbuseFilter variables!

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

@Huji no problem, of course :-) You are correct, we need 4A (the only urgent thing to do), 7 and 8, plus an i18n change (4C) and updating Flow variables docs (5). After 4A (which, I repeat, is a really urgent one) we can safely proceed with points 7 and 8, without any query or maintenance script (this codesearch is enough).

I have +2ed both 4A (which should be merged by Jenkins soon) and 8 (which won't be merged yet because it Depends-On an unmerged patch).

Change 454274 merged by jenkins-bot:
[mediawiki/extensions/Flow@master] Use new syntax for AbuseFilter variables and deprecate the old ones

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

Daimona updated the task description. (Show Details)EditedSep 7 2018, 7:41 AM
Daimona lowered the priority of this task from High to Normal.

@Raymond the last batch of messages (these ones) can now be moved. Many thanks again for your help!

Woah, already done tomorrow morning, I see. Thanks!

Daimona updated the task description. (Show Details)Sep 7 2018, 8:35 AM
SBisson moved this task from Inbox to External on the Growth-Team board.Sep 7 2018, 3:36 PM

Change 456089 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add full tests for deprecated variables

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

Change 467979 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@REL1_32] Remove workaround to complete phase 1 of variables migration

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

Change 467981 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/Flow@REL1_32] Remove superfluous parameter from AbuseFilter call

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