Page MenuHomePhabricator

[Wikistats2] The detail page for tops and maps metrics does not indicate time range
Closed, ResolvedPublic3 Story Points

Description

The detail page for tops metrics: https://stats.wikimedia.org/v2/#/all-projects/reading/pageviews-by-country doesn't display at any point the time interval that the data belongs to.
Sometimes, the user comes from the Dashboard and has that information already, but it would be important to show that in the detail page as well.
The other metrics, have the visual support of the line or bar charts that indicate the time span, but the tops metrics do not.
We should fix that :]

Details

Related Gerrit Patches:

Event Timeline

mforns created this task.Dec 15 2017, 2:38 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 15 2017, 2:38 PM
fdans moved this task from Wikistats Production to Backlog (Later) on the Analytics board.
fdans edited projects, added Analytics-Kanban; removed Analytics.Jan 8 2018, 4:52 PM
Nuria updated the task description. (Show Details)Feb 24 2018, 10:52 PM
mforns added a subscriber: Nuria.

From duplicate task:

"Pageviews by Country Monthly" should specify month in question
Right now on the map for pageviews per country the title does not give you any frame of reference as to the month you are seeing pageviews for, in other graphas that frame is provided
by the axis, in this case i think a more specific title is needed.

mforns renamed this task from [Wikistats2] The detail page for tops metrics does not indicate time range to [Wikistats2] The detail page for tops and maps metrics does not indicate time range.Mar 8 2018, 1:06 PM
mforns edited projects, added Analytics; removed Analytics-Kanban.Mar 8 2018, 6:07 PM
mforns moved this task from Backlog (Later) to Mentoring on the Analytics board.
sahil505 claimed this task.Mar 29 2018, 4:15 PM
This comment was removed by Amitjoki.

Change 423144 had a related patch set uploaded (by Amitjoki; owner: Amitjoki):
[analytics/wikistats2@master] Add time period for maps and tops metrics

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

@Amitjoki I admire your enthusiasm, but whenever you take a task please share it in comments or claim to work on the task. I was about to submit a patch for the same.

@sahil505 don't know. I thought claiming task was akin to issues "Assigned" in Github. Usually if a person is assigned a issue, only his work will be considered right? Whatever additional done is supplementary, that was my rationale.

I started working long before and thought it would do any harm if I submitted my patch too, thinking only your patch would be considered. I just uploaded my patch because I didn't want to waste my changes.

If you've a better approach for solving the problem, please do upload them. I'll be more than happy to consider abandoning my changes if the need be :]

I'll also remember to comment if I am working on the same problem the next time :)

Nuria added a comment.Mar 30 2018, 3:30 PM

@Amitjoki I think you need to rework your patch a bit, please take a look at code on dashboard that already prints current month for top metrics.

https://github.com/wikimedia/analytics-wikistats2/blob/master/src/components/dashboard/MetricWidget.vue#L19

  • data displayed on maps is only for the last month, your already have that data. See:

https://github.com/wikimedia/analytics-wikistats2/blob/master/src/components/detail/GraphPanel.vue#L161

@Amitjoki No worries please go through with it. Although, I did take a look at your code and I don't think you need to add new methods for this issues, and also as far as I have understood you don't need to display the year as well. As @Nuria correctly mentioned above you can take an example from Dashboard list of metrics. While you are at it so take a look at https://github.com/wikimedia/analytics-wikistats2/blob/master/src/components/dashboard/MetricListWidget.vue#L38, this might reduce your code even more. Try to extract information from already computed functions. For better understanding of the description please also look at T187389, you might be able to relate to this description more. Thanks.

@sahil505 thanks for the information. I've reduced the code as per your @Nuria suggestions :]

Nuria added a comment.Mar 30 2018, 5:56 PM

I have modified commit message and submitted a patch. Can you add a screenshot to this ticket with changes we should see from your patch? Thanks

Nuria added a comment.EditedMar 30 2018, 6:53 PM

I see where is the confusion, the legend should not be changed, it will not change per month rather header of page should change.

sahil505 added a comment.EditedMar 30 2018, 7:05 PM

@Amitjoki Maybe we can find a better placement for displaying the month in top metric table. Dedicating an entire row just to display month feels like not the best design practice? What do you think? @Nuria What are your views?

Nuria added a comment.EditedMar 30 2018, 7:11 PM

Maybe this visual explanation makes more sense.

@Nuria yeah. That makes sense. I've updated my changes. Here's the preview: https://www.useloom.com/share/9dbf7ff90a8c48b499a1053ffb520e0b Thanks :]

sahil505 added a comment.EditedApr 1 2018, 4:49 AM

Some suggestions I have, correct me if I'm wrong :)

  • 'for February' makes more sense than 'on February', maybe?
  • I feel like month has nothing to do with the page views hyperlink.
  • Maybe just displaying February would suffice, because it automatically implies that the data is monthly

@sahil505 yes, I too felt weird about the first two points. We could display "February" in place of "Monthly", that would make more sense. Will be submitting a patch for the same :]

Here's the newer version which I think is more pragmatic: https://www.useloom.com/share/ea829701049a46ab886caf02bd7269f7

Nuria added a comment.Apr 2 2018, 4:31 PM

'for February' makes more sense than 'on February', maybe?

The copy used in the dashboard is "Countries with the most views for March" So "Pageviews by Country For March" makes sense.

I feel like month has nothing to do with the page views hyperlink.
Maybe just displaying February would suffice, because it automatically implies that the data is monthly

Agreed. The "monthly" after the new copy should be removed.

mforns added a comment.Apr 3 2018, 2:08 PM

Please, @Amitjoki, do assign yourself to tasks you're working on.
Even if anyone can contribute to a currently open task, it's good to know that someone else is working on it.
This way you can decide to wait for their changes to appear on Gerrit, and then work on top of them; or rather work on another task.
It also avoids that 2 people work on the same thing twice, it reduces overall effort.
It really has no other meaning.

@sahil505 please feel free to submit your changes in a separate Gerrit patch, for this time.
Otherwise, you can work on top of Amil's change or work on another task.

@mforns : No worries at all :-) I'll find other tasks to work on.
Removing myself from 'Assigned To'.

sahil505 removed sahil505 as the assignee of this task.Apr 3 2018, 4:20 PM

@sahil505 and @mforns sorry! I'll make sure to let it be known if I am working on a task. Sorry for the inconvenience.

Amitjoki claimed this task.Apr 3 2018, 4:45 PM

Change 423144 merged by Mforns:
[analytics/wikistats2@master] Label map and top metrics with the month they belong to

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

mforns edited projects, added Analytics-Kanban; removed Analytics.Apr 11 2018, 3:56 PM
mforns set the point value for this task to 3.
mforns moved this task from Next Up to Ready to Deploy on the Analytics-Kanban board.
fdans moved this task from Ready to Deploy to Done on the Analytics-Kanban board.Apr 16 2018, 4:40 PM
Nuria closed this task as Resolved.Apr 17 2018, 2:58 AM