-
Notifications
You must be signed in to change notification settings - Fork 1
fix: improve debug and ignore blank lines in excel merchant list #32
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.
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
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a debug mode feature across the data processing layer. The base DataSource class now accepts a debugMode parameter that cascades to all concrete implementations (CTXSpendDataSource, CoinAtmRadarDataSource, DCGDataSource, PiggyCardsDataSource), enabling conditional logging behavior. The build version increments from 5 to 6, deployment timeout extends to 600 seconds, and minor messaging updates occur in the main application entry point. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 🧪 Unit Test Generation v2 is now available!We have significantly improved our unit test generation capabilities. To enable: Add this to your reviews:
finishing_touches:
unit_tests:
enabled: trueTry it out by using the Have feedback? Share your thoughts on our Discord thread! Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
deploy-gcf.sh (1)
15-22:⚠️ Potential issue | 🔴 CriticalFix timeout for Pub/Sub trigger compatibility.
This script uses
--trigger-topic(Pub/Sub trigger), which has a max timeout of 540s for both gen1 and gen2. The current 600s timeout will fail deployment. Reduce to 540s.Fix
-TIMEOUT="600s" +TIMEOUT="540s"src/main/kotlin/org/dash/mobile/explore/sync/SyncProcessor.kt (1)
140-190:⚠️ Potential issue | 🟠 MajorGuard debug logging from production to avoid leaking sensitive data in request/response bodies.
debugenables BODY level logging inCTXSpendDataSourceandPiggyCardsDataSource, which captures full request and response bodies (e.g., API keys, auth hashes, tokens). AlthoughAuthorizationheaders are redacted, BODY level logging still exposes sensitive data in request/response bodies and other headers. If-debugis used in production, secrets can land in logs. Create aneffectiveDebugvalue that disables debug in production mode.🔐 Suggested fix
private suspend fun importData(dbFile: File, locationsDbFile: File) { + val effectiveDebug = debug && mode != OperationMode.PRODUCTION - val ctxDataSource = CTXSpendDataSource(slackMessenger, debug) + val ctxDataSource = CTXSpendDataSource(slackMessenger, effectiveDebug) val ctxData = ctxDataSource.getDataList() val ctxReport = ctxDataSource.getReport() - val piggyCardsDataSource = PiggyCardsDataSource(slackMessenger, mode, debug) + val piggyCardsDataSource = PiggyCardsDataSource(slackMessenger, mode, effectiveDebug) val piggyCardsData = piggyCardsDataSource.getDataList() val piggyCardsReport = piggyCardsDataSource.getReport() val report = SyncReport(listOf(ctxReport, piggyCardsReport)) - if (debug) { + if (effectiveDebug) { saveMerchantDataToCsv(ctxData, "ctx.csv") saveMerchantDataToCsv(piggyCardsData, "piggycards.csv") } @@ - val dcgDataFlow = DCGDataSource(mode != OperationMode.PRODUCTION, slackMessenger, debug).getData(prepStatement) + val dcgDataFlow = DCGDataSource(mode != OperationMode.PRODUCTION, slackMessenger, effectiveDebug).getData(prepStatement)
🤖 Fix all issues with AI agents
In
`@src/main/kotlin/org/dash/mobile/explore/sync/process/PiggyCardsDataSource.kt`:
- Around line 254-260: The code mixes a case-insensitive check
(giftCard.name.lowercase().contains("(instant delivery)")) with a case-sensitive
one (!it.name.contains("(instant delivery)")), causing inconsistent behavior and
possible duplicates; update the filter in PiggyCardsDataSource (the
immediateDeliveryCards.addAll(...) line that filters giftCards where priceType
== "Fixed") to use a case-insensitive contains (e.g., .contains("(instant
delivery)", ignoreCase = true)) so both checks are consistent and duplicates are
avoided.
🧹 Nitpick comments (1)
src/main/kotlin/org/dash/mobile/explore/sync/process/PiggyCardsDataSource.kt (1)
291-306: Country-filtered locations are logged as "invalid", which may be misleading.Locations that don't match the target country are added to
invalidLocations(line 305) and logged as "invalid" at line 334. These locations aren't truly invalid—they're just filtered by country. This could confuse debugging efforts.Consider separating the tracking:
invalidLocationsfor genuinely invalid data (missing address/coordinates)- A separate collection for locations filtered by country
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.