Page MenuHomePhabricator

Don't claim logged out users are automatically subscribed to un-owned objects
ClosedPublic

Authored by Krenair on May 10 2015, 3:47 PM.

Details

Summary

The PHID for logged out users is NULL, but so is the PHID for un-owned objects.

Test Plan

Browse a non-owned object (i.e. unassigned task) while logged out, notice "Automatically subscribed" before this commit.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Krenair retitled this revision from to Don't claim logged out users are automatically subscribed to un-owned objects.
Krenair updated this object.
Krenair edited the test plan for this revision. (Show Details)
Krenair added a reviewer: epriestley.
epriestley edited edge metadata.

Instead of fixing this in every object, let's just fix it in PhabricatorSubscriptionsUIEventListener? Public users can never be subscribed to anything, and it will never make sense to let them subscribe to things (the operation is meaningless) so we don't need to test if they're automatically subscribed at all.

Since "0" isn't a valid PHID, it's sufficient to just test for $phid instead of !is_null($phid).

This revision now requires changes to proceed.May 10 2015, 3:51 PM
Krenair edited edge metadata.

Just fix the caller in PhabricatorSubscriptionsUIEventListener.

Fiddle around with user PHID variables to avoid >80 characters lint warning.

epriestley edited edge metadata.

Thanks!

src/applications/subscriptions/interface/PhabricatorSubscribableInterface.php
10–11 ↗(On Diff #30741)

I think this is obsolete, I'll just leave it out of the pull.

This revision is now accepted and ready to land.May 10 2015, 5:53 PM
This revision was automatically updated to reflect the committed changes.