Skip to content

feat(admin/logs): capture request user agent on log entries#241

Merged
akinwalexander merged 3 commits into
devfrom
feat/admin-logs-capture-user-agent
Jun 10, 2026
Merged

feat(admin/logs): capture request user agent on log entries#241
akinwalexander merged 3 commits into
devfrom
feat/admin-logs-capture-user-agent

Conversation

@ibraheembello

@ibraheembello ibraheembello commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Description

First of two stacked PRs adding location and device to the admin activity log. This PR is the write side only: it captures the request User-Agent so the data starts accumulating before the read side (PR 2) surfaces it. No API response changes here.

  • Adds a nullable user_agent column to admin_logs (varchar 512, mirrors the existing ip_address column).
  • LogService now reads req.headers['user-agent'] and stores it, capped at 512 chars. It is read synchronously because the request object is recycled before the deferred insert runs. Non-HTTP actions and absent headers store null.
  • The raw value is stored as-is; PR 2 parses it into a Browser Major . OS Version device label on read, so the parser can improve later without a backfill.

Related Issue

N/A. Requested directly by the frontend (admin activity log design needs location and device columns).

Type of Change

  • feat: New feature

How Has This Been Tested?

  • Unit tests
  • Manual tests

pnpm exec jest src/common/services/tests/log.service.spec.ts passes 14/14, including the new captures the User-Agent header when present and stores a null user_agent when the header is absent cases. Verified end to end against a local Postgres that the column is created and populated.

Test Evidence

image

Screenshot: 14/14 log.service spec run attached.

Additional Notes

PR 2 (expose location + device on GET /admin/logs) is stacked on this branch and will be retargeted to dev once this merges.

Summary by CodeRabbit

  • New Features

    • Admin logs now capture and store User-Agent (truncated to 512 chars) and surface two new fields: device (parsed from User-Agent) and location (IP-derived label).
  • Documentation

    • API docs updated to describe device and location formatting and nullability.
  • Tests

    • Added comprehensive tests for User-Agent capture, device parsing, geo-location resolution, and related list responses.

Add a nullable user_agent column to admin_logs and capture the request
User-Agent header in LogService at write time (the request object is
recycled before the deferred insert, so it must be read synchronously).

Stored raw and capped at 512 chars; the admin logs read side parses it
into a device label. Non-HTTP actions and absent headers store null.
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Captures HTTP User-Agent at log time (truncated to 512 chars) and persists it. Adds DB column and entity field, selects the value in log queries, introduces GeoLocationService (cached, batch resolve), parses device labels from stored User-Agent, and updates tests and Swagger to surface location and device.

Changes

User-Agent header capture & geo enrichment

