From 3ef5087f098add1c7e7aa7001f5a6aeaaeb8bb70 Mon Sep 17 00:00:00 2001 From: dwindown Date: Tue, 11 Nov 2025 16:05:21 +0700 Subject: [PATCH] fix: Critical data structure and mutation bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 🐛 Critical Fixes ### Issue 1: Toggling One Channel Affects Both **Problem:** Disabling email disabled both email and push **Root Cause:** Optimistic update with `onSettled` refetch caused race condition **Fix:** Removed optimistic update, use server response directly **Before:** ```ts onMutate: async () => { // Optimistic update queryClient.setQueryData(...) } onSettled: () => { // This refetch caused race condition queryClient.invalidateQueries(...) } ``` **After:** ```ts onSuccess: (data, variables) => { // Update cache with verified server response queryClient.setQueryData([...], (old) => old.map(channel => channel.id === variables.channelId ? { ...channel, enabled: data.enabled } : channel ) ); } ``` ### Issue 2: Events Cannot Be Enabled **Problem:** All event channels disabled and cannot be enabled **Root Cause:** Wrong data structure in `update_event()` **Before:** ```php $settings[$event_id][$channel_id] = [...]; // Saved as: { "order_placed": { "email": {...} } } ``` **After:** ```php $settings[$event_id]['channels'][$channel_id] = [...]; // Saves as: { "order_placed": { "channels": { "email": {...} } } } ``` ### Issue 3: POST Data Not Parsed **Problem:** Event updates not working **Root Cause:** Using `get_param()` instead of `get_json_params()` **Fix:** Changed to `get_json_params()` in `update_event()` ### What Was Fixed 1. ✅ Channel toggles work independently 2. ✅ No race conditions from optimistic updates 3. ✅ Event channel data structure matches get_events 4. ✅ Event toggles save correctly 5. ✅ POST data parsed properly 6. ✅ Boolean type enforcement ### Data Structure **Correct Structure:** ```php [ 'order_placed' => [ 'channels' => [ 'email' => ['enabled' => true, 'recipient' => 'admin'], 'push' => ['enabled' => false, 'recipient' => 'admin'] ] ] ] ``` --- **All toggles should now work correctly!** ✅ --- .../Settings/Notifications/Channels.tsx | 33 ++++++------------- includes/Api/NotificationsController.php | 21 +++++++----- 2 files changed, 23 insertions(+), 31 deletions(-) diff --git a/admin-spa/src/routes/Settings/Notifications/Channels.tsx b/admin-spa/src/routes/Settings/Notifications/Channels.tsx index 6165b94..1a494a9 100644 --- a/admin-spa/src/routes/Settings/Notifications/Channels.tsx +++ b/admin-spa/src/routes/Settings/Notifications/Channels.tsx @@ -47,38 +47,25 @@ export default function NotificationChannels() { // Toggle channel mutation const toggleChannelMutation = useMutation({ mutationFn: async ({ channelId, enabled }: { channelId: string; enabled: boolean }) => { - return api.post('/notifications/channels/toggle', { channelId, enabled }); + const response = await api.post('/notifications/channels/toggle', { channelId, enabled }); + return response; }, - onMutate: async ({ channelId, enabled }) => { - // Cancel outgoing refetches - await queryClient.cancelQueries({ queryKey: ['notification-channels'] }); - - // Snapshot previous value - const previousChannels = queryClient.getQueryData(['notification-channels']); - - // Optimistically update + onSuccess: (data, variables) => { + // Update cache with server response queryClient.setQueryData(['notification-channels'], (old: any) => { if (!old) return old; return old.map((channel: any) => - channel.id === channelId ? { ...channel, enabled } : channel + channel.id === variables.channelId + ? { ...channel, enabled: data.enabled } + : channel ); }); - - return { previousChannels }; - }, - onSuccess: () => { toast.success(__('Channel updated')); }, - onError: (error: any, variables, context: any) => { - // Rollback on error - if (context?.previousChannels) { - queryClient.setQueryData(['notification-channels'], context.previousChannels); - } - toast.error(error?.message || __('Failed to update channel')); - }, - onSettled: () => { - // Refetch after mutation + onError: (error: any) => { + // Refetch on error to sync with server queryClient.invalidateQueries({ queryKey: ['notification-channels'] }); + toast.error(error?.message || __('Failed to update channel')); }, }); diff --git a/includes/Api/NotificationsController.php b/includes/Api/NotificationsController.php index 0437129..350e3d2 100644 --- a/includes/Api/NotificationsController.php +++ b/includes/Api/NotificationsController.php @@ -297,10 +297,11 @@ class NotificationsController { * @return WP_REST_Response|WP_Error */ public function update_event(WP_REST_Request $request) { - $event_id = $request->get_param('eventId'); - $channel_id = $request->get_param('channelId'); - $enabled = $request->get_param('enabled'); - $recipient = $request->get_param('recipient'); + $params = $request->get_json_params(); + $event_id = isset($params['eventId']) ? $params['eventId'] : null; + $channel_id = isset($params['channelId']) ? $params['channelId'] : null; + $enabled = isset($params['enabled']) ? $params['enabled'] : null; + $recipient = isset($params['recipient']) ? $params['recipient'] : null; if (empty($event_id) || empty($channel_id)) { return new WP_Error( @@ -315,16 +316,20 @@ class NotificationsController { // Update settings if (!isset($settings[$event_id])) { - $settings[$event_id] = []; + $settings[$event_id] = ['channels' => []]; } - $settings[$event_id][$channel_id] = [ - 'enabled' => $enabled, + if (!isset($settings[$event_id]['channels'])) { + $settings[$event_id]['channels'] = []; + } + + $settings[$event_id]['channels'][$channel_id] = [ + 'enabled' => (bool) $enabled, 'recipient' => $recipient ?? 'admin', ]; // Save settings - update_option('woonoow_notification_settings', $settings); + update_option('woonoow_notification_settings', $settings, false); // Fire action for addons to react do_action('woonoow_notification_event_updated', $event_id, $channel_id, $enabled, $recipient);