Page MenuHomePhabricator

sectionNumbers and issuesSeverity should always be consistent in length
Closed, ResolvedPublic5 Estimated Story Points

Description

There are 2 remaining cases where sectionNumbers and issuesSeverity are of different length for pageLoaded and editClicked events.

Two issues inside the lead (no multiple issues)

Examples

https://reading-web-staging.wmflabs.org/w/index.php?title=Four_Horsemen_(drink)
https://en.wikipedia.org/wiki/Endurance_Kart_Championship

Visual

Screen Shot 2018-08-28 at 4.50.14 PM.png (268×1 px, 55 KB)

Current behaviour

sectionNumbers: [0] issuesSeverity: [HIGH, HIGH]

Expected behavior

we should log

sectionNumbers: [0, 0] issuesSeverity: [HIGH, HIGH]

Multiple issues template

Examples

Can be found here: https://en.wikipedia.org/w/index.php?title=Special%3AWhatLinksHere&target=Template%3AMultiple+issues&namespace=0
e.g.
https://reading-web-staging.wmflabs.org/w/index.php?title=Politics_of_Cyprus

Visual

Screen Shot 2018-08-28 at 5.00.10 PM.png (210×1 px, 42 KB)

Current behaviour

{ sectionNumbers: [0], issuesSeverity: [MEDIUM, MEDIUM] }

Expected behavior

we should log

{ sectionNumbers: [0], issuesSeverity: [MEDIUM] }

where MEDIUM is the maximum severity level.

combining both above

Example

https://reading-web-staging.wmflabs.org/w/index.php?title=Altitude_latitude_theory

Visual

Screen Shot 2018-08-28 at 4.54.41 PM.png (293×1 px, 64 KB)

Current behaviour

{ sectionNumbers: [0], issuesSeverity: ["HIGH", "MEDIUM", "LOW", "MEDIUM"]  }

Expected behavior

we should log

{ sectionNumbers: [0], issuesSeverity: [HIGH, MEDIUM] }

where MEDIUM is the maximum severity level of the multiple issues template and HIGH relates to the other box

Acceptance criteria

  • Behaviour is consistent between group A and group B
  • Clicking edit should log sectionNumbers and issuesSeverity per definition above
  • pageLoaded event should log sectionNumbers and issuesSeverity per definition above

Developer notes

getAllIssuesSections is the problematic function

/**
 * Returns an array of all the page sections that have issues.
 * @return {array}
 */
function getAllIssuesSections() {
        return Object.keys( allIssues ).filter( function ( section ) {
                return allIssues[ section ].length;
        } );
}

This will need to be modified. If allIssues[section] is length 2, we should return 2 section ids.
However, allIssues is also used to track all issues collapsed/not-collapsed.
We'll probably have to update the IssueSummary typedef to include a hidden field which will be set for issues hidden under multiple issues tag.

This is not going to be a trivial change.

Event Timeline

And for reference, this implements the following part of the schema description: "This array [sectionNumbers] should be consistent with severities, except on modalClose". Basically the n-th element in each array should correspond to the same issues template on the page, with special treatment for multiple issues templates as detailed in the schema description.

Change 456104 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Consistent sectionNumbers and issuesSeverity for page-issues.

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

The first patch above addresses the first scenario, multiple issues without the "multiple issues" template.
Tested on "List_of_residents_of_10_Downing_Street", "Four_Horsemen_(drink)" and "Roman_Catholic_Diocese_of_Darwin" articles. For groups A and B,
the pageLoaded and editClicked events return the following data:

Four_Horsemen_(drink) - pageLoaded and editClicked, groups A and B

"issuesSeverity":["HIGH","MEDIUM"],"sectionNumbers":[0,0]

List_of_residents_of_10_Downing_Street - pageLoaded and editClicked, groups A and B

"issuesSeverity":["LOW","LOW","MEDIUM"],"sectionNumbers":[0,0,0]

Roman_Catholic_Diocese_of_Darwin - pageLoaded and editClicked, groups B (new) - not group A (old)!*

"issuesSeverity":["MEDIUM","MEDIUM"],"sectionNumbers":[1,4]

*the reason group A doesn't return the same values as group B on the last article is that the old page-issues banner doesn't show up if issues aren't in the lead section :( (maybe a separate task for that?)

For scenario A
Two issues inside the lead (no multiple issues)
issue #1 high severity
issue #2 high severity

sectionNumbers: [0, 0] issuesSeverity: [HIGH, HIGH]

To be compatible with scenario C mixed 'multi-template' and non-multi-template as in "Altitude_latitude_theory"
issue #1 high severity
issue #2 (multiple) medium severity, low severity, low severity
I think the output has to be

sectionNumbers: [0, 0] issuesSeverity: [HIGH, MEDIUM]

instead of

sectionNumbers: [0], issuesSeverity: [HIGH, MEDIUM]

as in the description.

Change 456132 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] For page-issues pageLoaded and editClicked events, treat "multiple issues" templates as one issue.

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

ovasileva set the point value for this task to 5.Aug 29 2018, 4:54 PM

Change 456104 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Consistent sectionNumbers and issuesSeverity for page-issues.

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

Change 456132 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] For page-issues pageLoaded and editClicked events, treat "multiple issues" templates as one issue.

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

After reviewing with @Tbayer today, we noticed one error regarding the multiple issues template.
On the page Politics_of_Cyprus, which has a multiple issues template, the modalClosed event sends

"issuesSeverity":["MEDIUM","MEDIUM"],"sectionNumbers":[0]

Instead, there should be a section number corresponding to each issue in the modal.

"issuesSeverity":["MEDIUM","MEDIUM"],"sectionNumbers":[0,0]

Although it's kind of a given that the issues in modal are in the same section, we should still be explicit about this in the data.

Change 456764 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] page-issues sectionNumbers consistency on modalClose event

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

Change 458281 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Correct the link for page issues in the old treatment.

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

Change 456764 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] page-issues sectionNumbers consistency on modalClose event

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

Change 458281 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Correct the link for page issues in the old treatment.

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

In the old treatment, on https://reading-web-staging.wmflabs.org/w/index.php?title=Rover_(lawnmower)

Rover has 1 multiple issues box (containing 2 issues) and 1 deletion notice.
The pageLoaded event correctly identifies this to be the case.

However clicking "Page issues" triggers an event "issuesClicked" which has 2 sectionNumbers and 1 severity. When clicking the length should only be 1.

The modalClosed event works correctly showing the severities and sectionNumbers of all 3 issues in the page

Change 458619 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Issues instrumentation: issuesClicked events should always send 1 sectionNumber

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

Change 458619 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Issues instrumentation: issuesClicked events should always send 1 sectionNumber

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

I ran through this with @Tbayer @Jdrewniak and @Niedzielski and this appears to be implemented as expected
I've added some exploratory QA steps to T191532 to cover this scenario further.