Skip to content
This repository was archived by the owner on Jul 1, 2026. It is now read-only.

Day streak notification + refactoring#14

Merged
sleepkqq merged 5 commits into
masterfrom
feature/day-streak-notification
Feb 16, 2026
Merged

Day streak notification + refactoring#14
sleepkqq merged 5 commits into
masterfrom
feature/day-streak-notification

Conversation

@sleepkqq

@sleepkqq sleepkqq commented Feb 15, 2026

Copy link
Copy Markdown
Member

PR Type

Enhancement, Refactoring


Description

  • Implement day streak notification feature with i18n support

  • Refactor notification architecture: replace priority-based routing with source-based approach

  • Create new consumer classes for domain events (DayStreakExtended, LocaleUpdated, TasksSaved)

  • Introduce I18nService for multi-language notification messages

  • Remove unused Spring Retry and AOP dependencies

  • Update Spring Boot to 4.0.2 and dependencies to latest versions


Diagram Walkthrough

flowchart LR
  A["Domain Events<br/>DayStreakExtended<br/>LocaleUpdated<br/>TasksSaved"] -->|consumed by| B["New Consumers<br/>in kafka/consumer"]
  B -->|send via| C["NotificationProducer<br/>kafka/producer"]
  C -->|creates| D["Notification Events<br/>with source & type"]
  E["I18nService"] -->|provides localized| F["Messages<br/>messages.properties"]
  B -->|uses| E
  G["Removed Components<br/>SendNotificationConsumer<br/>ReceiveNotificationProducer<br/>AvroMapper<br/>RoutingStrategy"] -.->|replaced by| C
Loading

File Walkthrough

Relevant files
Refactoring
6 files
Application.kt
Remove unused @EnableRetry annotation                                       
+0/-2     
ReceiveNotificationProducer.kt
Removed in favor of unified NotificationProducer                 
+0/-18   
SendNotificationConsumer.kt
Removed legacy consumer implementation                                     
+0/-40   
AvroMapper.kt
Removed unused MapStruct mapper                                                   
+0/-23   
NotificationRoutingStrategy.kt
Removed priority-based routing interface                                 
+0/-8     
PriorityBasedRoutingStrategy.kt
Removed priority-based routing implementation                       
+0/-19   
Enhancement
6 files
DayStreakExtendedConsumer.kt
New consumer for day streak extended events                           
+47/-0   
LocaleUpdatedConsumer.kt
New consumer for locale updated events                                     
+43/-0   
TasksSavedConsumer.kt
New consumer for tasks saved events                                           
+50/-0   
NotificationProducer.kt
New unified notification producer with flexible routing   
+29/-0   
I18nService.kt
New service for message localization support                         
+18/-0   
LocalizationCode.kt
Localization code constants for notifications                       
+5/-0     
Documentation
2 files
messages.properties
English localization messages for notifications                   
+5/-0     
messages_ru.properties
Russian localization messages for notifications                   
+5/-0     
Dependencies
2 files
pom.xml
Update Spring Boot and dependency versions                             
+4/-4     
pom.xml
Remove spring-retry and aop, update testcontainers artifacts
+2/-10   

@qodo-code-review

qodo-code-review Bot commented Feb 15, 2026

Copy link
Copy Markdown

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Unhandled parse failure: The code parses event.txId via UUID.fromString(event.txId) without handling invalid/blank
values or adding contextual logging, which can crash processing and prevent graceful
degradation.

