Skip to content
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.

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

fix: Use correct offset in snapshot #2217

Merged
merged 6 commits into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 3 additions & 8 deletions packages/jsapi-components/src/useViewportData.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import type { dh as DhType } from '@deephaven/jsapi-types';
import dh from '@deephaven/jsapi-shim';
import {
OnTableUpdatedEvent,
ViewportRow,
generateEmptyKeyedItems,
isClosed,
ITEM_KEY_PREFIX,
Expand All @@ -23,13 +22,9 @@ jest.mock('@deephaven/react-hooks', () => ({
jest.mock('./useSetPaddedViewportCallback');
jest.mock('./useTableSize');

function mockViewportRow(offsetInSnapshot: number): ViewportRow {
return { offsetInSnapshot } as ViewportRow;
}

function mockUpdateEvent(
offset: number,
rows: ViewportRow[]
rows: DhType.Row[]
): OnTableUpdatedEvent {
return {
detail: {
Expand Down Expand Up @@ -213,14 +208,14 @@ it('should update state on dh.Table.EVENT_UPDATED event', () => {
);

const offset = 3;
const row = mockViewportRow(5);
const row = TestUtils.createMockProxy<DhType.Row>();
const event = mockUpdateEvent(offset, [row]);

act(() => {
updateEventHandler?.(event);
});

const expectedKeyIndex = offset + row.offsetInSnapshot;
const expectedKeyIndex = offset;
const expectedInitialItems = [...generateEmptyKeyedItems(0, table.size - 1)];
const expectedItems = [
...expectedInitialItems.slice(0, expectedKeyIndex),
Expand Down
36 changes: 9 additions & 27 deletions packages/jsapi-utils/src/ViewportDataUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import {
ITEM_KEY_PREFIX,
OnTableUpdatedEvent,
RowDeserializer,
ViewportRow,
createKeyFromOffsetRow,
createOnTableUpdatedHandler,
defaultRowDeserializer,
generateEmptyKeyedItems,
Expand All @@ -18,10 +16,6 @@ import {

const { asMock, createMockProxy } = TestUtils;

function mockViewportRow(offsetInSnapshot: number): ViewportRow {
return { offsetInSnapshot } as ViewportRow;
}

function mockColumn(name: string) {
return {
name,
Expand All @@ -44,28 +38,15 @@ describe('createdKeyedItemKey', () => {
});
});

describe('createKeyFromOffsetRow', () => {
it.each([
[{ offsetInSnapshot: 4 } as ViewportRow, 5, `${ITEM_KEY_PREFIX}_9`],
[{ offsetInSnapshot: 27 } as ViewportRow, 99, `${ITEM_KEY_PREFIX}_126`],
] as const)(
'should create a string key based on the actual row offset: %o',
(row, offset, expected) => {
const actual = createKeyFromOffsetRow(row, offset);
expect(actual).toEqual(expected);
}
);
});

describe('createOnTableUpdatedHandler', () => {
const mock = {
deserializeRow: jest.fn() as RowDeserializer<unknown>,
rows: [
createMockProxy<ViewportRow>({ offsetInSnapshot: 0 }),
createMockProxy<ViewportRow>({ offsetInSnapshot: 1 }),
createMockProxy<ViewportRow>({ offsetInSnapshot: 2 }),
createMockProxy<dh.Row>(),
createMockProxy<dh.Row>(),
createMockProxy<dh.Row>(),
],
updateEvent: (offset: number, rows: ViewportRow[], columns: dh.Column[]) =>
updateEvent: (offset: number, rows: dh.Row[], columns: dh.Column[]) =>
createMockProxy<OnTableUpdatedEvent>({
detail: {
offset,
Expand Down Expand Up @@ -107,10 +88,11 @@ describe('createOnTableUpdatedHandler', () => {

describe('defaultRowDeserializer', () => {
it('should map all columns with original names', () => {
const row = mockViewportRow(10);
// mock our get function by mapping capital column name to lowercase value
// e.g. A: 'a'
row.get = jest.fn(({ name }: { name: string }) => name.toLowerCase());
const row = createMockProxy<dh.Row>({
// mock our get function by mapping capital column name to lowercase value
// e.g. A: 'a'
get: jest.fn(({ name }: { name: string }) => name.toLowerCase()),
});

const actual = defaultRowDeserializer(row, [
mockColumn('A'),
Expand Down
27 changes: 5 additions & 22 deletions packages/jsapi-utils/src/ViewportDataUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,10 @@ export const ITEM_KEY_PREFIX = 'DH_ITEM_KEY';
export type OnTableUpdatedEvent = CustomEvent<{
offset: number;
columns: dh.Column[];
rows: ViewportRow[];
rows: dh.Row[];
}>;

export type RowDeserializer<T> = (row: ViewportRow, columns: dh.Column[]) => T;

export type ViewportRow = dh.Row & { offsetInSnapshot: number };
export type RowDeserializer<T> = (row: dh.Row, columns: dh.Column[]) => T;

const log = Log.module('ViewportDataUtils');

Expand All @@ -29,21 +27,6 @@ export function createKeyedItemKey(index: number): string {
return `${ITEM_KEY_PREFIX}_${index}`;
}

/**
* Create a unique string key for a row based on its ordinal position in its
* source table. This is calculated based on it's offset in the viewport
* (row.offsetInSnapshot) + the offset of the snapshot.
* @param row Row from a Table update event.
* @param offset Offset of the current viewport.
* @returns Unique string key for the ordinal position of the given row.
*/
export function createKeyFromOffsetRow(
row: ViewportRow,
offset: number
): string {
return createKeyedItemKey(row.offsetInSnapshot + offset);
}

/**
* Creates a handler function for a `dh.Table.EVENT_UPDATED` event. Rows that
* get passed to the handler will be bulk updated in the given `viewportData`
Expand All @@ -66,9 +49,9 @@ export function createOnTableUpdatedHandler<T>(

const updateKeyMap = new Map<Key, KeyedItem<T>>();

rows.forEach(row => {
rows.forEach((row, offsetInSnapshot) => {
const item = deserializeRow(row, columns);
const key = createKeyFromOffsetRow(row, offset);
const key = createKeyedItemKey(offset + offsetInSnapshot);
updateKeyMap.set(key, { key, item });
});

Expand All @@ -86,7 +69,7 @@ export function createOnTableUpdatedHandler<T>(
* @returns A key / value object for the row.
*/
export function defaultRowDeserializer<T>(
row: ViewportRow,
row: dh.Row,
columns: dh.Column[]
): T {
return columns.reduce((result, col) => {
Expand Down
Loading