Implement activity timeline for dashboard with hourly/daily aggregation#91
Implement activity timeline for dashboard with hourly/daily aggregation#91hhftechnology merged 3 commits intomainfrom
Conversation
Provide history-backed timeline data for dashboard charts. Adds HistoryActivity models (filter, bucket, response), store logic to produce zero-filled hourly/daily buckets (with SQL bucketing and latest snapshot), and a Service.GetHistoryActivity wrapper. Introduces parseHistoryActivityFilters and GetHistoryActivity API handler, registers the new route, and adds unit tests for parsing and store behavior. Frontend: update API client and types, fetch history activity in Dashboard, and replace client-side bucketing with server-provided buckets. Also updates mobile package override for @xmldom/xmldom.
Adjust daily activity window to end at the end of the current UTC day (today +24h) so the final daily bucket includes current-day data. Add handler and store-level tests to verify endAt and final bucket behavior. Update Dashboard to derive alert count from history activity buckets, expose isLoading flags, and render placeholders for detailed alert analysis while it is still loading. Update frontend tests and test helpers to reflect these changes.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 4 minutes and 41 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR introduces a new history activity feature that aggregates alert and decision counts across time windows (24-hour or 7-day) with hourly or daily granularity, exposing them via a backend API endpoint and integrating the data into mobile and web dashboards. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Web/Mobile Client
participant Handler as HTTP Handler<br/>(GetHistoryActivity)
participant Service as History Service
participant Store as History Store
participant DB as Database
Client->>Handler: GET /crowdsec/history/activity<br/>(window=24h, bucket=hour)
Handler->>Handler: Validate parameters<br/>Calculate UTC endAt boundary
Handler->>Service: GetActivityBuckets(window,<br/>bucket, endAt)
alt Service/Store unavailable
Service-->>Handler: ErrStoreUnavailable
Handler-->>Client: HTTP 503
else Valid request
Service->>Store: GetActivityBuckets(input)
Store->>DB: Query alert_history counts<br/>by bucket timestamp
DB-->>Store: Alert counts
Store->>DB: Query decision_history counts<br/>by bucket timestamp
DB-->>Store: Decision counts
Store->>DB: Query max activity timestamp
DB-->>Store: Latest timestamp
Store->>Store: Generate gap-filled buckets,<br/>aggregate counts
Store-->>Service: ActivityBuckets response
Service-->>Handler: ActivityBuckets response
Handler->>Handler: Build HistoryActivityResponse<br/>(include Window, Bucket,<br/>GeneratedAt, Buckets)
Handler-->>Client: HTTP 200 JSON
Client->>Client: Render activity<br/>metrics/charts
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Code Review
This pull request introduces a new history activity endpoint to provide bucketed counts of alerts and decisions for the dashboard, spanning both web and mobile platforms. The implementation includes backend logic for SQLite aggregation, API client updates, and UI enhancements to display recent activity. Feedback focuses on a high-severity issue regarding the disabling of minification and resource shrinking in the Android release build. Additionally, there are recommendations to resolve an inconsistency in time-window calculations for hourly buckets and to optimize database queries by removing redundant function calls that prevent the use of indexes.
| minifyEnabled false | ||
| shrinkResources false |
There was a problem hiding this comment.
Disabling minifyEnabled and shrinkResources in the release build type is discouraged for production applications. This increases the APK size and removes code obfuscation, making the application easier to reverse-engineer and potentially exposing internal logic.
minifyEnabled true
shrinkResources true
| endAt := nowUTC.Truncate(time.Hour) | ||
| if bucket == history.ActivityBucketDay { | ||
| todayUTC := time.Date(nowUTC.Year(), nowUTC.Month(), nowUTC.Day(), 0, 0, 0, 0, time.UTC) | ||
| endAt = todayUTC.Add(24 * time.Hour) | ||
| } |
There was a problem hiding this comment.
There is an inconsistency in how the end time (endAt) is calculated between bucket types. For daily buckets, it is set to the start of the next day, effectively including the current day's data. However, for hourly buckets, it is truncated to the current hour, which excludes the current hour's activity from the results. It is generally better to include the current partial bucket for a more up-to-date dashboard.
| endAt := nowUTC.Truncate(time.Hour) | |
| if bucket == history.ActivityBucketDay { | |
| todayUTC := time.Date(nowUTC.Year(), nowUTC.Month(), nowUTC.Day(), 0, 0, 0, 0, time.UTC) | |
| endAt = todayUTC.Add(24 * time.Hour) | |
| } | |
| endAt := nowUTC.Add(time.Hour).Truncate(time.Hour) | |
| if bucket == history.ActivityBucketDay { | |
| todayUTC := time.Date(nowUTC.Year(), nowUTC.Month(), nowUTC.Day(), 0, 0, 0, 0, time.UTC) | |
| endAt = todayUTC.Add(24 * time.Hour) | |
| } |
| bucketExpr := "strftime('%Y-%m-%dT%H:00:00Z', datetime(" + in.Column + "))" | ||
| if in.Bucket == ActivityBucketDay { | ||
| bucketExpr = "strftime('%Y-%m-%dT00:00:00Z', datetime(" + in.Column + "))" | ||
| } | ||
|
|
||
| query := fmt.Sprintf(` | ||
| SELECT %s AS bucket_start, COUNT(1) | ||
| FROM %s | ||
| WHERE datetime(%s) >= datetime(?) AND datetime(%s) < datetime(?) | ||
| GROUP BY bucket_start | ||
| ORDER BY bucket_start | ||
| `, bucketExpr, in.Table, in.Column, in.Column) |
There was a problem hiding this comment.
Using datetime() on column names in the WHERE clause prevents SQLite from utilizing indexes on those columns, which can lead to significant performance degradation as the history tables grow. Since the timestamps are stored in a sortable ISO8601 format, they can be compared directly as strings. Additionally, datetime() is redundant in the strftime call if the column already contains a valid date-time string.
bucketExpr := "strftime('%Y-%m-%dT%H:00:00Z', " + in.Column + ")"
if in.Bucket == ActivityBucketDay {
bucketExpr = "strftime('%Y-%m-%dT00:00:00Z', " + in.Column + ")"
}
query := fmt.Sprintf("SELECT %s AS bucket_start, COUNT(1) FROM %s WHERE %s >= ? AND %s < ? GROUP BY bucket_start ORDER BY bucket_start", bucketExpr, in.Table, in.Column, in.Column)| err := s.db.QueryRowContext(ctx, ` | ||
| SELECT MAX(ts) | ||
| FROM ( | ||
| SELECT MAX(strftime('%Y-%m-%dT%H:%M:%SZ', datetime(start_at))) AS ts | ||
| FROM alert_history | ||
| WHERE datetime(start_at) IS NOT NULL | ||
| UNION ALL | ||
| SELECT MAX(strftime('%Y-%m-%dT%H:%M:%SZ', datetime(created_at))) AS ts | ||
| FROM decision_history | ||
| WHERE datetime(created_at) IS NOT NULL | ||
| ) | ||
| `).Scan(&latest) |
There was a problem hiding this comment.
This query can be optimized by removing the strftime and datetime calls. SQLite can perform MAX() directly on ISO8601 string columns, and doing so allows the database to use indexes if available. The WHERE ... IS NOT NULL check is sufficient to filter out null values.
err := s.db.QueryRowContext(ctx, "SELECT MAX(ts) FROM (SELECT MAX(start_at) AS ts FROM alert_history WHERE start_at IS NOT NULL UNION ALL SELECT MAX(created_at) AS ts FROM decision_history WHERE created_at IS NOT NULL)").Scan(&latest)There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 172b16e223
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const activityAlertsCount = useMemo(() => { | ||
| if (!activityData?.buckets) return 0 | ||
| return activityData.buckets.reduce((sum, bucket) => sum + (bucket.alerts ?? 0), 0) | ||
| }, [activityData]) |
There was a problem hiding this comment.
Decouple Alerts (7d) stat from chart granularity
This count is now derived from activityData, which is fetched with window=24h when the user switches the chart to Hour mode, so the card labeled "Alerts (7d)" starts showing 24-hour totals instead of 7-day totals. That makes the headline metric and threshold coloring inaccurate whenever granularity is toggled, even though the label and description still claim a 7-day scope.
Useful? React with 👍 / 👎.
| if (decisionsSummaryRes.status === 'fulfilled' && decisionsSummaryRes.value) { | ||
| setDecisionsTotal(decisionsSummaryRes.value.count); | ||
| } |
There was a problem hiding this comment.
Restore fallback when decisions summary request fails
The dashboard now updates decisionsTotal only from decisionsSummary, but that call is wrapped with .catch(() => null), so any summary failure leaves the value unchanged (or at zero on first load) even when completeRes succeeded and already contains decisions. This is a regression from the previous behavior and causes stale/incorrect Active Decisions counts during transient API errors or partial backend incompatibility.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/api/handlers/history_activity_test.go`:
- Around line 243-263: The test is flaky because nowUTC is computed after
ServeHTTP and can differ from the handler's nowUTC across a UTC midnight; to
fix, compute the expected daily boundary before invoking router.ServeHTTP
(capture nowUTCPre := time.Now().UTC() and compute wantEndAt from nowUTCPre) or
accept either boundary by computing both possible wantEndAt values and asserting
service.input.EndAt equals one of them; locate and update
TestGetHistoryActivityDailyEndsAfterCurrentUTCDay and use service.input.EndAt /
getHistoryActivity handler context to compare against the precomputed expected
endAt(s).
- Around line 55-58: Remove the redundant per-iteration shadowing "tc := tc" in
the table-driven test loop: the explicit copy is unnecessary because Go now
creates per-iteration loop variables; update the test that iterates "for _, tc
:= range tests { ... t.Run(tc.name, func(t *testing.T) { t.Parallel()" by
deleting the "tc := tc" assignment (apply the same change at both occurrences
around the t.Run blocks).
In `@internal/api/handlers/history_activity.go`:
- Around line 62-68: The handler currently maps all errors from
loadActivityCounts to a 503 "history unavailable" response; change the error
handling so that if err is history.ErrStoreUnavailable you continue to return
503 as before, but for any other error emit the same logger.Warn call and return
a 500 with a distinct error message (e.g., "history error") while still calling
logHistoryActivityRequest with the chosen status; update the test in
history_activity_test.go that expects a 503 for a generic error to expect 500
(or the new message) so the behavior is validated.
In `@internal/history/activity_test.go`:
- Around line 129-135: The assertion is comparing result.LatestSnapshotAt to
event timestamps (e.g., endAt, startAt, CreatedAt) but fixtures set SnapshotAt
to endAt/now, so update the tests to assert LatestSnapshotAt equals the
corresponding snapshot's SnapshotAt value instead of the event time; locate the
checks around result.LatestSnapshotAt in activity_test.go (the blocks
referencing endAt, startAt, CreatedAt) and replace the expectedLatest
calculation and comparisons to use the snapshot's SnapshotAt field (or the
fixture variable that represents SnapshotAt) for the expected value for all
instances including the ones noted at lines 287-290, 327-330, and 383-386.
In `@internal/history/activity.go`:
- Around line 108-120: The loadActivityCounts function currently interpolates
in.Table and in.Column into SQL; add a defensive allow-list check at the top of
Store.loadActivityCounts to validate that in.Table is one of the expected table
names (e.g. "alert_history", "decision_history") and in.Column is one of the
expected column names (e.g. "start_at", "created_at"); if either value is not
allowed, return an error. Keep the existing bucketExpr logic and query
construction but only after the allow-list check succeeds (or alternatively
replace callers with an enum and map enum->(table,column) inside
loadActivityCounts to avoid direct interpolation). Ensure the check references
the loadActivityCountsInput fields to locate the code.
In `@mobile/android/app/build.gradle`:
- Around line 41-43: Change the release build configuration to enable R8 by
setting minifyEnabled true and shrinkResources true in the release block (the
symbols to update are minifyEnabled and shrinkResources inside the release {}
block) and ensure the proguard configuration referenced by proguardFiles
includes your proguard-rules.pro; if shrinking breaks behavior, add specific
-keep rules to proguard-rules.pro rather than disabling minification/shrinking
globally.
In `@mobile/src/pages/DashboardPage.tsx`:
- Around line 67-73: When one promise fails, derived dashboard state must be
reset or derived from other successful results: update the logic around
decisionsSummaryRes and activityRes so that if decisionsSummaryRes.status !==
'fulfilled' but completeRes.status === 'fulfilled' (and
completeRes.value.decisions exists) you call setDecisionsTotal using the
fallback count from completeRes.value.decisions.length (instead of leaving it at
0); and if activityRes.status !== 'fulfilled' ensure you call
setActivityBuckets([]) to clear stale buckets; locate and change the handling
around decisionsSummaryRes/completeRes and activityRes to implement these
fallbacks using setDecisionsTotal and setActivityBuckets.
In `@web/src/pages/Dashboard.tsx`:
- Around line 147-157: The Alerts (7d) stat must always sum a 7-day/day series
regardless of the current chart granularity; add a separate query that always
requests { window: '7d', bucket: 'day' } (e.g., useQuery with queryKey
['history-activity', '7d'] or ['history-activity-7d']) and call
api.crowdsec.getHistoryActivity with those params, keep the existing
activityData query for the chart (which uses granularity), and update the Alerts
(7d) card to read from the new 7-day query result instead of activityData so the
metric stays independent of granularity.
- Around line 82-95: formatHistoryActivityBuckets currently converts bucket.ts
to local time which shifts UTC bucket boundaries; update the formatting to
preserve UTC when creating the label in formatHistoryActivityBuckets by
formatting the Date using UTC-aware methods (e.g., pass timeZone: 'UTC' to
toLocaleTimeString/toLocaleDateString or use getUTC* getters) so hourly labels
use UTC hour/minute and daily labels use UTC month/day; ensure you still respect
the hourly flag and keep alerts/decisions mapping unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d1e25af3-41b2-4ac5-9a15-c1b5edfcc11f
⛔ Files ignored due to path filters (2)
mobile/package-lock.jsonis excluded by!**/package-lock.jsonweb/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
internal/api/handlers/history_activity.gointernal/api/handlers/history_activity_test.gointernal/api/routes.gointernal/history/activity.gointernal/history/activity_test.gointernal/history/store_test.gointernal/models/history_activity.gomobile/android/app/build.gradlemobile/package.jsonmobile/src/components/dashboard/SecurityOverviewPanel.tsxmobile/src/lib/api/crowdsec.tsmobile/src/lib/api/types.tsmobile/src/pages/DashboardPage.tsxweb/package.jsonweb/src/__tests__/Dashboard.test.tsxweb/src/lib/api/crowdsec.tsweb/src/lib/api/types.tsweb/src/pages/Dashboard.tsxweb/src/test/setup.tsweb/vite.config.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-and-push
- GitHub Check: Analyze (go)
🧰 Additional context used
🪛 golangci-lint (2.11.4)
internal/api/handlers/history_activity_test.go
[error] 56-56: The copy of the 'for' variable "tc" can be deleted (Go 1.22+)
(copyloopvar)
[error] 181-181: The copy of the 'for' variable "tc" can be deleted (Go 1.22+)
(copyloopvar)
🔇 Additional comments (2)
internal/history/store_test.go (1)
179-300: Good bucket-boundary coverage.These tests pin
EndAtto UTC bucket boundaries and assert exact bucket placement for both hour and day modes, which is the right shape to catch off-by-one regressions inGetActivityBuckets.internal/api/handlers/history_activity.go (1)
26-32: The current implementation is safe.SetHistoryService()is invoked at line 89 inmain.go, before route registration occurs at line 152 whereGetHistoryActivity()is called. The handler correctly captures the initializedhistoryServiceat route-setup time. No changes are needed.
| for _, tc := range tests { | ||
| tc := tc | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| t.Parallel() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the module's Go version supports loop-variable-per-iteration (>= 1.22).
fd -t f 'go.mod' | head -n 5 | xargs -I{} sh -c 'echo "=== {} ==="; cat "{}"'Repository: hhftechnology/crowdsec_manager
Length of output: 3531
🏁 Script executed:
head -n 185 internal/api/handlers/history_activity_test.go | tail -n +50 | cat -nRepository: hhftechnology/crowdsec_manager
Length of output: 4820
Remove redundant tc := tc loop-variable copies (per copyloopvar linter).
Go 1.23.0 supports per-iteration loop variable copies (since Go 1.22), making the explicit shadowing assignment tc := tc unnecessary. golangci-lint's copyloopvar rule flags these as unneeded.
♻️ Proposed fix
for _, tc := range tests {
- tc := tc
t.Run(tc.name, func(t *testing.T) {(apply at both lines 55–58 and 180–183)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for _, tc := range tests { | |
| tc := tc | |
| t.Run(tc.name, func(t *testing.T) { | |
| t.Parallel() | |
| for _, tc := range tests { | |
| t.Run(tc.name, func(t *testing.T) { | |
| t.Parallel() |
🧰 Tools
🪛 golangci-lint (2.11.4)
[error] 56-56: The copy of the 'for' variable "tc" can be deleted (Go 1.22+)
(copyloopvar)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/api/handlers/history_activity_test.go` around lines 55 - 58, Remove
the redundant per-iteration shadowing "tc := tc" in the table-driven test loop:
the explicit copy is unnecessary because Go now creates per-iteration loop
variables; update the test that iterates "for _, tc := range tests { ...
t.Run(tc.name, func(t *testing.T) { t.Parallel()" by deleting the "tc := tc"
assignment (apply the same change at both occurrences around the t.Run blocks).
| if result.LatestSnapshotAt == nil { | ||
| t.Fatalf("expected latest snapshot timestamp") | ||
| } | ||
| expectedLatest := endAt.Add(-1 * time.Hour) | ||
| if !result.LatestSnapshotAt.Equal(expectedLatest) { | ||
| t.Fatalf("latest snapshot mismatch: got %v want %v", result.LatestSnapshotAt, expectedLatest) | ||
| } |
There was a problem hiding this comment.
Assert LatestSnapshotAt against SnapshotAt, not the event timestamp.
These fixtures upsert every record with SnapshotAt: endAt / SnapshotAt: now, but the assertions expect LatestSnapshotAt to equal StartAt / CreatedAt. That changes the contract from “latest snapshot time” to “latest event time” and will mis-test the handler/model semantics.
Also applies to: 287-290, 327-330, 383-386
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/history/activity_test.go` around lines 129 - 135, The assertion is
comparing result.LatestSnapshotAt to event timestamps (e.g., endAt, startAt,
CreatedAt) but fixtures set SnapshotAt to endAt/now, so update the tests to
assert LatestSnapshotAt equals the corresponding snapshot's SnapshotAt value
instead of the event time; locate the checks around result.LatestSnapshotAt in
activity_test.go (the blocks referencing endAt, startAt, CreatedAt) and replace
the expectedLatest calculation and comparisons to use the snapshot's SnapshotAt
field (or the fixture variable that represents SnapshotAt) for the expected
value for all instances including the ones noted at lines 287-290, 327-330, and
383-386.
| func (s *Store) loadActivityCounts(ctx context.Context, in loadActivityCountsInput) error { | ||
| bucketExpr := "strftime('%Y-%m-%dT%H:00:00Z', datetime(" + in.Column + "))" | ||
| if in.Bucket == ActivityBucketDay { | ||
| bucketExpr = "strftime('%Y-%m-%dT00:00:00Z', datetime(" + in.Column + "))" | ||
| } | ||
|
|
||
| query := fmt.Sprintf(` | ||
| SELECT %s AS bucket_start, COUNT(1) | ||
| FROM %s | ||
| WHERE datetime(%s) >= datetime(?) AND datetime(%s) < datetime(?) | ||
| GROUP BY bucket_start | ||
| ORDER BY bucket_start | ||
| `, bucketExpr, in.Table, in.Column, in.Column) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Defensive: constrain Table/Column to a known allow-list.
loadActivityCountsInput.Table, Column, and the derived bucketExpr are interpolated directly into the SQL via fmt.Sprintf/string concatenation. Today all callers pass hard-coded literals ("alert_history", "decision_history", "start_at", "created_at"), so this is not exploitable. However, nothing in the type prevents a future caller from passing external input and introducing a SQL-injection vector.
Consider either:
- Making
loadActivityCountsa closed helper that switches on an enum (alerts/decisions) and selects the table/column internally, or - Validating
Table/Columnagainst a small allow-list at the top of the function.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/history/activity.go` around lines 108 - 120, The loadActivityCounts
function currently interpolates in.Table and in.Column into SQL; add a defensive
allow-list check at the top of Store.loadActivityCounts to validate that
in.Table is one of the expected table names (e.g. "alert_history",
"decision_history") and in.Column is one of the expected column names (e.g.
"start_at", "created_at"); if either value is not allowed, return an error. Keep
the existing bucketExpr logic and query construction but only after the
allow-list check succeeds (or alternatively replace callers with an enum and map
enum->(table,column) inside loadActivityCounts to avoid direct interpolation).
Ensure the check references the loadActivityCountsInput fields to locate the
code.
| release { | ||
| minifyEnabled true | ||
| shrinkResources true | ||
| minifyEnabled false | ||
| shrinkResources false |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the file and surrounding context
cat -n mobile/android/app/build.gradle | head -60Repository: hhftechnology/crowdsec_manager
Length of output: 2639
🏁 Script executed:
# Check if proguard-rules.pro exists and what's in it
find . -name "proguard-rules.pro" -type fRepository: hhftechnology/crowdsec_manager
Length of output: 114
🏁 Script executed:
# Check git diff to see what was changed from
cd mobile/android/app && git diff build.gradleRepository: hhftechnology/crowdsec_manager
Length of output: 56
🏁 Script executed:
# Look for any PR context or recent commits
git log --oneline -10 mobile/android/app/build.gradleRepository: hhftechnology/crowdsec_manager
Length of output: 121
🏁 Script executed:
# Show the previous version of the file before the current commit
git show HEAD~1:mobile/android/app/build.gradle | head -60Repository: hhftechnology/crowdsec_manager
Length of output: 111
🏁 Script executed:
# Read the proguard-rules.pro file
cat mobile/android/app/proguard-rules.proRepository: hhftechnology/crowdsec_manager
Length of output: 825
🏁 Script executed:
# Get the full diff for the build.gradle file in this commit
git show --no-patch mobile/android/app/build.gradle || git show 172b16e -- mobile/android/app/build.gradleRepository: hhftechnology/crowdsec_manager
Length of output: 716
🏁 Script executed:
# Check for any comments or context around the minifyEnabled/shrinkResources settings
sed -n '38,48p' mobile/android/app/build.gradleRepository: hhftechnology/crowdsec_manager
Length of output: 414
🏁 Script executed:
# Search for any TODOs or comments about minification across the project
rg -i "minify|shrink|proguard" mobile/android/ -A 2 -B 2Repository: hhftechnology/crowdsec_manager
Length of output: 1226
Enable R8 minification and resource shrinking in release builds.
The release build currently ships with both minifyEnabled and shrinkResources set to false, which increases APK size and removes obfuscation protection. The proguard-rules.pro file is configured but contains no active keep rules. If shrinking causes issues with this feature, add specific -keep rules to proguard-rules.pro rather than disabling the shrinker globally.
Suggested fix
release {
- minifyEnabled false
- shrinkResources false
+ minifyEnabled true
+ shrinkResources true
proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro'
if (project.hasProperty('RELEASE_STORE_FILE'))
signingConfig signingConfigs.release
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| release { | |
| minifyEnabled true | |
| shrinkResources true | |
| minifyEnabled false | |
| shrinkResources false | |
| release { | |
| minifyEnabled true | |
| shrinkResources true | |
| proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro' | |
| if (project.hasProperty('RELEASE_STORE_FILE')) | |
| signingConfig signingConfigs.release | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/android/app/build.gradle` around lines 41 - 43, Change the release
build configuration to enable R8 by setting minifyEnabled true and
shrinkResources true in the release block (the symbols to update are
minifyEnabled and shrinkResources inside the release {} block) and ensure the
proguard configuration referenced by proguardFiles includes your
proguard-rules.pro; if shrinking breaks behavior, add specific -keep rules to
proguard-rules.pro rather than disabling minification/shrinking globally.
Differentiate history error responses and use 7-day activity for dashboard alerts. Handler: return 503 with "history unavailable" when the store is unavailable, otherwise log and return 500 with "history error". Tests updated to expect the new statuses and error messages. Mobile: guard decision and activity totals to fall back to sensible defaults (use completeRes decisions length or 0, and empty buckets) and update tests. Web: format history bucket timestamps in UTC, add a separate query for 7d/day activity, include its timestamp in last-updated calculations, and use the 7d data to compute the "Alerts (7d)" stat; tests and fake data adjusted accordingly.
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores