Skip to content

Add /api/v1/banned_users/counts #907

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.

Sign up for GitHub

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

Merged
skyfallwastaken merged 2 commits into from
Feb 5, 2026
+31 −0
Merged

Add /api/v1/banned_users/counts #907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3262653
Add /api/v1/banned_users/counts
skyfallwastaken Feb 5, 2026
b9e47e0
fix: rubocop
skyfallwastaken Feb 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions app/controllers/api/v1/stats_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,35 @@ def trust_factor
render json: { trust_level: level, trust_value: User.trust_levels[level] }
end

def banned_users_counts
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

This endpoint lacks authentication/authorization. Similar stats endpoints in this controller either require authentication via the ensure_authenticated! before_action (like the show action on line 2) or are scoped to specific users. This endpoint exposes aggregate banned user statistics without any access control, which could be sensitive information. Consider adding authentication or moving this endpoint to the admin namespace (Api::Admin::V1) which has built-in admin authentication via AdminApiKey.

Copilot uses AI. Check for mistakes.
now = Time.current

day_ago = now - 1.day
week_ago = now - 1.week
month_ago = now - 1.month

day_count = TrustLevelAuditLog.where(new_trust_level: "red")
.where("created_at >= ?", day_ago)
.distinct
.count(:user_id)

week_count = TrustLevelAuditLog.where(new_trust_level: "red")
.where("created_at >= ?", week_ago)
.distinct
.count(:user_id)

month_count = TrustLevelAuditLog.where(new_trust_level: "red")
.where("created_at >= ?", month_ago)
.distinct
.count(:user_id)
Comment on lines +177 to +190
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The queries may not accurately count what the endpoint name suggests. Currently, these count distinct user_ids where a TrustLevelAuditLog entry with new_trust_level='red' was created in the time period. This means if a user was banned (red), unbanned (blue), and banned again (red) within the same period, they would be counted. Additionally, users who were already banned before the time period but had no new audit log entries during it won't be counted. Consider clarifying whether this should count: (1) users who were newly banned in the period, (2) total unique users who received any ban action in the period, or (3) users who are currently banned at the end of the period.

Copilot uses AI. Check for mistakes.
Comment on lines +177 to +190
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

This query pattern could have performance issues as the audit log table grows. The query filters by new_trust_level and created_at, but the database schema (db/schema.rb lines 555-568) doesn't have a composite index on (new_trust_level, created_at). The existing indexes are on (user_id, created_at) and (changed_by_id, created_at). Without an appropriate index, these queries will perform full table scans filtered by the WHERE clauses. Consider adding a composite index on (new_trust_level, created_at) to optimize this query pattern, or alternatively scope the query using existing indexes (e.g., by using user_id ranges with the indexed columns).

Copilot uses AI. Check for mistakes.
Comment on lines +177 to +190
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The three queries are nearly identical except for the time comparison. This can be refactored to reduce code duplication and make it easier to maintain. Consider extracting the common query logic into a helper method that takes a time parameter.

Copilot uses AI. Check for mistakes.

render json: {
day: day_count,
week: week_count,
month: month_count
}
end
Comment on lines +170 to +197
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

This new endpoint lacks test coverage. All other endpoints in spec/requests/api/v1/stats_spec.rb have corresponding test cases that document their behavior and response schemas. Add test coverage for this endpoint that verifies the response structure and behavior, including test cases for different time periods.

Copilot uses AI. Check for mistakes.

def user_projects
return render json: { error: "User not found" }, status: :not_found unless @user

Expand Down
2 changes: 2 additions & 0 deletions config/routes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,8 @@ def matches?(request)
get "users/lookup_email/:email", to: "users#lookup_email", constraints: { email: /[^\/]+/ }
get "users/lookup_slack_uid/:slack_uid", to: "users#lookup_slack_uid"

get "banned_users/counts", to: "stats#banned_users_counts"

# External service Slack OAuth integration
post "external/slack/oauth", to: "external_slack#create_user"

Expand Down