Skip to content

fix(admin): clarify Overview Recent Activity scope - #1079

Merged
cevheri merged 2 commits into
libredb:mainfrom
igorwfaoro:fix/992-overview-activity-scope
Sep 23, 2026
Merged

cevheri merged 2 commits into
libredb:mainfrom
igorwfaoro:fix/992-overview-activity-scope

Conversation

@igorwfaoro

@igorwfaoro igorwfaoro commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Description

The Overview tab's Recent Activity widget reads an application-recorded audit preview and browser-local query history. Because proxy-recorded boundary denials are not present in that preview, the old empty state could be read as a complete statement that no activity occurred.

This takes the middle option accepted in #992: keep the existing preview, provide an always-visible path to the Audit tab, and make the empty state explicit without changing the audit buffer or proxy.

Type of Change

  • Bug fix (non-breaking change which fixes a misleading UI state)
  • Test addition or update

Related Issue

Closes #992

Changes Made

  • Added an unconditional Audit link to the Overview Recent Activity card using the canonical admin route.
  • Added an inline comment explaining why the link remains visible in every feed state.
  • Changed the empty copy to: No recent activity here. The Audit tab explains what this feed does not capture.
  • Restored the empty-state data-testid and updated component coverage for the link, empty state, and exact copy.
  • Left the audit endpoint, ring buffer, src/proxy.ts, rate limits, and Audit tab unchanged.

Testing

  • I have tested this locally
  • I have added/updated tests
  • All existing tests pass

Test Environment

  • LibreDB Studio Version: 0.16.2
  • Browser: Playwright UAT against production build; component tests in jsdom
  • OS: Linux
  • Node.js/Bun Version: Node.js 22.17.0 / Bun 1.3.14
  • Database Type: n/a — admin dashboard UI

Checks run

  • bun tests/run-tests.ts tests/components/admin/OverviewTab.test.tsx — 25/25 passed
  • bun run format, lint, typecheck, knip, chart/channel/README/security checks — passed
  • bun run build — passed
  • Manual UAT verified the Audit link and navigation to /admin/audit.
  • Full bun run test is not green in this environment because unrelated tests require the repository's Bun 1.4.2 / Node 24+ environment and additional tools; the changed component test passes.

Screenshots (if applicable)

Not included — the behavior was verified through component tests and Playwright UAT.

Checklist

  • My code follows the project's code style guidelines
  • I have performed a self-review of the code
  • I have commented my code, particularly in hard-to-understand areas (the behavior is documented by the inline rationale comment)
  • I have updated the documentation accordingly (existing Audit and Security documentation already covers the limitation)
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective
  • New and existing unit tests pass locally (full suite is environment-blocked as described above)
  • The required CI test job passes the 100% line-coverage gate
  • If I changed src/lib/db/providers/, I updated the matching provider documentation and tests (n/a)
  • Any dependent changes have been merged and published (n/a)

Additional Notes

The PR intentionally does not attempt to make the per-process audit buffer complete. That is outside #992 and would require a separate audit-sink design.

@codecov

codecov Bot commented Sep 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cevheri
cevheri marked this pull request as ready for review September 22, 2026 19:25
@cevheri

cevheri commented Sep 22, 2026

Copy link
Copy Markdown
Member

Hi @igorwfaoro, thanks for picking this up, and the direction is right: the middle option is what the issue asked for.

Before I review it properly, one thing to sort out. This diff is the same change as #1002, which I closed on 19 Sep. Not the same approach, the same code: the same wrapper classes, the same data-testid="overview-audit-link", the same inline-flex items-center gap-1 text-xs text-fg-subtle hover:text-fg-tertiary transition-colors, the same empty-state string, and the same two test cases with the same assertions. The differences are an import line moved, two rationale comments dropped, one data-testid on the empty state dropped, and a test renamed.

That PR is closed but public, so there is nothing hidden here. I just want the PR to say where the code came from. If you built on #1002, please note that in the body and credit it. If you did not, tell me and I will take your word for it.

Why I am asking at all, rather than just merging working code. I closed #1002 because it came from an account that opened 171 PRs in one day across about 50 repos. The good first issue label is not a queue to be drained. It exists so someone new to this codebase can learn it by fixing one thing, and so I can spend real review time on that one person. If the label just routes existing patches through a new account, it has cost the people it was reserved for and bought nobody anything. I would rather have your version of this change, even if it lands slower and looks different, than a faster copy of someone else's.

Two notes either way, since #1002 was better on both: keep the empty-state data-testid, it pins the state by id rather than by copy, and keep the comment explaining why the link is unconditional.

Separately, "No application-recorded events in this preview." points at a "preview" that is not named anywhere on the card. The card reads "Recent Activity". Something like "No recent activity here. The Audit tab has the full record." says the same thing without the dangling referent.

@igorwfaoro

Copy link
Copy Markdown
Contributor Author

@cevheri Thanks for pointing this out. I did see that #1002 existed, but I didn't read or use its code as a reference for my implementation. I worked from #992 and used an AI coding agent (OpenCode) to help implement and test the change. So I wasn't aware that the resulting code was this similar to #1002.

I understand your concern, especially given the context you explained around good first issues. I'll make the changes you suggested: restore the empty-state data-testid, add the comment explaining why the link is unconditional, and improve the empty-state wording.

Thanks for taking the time to explain the reasoning behind it.

@cevheri

cevheri commented Sep 22, 2026

Copy link
Copy Markdown
Member

Thanks, I'll take your word for it, and thanks for picking up the three changes.

One correction to my own suggestion before you push it: "The Audit tab has the full record" is wrong. The Audit tab reads the same buffer and says so in its own disclosure (proxy-recorded denials are missing there too), which is the exact gap #992 is about. Something like "No recent activity here. The Audit tab explains what this feed does not capture." keeps the pointer without claiming completeness. Any wording that does not call the Audit tab complete is fine.

@cevheri cevheri added the enhancement New feature or request label Sep 23, 2026
@igorwfaoro

Copy link
Copy Markdown
Contributor Author

Applied the requested follow-ups in commit 553bf594: restored the empty-state data-testid, added the rationale for the unconditional Audit link, and updated the empty-state copy to No recent activity here. The Audit tab explains what this feed does not capture. Updated the PR description as well.

@cevheri

cevheri commented Sep 23, 2026

Copy link
Copy Markdown
Member

thanks, merging after finishing CI

@cevheri
cevheri merged commit 53e6ce8 into libredb:main Sep 23, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Admin Overview 'Recent Activity' reads the same per-process buffer that #851 showed is incomplete

2 participants