Calendar Import: calendar uploader is not anymore an alien
Summary: If one of *your* verified email addresses is invited in *your* ICS Calendar, you are now imported as yourself, instead of being a "Private User". For example, if you own a Google Calendar, and if you import that in Phorge, and if your email is mentioned in the Invitees: - you are not shown anymore as "Private User 1" but as yourself - the "Busy" or "Available" badge is shown from your Profile (instead of nothing), respecting the "Time Transparency" RFC 5545 section 3.8.2.7 from your ICS event https://icalendar.org/iCalendar-RFC-5545/3-8-2-7-time-transparency.html - the widget "Profile Calendar" in your user page shows your imported Event, instead of nothing. No "Clear Sailing ahead" anymore. As usual - this happens only if the event happens today, tomorrow, or the day after tomorrow. Example situation: User "test" imports a Calendar. An Event has two invited emails: - 1 email is verified and belongs to the very same user "test" - 1 email belongs to another user | Before | After | |-----------|-----------| | {F324892} | {F324893} | See that the calendar importer named "test" is not an alien anymore. Allowing to match yourself makes sense because you trust your imported Calendar file, and we trust your verified email addresses. WE DO NOT MATCH OTHER USERS BUT THE CALENDAR OWNER. Matching other users must involve serious privacy measures, coherent with the rest of Phorge. Closes T15564 Closes T15941 Test Plan: Download this example ICS file: {F2599125} Replace the email `boz+asdlol@reyboz.it` with one of your verified email of your Phorge account. Import the event in Phorge using {nav Calendar > Imports > Import Events > Import .ics File} /calendar/import/edit/?importType=icsuri Two imported events are created successfully: - In the event "Very busy opaque" (25 December 2024): - you are finally shown as "Busy" - there is also another "Private user" - In the other event "Very available transparent" (25 December 2024) - you are finall shown as "Available" - there is also another "Private user" Then nuke these example events by visiting the import and "Delete Imported Events". --- Try again from scratch in these alternatives: - if you import the ICS file as-is: - you get two "Private User" in all events (since none of the invitees matches one of your verified emails) - if you import the ICS file, setting one of your un-verified emails: - you get two "Private User" in all events (since none of the invitees matches one of your verified emails) - if you import the ICS file, setting a verified email of *another* user: - you get two "Private User" in all events (since none of the invitees matches one of your verified emails) As additional test, from the file you can also manually set these events to today, tomorrow, or the day after tomorrow; so you can test the user profile's calendar widget, and see that finally it does not show "Clear sailing" anymore, but it shows your calendar invitations (if the ICS file contains one of your verified email - as already said): {F2599178} Reviewers: O1 Blessed Committers, aklapper Reviewed By: O1 Blessed Committers, aklapper Subscribers: aklapper, avivey, speck, tobiaswiese, Matthew, Cigaryno Maniphest Tasks: T15564, T15941 Differential Revision: https://we.phorge.it/D25363
This commit is contained in:
parent
ee9991d3c9
commit
c1e2e864e8
|
@ -166,6 +166,7 @@ final class PhabricatorCalendarEventViewController
|
|||
}
|
||||
|
||||
$availability_select->setDropdownMenu($dropdown);
|
||||
$availability_select->setDisabled($event->isImportedEvent());
|
||||
$header->addActionLink($availability_select);
|
||||
}
|
||||
|
||||
|
@ -629,6 +630,7 @@ final class PhabricatorCalendarEventViewController
|
|||
->setIcon('fa-times grey')
|
||||
->setHref($this->getApplicationURI("/event/decline/{$id}/"))
|
||||
->setWorkflow(true)
|
||||
->setDisabled($event->isImportedEvent())
|
||||
->setText(pht('Decline'));
|
||||
|
||||
$accept_button = id(new PHUIButtonView())
|
||||
|
@ -636,6 +638,7 @@ final class PhabricatorCalendarEventViewController
|
|||
->setIcon('fa-check green')
|
||||
->setHref($this->getApplicationURI("/event/accept/{$id}/"))
|
||||
->setWorkflow(true)
|
||||
->setDisabled($event->isImportedEvent())
|
||||
->setText(pht('Accept'));
|
||||
|
||||
return array($decline_button, $accept_button);
|
||||
|
|
|
@ -207,10 +207,24 @@ abstract class PhabricatorCalendarImportEngine
|
|||
$events = null;
|
||||
}
|
||||
|
||||
// Verified emails of the Event Uploader, to be eventually matched.
|
||||
// Phorge loves privacy, so emails are generally private.
|
||||
// This just covers a corner case: yourself importing yourself.
|
||||
// NOTE: We are using the omnipotent user since we already have
|
||||
// withUserPHIDs() limiting to a specific person (you).
|
||||
$author_verified_emails = id(new PhabricatorPeopleUserEmailQuery())
|
||||
->setViewer(PhabricatorUser::getOmnipotentUser())
|
||||
->withUserPHIDs(array($import->getAuthorPHID()))
|
||||
->withIsVerified(true)
|
||||
->execute();
|
||||
$author_verified_emails = mpull($author_verified_emails, 'getAddress');
|
||||
$author_verified_emails = array_fuse($author_verified_emails);
|
||||
|
||||
$xactions = array();
|
||||
$update_map = array();
|
||||
$invitee_map = array();
|
||||
$attendee_map = array();
|
||||
$attendee_name_map = array(); // map[eventUID][email from] = Attendee
|
||||
$attendee_user_map = array(); // map[eventUID][userPHID ] = Attendee
|
||||
foreach ($node_map as $full_uid => $node) {
|
||||
$event = idx($events, $full_uid);
|
||||
if (!$event) {
|
||||
|
@ -227,7 +241,8 @@ abstract class PhabricatorCalendarImportEngine
|
|||
$xactions[$full_uid] = $this->newUpdateTransactions($event, $node);
|
||||
$update_map[$full_uid] = $event;
|
||||
|
||||
$attendee_map[$full_uid] = array();
|
||||
$attendee_name_map[$full_uid] = array();
|
||||
$attendee_user_map[$full_uid] = array();
|
||||
$attendees = $node->getAttendees();
|
||||
$private_index = 1;
|
||||
foreach ($attendees as $attendee) {
|
||||
|
@ -236,8 +251,16 @@ abstract class PhabricatorCalendarImportEngine
|
|||
// of the product.
|
||||
$name = $attendee->getName();
|
||||
if (phutil_nonempty_string($name) && preg_match('/@/', $name)) {
|
||||
$name = new PhutilEmailAddress($name);
|
||||
$name = $name->getDisplayName();
|
||||
$attendee_mail = new PhutilEmailAddress($name);
|
||||
$name = $attendee_mail->getDisplayName();
|
||||
$address = $attendee_mail->getAddress();
|
||||
|
||||
// Skip creation of dummy "Private User" if it's me, the uploader.
|
||||
if ($address && isset($author_verified_emails[$address])) {
|
||||
$attendee_user_map[$full_uid][$import->getAuthorPHID()] =
|
||||
$attendee;
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
// If we don't have a name or the name still looks like it's an
|
||||
|
@ -247,12 +270,12 @@ abstract class PhabricatorCalendarImportEngine
|
|||
$private_index++;
|
||||
}
|
||||
|
||||
$attendee_map[$full_uid][$name] = $attendee;
|
||||
$attendee_name_map[$full_uid][$name] = $attendee;
|
||||
}
|
||||
}
|
||||
|
||||
$attendee_names = array();
|
||||
foreach ($attendee_map as $full_uid => $event_attendees) {
|
||||
foreach ($attendee_name_map as $full_uid => $event_attendees) {
|
||||
foreach ($event_attendees as $name => $attendee) {
|
||||
$attendee_names[$name] = $attendee;
|
||||
}
|
||||
|
@ -331,7 +354,8 @@ abstract class PhabricatorCalendarImportEngine
|
|||
|
||||
$update_map = array_select_keys($update_map, $insert_order);
|
||||
foreach ($update_map as $full_uid => $event) {
|
||||
$parent_uid = $this->getParentNodeUID($node_map[$full_uid]);
|
||||
$node = $node_map[$full_uid];
|
||||
$parent_uid = $this->getParentNodeUID($node);
|
||||
if ($parent_uid) {
|
||||
$parent_phid = $update_map[$parent_uid]->getPHID();
|
||||
} else {
|
||||
|
@ -356,19 +380,28 @@ abstract class PhabricatorCalendarImportEngine
|
|||
// We're just forcing attendees to the correct values here because
|
||||
// transactions intentionally don't let you RSVP for other users. This
|
||||
// might need to be turned into a special type of transaction eventually.
|
||||
$attendees = $attendee_map[$full_uid];
|
||||
$attendees_name = $attendee_name_map[$full_uid];
|
||||
$attendees_user = $attendee_user_map[$full_uid];
|
||||
$old_map = $event->getInvitees();
|
||||
$old_map = mpull($old_map, null, 'getInviteePHID');
|
||||
|
||||
$new_map = array();
|
||||
foreach ($attendees as $name => $attendee) {
|
||||
$phid = $external_invitees[$name]->getPHID();
|
||||
$phid_invitees = array();
|
||||
foreach ($attendees_name as $name => $attendee) {
|
||||
$attendee_phid = $external_invitees[$name]->getPHID();
|
||||
$phid_invitees[$attendee_phid] = $attendee;
|
||||
}
|
||||
foreach ($attendees_user as $phid_user_attendee => $attendee) {
|
||||
$phid_invitees[$phid_user_attendee] = $attendee;
|
||||
}
|
||||
|
||||
$invitee = idx($old_map, $phid);
|
||||
$new_map = array();
|
||||
foreach ($phid_invitees as $phid_invitee => $attendee) {
|
||||
|
||||
$invitee = idx($old_map, $phid_invitee);
|
||||
if (!$invitee) {
|
||||
$invitee = id(new PhabricatorCalendarEventInvitee())
|
||||
->setEventPHID($event->getPHID())
|
||||
->setInviteePHID($phid)
|
||||
->setInviteePHID($phid_invitee)
|
||||
->setInviterPHID($import->getPHID());
|
||||
}
|
||||
|
||||
|
@ -381,15 +414,26 @@ abstract class PhabricatorCalendarImportEngine
|
|||
break;
|
||||
case PhutilCalendarUserNode::STATUS_INVITED:
|
||||
default:
|
||||
$status = PhabricatorCalendarEventInvitee::STATUS_INVITED;
|
||||
// Is me importing myself? I'm coming!
|
||||
if ($phid_invitee === $import->getAuthorPHID()) {
|
||||
$status = PhabricatorCalendarEventInvitee::STATUS_ATTENDING;
|
||||
} else {
|
||||
$status = PhabricatorCalendarEventInvitee::STATUS_INVITED;
|
||||
}
|
||||
break;
|
||||
}
|
||||
$invitee->setStatus($status);
|
||||
// Import "busy/available", very useful for myself to tell this
|
||||
// to coworkers. This is probably somehow very un-useful for most
|
||||
// "Private user(s)", but let's add it for them too since it
|
||||
// doesn't hurt them.
|
||||
$invitee->importAvailabilityFromTimeTransparency(
|
||||
$node->getTimeTransparency());
|
||||
$invitee->save();
|
||||
|
||||
$new_map[$phid] = $invitee;
|
||||
$new_map[$phid_invitee] = $invitee;
|
||||
}
|
||||
|
||||
// Remove old Invitees if they are not invited anymore.
|
||||
foreach ($old_map as $phid => $invitee) {
|
||||
if (empty($new_map[$phid])) {
|
||||
$invitee->delete();
|
||||
|
|
|
@ -56,7 +56,7 @@ final class CalendarImportTestCase extends PhabricatorTestCase {
|
|||
'fileAuthor' => $lincoln_verified,
|
||||
'expectedInvitees' => 3,
|
||||
'expectedInviteesTests' => array(
|
||||
// array($lincoln_verified, true), // Self-invitation. T15564
|
||||
array($lincoln_verified, true), // Self-invitation. T15564
|
||||
array($alice_unverified, false),
|
||||
array($alien_unverified, false),
|
||||
array($alien_verified, false),
|
||||
|
|
|
@ -15,6 +15,7 @@ final class PhutilCalendarEventNode
|
|||
private $modifiedDateTime;
|
||||
private $organizer;
|
||||
private $attendees = array();
|
||||
private $timeTransparency;
|
||||
private $recurrenceRule;
|
||||
private $recurrenceExceptions = array();
|
||||
private $recurrenceDates = array();
|
||||
|
@ -130,6 +131,24 @@ final class PhutilCalendarEventNode
|
|||
return $this;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the "time transparency" as described by RFC 5545 3.8.2.7.
|
||||
* @return string|null
|
||||
*/
|
||||
public function getTimeTransparency() {
|
||||
return $this->timeTransparency;
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the "time transparency" as described by RFC 5545 3.8.2.7.
|
||||
* @param string|null $time_transparency
|
||||
* @return self
|
||||
*/
|
||||
public function setTimeTransparency($time_transparency) {
|
||||
$this->timeTransparency = $time_transparency;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function setRecurrenceRule(
|
||||
PhutilCalendarRecurrenceRule $recurrence_rule) {
|
||||
$this->recurrenceRule = $recurrence_rule;
|
||||
|
|
|
@ -673,6 +673,10 @@ final class PhutilICSParser extends Phobject {
|
|||
$attendee = $this->newAttendeeFromProperty($parameters, $value);
|
||||
$node->addAttendee($attendee);
|
||||
break;
|
||||
case 'TRANSP':
|
||||
$transp = $this->newTextFromProperty($parameters, $value);
|
||||
$node->setTimeTransparency($transp);
|
||||
break;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -213,6 +213,17 @@ final class PhutilICSWriter extends Phobject {
|
|||
}
|
||||
}
|
||||
|
||||
// In the future you may want to add export support
|
||||
// to the "Time Trasparency" field. In case, please tell us why.
|
||||
// No one needs it at the moment. This is not even persisted
|
||||
// in the event object, so, this cannot be exported.
|
||||
// $transp = $event->getTimeTransparency();
|
||||
// if ($transp) {
|
||||
// $properties[] = $this->newTextProperty(
|
||||
// 'TRANSP',
|
||||
// $transp);
|
||||
// }
|
||||
|
||||
$rrule = $event->getRecurrenceRule();
|
||||
if ($rrule) {
|
||||
$properties[] = $this->newRRULEProperty(
|
||||
|
|
|
@ -93,6 +93,17 @@ final class PhutilICSParserTestCase extends PhutilTestCase {
|
|||
'raw' => 'This is a simple event.',
|
||||
),
|
||||
),
|
||||
array(
|
||||
'name' => 'TRANSP',
|
||||
'parameters' => array(),
|
||||
'value' => array(
|
||||
'type' => 'TEXT',
|
||||
'value' => array(
|
||||
'OPAQUE',
|
||||
),
|
||||
'raw' => 'OPAQUE',
|
||||
),
|
||||
),
|
||||
),
|
||||
$event->getAttribute('ics.properties'));
|
||||
|
||||
|
|
|
@ -8,5 +8,6 @@ DTSTART;TZID=America/Los_Angeles:20160915T090000
|
|||
DTEND;TZID=America/Los_Angeles:20160915T100000
|
||||
SUMMARY:Simple Event
|
||||
DESCRIPTION:This is a simple event.
|
||||
TRANSP:OPAQUE
|
||||
END:VEVENT
|
||||
END:VCALENDAR
|
||||
|
|
|
@ -69,6 +69,33 @@ final class PhabricatorCalendarEventInvitee extends PhabricatorCalendarDAO
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Import the invitee availability from the Time Transparency
|
||||
* field in an ICS calendar event as per RFC 5545 section 3.8.2.7.
|
||||
* @param wild $time_transp Time transparency like 'OPAQUE'
|
||||
* or 'TRANSPARENT' or null.
|
||||
* @return void
|
||||
*/
|
||||
public function importAvailabilityFromTimeTransparency($time_transp) {
|
||||
// How to understand RFC 5545 suburbs. Example conversation:
|
||||
// "Hey dude
|
||||
// I'm a bit *opaque* on this event so I'm not *transparent*"
|
||||
// Means:
|
||||
// "Good morning Sir,
|
||||
// I'm a bit *busy* on this business so I'm not *available*"
|
||||
static $transparency_2_availability = array(
|
||||
'OPAQUE' => self::AVAILABILITY_BUSY,
|
||||
'TRANSPARENT' => self::AVAILABILITY_AVAILABLE,
|
||||
);
|
||||
|
||||
// Note that idx($array, $key) likes a null $key.
|
||||
$availability = idx($transparency_2_availability, $time_transp);
|
||||
if ($availability) {
|
||||
$this->setAvailability($availability);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
public static function getAvailabilityMap() {
|
||||
return array(
|
||||
self::AVAILABILITY_AVAILABLE => array(
|
||||
|
|
|
@ -5,6 +5,8 @@ final class PhabricatorPeopleUserEmailQuery
|
|||
|
||||
private $ids;
|
||||
private $phids;
|
||||
private $userPhids;
|
||||
private $isVerified;
|
||||
|
||||
public function withIDs(array $ids) {
|
||||
$this->ids = $ids;
|
||||
|
@ -16,6 +18,24 @@ final class PhabricatorPeopleUserEmailQuery
|
|||
return $this;
|
||||
}
|
||||
|
||||
/**
|
||||
* With the specified User PHIDs.
|
||||
* @param null|array $phids User PHIDs
|
||||
*/
|
||||
public function withUserPHIDs(array $phids) {
|
||||
$this->userPhids = $phids;
|
||||
return $this;
|
||||
}
|
||||
|
||||
/**
|
||||
* With a verified email or not.
|
||||
* @param bool|null $isVerified
|
||||
*/
|
||||
public function withIsVerified($verified) {
|
||||
$this->isVerified = $verified;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function newResultObject() {
|
||||
return new PhabricatorUserEmail();
|
||||
}
|
||||
|
@ -41,6 +61,20 @@ final class PhabricatorPeopleUserEmailQuery
|
|||
$this->phids);
|
||||
}
|
||||
|
||||
if ($this->userPhids !== null) {
|
||||
$where[] = qsprintf(
|
||||
$conn,
|
||||
'email.userPHID IN (%Ls)',
|
||||
$this->userPhids);
|
||||
}
|
||||
|
||||
if ($this->isVerified !== null) {
|
||||
$where[] = qsprintf(
|
||||
$conn,
|
||||
'email.isVerified = %d',
|
||||
(int)$this->isVerified);
|
||||
}
|
||||
|
||||
return $where;
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in a new issue