Page MenuHomePhabricator

Provide access to Notifications for anonymous users
Open, NormalPublic

Description

Enabling Echo for anonymous users is something that eventually should get done.

At the very least a schema change will be needed on the echo_notification table.

There are quite a few calls to if ( $user->isAnon() ) { abort; } which need to be updated.

It would be interesting to investigate expiring notifications for dynamic IP addresses.


See Also:
T63022: Add ability to thank anonymous/IP users

Details

Reference
bz56828

Event Timeline

bzimport raised the priority of this task from to Lowest.Nov 22 2014, 2:23 AM
bzimport added a project: Notifications.
bzimport set Reference to bz56828.
bzimport added a subscriber: Unknown Object (MLST).
Legoktm created this task.Nov 9 2013, 7:41 AM

The WMF core features team tracks this bug on Mingle card https://mingle.corp.wikimedia.org/projects/flow/cards/487, but people from the community are welcome to contribute here and in Gerrit.

Apparently WikiHow is working on this (or already has it done?)...

(In reply to Kunal Mehta (Legoktm) from comment #2)

Apparently WikiHow is working on this (or already has it done?)...

Compare http://www.wikihow.com/Special:Notifications versus https://en.wikipedia.org/wiki/Special:Notifications (visit both as logged out users).

I uploaded Icfafb4e059baadff8bcda93594835a4ec6b47264 which is a diff of the WikiHow code from current master of Echo. I'll work on cutting down the irrelevant parts.

Change 135396 had a related patch set uploaded by Legoktm:
[WIP] Allow anonymous users to receive notifications

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

Legoktm moved this task from Backlog to Needs plan on the Notifications board.Jul 6 2015, 9:02 AM
Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptJul 6 2015, 9:02 AM
Jdforrester-WMF set Security to None.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 18 2016, 11:12 PM
matej_suchanek removed a subscriber: wikibugs-l-list.
Jdforrester-WMF renamed this task from Echo for anonymous users to Provide access to Notifications for anonymous users.Feb 5 2016, 9:55 PM
Qgil removed a subscriber: Qgil.Feb 11 2016, 12:32 PM

I'm planing on submitting a patch for this. This would be based on the following ideas:

  • we don't really need persistent storage for anon notifications, so we can store them in the WAN object cache for say a month
  • we can't notify all users using a given IP, so we need to know which ones to notify, for this we can use a cookie which is added when editing (and maybe in other circumstances).

Implementation-wise, we would have:

  • a new class NotifAnon to handle cookies and marking as read, a new class AnonNotificationMapper to fetch notifications from cache
  • two cookies storing the latest seen timestamp for alerts and notices.

Limitations:

  • temporary
  • will not notify if user clears cookies or changes browser/platform
  • no crosswiki
  • limited to say 10 notifications per type to quickly fetch from cache

Use cases: reviews, thanks, reverts, active deferrals

Cenarium raised the priority of this task from Lowest to Normal.Dec 27 2016, 10:19 AM
Aklapper assigned this task to Cenarium.Dec 27 2016, 3:38 PM

I'm planing on submitting a patch for this.

Thanks. Setting assignee field accordingly.

Change 329374 had a related patch set uploaded (by Matěj Suchánek):
Support limited anon notifications

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

Change 329374 had a related patch set uploaded (by Matěj Suchánek):

By @Cenarium, of course.

Change 331487 had a related patch set uploaded (by Cen.temp):
Support anon notifications

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

Thanks for your work, @Cenarium !

Some of this probably requires some design and product decisions; I'm pinging @jmatazzoni, @Pginer-WMF and @Catrope to make sure they're on it, and are chipping in if/when necessary.

Issues to discuss include:

  • Random expiration when server caches expires
  • Receiving notifications not intended for you
  • Limited notification count (10 per type)
  • UI (Should we hide notifications UI if there are no unread/cookies indicate no unread, as it currently does?)
    • Other UI questions, such as warning users they may receive notifications that are not intended for them
  • Current patch says, “there is no page filtering, they are local only and can't be marked unread” (that means Special:Notifications has limitations) and “Initial support is provided for new messages, mentions and reverts.” (We’ve never limited an Echo UI/delivery method to one type of notification before).

The project we have in hand and for which this patch was presumably intended is Deferred Changes. One of the outstanding issues we have to deal with there is to provide anonymous users with feedback letting them know that the change they just saved is being actively deferred. Meaning it's not being published pending review.

While a notification would be one way to clue these IP users in, it's not clear that it's the most effective way, since new and IP users may not even know about notifications. And given some of the issues that have been raised about anonymous notifications (there's a reason someone assigned this the Epic tag), it's probably not the easiest way, either.

There are a number of good reasons I'm sure to extend Notifications to anon users. But, as I say, that's not the project we're working on now. I think we need to take a step back and consider what our goals for communicating with users impacted by Deferred Changes are, and how we can best achieve them. I've created a task for that at T156083. Input from all is welcome there.

Issues to discuss include:

  • Random expiration when server caches expires

At first I only wanted this to cover the deferred changes case, but we might as well build a full fledged system. I think the cache would be enough in the vast majority of cases for the IP to get the notification. And having an archive of notifications for IPs is IMO not essential, but sure if we can, it would be preferable overall to use the db. My only concern is whether we can do so, as I was under the impression that some tables are too sensitive to be altered in some ways.

@jcrespo: Would it be possible to alter the echo_notification table this way:

ALTER TABLE echo_notification ADD COLUMN notification_anon_ip varchar(15) not null default '';
  • Receiving notifications not intended for you

I believe this is largely addressed by the cookie. Users see the notification only if they have the cookie. The cookie is added when an IP edits. To get a notification not intended for you, one would need to have edited recently and inherit an IP that a previous unrelated user edited from. This is possible, but even then the new user should probably be made aware of the notification.

  • UI (Should we hide notifications UI if there are no unread/cookies indicate no unread, as it currently does?)

It isn't the cookie that indicates no unread. The cookie indicates that the user is a contributor (has recently edited) and thus should be made aware of notifications on this IP (it also contains the last seen timestamps). I've done this for performance, to avoid having to needlessly query for notifications when the user isn't a contributor.

Assuming we can alter the db, I've modified the patch set to offer the same level of functionality than notifications for logged in users. They can be marked read / unread and filtered by titles, and cross wiki notifications are possible. The last one requires caching the IP address in a global key.

echo_notification table for enwiki:

CREATE TABLE `echo_notification` (
  `notification_event` int(10) unsigned NOT NULL,
  `notification_user` int(10) unsigned NOT NULL,
  `notification_timestamp` binary(14) NOT NULL,
  `notification_read_timestamp` binary(14) DEFAULT NULL,
  `notification_bundle_base` tinyint(1) NOT NULL DEFAULT '1',
  `notification_bundle_hash` varchar(32) CHARACTER SET latin1 COLLATE latin1_bin NOT NULL,
  `notification_bundle_display_hash` varchar(32) CHARACTER SET latin1 COLLATE latin1_bin NOT NULL,
  `notification_seen` tinyint(1) NOT NULL DEFAULT '0',
  UNIQUE KEY `user_event` (`notification_user`,`notification_event`),
  KEY `user_timestamp` (`notification_user`,`notification_timestamp`),
  KEY `echo_notification_user_base_read_timestamp` (`notification_user`,`notification_bundle_base`,`notification_read_timestamp`),
  KEY `echo_notification_user_base_timestamp` (`notification_user`,`notification_bundle_base`,`notification_timestamp`,`notification_event`),
  KEY `echo_notification_user_hash_timestamp` (`notification_user`,`notification_bundle_hash`,`notification_timestamp`),
  KEY `echo_notification_user_hash_base_timestamp` (`notification_user`,`notification_bundle_display_hash`,`notification_bundle_base`,`notification_t
  KEY `echo_notification_event` (`notification_event`),
  KEY `echo_notification_user_read_timestamp` (`notification_user`,`notification_read_timestamp`)
) ENGINE=InnoDB DEFAULT CHARSET=latin1

It has not primary key. All tables must have a primary key. T136428 is a blocker.

Aside from that, I do not see why not. Consider using a separate table with a key if most of those fields are going to be null and unused.

Change 331487 abandoned by Jdlrobson:
Add anon notifications

Reason:
This will need to be re-submitted against the MinervaNeue skin.

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

Restricted Application added a project: Growth-Team. · View Herald TranscriptSep 4 2018, 2:26 PM
jmatazzoni added a subscriber: jmatazzoni.
jmatazzoni removed a subscriber: jmatazzoni.