Maniphest T203013

Page previews should use mw.user.getPageviewToken()
Closed, ResolvedPublic2 Estimated Story Points
Actions

Description

Now we have a standard way of accessing page tokens, we should use that everywhere in the schemas we make use of.

Acceptance criteria

Replace usage of mw.user.generateRandomSessionId for accessing a page token with mw.user.getPageviewToken
- pageToken is set here
When refactoring, please keep in mind, that in Page previews repo, when we create a pageInteractionToken -> this token has to be unique for each preview. We cannot use the same token if the user dwells over the same link twice. Please DOCUMENT this in page previews codebase (Popups extension) as it's not correctly documented. The goal is to distinguish between two hovers on the same link.

Note: be careful as page previews also has a concept of a linkInteractionToken for which there is no shared/well-defined API.

Details

	Subject	Repo	Branch	Lines +/-
	Doc: add linkInteractionToken description	mediawiki/extensions/Popups	master	+5 -1
	Use getPageviewToken api	mediawiki/extensions/Popups	master	+9 -13

Customize query in gerrit

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Resolved		Jdlrobson	T203013 Page previews should use mw.user.getPageviewToken()
		Resolved		• Niedzielski	T203522 Popups php unit tests mysteriously failing and blocking merges

Event Timeline

Jdlrobson created this task.Aug 28 2018, 5:51 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 28 2018, 5:51 PM

Change 454435 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/Popups@master] Use getPageviewToken api

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

gerritbot added a project: Patch-For-Review.Aug 28 2018, 5:51 PM

Jdlrobson moved this task from Incoming to Triaged but Future on the Web-Team-Backlog board.Aug 28 2018, 5:51 PM

Jdlrobson moved this task from Triaged but Future to Upcoming on the Web-Team-Backlog board.Aug 28 2018, 9:37 PM

ovasileva triaged this task as Medium priority.Aug 29 2018, 4:02 PM

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

Note: be careful as page previews also has a concept of a linkInteractionToken for which there is no shared/well-defined API.

As I said during today's prioritization ritual:

The link interaction token (the "LIT") is used to identify a unique link interaction (the user dwelling on a link with their mouse) in the EventLogging instrumentation and we use mw.user.generateRandomSessionId to generate them.

In all of the analysis of the data that I've seen thus far, we've combined LITs with a token that uniquely identifies the pageview (the pageToken property of a Popups event). Consequently, LITs only have to be unique per pageview and not, say, globally unique. Perhaps, @Tbayer can confirm this in case I've either missed something or misunderstood.

Given that the entropy of mw.user.generateRandomSessionId was doubled recently, I think we can safely continue relying on it to generate LITs.

However, you should make sure to update the use of mw.user.generateRandomSessionId here.

phuedx updated the task description. (Show Details)Aug 29 2018, 5:28 PM

phuedx added a subscriber: • Tbayer.Aug 29 2018, 5:32 PM

In T203013#4542988, @phuedx wrote:

The link interaction token (the "LIT") is used to identify a unique link interaction (the user dwelling on a link with their mouse) in the EventLogging instrumentation and we use mw.user.generateRandomSessionId to generate them.

In all of the analysis of the data that I've seen thus far, we've combined LITs with a token that uniquely identifies the pageview (the pageToken property of a Popups event). Consequently, LITs only have to be unique per pageview and not, say, globally unique. Perhaps, @Tbayer can confirm this in case I've either missed something or misunderstood.

I'm not sure I understand the use case for the LIT. Every event is its own entry in the EventLogging database. If a globally unique ID is added to every beacon, and that ID is not meant to correlate with anything else, what purpose does it serve?

Note that the EventLogging databases already have a primary key for the storage of events. Every event beacon received and processed by an EventLogging server is given a UUID (see EventCapsule). This is globally unique and follows a format with much stronger guarantees than the random generator we user client-side (UUID is 128bit with both server identify and and datetime embedded, in addition to rand; compared to just 32bit or 80bit pure rand from the client).

Also, if all we need is to differentiate a handful of events beaconed from the same page view, we may want to consider a JavaScript number that increments. Would that suffice?

In T203013#4543477, @Krinkle wrote:

Also, if all we need is to differentiate a handful of events beaconed from the same page view, we may want to consider a JavaScript number that increments. Would that suffice?

I was thinking that as I wrote my comment above.

Jdlrobson updated the task description. (Show Details)Sep 4 2018, 5:11 PM

ovasileva edited projects, added Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q1); removed Web-Team-Backlog.Sep 4 2018, 5:11 PM

Jdlrobson mentioned this in T201124: Provide standard/reproducible way to access a PageToken.Sep 4 2018, 5:13 PM

Jdlrobson moved this task from To Do to Needs Code Review on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q1) board.

LGTM, ready for merge pending commit dependency on T202748.

Change 457988 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/Popups@master] Doc: add linkInteractionToken description

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