Skip to content

Restore security alert unsubscribe fallback #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jasonlong merged 1 commit into from
Jul 13, 2026
Merged

Restore security alert unsubscribe fallback #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 14 additions & 18 deletions Octodot/App/AppState.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ final class AppState {
private static let inboxModeStorageKey = "AppState.inboxMode.v1"
private static let groupByRepoStorageKey = "AppState.groupByRepo.v1"
private static let visibleSubjectStateBatchSize = 20
private static let unsupportedSecurityAlertActionMessage =
"Security alerts can only be opened or marked done"
static let pageJumpCount = 8
static let halfPageJumpCount = 4

Expand Down Expand Up @@ -504,10 +502,13 @@ final class AppState {
if let batch = checkedNotificationsBatch() {
let originalVisibleOrder = filteredNotifications
let originalSelectionID = selectedNotificationID
let hasUnsupportedAlerts = batch.contains { $0.source == .dependabotAlert }
var acceptedItems: [GitHubNotification] = []
let securityAlerts = batch.filter { $0.source == .dependabotAlert }
var unsubscribedItems: [GitHubNotification] = []
clearChecked()

for notification in securityAlerts {
dismissSecurityAlert(notification, updatesSelection: false)
}
for group in groupedThreadNotifications(from: batch) {
guard let representative = group.first else { continue }
if startThreadAction(
Expand All @@ -518,26 +519,23 @@ final class AppState {
) {
inboxStore.muteThread(representative.threadId)
clampSelection()
acceptedItems.append(contentsOf: group)
unsubscribedItems.append(contentsOf: group)
}
}

restoreSelectionAfterBulkMutation(
originalSelectionID: originalSelectionID,
originalVisibleOrder: originalVisibleOrder
)
presentActionToast(verb: .unsub, items: acceptedItems)
if hasUnsupportedAlerts {
errorMessage = Self.unsupportedSecurityAlertActionMessage
}
presentActionToast(verb: .unsub, items: unsubscribedItems)
presentActionToast(verb: .done, items: securityAlerts)
return
}
guard let notification = selectedNotification else { return }
guard notification.source == .thread else {
errorMessage = Self.unsupportedSecurityAlertActionMessage
return
}
if startThreadAction(.unsubscribe) {
if notification.source == .dependabotAlert {
dismissSecurityAlert(notification)
presentActionToast(verb: .done, items: [notification])
} else if startThreadAction(.unsubscribe) {
inboxStore.muteThread(notification.threadId)
clampSelection()
presentActionToast(verb: .unsub, items: [notification])
Expand Down Expand Up @@ -933,7 +931,7 @@ final class AppState {
guard let client = apiClient else { return false }
guard let target = explicitTarget ?? selectedNotification else { return false }
guard target.source == .thread else {
errorMessage = Self.unsupportedSecurityAlertActionMessage
errorMessage = "Security alerts can only be opened or marked done"
return false
}
guard !threadActions.hasPendingAction(for: target.threadId) else { return false }
Expand Down Expand Up @@ -1101,9 +1099,7 @@ final class AppState {
"target.thread=\(pending.notification.threadId) server=\(serverNotifications.map(\.id).joined(separator: ","))"
)

if errorMessage != Self.unsupportedSecurityAlertActionMessage {
errorMessage = nil
}
errorMessage = nil
rebuildDerivedState()
DebugTrace.log(
"after rebuild success kind=\(pending.kind.rawValue) selected=\(selectedNotificationID ?? "nil") " +
Expand Down
71 changes: 62 additions & 9 deletions OctodotTests/AppStateTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,58 @@ struct AppStateTests {
#expect(state.filteredNotifications.contains { $0.id == alertID })
}

@Test func unsubscribeCommandMarksSecurityAlertDone() async {
let session = StubNetworkSession(results: [
.success((
Self.notificationsPayload(ids: ["1"]),
Self.httpResponse(
url: "https://api.github.com/notifications",
statusCode: 200,
headers: ["Last-Modified": "Wed, 01 Apr 2026 12:00:00 GMT"]
)
)),
.success((
Data("[]".utf8),
Self.httpResponse(
url: "https://api.github.com/notifications?all=true",
statusCode: 200
)
)),
.success((
Self.dependabotAlertsPayload(),
Self.httpResponse(
url: "https://api.github.com/repos/acme/alpha/dependabot/alerts",
statusCode: 200
)
)),
])
let client = GitHubAPIClient(token: "ghp_secret", session: session, useGraphQLForSubjectMetadata: false)
let state = AppState(
notifications: [],
authStatus: .signedIn(username: "octodot"),
apiClient: client,
userDefaults: Self.makeIsolatedUserDefaults()
)
state.groupByRepo = false
state.isPanelVisible = true

await state.loadNotifications(force: true)
let alertID = "dependabot:acme/alpha:7"
await Self.waitUntil {
await MainActor.run {
state.filteredNotifications.contains { $0.id == alertID }
}
}
state.selectNotification(id: alertID)

state.unsubscribeFromThread()

#expect(state.filteredNotifications.contains { $0.id == alertID } == false)
#expect(state.actionToasts.last?.message == "Marked acme/alpha done")
#expect(state.errorMessage == nil)
#expect((await session.recordedRequests()).count == 3)
}

@Test func openInBrowserMarksSecurityAlertReadLocally() async {
let session = StubNetworkSession(results: [
.success((
Expand Down Expand Up @@ -3511,7 +3563,7 @@ struct AppStateTests {
#expect(relaunched.filteredNotifications.isEmpty)
}

@Test func bulkUnsubscribeSkipsSecurityAlertsAndReportsOnlyAcceptedThreads() async {
@Test func bulkUnsubscribeMarksSecurityAlertsDoneAndReportsBothOutcomes() async {
let thread = Self.makeNotification(id: 0)
let alert = Self.makeSecurityAlert()
let (state, session) = Self.makeAuthedState(
Expand All @@ -3528,14 +3580,16 @@ struct AppStateTests {

state.unsubscribeFromThread()

#expect(state.filteredNotifications.map(\.id) == [alert.id])
#expect(state.actionToasts.last?.message == "Unsubscribed from acme/alpha#0")
#expect(state.errorMessage == "Security alerts can only be opened or marked done")
#expect(state.actionToasts.map(\.message) == [
"Unsubscribed from acme/alpha#0",
"Marked acme/alpha done",
])
#expect(state.errorMessage == nil)
await Self.waitUntil { await session.recordedRequests().count == 2 }
#expect(state.errorMessage == "Security alerts can only be opened or marked done")
#expect(state.errorMessage == nil)
}

@Test func singleSecurityAlertUnsubscribeIsRejectedWithoutSuccessFeedback() {
@Test func singleSecurityAlertUnsubscribeUsesDoneFallback() {
let alert = Self.makeSecurityAlert()
let (state, _) = Self.makeAuthedState(notifications: [alert])
state.inboxMode = .inbox
Expand All @@ -3544,9 +3598,8 @@ struct AppStateTests {

state.unsubscribeFromThread()

#expect(state.filteredNotifications.map(\.id) == [alert.id])
#expect(state.actionToasts.isEmpty)
#expect(state.errorMessage == "Security alerts can only be opened or marked done")
#expect(state.actionToasts.last?.message == "Marked acme/alpha done")
#expect(state.errorMessage == nil)
}

@Test func securityAlertDoneProducesAccurateSingleAndMixedFeedback() {
Expand Down
Loading