Layer / File(s) Summary
Entity schema and DB migration
src/modules/admin/logs/entities/admin-log.entity.ts, src/database/migrations/1781308800000-AddUserAgentToAdminLogs.ts
AdminLog adds nullable user_agent: string | null mapped to varchar(512); migration adds/drops the column idempotently.
User-Agent capture and persistence in LogService
src/common/services/log.service.ts
Adds MAX_USER_AGENT_LENGTH, extractUserAgent(req), captures UA synchronously in log(), and persists user_agent on AdminLog insert.
Query selection and contract
src/modules/admin/logs/actions/admin-logs-list.action.ts, src/modules/admin/logs/interfaces/admin-logs.interfaces.ts
Raw query selects log.user_agent as log_user_agent; AdminLogItem gains nullable location and device fields.
GeoLocationService and AdminLogsService enrichment
src/modules/admin/logs/services/geo-location.service.ts, src/modules/admin/logs/admin-logs.module.ts, src/modules/admin/logs/admin-logs.service.ts
New GeoLocationService with per-process cache, timed fetch to freeipapi, and resolveMany; module registers the provider; AdminLogsService batch-resolves locations and maps device via formatDevice.
User-Agent parser utility
src/modules/admin/logs/utils/parse-user-agent.util.ts
Adds formatDevice(userAgent) that parses browser and OS into a compact label or returns null when unrecognizable.
Tests, controller fixtures, and docs
src/common/services/tests/log.service.spec.ts, src/modules/admin/logs/tests/*, src/modules/admin/logs/docs/admin-logs-swagger.doc.ts
Update LogService tests to assert user_agent; add GeoLocationService unit tests, parse-user-agent tests, expand AdminLogsService/controller tests to include location and device, and refresh Swagger example/docs.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • elijaharhinful
  • Nuel-09
  • thaArcadeGuy
  • nsien-prestige
  • akinwalexander
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately summarizes the main change: capturing HTTP request User-Agent headers and storing them in admin log entries.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/admin-logs-capture-user-agent

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

GET /admin/logs now returns two derived fields per entry:

- device: the stored user agent parsed into a 'Browser Major . OS Version'
  label by a dependency-free parser (Chrome, Firefox, Safari, Edge, Opera
  across Windows, macOS, iOS, Android, Linux).
- location: a 'Region, CC' label resolved from ip_address via freeipapi.com,
  a keyless HTTPS endpoint, over the built-in fetch. Cached per IP and
  degrades to null on any failure so the feed never breaks. The offline
  geo-IP database that would avoid the network call is a banned dependency.

Both fields are null when they cannot be resolved (no IP, no user agent,
private/loopback address, or provider unreachable).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/common/services/tests/log.service.spec.ts`:
- Around line 65-87: Add a test that verifies LogService.log truncates
user_agent to 512 characters: create a request via makeRequest(...) with a
'user-agent' header longer than 512 chars, call service.log (e.g.,
service.log('user-1', AdminLogActionType.LOGIN, 'x', req,
AdminLogStatus.SUCCESS)), await flushImmediates(), and assert
mockAdminLogRepository.create was called with expect.objectContaining({
user_agent: longUserAgent.slice(0, 512) }) to confirm the truncation boundary is
enforced.
🪄 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: 953f5239-5a08-4286-a6cd-73b6292de2ba

📥 Commits

Reviewing files that changed from the base of the PR and between a73919a and bca113f.

📒 Files selected for processing (4)
  • src/common/services/log.service.ts
  • src/common/services/tests/log.service.spec.ts
  • src/database/migrations/1781308800000-AddUserAgentToAdminLogs.ts
  • src/modules/admin/logs/entities/admin-log.entity.ts

Comment thread src/common/services/tests/log.service.spec.ts
akinwalexander
akinwalexander previously approved these changes Jun 10, 2026
…ion-device

feat(admin/logs): expose location and device on admin logs feed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/modules/admin/logs/services/geo-location.service.ts`:
- Around line 101-111: The isNonRoutable(ip: string) guard misses IPv4-mapped
IPv6 and IPv6 link-local ranges; update isNonRoutable to
normalize/case-insensitively check for "::ffff:" mapped addresses (e.g. strings
starting with "::ffff:" and then an IPv4 pattern) and treat them as non-routable
when the mapped IPv4 is private/loopback, and also add a check for IPv6
link-local prefix "fe80::/10" (e.g. ip.toLowerCase().startsWith('fe80:') or a
/^fe8[0-9]:/i pattern). Keep existing checks (127.0.0.1, ::1, 10., 192.168.,
169.254., 172.16-31, fc/ fd) and apply case-insensitive comparisons; implement
these checks inside the same isNonRoutable function.

In `@src/modules/admin/logs/tests/geo-location.service.spec.ts`:
- Line 22: Test suite is leaving mocked global.fetch assigned (e.g.,
global.fetch = jest.fn()), which jest.restoreAllMocks() doesn't revert; capture
the original fetch reference (const originalFetch = global.fetch) before tests
run (in a top-level beforeAll or beforeEach) and then restore it in the existing
afterEach (or add an afterEach) by setting global.fetch = originalFetch or
deleting global.fetch so the mock doesn't leak to other suites; reference the
existing afterEach(() => jest.restoreAllMocks()) and any places assigning
global.fetch = jest.fn() to implement the save/restore.
🪄 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: 8aedb12a-6893-4ed5-91f0-95ecc0666bf3

📥 Commits

Reviewing files that changed from the base of the PR and between bca113f and 7dfa74e.

📒 Files selected for processing (11)
  • src/modules/admin/logs/actions/admin-logs-list.action.ts
  • src/modules/admin/logs/admin-logs.module.ts
  • src/modules/admin/logs/admin-logs.service.ts
  • src/modules/admin/logs/docs/admin-logs-swagger.doc.ts
  • src/modules/admin/logs/interfaces/admin-logs.interfaces.ts
  • src/modules/admin/logs/services/geo-location.service.ts
  • src/modules/admin/logs/tests/admin-logs.controller.spec.ts
  • src/modules/admin/logs/tests/admin-logs.service.spec.ts
  • src/modules/admin/logs/tests/geo-location.service.spec.ts
  • src/modules/admin/logs/tests/parse-user-agent.util.spec.ts
  • src/modules/admin/logs/utils/parse-user-agent.util.ts

Comment thread src/modules/admin/logs/services/geo-location.service.ts
Comment thread src/modules/admin/logs/tests/geo-location.service.spec.ts
@akinwalexander akinwalexander merged commit a6b8aed into dev Jun 10, 2026
7 checks passed
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