Page MenuHomePhabricator

Remove 'ipinfo-enable' preference while IPInfo is a BetaFeature
Closed, ResolvedPublic3 Estimated Story Points

Description

Background

The 'ipinfo-enable' preference is an unnecessary extra step that is causing bugs such as T309365 and T302398.

For now, IPInfo could be considered enabled if the BetaFeature is enabled, and disabled if the BetaFeature is disabled. (The user must still agree to the terms of use before using IPInfo.)

Once IPInfo is no longer a beta feature, we should re-introduce the preference for enabling it.

Acceptance criteria
  • remove "enable IPInfo" checkbox from Special:Preferences main page
Testing notes

The following should be tested...

Basic functioning:

  • When the beta feature is enabled and the agreement is accepted, IPInfo is visible on Special:Contributions and Special:Log
  • When the beta feature is enabled but not accepted, the agreement checkbox appears on Special:Contributions and IPInfo icons do not appear on Special:Log
  • When the beta feature is disabled, the IPInfo infobox does not appear on Special:Contributions and IPInfo icons do not appear on Special:Log. The agreement checkbox does not appear on Special:Preferences#mw-prefsection-personal

Logging:

  • When the beta feature is first enabled (and the agreement is never accepted at this stage), nothing is logged (whether enabling directly or via checking auto-enroll)
  • When the agreement is accepted, 'enable_ipinfo' is logged
  • When the agreement is un-accepted, 'disable_ipinfo' is logged
  • If IPInfo is already enabled, then when the beta feature disabled, 'disable_ipinfo' is logged
  • If IPInfo is already enabled, then when the preferences are reset via Special:Preferences/reset, 'disable_ipinfo' is logged
Notes

The following documentation may be useful:

Event Timeline

Tchanders set the point value for this task to 3.Aug 1 2022, 5:53 PM

Change 824285 had a related patch set uploaded (by TsepoThoabala; author: TsepoThoabala):

[mediawiki/extensions/IPInfo@master] Remove 'enable-ipinfo' preference while IPInfo is a BetaFeature

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

@Prtksxna do we need to update this copy, as now IPInfo enables/disables when the beta feature enables/disables

pref.png (684×2 px, 244 KB)

@TThoabala, nice catch! Let us keep the copy the same and change the link to go to the Beta features tab in Preferences. Could you check where it links to at the moment?

@Prtksxna it was linkking to Special:Preferences, I have updated it to link to Special:Preferences#mw-prefsection-betafeatures as suggested.

We are currently logging when IPInfo enables or disables the current actions we have been logging are enable_ipinfo and disable_ipinfo. We currently log when IPinfo changes state(enable/disable), since we have removed that in this ticket, we will now be logging when beta feature enables/disables. @Niharika does this sound ok?

We are currently logging when IPInfo enables or disables the current actions we have been logging are enable_ipinfo and disable_ipinfo. We currently log when IPinfo changes state(enable/disable), since we have removed that in this ticket, we will now be logging when beta feature enables/disables. @Niharika does this sound ok?

@TThoabala Good question, thanks for highlighting. We should still log when the feature becomes enabled/disabled for the user, i.e.:

  • don't log ipinfo_enable if the beta feature is enabled, but the agreement isn't accepted, because the user won't be able to see IPInfo
  • log ipinfo_enable when the beta feature is enabled AND the agreement is accepted (whether or not the beta feature was enabled earlier or just now)
  • log ipinfo_disable when the beta feature is disabled, or the agreement is unchecked; i.e. anything that causes the user to cease being able to see IPInfo

I've commented some more detail on the patch.

@jwang You might be interested to see that the UI workflow is going to change after this task. It shouldn't change anything from the logging perspective though.

I tested the points from the task description, and things seem to be working as expected.

I did notice that sometimes when enabling the beta feature, there is a delay before the agreement checkbox appears on Special:Preferences (and before the IPInfo infobox is visible on Special:Contributions). This has the same cause as the issue noted in T302398#7887863 and isn't fixable, for the reasons discussed in that task. (Enabling any beta feature can be susceptible to a delay.) However, we have improved the situation by removing the 'ipinfo-enable' checkbox step in this task: once the beta feature is enabled, the infobox will be available on Special:Contributions. The user won't need to return to Special:Preferences to check another checkbox.

The unused preference in the database would be cleaned up via T300371: Drop now unused user preferences from production database(s). If there's a problem, we could run a script separately to clean it up.

