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 @@ + 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; }