Skip to content

Commit c27878b

Browse files
authored
fix: notifications bugs (#140)
fix: muted collapsed notifications now silent, prevent dismiss re-alerts - Use silent channel when all collapsed events are muted - Add setOnlyAlertOnce to prevent alerts on notification reorganization - Exclude muted/task alarms from hasAlarms calculation - Extract notification logic into testable helper functions - Add Robolectric tests and architecture documentation * fix: silent reminders when all active (unsnoozed events) are muted wp * fix: maybe notify on every dismiss * test: update * fix: make the test use the real code * fix: test the real production codes * fix: maybe more respect mute * docs: notfication arch * fix: another notification fix * docs: notification_architecture more * docs: notification_architecture update * docs: all states appendix * test: more * fix: computeNotificationMode * fix: refactor use enum
1 parent 1da340f commit c27878b

4 files changed

Lines changed: 1489 additions & 27 deletions

File tree

android/app/src/main/java/com/github/quarck/calnotify/notification/EventNotificationManager.kt

Lines changed: 198 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -159,30 +159,43 @@ open class EventNotificationManager : EventNotificationManagerInterface {
159159
events: List<EventAlertRecord>,
160160
settings: Settings
161161
): Pair<List<EventAlertRecord>, List<EventAlertRecord>> {
162-
163-
if (events.size >= Consts.MAX_NUM_EVENTS_BEFORE_COLLAPSING_EVERYTHING) {
164-
// short-cut to avoid heavy memory load on dealing with lots of requests...
165-
return Pair(listOf(), events)
166-
}
167-
168162
val activeEvents = events.sortedBy { it.lastStatusChangeTime }
169-
170-
val maxNotifications = settings.maxNotifications
171-
val collapseEverything = settings.collapseEverything
172-
173-
var recentEvents = activeEvents.takeLast(maxNotifications - 1)
174-
var collapsedEvents = activeEvents.take(activeEvents.size - recentEvents.size)
175-
176-
if (collapsedEvents.size == 1) {
177-
recentEvents += collapsedEvents
178-
collapsedEvents = listOf()
179-
}
180-
else if (collapseEverything && !collapsedEvents.isEmpty()) {
181-
collapsedEvents = recentEvents + collapsedEvents
182-
recentEvents = listOf()
163+
164+
// Use computeNotificationMode as single source of truth for mode decision
165+
val mode = computeNotificationMode(
166+
eventCount = events.size,
167+
collapseEverything = settings.collapseEverything,
168+
maxNotifications = settings.maxNotifications
169+
)
170+
171+
return when (mode) {
172+
NotificationMode.ALL_COLLAPSED -> {
173+
// All events go to collapsed
174+
Pair(listOf(), activeEvents)
175+
}
176+
NotificationMode.PARTIAL_COLLAPSE -> {
177+
// Split: recent events shown individually, rest collapsed
178+
val recentEvents = activeEvents.takeLast(settings.maxNotifications - 1)
179+
val collapsedEvents = activeEvents.take(activeEvents.size - recentEvents.size)
180+
Pair(recentEvents, collapsedEvents)
181+
}
182+
NotificationMode.INDIVIDUAL -> {
183+
// All events shown individually
184+
Pair(activeEvents, listOf())
185+
}
183186
}
187+
}
184188

185-
return Pair(recentEvents, collapsedEvents)
189+
/**
190+
* Notification display modes based on event arrangement.
191+
*/
192+
enum class NotificationMode {
193+
/** Each event gets its own notification */
194+
INDIVIDUAL,
195+
/** Some events shown individually, rest collapsed into "X more" summary */
196+
PARTIAL_COLLAPSE,
197+
/** All events collapsed into a single notification */
198+
ALL_COLLAPSED
186199
}
187200

188201
private fun arrangeEvents(
@@ -222,7 +235,9 @@ open class EventNotificationManager : EventNotificationManagerInterface {
222235

223236
val (recentEvents, collapsedEvents) = arrangeEvents(db, currentTime, settings)
224237

225-
val anyAlarms = recentEvents.any { it.isAlarm } || collapsedEvents.any { it.isAlarm }
238+
// Only count unmuted, non-task alarms (muted alarms should stay silent)
239+
val anyAlarms = recentEvents.any { it.isAlarm && !it.isTask && !it.isMuted } ||
240+
collapsedEvents.any { it.isAlarm && !it.isTask && !it.isMuted }
226241

227242
if (!recentEvents.isEmpty())
228243
collapseDisplayedNotifications(
@@ -482,8 +497,8 @@ open class EventNotificationManager : EventNotificationManagerInterface {
482497
}
483498
}
484499

485-
if (playReminderSound)
486-
shouldPlayAndVibrate = shouldPlayAndVibrate || !isQuietPeriodActive || hasAlarms
500+
// Apply reminder sound override using shared helper (testable)
501+
shouldPlayAndVibrate = applyReminderSoundOverride(shouldPlayAndVibrate, playReminderSound, hasAlarms)
487502

488503
// now build actual notification and notify
489504
val intent = Intent(context, MainActivity::class.java)
@@ -513,8 +528,8 @@ open class EventNotificationManager : EventNotificationManagerInterface {
513528
}
514529
.toString()
515530

516-
// Use alarm channel if there are alarms, otherwise default
517-
val channelId = if (hasAlarms) NotificationChannels.CHANNEL_ID_ALARM else NotificationChannels.CHANNEL_ID_DEFAULT
531+
// Use appropriate channel using shared helper (testable)
532+
val channelId = computeCollapsedChannelId(events, hasAlarms, playReminderSound)
518533

519534
val builder =
520535
NotificationCompat.Builder(context, channelId)
@@ -530,6 +545,7 @@ open class EventNotificationManager : EventNotificationManagerInterface {
530545
.setContentIntent(pendingIntent)
531546
.setAutoCancel(false)
532547
.setOngoing(true)
548+
.setOnlyAlertOnce(!shouldPlayAndVibrate)
533549
.setStyle(NotificationCompat.BigTextStyle().bigText(bigText))
534550
.setNumber(numEvents)
535551
.setShowWhen(false)
@@ -991,6 +1007,7 @@ open class EventNotificationManager : EventNotificationManagerInterface {
9911007
)
9921008
.setWhen(clock.currentTimeMillis())
9931009
.setShowWhen(false)
1010+
.setOnlyAlertOnce(true)
9941011
.setNumber(numTotalEvents)
9951012
.setVisibility(NotificationCompat.VISIBILITY_PRIVATE)
9961013

@@ -1130,6 +1147,7 @@ open class EventNotificationManager : EventNotificationManagerInterface {
11301147
.setOngoing(
11311148
!notificationSettings.behavior.allowNotificationSwipe
11321149
)
1150+
.setOnlyAlertOnce(isForce || wasCollapsed)
11331151
.setStyle(
11341152
NotificationCompat.BigTextStyle().bigText(notificationTextString)
11351153
)
@@ -1474,15 +1492,19 @@ open class EventNotificationManager : EventNotificationManagerInterface {
14741492
}
14751493
.toString()
14761494

1495+
// Use silent channel if all collapsed events are muted
1496+
val channelId = computePartialCollapseChannelId(events)
1497+
14771498
val builder =
1478-
NotificationCompat.Builder(context, NotificationChannels.CHANNEL_ID_DEFAULT)
1499+
NotificationCompat.Builder(context, channelId)
14791500
.setContentTitle(title)
14801501
.setContentText(text)
14811502
.setSmallIcon(com.github.quarck.calnotify.R.drawable.stat_notify_calendar_multiple)
14821503
.setPriority(Notification.PRIORITY_LOW) // always LOW regardless of other settings for regular notifications, so it is always last
14831504
.setContentIntent(pendingIntent)
14841505
.setAutoCancel(false)
14851506
.setOngoing(true)
1507+
.setOnlyAlertOnce(true)
14861508
.setStyle(NotificationCompat.BigTextStyle().bigText(bigText))
14871509
.setShowWhen(false)
14881510
.setCategory(
@@ -1612,5 +1634,154 @@ open class EventNotificationManager : EventNotificationManagerInterface {
16121634
const val MAIN_ACTIVITY_NUM_NOTIFICATIONS_COLLAPSED_CODE = 1
16131635
const val MAIN_ACTIVITY_REMINDER_CODE = 2
16141636
const val MAIN_ACTIVITY_GROUP_NOTIFICATION_CODE = 3
1637+
1638+
/**
1639+
* Applies the reminder sound override logic.
1640+
* THIS IS THE ACTUAL PRODUCTION CODE - used by postEverythingCollapsed.
1641+
*
1642+
* Fixed bug: Previously was `currentValue || !isQuietPeriodActive || hasAlarms`
1643+
* which would force sound when NOT in quiet period, ignoring muted status.
1644+
* Now only hasAlarms can override the muted status.
1645+
*
1646+
* @param currentShouldPlayAndVibrate The value computed by the event loop
1647+
* @param playReminderSound Whether this is a reminder notification
1648+
* @param hasAlarms Whether there are non-muted alarm events
1649+
* @return Final shouldPlayAndVibrate value
1650+
*/
1651+
fun applyReminderSoundOverride(
1652+
currentShouldPlayAndVibrate: Boolean,
1653+
playReminderSound: Boolean,
1654+
hasAlarms: Boolean
1655+
): Boolean {
1656+
return if (playReminderSound) {
1657+
currentShouldPlayAndVibrate || hasAlarms
1658+
} else {
1659+
currentShouldPlayAndVibrate
1660+
}
1661+
}
1662+
1663+
/**
1664+
* Computes whether sound/vibration should play for collapsed notification.
1665+
* Uses a simplified loop (only checks muted status) + applyReminderSoundOverride.
1666+
*
1667+
* Note: The loop here is simplified compared to postEverythingCollapsed which also
1668+
* handles force, displayStatus, and isQuietPeriodActive. This function is used by
1669+
* tests to verify muted event behavior without needing full production dependencies.
1670+
*
1671+
* @param events List of events to display
1672+
* @param playReminderSound Whether this is a reminder (affects sound logic)
1673+
* @param hasAlarms Whether there are non-muted alarm events
1674+
* @return true if sound should play, false otherwise
1675+
*/
1676+
fun computeShouldPlayAndVibrateForCollapsed(
1677+
events: List<EventAlertRecord>,
1678+
playReminderSound: Boolean,
1679+
hasAlarms: Boolean
1680+
): Boolean {
1681+
// Simplified loop logic - checks muted status (matches production for basic cases)
1682+
var shouldPlayAndVibrate = false
1683+
for (event in events) {
1684+
if (event.snoozedUntil == 0L) {
1685+
val shouldBeQuiet = event.isMuted
1686+
shouldPlayAndVibrate = shouldPlayAndVibrate || !shouldBeQuiet
1687+
} else {
1688+
shouldPlayAndVibrate = shouldPlayAndVibrate || !event.isMuted
1689+
}
1690+
}
1691+
1692+
// Apply override using ACTUAL PRODUCTION CODE
1693+
return applyReminderSoundOverride(shouldPlayAndVibrate, playReminderSound, hasAlarms)
1694+
}
1695+
1696+
/**
1697+
* Computes the notification mode based on event count and settings.
1698+
* THIS IS ACTUAL PRODUCTION CODE - reflects the logic in arrangeEvents().
1699+
*
1700+
* The logic order matters and matches production:
1701+
* 1. Safety limit check (≥50 events)
1702+
* 2. Split into recent (maxNotifications-1) and collapsed (rest)
1703+
* 3. Special case: if only 1 would be collapsed, fold into recent
1704+
* 4. collapseEverything check (only if we still have collapsed events)
1705+
*
1706+
* @param eventCount Number of events to display
1707+
* @param collapseEverything User setting to collapse all events
1708+
* @param maxNotifications Maximum individual notifications before overflow
1709+
* @return The notification mode that will be used
1710+
*/
1711+
fun computeNotificationMode(
1712+
eventCount: Int,
1713+
collapseEverything: Boolean,
1714+
maxNotifications: Int
1715+
): NotificationMode {
1716+
// short-cut to avoid heavy memory load on dealing with lots of requests...
1717+
// Safety limit - always collapse at 50+
1718+
if (eventCount >= Consts.MAX_NUM_EVENTS_BEFORE_COLLAPSING_EVERYTHING) {
1719+
return NotificationMode.ALL_COLLAPSED
1720+
}
1721+
1722+
// Calculate how the split would work
1723+
// recentEvents = takeLast(maxNotifications - 1)
1724+
// collapsedEvents = rest
1725+
val recentCount = minOf(eventCount, maxNotifications - 1)
1726+
var collapsedCount = eventCount - recentCount
1727+
1728+
// Special case: if only 1 event would be collapsed, show it individually instead
1729+
if (collapsedCount == 1) {
1730+
collapsedCount = 0
1731+
}
1732+
1733+
// Now check collapseEverything - only applies if we have events to collapse
1734+
if (collapseEverything && collapsedCount > 0) {
1735+
return NotificationMode.ALL_COLLAPSED
1736+
}
1737+
1738+
// Determine mode based on whether we have collapsed events
1739+
return when {
1740+
collapsedCount == 0 -> NotificationMode.INDIVIDUAL
1741+
else -> NotificationMode.PARTIAL_COLLAPSE
1742+
}
1743+
}
1744+
1745+
/**
1746+
* Computes the notification channel ID for collapsed notifications (postEverythingCollapsed).
1747+
* THIS IS ACTUAL PRODUCTION CODE - called by postEverythingCollapsed.
1748+
*
1749+
* @param events List of events to display
1750+
* @param hasAlarms Whether there are non-muted alarm events
1751+
* @param isReminder Whether this is a reminder notification
1752+
* @return The appropriate channel ID
1753+
*/
1754+
fun computeCollapsedChannelId(
1755+
events: List<EventAlertRecord>,
1756+
hasAlarms: Boolean,
1757+
isReminder: Boolean
1758+
): String {
1759+
val allEventsMuted = events.all { it.isMuted }
1760+
return NotificationChannels.getChannelId(
1761+
isAlarm = hasAlarms,
1762+
isMuted = allEventsMuted,
1763+
isReminder = isReminder
1764+
)
1765+
}
1766+
1767+
/**
1768+
* Computes the notification channel ID for partial collapse notifications (postNumNotificationsCollapsed).
1769+
* THIS IS ACTUAL PRODUCTION CODE - called by postNumNotificationsCollapsed.
1770+
*
1771+
* This is for the "X more events" summary notification when some events are shown
1772+
* individually and older ones are collapsed. Uses DEFAULT or SILENT channel only
1773+
* (no alarm/reminder variants since this is a passive summary).
1774+
*
1775+
* @param events List of collapsed events
1776+
* @return SILENT if all events are muted, DEFAULT otherwise
1777+
*/
1778+
fun computePartialCollapseChannelId(events: List<EventAlertRecord>): String {
1779+
val allEventsMuted = events.all { it.isMuted }
1780+
return if (allEventsMuted) {
1781+
NotificationChannels.CHANNEL_ID_SILENT
1782+
} else {
1783+
NotificationChannels.CHANNEL_ID_DEFAULT
1784+
}
1785+
}
16151786
}
16161787
}

0 commit comments

Comments
 (0)