Skip to content

Commit 3581c7e

Browse files
ref(logs): Unify pinned-log fetching into a single query+cache
The previous strategy paired per-id useQueries(queryFn: skipToken) with a separate driver query that imperatively seeded their caches via setQueryData. Those per-id entries had no queryFn, so once garbage-collected they could not be refetched, leaving rows blank or stuck pending — a latent runtime invariant, and an anti-pattern of sourcing a query's data from a sibling query's side effects. Collapse it into one useQuery whose queryFn runs the same two-step fetch (selected range, then a UUIDv7-derived wide window) and returns its rows directly. placeholderData: keepPreviousData preserves the no-flicker-on-unpin behavior the per-id cache was hacked in to provide. The unpin effect now only removes ids that are still missing, so retained notFoundIds can't re-trigger a removal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3e79749 commit 3581c7e

2 files changed

Lines changed: 283 additions & 131 deletions

File tree

static/app/views/explore/logs/pinning/usePinnedLogsQuery.spec.tsx

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,44 @@ describe('usePinnedLogsQuery', () => {
433433
expect(logsPinning.removePinnedRows).not.toHaveBeenCalled();
434434
});
435435

436+
it('caches rows found by a partial wide scan without unpinning the stragglers', async () => {
437+
const partialLog = LogFixture({
438+
[OurLogKnownFieldKey.ID]: 'log-partial',
439+
[OurLogKnownFieldKey.PROJECT_ID]: String(project.id),
440+
[OurLogKnownFieldKey.ORGANIZATION_ID]: Number(organization.id),
441+
});
442+
443+
MockApiClient.addMockResponse({
444+
url: `/organizations/${organization.slug}/events/`,
445+
method: 'GET',
446+
body: {data: [], meta: {fields: {id: 'string'}, units: {}}},
447+
match: [MockApiClient.matchQuery({statsPeriod: '14d'})],
448+
});
449+
MockApiClient.addMockResponse({
450+
url: `/organizations/${organization.slug}/events/`,
451+
method: 'GET',
452+
body: {
453+
data: [partialLog],
454+
meta: {fields: {id: 'string'}, units: {}, dataScanned: 'partial'},
455+
},
456+
match: [MockApiClient.matchQuery({statsPeriod: '9999d'})],
457+
});
458+
459+
const logsPinning = makeLogsPinning(['log-partial']);
460+
461+
const {result} = renderHookWithProviders(
462+
() => usePinnedLogsQuery({allRows: [], logsPinning}),
463+
{organization, additionalWrapper: AdditionalWrapper}
464+
);
465+
466+
await waitFor(() => {
467+
expect(result.current.fetchedRows).toHaveLength(1);
468+
});
469+
470+
expect(result.current.fetchedRows[0]?.[OurLogKnownFieldKey.ID]).toBe('log-partial');
471+
expect(logsPinning.removePinnedRows).not.toHaveBeenCalled();
472+
});
473+
436474
it('calls removePinnedRows with every id not found in the API response', async () => {
437475
const foundLog = LogFixture({
438476
[OurLogKnownFieldKey.ID]: 'log-found',
@@ -521,6 +559,87 @@ describe('usePinnedLogsQuery', () => {
521559
});
522560
});
523561

562+
it('keeps already-fetched rows for the remaining pins when a pin is removed', async () => {
563+
const logA = LogFixture({
564+
[OurLogKnownFieldKey.ID]: 'log-a',
565+
[OurLogKnownFieldKey.PROJECT_ID]: String(project.id),
566+
[OurLogKnownFieldKey.ORGANIZATION_ID]: Number(organization.id),
567+
});
568+
const logB = LogFixture({
569+
[OurLogKnownFieldKey.ID]: 'log-b',
570+
[OurLogKnownFieldKey.PROJECT_ID]: String(project.id),
571+
[OurLogKnownFieldKey.ORGANIZATION_ID]: Number(organization.id),
572+
});
573+
574+
MockApiClient.addMockResponse({
575+
url: `/organizations/${organization.slug}/events/`,
576+
method: 'GET',
577+
body: {data: [logA, logB], meta: {fields: {id: 'string'}, units: {}}},
578+
});
579+
580+
const {result, rerender} = renderHookWithProviders(
581+
({ids}: {ids: string[]}) =>
582+
usePinnedLogsQuery({allRows: [], logsPinning: makeLogsPinning(ids)}),
583+
{
584+
organization,
585+
additionalWrapper: AdditionalWrapper,
586+
initialProps: {ids: ['log-a', 'log-b']},
587+
}
588+
);
589+
590+
await waitFor(() => {
591+
expect(result.current.fetchedRows).toHaveLength(2);
592+
});
593+
594+
rerender({ids: ['log-b']});
595+
596+
expect(result.current.fetchedRows.map(row => row[OurLogKnownFieldKey.ID])).toEqual([
597+
'log-b',
598+
]);
599+
});
600+
601+
it('refetches the pinned rows with the new fields when the visible columns change', async () => {
602+
const pinnedLog = LogFixture({
603+
[OurLogKnownFieldKey.ID]: 'log-cols',
604+
[OurLogKnownFieldKey.PROJECT_ID]: String(project.id),
605+
[OurLogKnownFieldKey.ORGANIZATION_ID]: Number(organization.id),
606+
});
607+
608+
const eventsRequest = MockApiClient.addMockResponse({
609+
url: `/organizations/${organization.slug}/events/`,
610+
method: 'GET',
611+
body: {data: [pinnedLog], meta: {fields: {id: 'string'}, units: {}}},
612+
});
613+
614+
const logsPinning = makeLogsPinning(['log-cols']);
615+
616+
const {result, router} = renderHookWithProviders(
617+
() => usePinnedLogsQuery({allRows: [], logsPinning}),
618+
{
619+
organization,
620+
additionalWrapper: AdditionalWrapper,
621+
initialRouterConfig: {location: {pathname: '/', query: {logsFields: 'message'}}},
622+
}
623+
);
624+
625+
const fieldsForCall = (call: unknown[]) =>
626+
(call[1] as {query: {field: string[]}}).query.field;
627+
628+
await waitFor(() => {
629+
expect(result.current.fetchedRows).toHaveLength(1);
630+
});
631+
expect(eventsRequest).toHaveBeenCalledTimes(1);
632+
633+
act(() => {
634+
router.navigate('/?logsFields=message&logsFields=my.custom.attr');
635+
});
636+
637+
await waitFor(() => {
638+
expect(eventsRequest).toHaveBeenCalledTimes(2);
639+
});
640+
expect(fieldsForCall(eventsRequest.mock.calls[1])).toContain('my.custom.attr');
641+
});
642+
524643
it('does not fetch when pinned ids are already in allRows', () => {
525644
const existingLog = LogFixture({
526645
[OurLogKnownFieldKey.ID]: 'log-existing',

0 commit comments

Comments
 (0)