Repository: gitify-app/gitify
Pull request #1007 (2024-04-15 18:21:20)

fix(settings): allow showBots only when detailedNotifications is selected
Also make it null safe for notifications that don't have users (ie: workflow runs)

setchy: > Seems logical I suppose!

now with added tests :) 

Patch:
diff --git a/src/hooks/useNotifications.ts b/src/hooks/useNotifications.ts
index f830be06..5565ba13 100644
--- a/src/hooks/useNotifications.ts
+++ b/src/hooks/useNotifications.ts
@@ -174,7 +174,7 @@ export const useNotifications = (): NotificationsState => {
                         return notifications.filter((notification) => {
                           if (
                             !settings.showBots &&
-                            notification.subject?.user.type === 'Bot'
+                            notification.subject?.user?.type === 'Bot'
                           ) {
                             return false;
                           }
diff --git a/src/routes/Settings.tsx b/src/routes/Settings.tsx
index 28228dfc..9928b607 100644
--- a/src/routes/Settings.tsx
+++ b/src/routes/Settings.tsx
@@ -181,8 +181,24 @@ export const SettingsRoute: FC = () => {
           <Checkbox
             name="showBots"
             label="Show notifications from Bot accounts"
-            checked={settings.showBots}
-            onChange={(evt) => updateSetting('showBots', evt.target.checked)}
+            checked={!settings.detailedNotifications || settings.showBots}
+            onChange={(evt) =>
+              settings.detailedNotifications &&
+              updateSetting('showBots', evt.target.checked)
+            }
+            disabled={!settings.detailedNotifications}
+            tooltip={
+              <div>
+                <div className="pb-3">
+                  Show or hide notifications from Bot accounts, such as
+                  @dependabot, @renovatebot, etc
+                </div>
+                <div className="text-orange-600">
+                  ⚠️ This setting requires{' '}
+                  <strong>Detailed Notifications</strong> to be enabled.
+                </div>
+              </div>
+            }
           />
           <Checkbox
             name="markAsDoneOnOpen"


Test patch:
diff --git a/src/routes/Settings.test.tsx b/src/routes/Settings.test.tsx
index afee0c47..a5155f5d 100644
--- a/src/routes/Settings.test.tsx
+++ b/src/routes/Settings.test.tsx
@@ -119,12 +119,16 @@ describe('routes/Settings.tsx', () => {
     expect(updateSetting).toHaveBeenCalledWith('participating', false);
   });
 
-  it('should toggle the showBots checkbox', async () => {
+  it('should not be able to toggle the showBots checkbox when detailedNotifications is disabled', async () => {
     await act(async () => {
       render(
         <AppContext.Provider
           value={{
-            settings: mockSettings,
+            settings: {
+              ...mockSettings,
+              detailedNotifications: false,
+              showBots: true,
+            },
             accounts: mockAccounts,
             updateSetting,
           }}
@@ -136,15 +140,65 @@ describe('routes/Settings.tsx', () => {
       );
     });
 
+    expect(
+      screen
+        .getByLabelText('Show notifications from Bot accounts')
+        .closest('input'),
+    ).toHaveProperty('disabled', true);
+
+    // click the checkbox
     fireEvent.click(
       screen.getByLabelText('Show notifications from Bot accounts'),
-      {
-        target: { checked: true },
-      },
     );
 
-    expect(updateSetting).toHaveBeenCalledTimes(1);
+    // check if the checkbox is still unchecked
+    expect(updateSetting).not.toHaveBeenCalled();
+
+    expect(
+      screen.getByLabelText('Show notifications from Bot accounts').parentNode
+        .parentNode,
+    ).toMatchSnapshot();
+  });
+
+  it('should be able to toggle the showBots checkbox when detailedNotifications is enabled', async () => {
+    await act(async () => {
+      render(
+        <AppContext.Provider
+          value={{
+            settings: {
+              ...mockSettings,
+              detailedNotifications: true,
+              showBots: true,
+            },
+            accounts: mockAccounts,
+            updateSetting,
+          }}
+        >
+          <MemoryRouter>
+            <SettingsRoute />
+          </MemoryRouter>
+        </AppContext.Provider>,
+      );
+    });
+
+    expect(
+      screen
+        .getByLabelText('Show notifications from Bot accounts')
+        .closest('input'),
+    ).toHaveProperty('disabled', false);
+
+    // click the checkbox
+    fireEvent.click(
+      screen.getByLabelText('Show notifications from Bot accounts'),
+    );
+
+    // check if the checkbox is still unchecked
     expect(updateSetting).toHaveBeenCalledWith('showBots', false);
+
+    expect(
+      screen.getByLabelText('Show notifications from Bot accounts').parentNode
+        .parentNode,
+    ).toMatchSnapshot();
   });
 
   it('should toggle the showNotificationsCountInTray checkbox', async () => {
diff --git a/src/routes/__snapshots__/Settings.test.tsx.snap b/src/routes/__snapshots__/Settings.test.tsx.snap
index 0f3cbb6d..0dbaa19e 100644
--- a/src/routes/__snapshots__/Settings.test.tsx.snap
+++ b/src/routes/__snapshots__/Settings.test.tsx.snap
@@ -1,5 +1,51 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`routes/Settings.tsx should be able to toggle the showBots checkbox when detailedNotifications is enabled 1`] = `
+<div
+  class="flex items-start"
+>
+  <div
+    class="flex items-center h-5"
+  >
+    <input
+      checked=""
+      class="focus:ring-indigo-500 h-4 w-4 text-indigo-600 border-gray-300 rounded"
+      id="showBots"
+      type="checkbox"
+    />
+  </div>
+  <div
+    class="ml-3 "
+  >
+    <label
+      class="font-medium text-gray-700 dark:text-gray-200"
+      for="showBots"
+    >
+      Show notifications from Bot accounts
+      <span
+        class="relative"
+        title="tooltip"
+      >
+        <svg
+          aria-hidden="true"
+          class="text-blue-500 ml-1"
+          fill="currentColor"
+          focusable="false"
+          height="16"
+          style="display: inline-block; user-select: none; vertical-align: text-bottom; overflow: visible;"
+          viewBox="0 0 16 16"
+          width="16"
+        >
+          <path
+            d="M0 8a8 8 0 1 1 16 0A8 8 0 0 1 0 8Zm8-6.5a6.5 6.5 0 1 0 0 13 6.5 6.5 0 0 0 0-13ZM6.92 6.085h.001a.749.749 0 1 1-1.342-.67c.169-.339.436-.701.849-.977C6.845 4.16 7.369 4 8 4a2.756 2.756 0 0 1 1.637.525c.503.377.863.965.863 1.725 0 .448-.115.83-.329 1.15-.205.307-.47.513-.692.662-.109.072-.22.138-.313.195l-.006.004a6.24 6.24 0 0 0-.26.16.952.952 0 0 0-.276.245.75.75 0 0 1-1.248-.832c.184-.264.42-.489.692-.661.103-.067.207-.132.313-.195l.007-.004c.1-.061.182-.11.258-.161a.969.969 0 0 0 .277-.245C8.96 6.514 9 6.427 9 6.25a.612.612 0 0 0-.262-.525A1.27 1.27 0 0 0 8 5.5c-.369 0-.595.09-.74.187a1.01 1.01 0 0 0-.34.398ZM9 11a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z"
+          />
+        </svg>
+      </span>
+    </label>
+  </div>
+</div>
+`;
+
 exports[`routes/Settings.tsx should not be able to enable detailed notifications due to missing scope 1`] = `
 <div
   class="flex items-start"
@@ -47,6 +93,54 @@ exports[`routes/Settings.tsx should not be able to enable detailed notifications
 </div>
 `;
 
+exports[`routes/Settings.tsx should not be able to toggle the showBots checkbox when detailedNotifications is disabled 1`] = `
+<div
+  class="flex items-start"
+>
+  <div
+    class="flex items-center h-5"
+  >
+    <input
+      checked=""
+      class="focus:ring-indigo-500 h-4 w-4 text-indigo-600 border-gray-300 rounded"
+      disabled=""
+      id="showBots"
+      type="checkbox"
+    />
+  </div>
+  <div
+    class="ml-3 "
+  >
+    <label
+      class="font-medium text-gray-700 dark:text-gray-200"
+      for="showBots"
+      style="text-decoration: line-through;"
+    >
+      Show notifications from Bot accounts
+      <span
+        class="relative"
+        title="tooltip"
+      >
+        <svg
+          aria-hidden="true"
+          class="text-blue-500 ml-1"
+          fill="currentColor"
+          focusable="false"
+          height="16"
+          style="display: inline-block; user-select: none; vertical-align: text-bottom; overflow: visible;"
+          viewBox="0 0 16 16"
+          width="16"
+        >
+          <path
+            d="M0 8a8 8 0 1 1 16 0A8 8 0 0 1 0 8Zm8-6.5a6.5 6.5 0 1 0 0 13 6.5 6.5 0 0 0 0-13ZM6.92 6.085h.001a.749.749 0 1 1-1.342-.67c.169-.339.436-.701.849-.977C6.845 4.16 7.369 4 8 4a2.756 2.756 0 0 1 1.637.525c.503.377.863.965.863 1.725 0 .448-.115.83-.329 1.15-.205.307-.47.513-.692.662-.109.072-.22.138-.313.195l-.006.004a6.24 6.24 0 0 0-.26.16.952.952 0 0 0-.276.245.75.75 0 0 1-1.248-.832c.184-.264.42-.489.692-.661.103-.067.207-.132.313-.195l.007-.004c.1-.061.182-.11.258-.161a.969.969 0 0 0 .277-.245C8.96 6.514 9 6.427 9 6.25a.612.612 0 0 0-.262-.525A1.27 1.27 0 0 0 8 5.5c-.369 0-.595.09-.74.187a1.01 1.01 0 0 0-.34.398ZM9 11a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z"
+          />
+        </svg>
+      </span>
+    </label>
+  </div>
+</div>
+`;
+
 exports[`routes/Settings.tsx should render itself & its children 1`] = `
 <div
   class="flex flex-1 flex-col h-screen dark:bg-gray-dark dark:text-white"
@@ -293,6 +387,7 @@ exports[`routes/Settings.tsx should render itself & its children 1`] = `
             <input
               checked=""
               class="focus:ring-indigo-500 h-4 w-4 text-indigo-600 border-gray-300 rounded"
+              disabled=""
               id="showBots"
               type="checkbox"
             />
@@ -303,8 +398,28 @@ exports[`routes/Settings.tsx should render itself & its children 1`] = `
             <label
               class="font-medium text-gray-700 dark:text-gray-200"
               for="showBots"
+              style="text-decoration: line-through;"
             >
               Show notifications from Bot accounts
+              <span
+                class="relative"
+                title="tooltip"
+              >
+                <svg
+                  aria-hidden="true"
+                  class="text-blue-500 ml-1"
+                  fill="currentColor"
+                  focusable="false"
+                  height="16"
+                  style="display: inline-block; user-select: none; vertical-align: text-bottom; overflow: visible;"
+                  viewBox="0 0 16 16"
+                  width="16"
+                >
+                  <path
+                    d="M0 8a8 8 0 1 1 16 0A8 8 0 0 1 0 8Zm8-6.5a6.5 6.5 0 1 0 0 13 6.5 6.5 0 0 0 0-13ZM6.92 6.085h.001a.749.749 0 1 1-1.342-.67c.169-.339.436-.701.849-.977C6.845 4.16 7.369 4 8 4a2.756 2.756 0 0 1 1.637.525c.503.377.863.965.863 1.725 0 .448-.115.83-.329 1.15-.205.307-.47.513-.692.662-.109.072-.22.138-.313.195l-.006.004a6.24 6.24 0 0 0-.26.16.952.952 0 0 0-.276.245.75.75 0 0 1-1.248-.832c.184-.264.42-.489.692-.661.103-.067.207-.132.313-.195l.007-.004c.1-.061.182-.11.258-.161a.969.969 0 0 0 .277-.245C8.96 6.514 9 6.427 9 6.25a.612.612 0 0 0-.262-.525A1.27 1.27 0 0 0 8 5.5c-.369 0-.595.09-.74.187a1.01 1.01 0 0 0-.34.398ZM9 11a1 1 0 1 1-2 0 1 1 0 0 1 2 0Z"
+                  />
+                </svg>
+              </span>
             </label>
           </div>
         </div>