Referred Code
override fun processEvent(event: DayStreakExtendedEvent) {
	notificationProducer.send(
		txId = UUID.fromString(event.txId),
		userId = event.userId,
		message = i18nService.getMessage(LocalizationCode.DAY_STREAK_EXTENDED_INFO),
		source = NotificationSource.DAY_STREAK,
		topics = setOf(KafkaTaskTopics.UI_NOTIFICATION_TOPIC),
	)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Missing input validation: The consumer uses external event fields (event.txId, event.userId) directly (including
UUID.fromString(event.txId)) without validating format/range, risking runtime exceptions
and unsafe propagation of malformed inputs.

Referred Code
override fun processEvent(event: LocaleUpdatedEvent) {
	notificationProducer.send(
		txId = UUID.fromString(event.txId),
		userId = event.userId,
		source = NotificationSource.LOCALE,
		topics = setOf(KafkaTaskTopics.UI_NOTIFICATION_TOPIC, KafkaTaskTopics.TG_NOTIFICATION_TOPIC),
	)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing audit context: The new notification-producing consumers do not visibly record audit logs including
userId, action description, and outcome, and it is unclear if AbstractKafkaConsumer
provides sufficient audit-trail logging.

Referred Code
override fun processEvent(event: DayStreakExtendedEvent) {
	notificationProducer.send(
		txId = UUID.fromString(event.txId),
		userId = event.userId,
		message = i18nService.getMessage(LocalizationCode.DAY_STREAK_EXTENDED_INFO),
		source = NotificationSource.DAY_STREAK,
		topics = setOf(KafkaTaskTopics.UI_NOTIFICATION_TOPIC),
	)

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review

qodo-code-review Bot commented Feb 15, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Rethink internationalization in Kafka consumers

The I18nService incorrectly uses LocaleContextHolder to get the user's language,
which is incompatible with the Kafka consumer environment. To fix this, the
user's locale should be passed within the Kafka event and used explicitly in the
I18nService.

Examples:

solo-leveling-notification-service/src/main/kotlin/com/sleepkqq/sololeveling/notification/service/i18n/I18nService.kt [12-17]
	fun getMessage(code: String, args: List<Any> = emptyList()): String =
		messageSource.getMessage(
			code,
			args.toTypedArray().takeIf { it.isNotEmpty() },
			LocaleContextHolder.getLocale()
		)
solo-leveling-notification-service/src/main/kotlin/com/sleepkqq/sololeveling/notification/kafka/consumer/DayStreakExtendedConsumer.kt [42]
			message = i18nService.getMessage(LocalizationCode.DAY_STREAK_EXTENDED_INFO),

Solution Walkthrough:

Before:

// I18nService.kt
class I18nService(private val messageSource: MessageSource) {
    fun getMessage(code: String): String {
        // Relies on thread-local locale, which is not available in Kafka consumers
        return messageSource.getMessage(code, null, LocaleContextHolder.getLocale())
    }
}

// DayStreakExtendedConsumer.kt
class DayStreakExtendedConsumer(private val i18nService: I18nService, ...) {
    override fun processEvent(event: DayStreakExtendedEvent) {
        // The user's locale is unknown here, so it uses the server's default
        val message = i18nService.getMessage("day-streak.extended.info")
        // ... send notification
    }
}

After:

// I18nService.kt
class I18nService(private val messageSource: MessageSource) {
    // Accepts an explicit Locale
    fun getMessage(code: String, locale: Locale): String {
        return messageSource.getMessage(code, null, locale)
    }
}

// DayStreakExtendedEvent (conceptual change)
// event should now contain user's locale, e.g., val userLocale: String

// DayStreakExtendedConsumer.kt
class DayStreakExtendedConsumer(private val i18nService: I18nService, ...) {
    override fun processEvent(event: DayStreakExtendedEvent) {
        // Locale is retrieved from the event payload
        val userLocale = Locale.forLanguageTag(event.userLocale) 
        val message = i18nService.getMessage("day-streak.extended.info", userLocale)
        // ... send notification with correctly localized message
    }
}
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical design flaw in the new internationalization feature, where using LocaleContextHolder in a Kafka consumer will result in incorrect language selection for notifications.

High
Possible issue
Safely parse UUID from event

Add a try-catch block when parsing event.txId to a UUID to handle potential
IllegalArgumentException and prevent the consumer from failing on malformed
data.

solo-leveling-notification-service/src/main/kotlin/com/sleepkqq/sololeveling/notification/kafka/consumer/DayStreakExtendedConsumer.kt [38-46]

 override fun processEvent(event: DayStreakExtendedEvent) {
+	val txId = try {
+		UUID.fromString(event.txId)
+	} catch (e: IllegalArgumentException) {
+		log.error("Invalid txId format for DayStreakExtendedEvent: ${event.txId}. Skipping event.", e)
+		return
+	}
 	notificationProducer.send(
-		txId = UUID.fromString(event.txId),
+		txId = txId,
 		userId = event.userId,
 		message = i18nService.getMessage(LocalizationCode.DAY_STREAK_EXTENDED_INFO),
 		source = NotificationSource.DAY_STREAK,
 		topics = setOf(KafkaTaskTopics.UI_NOTIFICATION_TOPIC),
 	)
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that UUID.fromString can throw an exception and proposes a robust error handling mechanism, which is a good practice for consumer resilience.

Medium
  • Update

@sleepkqq
sleepkqq merged commit 5b07f2b into master Feb 16, 2026
2 checks passed
@sleepkqq
sleepkqq deleted the feature/day-streak-notification branch February 16, 2026 17:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant