Chandler bug: Updating a Reminder Removes Associated user_notification_channels

[NixFey]

Describe the bug
Using the most recent build of Chandler, a pretty major issue occurs when updating a contact reminder which has been scheduled for a notification channel. While going through the update handler, all notification channels which that reminder is using are removed. Note: Not just the contact_reminder_scheduled records, but the overall user_notification_channel records. This is most definitely not the expected behavior. DB Engine: MySQL, hosting: Docker

Screenshots

Additional context
Relevant code snippet: https://github.com/monicahq/monica/blob/chandler/app/Domains/Contact/ManageReminders/Services/UpdateContactReminder.php#L84-L87

[djaiss]

This is really bad indeed. I'll fix this. Thanks so much for raising this bug.

[djaiss]

Hang on. This is actually working as expected.
Yes, all the notifications channels for the given reminder are deleted. BUT right after, they are all generated again.

The reason is that it's actually easier to delete all the notif channels and generate them all again rather than going through them one by one and modify them.

See https://github.com/monicahq/monica/blob/main/app/Domains/Contact/ManageReminders/Services/UpdateContactReminder.php#L89C7-L89C7 for reference.

[djaiss]

I'll close it but feel free to open it again if you feel this is still not right.

[NixFey]

Please reopen 🙂

Thanks for looking into this @djaiss! I know what you're getting at when you say that, but I think there's a bit of a misunderstanding. When that bit of code runs, it completely wipes out the Telegram connection that I have set up, I need to go back in to the settings and set up Telegram from scratch. It seems that Laravel is not just deleting the record in the pivot table, but actually going one level deeper and deleting that too, if that makes sense?

The code was refactored back in September 2022, and I have a hunch it might have been broken since then 3fa0691#diff-5320638e148e1d8cac8cd9f8c6d7b02590b459bf3a8315e95efe882a14ba0225L94-R93

I've included below the full set of patches I had to make to Monica to get Telegram working. I considered opening a PR with this but I wasn't confident enough that this was all that needed to be done

diff --git a/app/Domains/Contact/ManageReminders/Services/UpdateContactReminder.php b/app/Domains/Contact/ManageReminders/Services/UpdateContactReminder.php
index e15752ec2..c6747f2cf 100644
--- a/app/Domains/Contact/ManageReminders/Services/UpdateContactReminder.php
+++ b/app/Domains/Contact/ManageReminders/Services/UpdateContactReminder.php
@@ -6,6 +6,7 @@ use App\Interfaces\ServiceInterface;
 use App\Models\ContactReminder;
 use App\Services\BaseService;
 use Carbon\Carbon;
+use Illuminate\Support\Facades\DB;

 class UpdateContactReminder extends BaseService implements ServiceInterface
 {
@@ -83,7 +84,7 @@ class UpdateContactReminder extends BaseService implements ServiceInterface

     private function deleteOldScheduledReminders(): void
     {
-        $this->reminder->userNotificationChannels->each->delete();
+        DB::table('contact_reminder_scheduled')->where('contact_reminder_id', $this->reminder->id)->delete();
     }

     private function scheduledReminderForAllUsersInVault(): void
diff --git a/app/Domains/Settings/ManageNotificationChannels/Web/Controllers/TelegramWebhookController.php b/app/Domains/Settings/ManageNotificationChannels/Web/Controllers/TelegramWebhookController.php
index 509e58d42..93156164a 100644
--- a/app/Domains/Settings/ManageNotificationChannels/Web/Controllers/TelegramWebhookController.php
+++ b/app/Domains/Settings/ManageNotificationChannels/Web/Controllers/TelegramWebhookController.php
@@ -4,9 +4,11 @@ namespace App\Domains\Settings\ManageNotificationChannels\Web\Controllers;

 use App\Http\Controllers\Controller;
 use App\Models\UserNotificationChannel;
+use App\Domains\Settings\ManageNotificationChannels\Services\ScheduleAllContactRemindersForNotificationChannel;
 use Exception;
 use Illuminate\Http\Request;
 use Illuminate\Support\Str;
+use Illuminate\Support\Facades\Auth;

 class TelegramWebhookController extends Controller
 {
@@ -52,6 +54,12 @@ class TelegramWebhookController extends Controller
         $channel->active = true;
         $channel->save();

+	(new ScheduleAllContactRemindersForNotificationChannel())->execute([
+            'account_id' => $channel->user->account_id,
+            'author_id' => $channel->user->id,
+            'user_notification_channel_id' => $channel->id,
+        ]);
+
         return response('Success', 200);
     }
 }

[github-actions]

🎉 This issue has been resolved in version 5.0.0-beta.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