The users should experience no changes to whether IPInfo is switched on or off:

  • If they had IPInfo switched on, they must have had preferences ipinfo-enable, ipinfo-beta-feature-enable and ipinfo-use-agreement all true. After this task, ipinfo-beta-feature-enable and ipinfo-use-agreement will still be true, so IPInfo will still be switched on.
  • If they had the beta feature enabled but ipinfo-enable set to false, then ipinfo-use-agreement must also have been false. After this task, ipinfo-use-agreement will still be false, so IPInfo will still be switched off.
  • If they had the beta feature disabled, then likewise ipinfo-use-agreement must have been false. After this task, ipinfo-use-agreement will still be false, so IPInfo will still be switched off.
Tchanders renamed this task from Remove 'enable-ipinfo' preference while IPInfo is a BetaFeature to Remove 'ipinfo-enable' preference while IPInfo is a BetaFeature.Sep 21 2022, 4:56 PM
Tchanders updated the task description. (Show Details)

Change 824285 merged by jenkins-bot:

[mediawiki/extensions/IPInfo@master] Remove 'ipinfo-enable' preference while IPInfo is a BetaFeature

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

@TThoabala I think we might need to update the WikimediaMessages repo as well after this change (see here).

I am no longer seeing Global Contributions, XTools and Leave Feedback:

T310819_missing_links.png (365×1 px, 40 KB)

Remember to test with a user who has never enabled IPInfo before (because an existing user may still have ipinfo-enable in the database).

Change 834047 had a related patch set uploaded (by Tchanders; author: Tchanders):

[mediawiki/extensions/WikimediaMessages@master] Stop checking for ipinfo-enable preference from hook handler

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

Basic functioning:

  • When the beta feature is enabled and the agreement is accepted, IPInfo is visible on Special:Contributions and Special:Log
  • When the beta feature is enabled but not accepted, the agreement checkbox appears on Special:Contributions and IPInfo icons do not appear on Special:Log
  • When the beta feature is disabled, the IPInfo infobox does not appear on Special:Contributions and IPInfo icons do not appear on Special:Log. The agreement checkbox does not appear on Special:Preferences#mw-prefsection-personal

Logging:

  • When the beta feature is first enabled (and the agreement is never accepted at this stage), nothing is logged (whether enabling directly or via checking auto-enroll)
  • When the agreement is accepted, 'enable_ipinfo' is logged
  • When the agreement is un-accepted, 'disable_ipinfo' is logged
  • If IPInfo is already enabled, then when the beta feature disabled, 'disable_ipinfo' is logged
  • If IPInfo is already enabled, then when the preferences are reset via Special:Preferences/reset, 'disable_ipinfo' is logged

As far as I could tell, all the above are true.

I tested various combinations of user preferences related to IPInfo. This included where a user still had ipinfo-enable set in the database.

For each combination, I checked the state of the infobox on Special:Contributions, the popup, the IP Information log and EventLogging (where possible).

I did not always check whether the agreement checkbox appeared in Special:Preferences.

I could not always check EventLogging when I was testing beta, as I don't know how to monitor server-side events. However, I could do so when I was testing locally.

There were a couple of bugs related to Preferences and Global Preferences which may have preexisted this, which I will raise separately.

I will move this into Code Review again just to make sure the developers are aware of the current state.

Test environments:

Change 834047 merged by jenkins-bot:

[mediawiki/extensions/WikimediaMessages@master] Stop checking for ipinfo-enable preference from hook handler

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

I will move this into Code Review again just to make sure the developers are aware of the current state.

Thanks - worked a charm!

@TThoabala I think we might need to update the WikimediaMessages repo as well after this change (see here).

I am no longer seeing Global Contributions, XTools and Leave Feedback:

T310819_missing_links.png (365×1 px, 40 KB)

Remember to test with a user who has never enabled IPInfo before (because an existing user may still have ipinfo-enable in the database).

The latest patch should fix this problem.

@TThoabala I think we might need to update the WikimediaMessages repo as well after this change (see here).

I am no longer seeing Global Contributions, XTools and Leave Feedback:

T310819_missing_links.png (365×1 px, 40 KB)

Remember to test with a user who has never enabled IPInfo before (because an existing user may still have ipinfo-enable in the database).

The latest patch should fix this problem.

Thanks. On beta, I now see the Global Contributions, XTools and Leave Feedback regardless of whether or not the user has ipinfo-enable in the database.

Prtksxna moved this task from In Progress to Closed on the IP Info board.