Skip to content

Conversation

@daohoangson
Copy link
Owner

No description provided.

@daohoangson daohoangson requested a review from Copilot July 31, 2025 02:50
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the notification filtering configuration system to use a simplified rule-based approach with regex patterns. The main changes replace the complex multi-rule filtering system with direct package matching using regular expressions, and update the default configuration to focus on bank notifications while ignoring social media and system apps.

  • Simplified notification filtering logic from complex rule-based system to direct regex package matching
  • Updated default configuration to focus on Vietnamese bank apps with comprehensive ignore lists
  • Enhanced test coverage with specific scenarios for bank apps and edge cases

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
NotificationFilterEngineTest.kt Complete test suite rewrite with focused bank app testing and social media ignore scenarios
NetworkModule.kt Added hardcoded base URL for development/testing
AppDatabase.kt Modified database migration configuration
WebhookConfig.kt Restructured data models and default configuration for regex-based filtering
NotificationFilterEngine.kt Simplified filtering logic to use regex matching instead of complex rules
app/build.gradle.kts Added build configuration for webhook URLs and signing
README.md Updated documentation to reflect new architecture and features
Comments suppressed due to low confidence (2)

app/src/main/java/com/daohoangson/n8n/notificationlistener/data/database/AppDatabase.kt:28

  • The parameter 'dropAllTables = true' does not exist in Room's fallbackToDestructiveMigration() method. This method takes no parameters and will cause a compilation error.
                ).fallbackToDestructiveMigration(dropAllTables = true)

app/src/test/java/com/daohoangson/n8n/notificationlistener/config/NotificationFilterEngineTest.kt:47

  • The package name 'com.Slack' uses incorrect capitalization. Android package names are typically lowercase, so this should be 'com.slack' to match real-world package naming conventions.
            packageName = "com.Slack",

.build()

private val retrofit = Retrofit.Builder()
.baseUrl("https://google.com")
Copy link

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardcoded base URL 'https://google.com' appears to be a placeholder or debugging value. This should be replaced with a proper configuration or removed if not needed.

Copilot uses AI. Check for mistakes.
Comment on lines +6 to +10
val urls: List<WebhookUrl>, val ignoredPackages: List<Regex>
)

data class WebhookUrl(
val url: String,
val name: String,
val rules: List<FilterRule>
)

data class FilterRule(
val packageName: String,
val titleRegex: Regex? = null,
val textRegex: Regex? = null
val url: String, val name: String, val packages: List<Regex>
Copy link

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Data class parameters should be placed on separate lines when there are multiple parameters for better readability and maintainability.

Copilot uses AI. Check for mistakes.
val packageName: String,
val titleRegex: Regex? = null,
val textRegex: Regex? = null
val url: String, val name: String, val packages: List<Regex>
Copy link

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Data class parameters should be placed on separate lines when there are multiple parameters for better readability and maintainability.

Suggested change
val url: String, val name: String, val packages: List<Regex>
val url: String,
val name: String,
val packages: List<Regex>

Copilot uses AI. Check for mistakes.
proguardFiles(
getDefaultProguardFile("proguard-android-optimize.txt"),
"proguard-rules.pro"
getDefaultProguardFile("proguard-android-optimize.txt"), "proguard-rules.pro"
Copy link

Copilot AI Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Function parameters should be placed on separate lines when there are multiple parameters for better readability and maintainability.

Suggested change
getDefaultProguardFile("proguard-android-optimize.txt"), "proguard-rules.pro"
getDefaultProguardFile("proguard-android-optimize.txt"),
"proguard-rules.pro"

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants