Page MenuHomePhabricator
Differential D12788

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

Authored by Krenair on May 10 2015, 3:47 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 27, 8:36 PM
Unknown Object (File)
Wed, Nov 27, 6:51 AM
Unknown Object (File)
Nov 18 2024, 11:04 PM
Unknown Object (File)
Oct 24 2024, 10:06 AM
Unknown Object (File)
Oct 24 2024, 10:01 AM
Unknown Object (File)
Oct 24 2024, 9:58 AM
Unknown Object (File)
Oct 24 2024, 9:57 AM
Unknown Object (File)
Oct 24 2024, 9:54 AM
View All Files
Subscribers
epriestley
Korvin

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Commits
Restricted Diffusion Commit
rPa03a488ba61b: Don't claim logged out users are automatically subscribed to un-owned objects
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 updated this revision to Diff 30740.May 10 2015, 3:47 PM
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.
Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptMay 10 2015, 3:47 PM
Herald added subscribers: epriestley, Korvin. · View Herald Transcript
epriestley requested changes to this revision.May 10 2015, 3:51 PM
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 updated this revision to Diff 30741.May 10 2015, 4:08 PM
Krenair edited edge metadata.

Just fix the caller in PhabricatorSubscriptionsUIEventListener.

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

epriestley accepted this revision.May 10 2015, 5:53 PM
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
Closed by commit rPa03a488ba61b: Don't claim logged out users are automatically subscribed to un-owned objects (authored by Krenair, committed by epriestley). · Explain WhyMay 10 2015, 5:54 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
src/
applications/
subscriptions/
events/
10 lines

Diff 30746

View Options

src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php

Loading...
Phabricator · Built by Phacility