Skip to content

Commit 86da335

Browse files
authored
fix(settings): allow showBots only when detailedNotifications is selected (#1007)
* fix: allow showBots only when detailedNotifications is selected. make null safe * fix: allow showBots only when detailedNotifications is selected. make null safe
1 parent 02a7a38 commit 86da335

File tree

4 files changed

+194
-9
lines changed

4 files changed

+194
-9
lines changed

src/hooks/useNotifications.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ export const useNotifications = (): NotificationsState => {
174174
return notifications.filter((notification) => {
175175
if (
176176
!settings.showBots &&
177-
notification.subject?.user.type === 'Bot'
177+
notification.subject?.user?.type === 'Bot'
178178
) {
179179
return false;
180180
}

src/routes/Settings.test.tsx

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,16 @@ describe('routes/Settings.tsx', () => {
119119
expect(updateSetting).toHaveBeenCalledWith('participating', false);
120120
});
121121

122-
it('should toggle the showBots checkbox', async () => {
122+
it('should not be able to toggle the showBots checkbox when detailedNotifications is disabled', async () => {
123123
await act(async () => {
124124
render(
125125
<AppContext.Provider
126126
value={{
127-
settings: mockSettings,
127+
settings: {
128+
...mockSettings,
129+
detailedNotifications: false,
130+
showBots: true,
131+
},
128132
accounts: mockAccounts,
129133
updateSetting,
130134
}}
@@ -136,15 +140,65 @@ describe('routes/Settings.tsx', () => {
136140
);
137141
});
138142

143+
expect(
144+
screen
145+
.getByLabelText('Show notifications from Bot accounts')
146+
.closest('input'),
147+
).toHaveProperty('disabled', true);
148+
149+
// click the checkbox
139150
fireEvent.click(
140151
screen.getByLabelText('Show notifications from Bot accounts'),
141-
{
142-
target: { checked: true },
143-
},
144152
);
145153

146-
expect(updateSetting).toHaveBeenCalledTimes(1);
154+
// check if the checkbox is still unchecked
155+
expect(updateSetting).not.toHaveBeenCalled();
156+
157+
expect(
158+
screen.getByLabelText('Show notifications from Bot accounts').parentNode
159+
.parentNode,
160+
).toMatchSnapshot();
161+
});
162+
163+
it('should be able to toggle the showBots checkbox when detailedNotifications is enabled', async () => {
164+
await act(async () => {
165+
render(
166+
<AppContext.Provider
167+
value={{
168+
settings: {
169+
...mockSettings,
170+
detailedNotifications: true,
171+
showBots: true,
172+
},
173+
accounts: mockAccounts,
174+
updateSetting,
175+
}}
176+
>
177+
<MemoryRouter>
178+
<SettingsRoute />
179+
</MemoryRouter>
180+
</AppContext.Provider>,
181+
);
182+
});
183+
184+
expect(
185+
screen
186+
.getByLabelText('Show notifications from Bot accounts')
187+
.closest('input'),
188+
).toHaveProperty('disabled', false);
189+
190+
// click the checkbox
191+
fireEvent.click(
192+
screen.getByLabelText('Show notifications from Bot accounts'),
193+
);
194+
195+
// check if the checkbox is still unchecked
147196
expect(updateSetting).toHaveBeenCalledWith('showBots', false);
197+
198+
expect(
199+
screen.getByLabelText('Show notifications from Bot accounts').parentNode
200+
.parentNode,
201+
).toMatchSnapshot();
148202
});
149203

150204
it('should toggle the showNotificationsCountInTray checkbox', async () => {

src/routes/Settings.tsx

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,24 @@ export const SettingsRoute: FC = () => {
181181
<Checkbox
182182
name="showBots"
183183
label="Show notifications from Bot accounts"
184-
checked={settings.showBots}
185-
onChange={(evt) => updateSetting('showBots', evt.target.checked)}
184+
checked={!settings.detailedNotifications || settings.showBots}
185+
onChange={(evt) =>
186+
settings.detailedNotifications &&
187+
updateSetting('showBots', evt.target.checked)
188+
}
189+
disabled={!settings.detailedNotifications}
190+
tooltip={
191+
<div>
192+
<div className="pb-3">
193+
Show or hide notifications from Bot accounts, such as
194+
@dependabot, @renovatebot, etc
195+
</div>
196+
<div className="text-orange-600">
197+
⚠️ This setting requires{' '}
198+
<strong>Detailed Notifications</strong> to be enabled.
199+
</div>
200+
</div>
201+
}
186202
/>
187203
<Checkbox
188204
name="markAsDoneOnOpen"

src/routes/__snapshots__/Settings.test.tsx.snap

Lines changed: 115 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)