Since the infobox (our name for the accordion) loads ip information if it can on page load, some changes should be made to when that information is loaded so that we can log on-load (as opposed to downstream on user action). The user doesn't necessarily have to see the information (eg. if they keep the accordion closed) so part of the goal of this proposal is to reduce conflating intent vs accidental exposure. I propose:

• No longer loading on-load in all cases
• If the user has the accordion preference set to 'open', the widget should check for that and load if so
• if the user has no preference or the preference set to 'closed', then the widget should not load
• only when the user opens the accordion should the ip info load

That way, we can log on every ip info api GET and know that, roughly, this corresponds to the user viewing ip information.

In T294658#7469482, @STran wrote:

That way, we can log on every ip info api GET and know that, roughly, this corresponds to the user viewing ip information.

A hearty +1 to this point. We shouldn't fool ourselves into thinking that our UI will be the only way that our API is used – a user could create a user script and/or gadget, for example.

When we say "log", do we mean a Special:Log log or something on the backend? That's not obvious to me as a wiki user. (I have no opinion on which would be preferred and I don't know that has shown up in discussions in consultation.)

In T294658#7472395, @Izno wrote:

When we say "log", do we mean a Special:Log log or something on the backend? That's not obvious to me as a wiki user. (I have no opinion on which would be preferred and I don't know that has shown up in discussions in consultation.)

@Izno: Thanks for the clarifying question. Based on the outcome of T263756: Create a table to store which users have access to IPInfo, and the timestamp when access was granted [L], we mean Special:Log.

Change 736267 had a related patch set uploaded (by Phuedx; author: Phuedx):

[mediawiki/extensions/IPInfo@master] WIP: Add IPInfo log to Special:Log

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

Change 748184 had a related patch set uploaded (by STran; author: STran):

[mediawiki/extensions/IPInfo@master] [WIP] Add access logging

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

Hm, it occurs to me that whether or not we load from the infobox or the popup, we pass along the same full set of information from the API. Would it be better then to log anytime anyone access the REST point? As @phuedx pointed out:

We shouldn't fool ourselves into thinking that our UI will be the only way that our API is used – a user could create a user script and/or gadget, for example.

I've uploaded a proof of concept as to how this could work (which also takes into account my concerns around conflating intent vs accidental exposure see: T294658#7469482)
cc @Prtksxna and maybe @Tchanders?

@STran I'm not sure I understand this. Would it mean that that a popup view (T294657) and an accordion view would be treated as the same thing?

Could these be considered two different calls one which has limited information and the other which has the full info? If they choose to, gadgets and tools could then decide which one they need.

@Prtksxna

In T294658#7593510, @Prtksxna wrote:

@STran I'm not sure I understand this. Would it mean that that a popup view (T294657) and an accordion view would be treated as the same thing?

As far as "access to data" goes, yes. For instance, if I get this popup:

There's nothing stopping me from investigating the data object that got passed along, which contains more information:

Could these be considered two different calls one which has limited information and the other which has the full info? If they choose to, gadgets and tools could then decide which one they need.

They could be but I think that's a new ticket. I'm happy to write that up if that's what we want to go with. To confirm though, does this mean we're logging with the intent to track access?

In T294658#7603503, @STran wrote:

To confirm though, does this mean we're logging with the intent to track access?

This was my understanding (though debouncing the log entries for a viewing the popup and the infobox makes this a little fuzzy).

/cc @Niharika

In T294658#7603503, @STran wrote:

They could be but I think that's a new ticket. I'm happy to write that up if that's what we want to go with.

Yep, that is what we want to go with. You can create the new tickets, thanks!

The distinction between logging infobox and popup access isn't the same now that logging is done via the job. From the patch on this task:

$this->jobQueueGroup->push(
	new JobSpecification(
		'ipinfoLogIPInfoAccess',
		[
			'performer' => $user->getName(),
			'ip' => $author->getName(),
			'dataContext' => $dataContext
		],
		[],
		null
	)
);

The data context is what determines whether the job calls logViewInfobox or logViewPopup.

So separating this task out from T294657: Create a log entry when the LogHandler is called no longer makes sense. We could re-purpose them as logging via the RevisionHandler (this task) vs the LogHandler. Or we could merge the tasks and do it all at once. What do you think @STran ?

Let's split it up like you suggest (RevisionHandler vs LogHandler). I think it'll be good practice for someone.

Change 748184 merged by jenkins-bot:

[mediawiki/extensions/IPInfo@master] Add RevisionHandler access logging

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

QA front end testing will be done in https://phabricator.wikimedia.org/T295017