diff --git a/apps/dav/lib/CalDAV/BirthdayService.php b/apps/dav/lib/CalDAV/BirthdayService.php index 95176283f11d4..a34b5e0d33196 100644 --- a/apps/dav/lib/CalDAV/BirthdayService.php +++ b/apps/dav/lib/CalDAV/BirthdayService.php @@ -317,6 +317,11 @@ public function syncUser(string $user):void { } /** + * The birthday event is considered changed if either + * - the start date has changed + * - the title has changed + * - the time for the alarm has changed + * * @param string $existingCalendarData * @param VCalendar $newCalendarData * @return bool @@ -331,7 +336,8 @@ public function birthdayEvenChanged(string $existingCalendarData, return ( $newCalendarData->VEVENT->DTSTART->getValue() !== $existingBirthday->VEVENT->DTSTART->getValue() || - $newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue() + $newCalendarData->VEVENT->SUMMARY->getValue() !== $existingBirthday->VEVENT->SUMMARY->getValue() || + ($newCalendarData->VEVENT->VALARM && $existingBirthday->VEVENT->VALARM && $newCalendarData->VEVENT->VALARM->TRIGGER->getValue() !== $existingBirthday->VEVENT->VALARM->TRIGGER->getValue()) ); } diff --git a/apps/dav/lib/Migration/RegenerateBirthdayCalendars.php b/apps/dav/lib/Migration/RegenerateBirthdayCalendars.php index a8138d876e907..806eaf04c24a8 100644 --- a/apps/dav/lib/Migration/RegenerateBirthdayCalendars.php +++ b/apps/dav/lib/Migration/RegenerateBirthdayCalendars.php @@ -4,6 +4,7 @@ * * @author Arthur Schiwon * @author Georg Ehrke + * @author Thomas Citharel * * @license GNU AGPL version 3 or any later version * @@ -30,36 +31,22 @@ use OCP\Migration\IRepairStep; class RegenerateBirthdayCalendars implements IRepairStep { + private IJobList $jobList; + private IConfig $config; - /** @var IJobList */ - private $jobList; - - /** @var IConfig */ - private $config; - - /** - * @param IJobList $jobList - * @param IConfig $config - */ public function __construct(IJobList $jobList, IConfig $config) { $this->jobList = $jobList; $this->config = $config; } - /** - * @return string - */ - public function getName() { + public function getName(): string { return 'Regenerating birthday calendars to use new icons and fix old birthday events without year'; } - /** - * @param IOutput $output - */ - public function run(IOutput $output) { + public function run(IOutput $output): void { // only run once - if ($this->config->getAppValue('dav', 'regeneratedBirthdayCalendarsForYearFix') === 'yes') { + if ($this->config->getAppValue('dav', 'regeneratedBirthdayCalendarsForYearFix') === 'yes' && $this->config->getAppValue('dav', 'regeneratedBirthdayCalendarsForAlarmFix') === 'yes') { $output->info('Repair step already executed'); return; } @@ -69,5 +56,6 @@ public function run(IOutput $output) { // if all were done, no need to redo the repair during next upgrade $this->config->setAppValue('dav', 'regeneratedBirthdayCalendarsForYearFix', 'yes'); + $this->config->setAppValue('dav', 'regeneratedBirthdayCalendarsForAlarmFix', 'yes'); } } diff --git a/apps/dav/tests/unit/Migration/RegenerateBirthdayCalendarsTest.php b/apps/dav/tests/unit/Migration/RegenerateBirthdayCalendarsTest.php index 5305abc92996f..bc36df93da62c 100644 --- a/apps/dav/tests/unit/Migration/RegenerateBirthdayCalendarsTest.php +++ b/apps/dav/tests/unit/Migration/RegenerateBirthdayCalendarsTest.php @@ -31,18 +31,17 @@ use OCP\BackgroundJob\IJobList; use OCP\IConfig; use OCP\Migration\IOutput; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; class RegenerateBirthdayCalendarsTest extends TestCase { - - /** @var IJobList | \PHPUnit\Framework\MockObject\MockObject */ + /** @var IJobList | MockObject */ private $jobList; - /** @var IConfig | \PHPUnit\Framework\MockObject\MockObject */ + /** @var IConfig | MockObject */ private $config; - /** @var RegenerateBirthdayCalendars */ - private $migration; + private RegenerateBirthdayCalendars $migration; protected function setUp(): void { parent::setUp(); @@ -61,41 +60,47 @@ public function testGetName(): void { ); } - public function testRun(): void { - $this->config->expects($this->once()) + public function dataForTestRun(): array { + return [ + ['', '', true], + ['yes', '', true], + ['yes', 'yes', false] + ]; + } + + /** + * @dataProvider dataForTestRun + */ + public function testRun(string $yearFix, string $alarmFix, bool $run): void { + $this->config->expects($this->exactly($yearFix === '' ? 1 : 2)) ->method('getAppValue') - ->with('dav', 'regeneratedBirthdayCalendarsForYearFix') - ->willReturn(null); + ->withConsecutive(['dav', 'regeneratedBirthdayCalendarsForYearFix'], ['dav', 'regeneratedBirthdayCalendarsForAlarmFix']) + ->willReturnOnConsecutiveCalls($yearFix, $alarmFix); $output = $this->createMock(IOutput::class); - $output->expects($this->once()) - ->method('info') - ->with('Adding background jobs to regenerate birthday calendar'); - - $this->jobList->expects($this->once()) - ->method('add') - ->with(RegisterRegenerateBirthdayCalendars::class); - $this->config->expects($this->once()) - ->method('setAppValue') - ->with('dav', 'regeneratedBirthdayCalendarsForYearFix', 'yes'); + if ($run) { + $output->expects($this->once()) + ->method('info') + ->with('Adding background jobs to regenerate birthday calendar'); - $this->migration->run($output); - } + $this->jobList->expects($this->once()) + ->method('add') + ->with(RegisterRegenerateBirthdayCalendars::class); - public function testRunSecondTime(): void { - $this->config->expects($this->once()) - ->method('getAppValue') - ->with('dav', 'regeneratedBirthdayCalendarsForYearFix') - ->willReturn('yes'); + $this->config->expects($this->exactly(2)) + ->method('setAppValue') + ->withConsecutive(['dav', 'regeneratedBirthdayCalendarsForYearFix', 'yes'], ['dav', 'regeneratedBirthdayCalendarsForAlarmFix', 'yes']); + } else { + $output->expects($this->once()) + ->method('info') + ->with('Repair step already executed'); - $output = $this->createMock(IOutput::class); - $output->expects($this->once()) - ->method('info') - ->with('Repair step already executed'); + $this->jobList->expects($this->never()) + ->method('add'); - $this->jobList->expects($this->never()) - ->method('add'); + $this->config->expects($this->never())->method('setAppValue'); + } $this->migration->run($output); }