Skip to content

feat(ourlogs): fetch not-yet-available pinned logs and invalidate on filter changes#116614

Merged
JoshuaKGoldberg merged 3 commits into
masterfrom
feat/logs-781-fetch-not-yet-available-pinned-logs
Jun 5, 2026
Merged

feat(ourlogs): fetch not-yet-available pinned logs and invalidate on filter changes#116614
JoshuaKGoldberg merged 3 commits into
masterfrom
feat/logs-781-fetch-not-yet-available-pinned-logs

Conversation

@JoshuaKGoldberg
Copy link
Copy Markdown
Member

@JoshuaKGoldberg JoshuaKGoldberg commented Jun 1, 2026

Summary

Part 2 of 2 for log pinning implementation (part 1: #115102). Adds a new usePinnedLogsQuery hook that sets up a query, which is then used in the existing PinnedLogs component.

Closes LOGS-781

@JoshuaKGoldberg JoshuaKGoldberg requested a review from a team as a code owner June 1, 2026 19:22
@linear-code
Copy link
Copy Markdown

linear-code Bot commented Jun 1, 2026

LOGS-781

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jun 1, 2026
Comment thread static/app/views/explore/logs/pinning/useLogsPinning.tsx Outdated
Comment thread static/app/views/explore/logs/pinning/usePinnedLogsQuery.tsx
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 1, 2026

📊 Type Coverage Diff

✅ no issues found

Comment thread static/app/views/explore/logs/pinning/useLogsPinning.tsx Outdated
@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as draft June 1, 2026 19:45
@JoshuaKGoldberg JoshuaKGoldberg force-pushed the feat/logs-781-fetch-not-yet-available-pinned-logs branch from ebdd212 to 24d8526 Compare June 1, 2026 20:43
@JoshuaKGoldberg JoshuaKGoldberg force-pushed the feat/logs-781-fetch-not-yet-available-pinned-logs branch from 24d8526 to 00fd77e Compare June 1, 2026 21:21
…filter changes

Adds data fetching for pinned log rows that are in the URL but not yet
loaded in the infinite scroll table (e.g. the virtualizer hasn't reached
them yet). Also removes pinned IDs from the URL when they can't be found
within the current page filters (date range, project, environment).

- Add removePinnedRow to LogsPinning interface (idempotent removal)
- New usePinnedLogsQuery hook: fetches missing pinned log rows via the
  events API, then removes any IDs that aren't returned (invalidation)
- PinnedLogs now accepts fetchedPinnedRows/isFetchingPinnedRows props,
  merges fetched rows with existing allRows, and shows a loading indicator
  while rows are being fetched
- logsInfiniteTable calls usePinnedLogsQuery and passes results down

Refs LOGS-781
Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
@JoshuaKGoldberg JoshuaKGoldberg force-pushed the feat/logs-781-fetch-not-yet-available-pinned-logs branch from 00fd77e to 394b9bc Compare June 1, 2026 21:26
JoshuaKGoldberg and others added 2 commits June 3, 2026 16:14
Replace removePinnedRow with removePinnedRows so invalidation removes all
not-yet-available pinned ids in a single state update. Calling the
single-id setter in a loop dropped removals: nuqs runs setState updaters
at render, so each synchronous call saw the same committed state and only
the last write survived.

Include the utc flag in the pinned-log fetch for absolute date ranges so
it matches the main logs query's time window and avoids spurious pin
invalidation.

Also fix pre-existing typecheck failures (LogFixture missing
ORGANIZATION_ID, PinnedLogs wrapper row type) and a require-await lint
error in the specs.

Refs LOGS-781
Co-Authored-By: Claude <noreply@anthropic.com>
@JoshuaKGoldberg JoshuaKGoldberg changed the title feat(ourlogs): Fetch not-yet-available pinned logs and invalidate on filter changes feat(ourlogs): fetch not-yet-available pinned logs and invalidate on filter changes Jun 3, 2026
@JoshuaKGoldberg
Copy link
Copy Markdown
Member Author

@cursor review

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 90715c1. Configure here.

Comment thread static/app/views/explore/logs/pinning/usePinnedLogsQuery.tsx
@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as ready for review June 3, 2026 20:30
Comment thread static/app/views/explore/logs/pinning/usePinnedLogsQuery.tsx
Copy link
Copy Markdown
Member

@narsaynorath narsaynorath left a comment

Choose a reason for hiding this comment

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

lgtm! One thing I noticed when testing it was that the floating button to move to the top of the table occupies the same space as the pinned logs. Not sure if this is already known or is fine to ignore, but just wanted to raise it 👍

Image

Comment thread static/app/views/explore/logs/pinning/PinnedLogs.tsx
@JoshuaKGoldberg
Copy link
Copy Markdown
Member Author

floating button to move to the top of the table occupies

Great spot! No, I hadn't noticed 🙈. Filed: LOGS-837

@JoshuaKGoldberg JoshuaKGoldberg merged commit ab6305d into master Jun 5, 2026
72 checks passed
@JoshuaKGoldberg JoshuaKGoldberg deleted the feat/logs-781-fetch-not-yet-available-pinned-logs branch June 5, 2026 13:35
@JoshuaKGoldberg
Copy link
Copy Markdown
Member Author

JoshuaKGoldberg commented Jun 5, 2026

Oh, dangit, my rename commit push failed and then I merged. Blagh. Sent: #116966.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants