Page MenuHomePhorge
Files F2695521

D25363.1734846296.diff
No OneTemporary
Actions

D25363.1734846296.diff
View Options

diff --git a/.arclint b/.arclint
--- a/.arclint
+++ b/.arclint
@@ -65,7 +65,7 @@
"text": {
"type": "text",
"exclude": [
- "(^src/(.*/)?__tests__/[^/]+/.*\\.(txt|json|expect))"
+ "(^src/(.*/)?__tests__/[^/]+/.*\\.(txt|json|expect|ics))"
]
},
"text-without-length": {
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -311,6 +311,7 @@
'BulkSelectParameterType' => 'applications/transactions/bulk/type/BulkSelectParameterType.php',
'BulkStringParameterType' => 'applications/transactions/bulk/type/BulkStringParameterType.php',
'BulkTokenizerParameterType' => 'applications/transactions/bulk/type/BulkTokenizerParameterType.php',
+ 'CalendarImportTestCase' => 'applications/calendar/import/__tests__/CalendarImportTestCase.php',
'CalendarTimeUtil' => 'applications/calendar/util/CalendarTimeUtil.php',
'CalendarTimeUtilTestCase' => 'applications/calendar/__tests__/CalendarTimeUtilTestCase.php',
'CelerityAPI' => 'applications/celerity/CelerityAPI.php',
@@ -6317,6 +6318,7 @@
'BulkSelectParameterType' => 'BulkParameterType',
'BulkStringParameterType' => 'BulkParameterType',
'BulkTokenizerParameterType' => 'BulkParameterType',
+ 'CalendarImportTestCase' => 'PhabricatorTestCase',
'CalendarTimeUtil' => 'Phobject',
'CalendarTimeUtilTestCase' => 'PhabricatorTestCase',
'CelerityAPI' => 'Phobject',
diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php
--- a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php
+++ b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php
@@ -166,6 +166,7 @@
}
$availability_select->setDropdownMenu($dropdown);
+ $availability_select->setDisabled($event->isImportedEvent());
$header->addActionLink($availability_select);
}
@@ -629,6 +630,7 @@
->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 @@
->setIcon('fa-check green')
->setHref($this->getApplicationURI("/event/accept/{$id}/"))
->setWorkflow(true)
+ ->setDisabled($event->isImportedEvent())
->setText(pht('Accept'));
return array($decline_button, $accept_button);
diff --git a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php
--- a/src/applications/calendar/import/PhabricatorCalendarImportEngine.php
+++ b/src/applications/calendar/import/PhabricatorCalendarImportEngine.php
@@ -207,10 +207,24 @@
$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 inviting 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(); // [eventUID][email from] = Attendee
+ $attendee_user_map = array(); // [eventUID][userPHID ] = Attendee
foreach ($node_map as $full_uid => $node) {
$event = idx($events, $full_uid);
if (!$event) {
@@ -227,7 +241,8 @@
$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 @@
// 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 @@
$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;
}
@@ -356,19 +379,28 @@
// 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');
+ $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;
+ }
+
$new_map = array();
- foreach ($attendees as $name => $attendee) {
- $phid = $external_invitees[$name]->getPHID();
+ foreach ($phid_invitees as $phid_invitee => $attendee) {
- $invitee = idx($old_map, $phid);
+ $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 +413,20 @@
break;
case PhutilCalendarUserNode::STATUS_INVITED:
default:
- $status = PhabricatorCalendarEventInvitee::STATUS_INVITED;
+ if ($phid_invitee === $import->getAuthorPHID()) {
+ $status = PhabricatorCalendarEventInvitee::STATUS_ATTENDING;
+ } else {
+ $status = PhabricatorCalendarEventInvitee::STATUS_INVITED;
+ }
break;
}
$invitee->setStatus($status);
$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();
diff --git a/src/applications/calendar/import/__tests__/CalendarImportTestCase.php b/src/applications/calendar/import/__tests__/CalendarImportTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/applications/calendar/import/__tests__/CalendarImportTestCase.php
@@ -0,0 +1,231 @@
+<?php
+
+final class CalendarImportTestCase extends PhabricatorTestCase {
+
+ protected function getPhabricatorTestCaseConfiguration() {
+ return array(
+ self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES => true,
+ );
+ }
+
+ // Indexes of the "expectedInviteesTests" test.
+ const INVITED_USER = 0;
+ const INVITED_EXPECTED = 1;
+ const INVITED_RESULT = 2;
+
+ public function testIcsFileImportWithGuestThatIsHost() {
+ $alice_unverified =
+ $this->generateTestUserWithVerifiedMail(
+ 'alice@example.com',
+ 0);
+ $lincoln_verified =
+ $this->generateTestUserWithVerifiedMail(
+ 'a.lincoln@example.com',
+ 1);
+ $alien_unverified =
+ $this->generateTestUserWithVerifiedMail(
+ 'alien.unferified@example.com',
+ 0);
+ $alien_verified =
+ $this->generateTestUserWithVerifiedMail(
+ 'alien.verified@example.com',
+ 1);
+
+ // Tests are event-based. Each event has their expected invitees.
+ $tests = array(
+ // Test zero. Alice imports an event with A.Lincoln.
+ array(
+ 'test' => pht('alice invites a.lincoln via verified email'),
+ 'file' => 'simple-event-alincoln-guest.ics',
+ 'fileAuthor' => $alice_unverified,
+ 'expectedInvitees' => 3,
+ 'expectedInviteesTests' => array(
+ // Documentation:
+ // Array 0 (INVITED_USER): User object
+ // Array 1 (INVITED_EXPECTED): Presence (bool)
+ array($lincoln_verified, false),
+ array($alice_unverified, false),
+ array($alien_unverified, false),
+ array($alien_verified, false),
+ ),
+ ),
+ // Test one. A.Lincoln imports an event with A.Lincoln.
+ array(
+ 'test' => pht('a.lincoln self-invite via verified email'),
+ 'file' => 'simple-event-alincoln-guest.ics',
+ 'fileAuthor' => $lincoln_verified,
+ 'expectedInvitees' => 3,
+ 'expectedInviteesTests' => array(
+ array($lincoln_verified, true), // Self-invitation. T15564
+ array($alice_unverified, false),
+ array($alien_unverified, false),
+ array($alien_verified, false),
+ ),
+ ),
+ );
+
+ foreach ($tests as $test) {
+ $this->runIcsFileImportTestWithExpectedResults(
+ $test['test'],
+ $test['file'],
+ $test['fileAuthor'],
+ $test['expectedInvitees'],
+ $test['expectedInviteesTests']);
+ }
+ }
+
+ private function runIcsFileImportTestWithExpectedResults(
+ $test, $file, $importer_author, $expecteds, $invitees_tests) {
+
+ $ics_path = __DIR__.'/events/'.$file;
+
+ // Prepare a calendar import.
+ $import_type = new PhabricatorCalendarICSFileImportEngine();
+ $calendar_import = PhabricatorCalendarImport::initializeNewCalendarImport(
+ $importer_author,
+ clone $import_type);
+
+ // Create the File containing the ICS example.
+ $file_data = Filesystem::readFile($ics_path);
+ $file_test_engine = new PhabricatorTestStorageEngine();
+ $file_params = array(
+ 'name' => $file,
+ 'viewPolicy' => PhabricatorPolicies::POLICY_USER,
+ 'authorPHID' => $importer_author->getPHID(),
+ 'storageEngines' => array($file_test_engine),
+ );
+ $file_up = PhabricatorFile::newFromFileData($file_data, $file_params);
+
+ // Create a calendar import with our ICS file.
+ $import_xactions = array();
+ $import_xactions[] = id(new PhabricatorCalendarImportTransaction())
+ ->setTransactionType(
+ PhabricatorCalendarImportICSFileTransaction::TRANSACTIONTYPE)
+ ->setNewValue($file_up->getPHID());
+
+ // Persist the calendar import and get it.
+ id(new PhabricatorCalendarImportEditor())
+ ->setActor($importer_author)
+ ->setContentSource($this->newContentSource())
+ ->applyTransactions($calendar_import, $import_xactions);
+
+ $import_type->importEventsFromSource(
+ $importer_author,
+ $calendar_import,
+ false);
+
+ // Find imported events from the perspective of the importer author itself.
+ // So we check if we gained some extra people email visibility by mistake.
+ // The backend does not support attachInvitees() and it's done by default.
+ $events = (new PhabricatorCalendarEventQuery())
+ ->setViewer($importer_author)
+ ->withImportSourcePHIDs(array($calendar_import->getPHID()))
+ ->execute();
+
+ // At the moment test cases are hardcoded with one event.
+ $this->assertEqual(
+ 1,
+ count($events),
+ pht('Unexpected events in file "%s" on test "%s"',
+ $file,
+ $test));
+
+ // Take the first event.
+ $event = head($events);
+
+ // How many people we invited in this event.
+ $this->assertEqual(
+ $expecteds,
+ count($event->getInvitees()),
+ pht('Unexpected invitees in file "%s" on test "%s"', $file, $test));
+
+ foreach ($invitees_tests as $invitees_test) {
+ $this->assertMatchingInvitees($test, $file, $event, $invitees_tests);
+ }
+
+ $event->delete();
+ }
+
+ /**
+ * Check if the invitees matches.
+ */
+ private function assertMatchingInvitees($test, $file, $event, $expecteds) {
+
+ // Index what we expect, by user PHID.
+ $expected_users_by_phid = [];
+ foreach ($expecteds as $expected_invited_data) {
+ $user_phid = $expected_invited_data[self::INVITED_USER]
+ ->getPHID();
+ $expected_users_by_phid[$user_phid]
+ = $expected_invited_data;
+ }
+
+ // Get current invitees (mixed between "PHID-CXNV" and "PHID-USER").
+ $actuals_phids_mixed = mpull($event->getInvitees(), 'getInviteePHID');
+
+ // Get just actual users.
+ $actual_users = (new PhabricatorUser())->loadAllWhere(
+ 'phid IN (%Ls)',
+ $actuals_phids_mixed);
+ $actual_users = mpull($actual_users, null, 'getPHID');
+
+ // Map actual users with the expected ones.
+ foreach ($actual_users as $actual_user) {
+ $user_phid = $actual_user->getPHID();
+ $found = isset($expected_users_by_phid[$user_phid]);
+ if (!$found) {
+ $expected_users_by_phid[$user_phid] = array();
+ }
+ $expected_users_by_phid[$user_phid][self::INVITED_RESULT]
+ = $found;
+ }
+
+// In the future it may be useful to also check external users
+// by their email. In case, start from here!
+// but note that the 'getURI()' returns 'mailto:' stuff.
+// $actual_externals = (new PhabricatorCalendarExternalInvitee())
+// ->loadAllWhere(
+// 'phid IN (%Ls)',
+// $actuals_phids_mixed);
+// $actual_externals = mpull($actual_externals, null, 'getURI');
+
+ // Check results (matched or not).
+ foreach ($expected_users_by_phid as $phid => $expecteds_data) {
+ $expected = idx($expecteds_data, self::INVITED_EXPECTED, null);
+ $result = idx($expecteds_data, self::INVITED_RESULT, false);
+ $user = idx($expecteds_data, self::INVITED_USER, null);
+ if ($expected !== null) {
+ $this->assertEqual(
+ $expected,
+ $result,
+ pht('Unexpected presence of user "%s" in file "%s" on test "%s"',
+ $user == null ? '(unknown)' : $user->loadPrimaryEmailAddress(),
+ $file,
+ $test));
+ }
+ }
+ }
+
+ /**
+ * Generate a test user with a specific verified (or not) email.
+ * @param string $mail Email address
+ * @param int $is_verified 0: unverified, 1: verified
+ * @return PhabricatorUser
+ */
+ private function generateTestUserWithVerifiedMail($mail, $is_verified) {
+ $user = $this->generateNewTestUser();
+
+ // Set our primary address as verified or not.
+ $email = id(new PhabricatorUserEmail())->loadOneWhere(
+ 'userPHID = %s',
+ $user->getPHID());
+
+ $email->setAddress($mail);
+ $email->setIsVerified($is_verified);
+ $email->setIsPrimary(true);
+ $email->save();
+
+ return $user;
+ }
+
+}
diff --git a/src/applications/calendar/import/__tests__/events/simple-event-alincoln-guest.ics b/src/applications/calendar/import/__tests__/events/simple-event-alincoln-guest.ics
new file mode 100644
--- /dev/null
+++ b/src/applications/calendar/import/__tests__/events/simple-event-alincoln-guest.ics
@@ -0,0 +1,23 @@
+BEGIN:VCALENDAR
+VERSION:2.0
+PRODID:-//Phacility//Phabricator//EN
+BEGIN:VEVENT
+DTSTART;VALUE=DATE:20240205
+DTEND;VALUE=DATE:20240206
+DTSTAMP:20240411T230207Z
+ORGANIZER;CN=asd@group.calendar.google.com:mailto:lol@group.calendar.google.com
+UID:blabla@google.com
+ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;CN=
+ foo@example.com;X-NUM-GUESTS=0:mailto:foo@example.com
+ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;CN=a.
+ lincoln@example.com;X-NUM-GUESTS=0:mailto:a.lincoln@example.com
+ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;CN=
+ alien@example.com;X-NUM-GUESTS=0:mailto:alien@example.com
+CREATED:20240123T154250Z
+LAST-MODIFIED:20240123T162620Z
+SEQUENCE:0
+STATUS:CONFIRMED
+SUMMARY:A Lincoln, Foo and Alien to FOSDEM 2024
+TRANSP:TRANSPARENT
+END:VEVENT
+END:VCALENDAR
diff --git a/src/applications/people/query/PhabricatorPeopleUserEmailQuery.php b/src/applications/people/query/PhabricatorPeopleUserEmailQuery.php
--- a/src/applications/people/query/PhabricatorPeopleUserEmailQuery.php
+++ b/src/applications/people/query/PhabricatorPeopleUserEmailQuery.php
@@ -5,6 +5,8 @@
private $ids;
private $phids;
+ private $userPhids;
+ private $isVerified;
public function withIDs(array $ids) {
$this->ids = $ids;
@@ -16,6 +18,24 @@
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 @@
$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;
}

File Metadata

Mime Type
text/plain
Expires
Sun, Dec 22, 05:44 (43 m, 28 s)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1025397
Default Alt Text
D25363.1734846296.diff (18 KB)

Event Timeline

Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0