Skip to content
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
146 changes: 115 additions & 31 deletions src/engine/rowFinder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,67 @@ export class RowFinder<T = any> {
this.resolve = resolve;
}

private splitFilters(filters: Record<string, FilterValue>): {
domFilters: Record<string, FilterValue>;
overrideFilters: Record<string, FilterValue>;
} {
const domFilters: Record<string, FilterValue> = {};
const overrideFilters: Record<string, FilterValue> = {};
for (const [key, value] of Object.entries(filters)) {
if (this.config.columnOverrides?.[key as keyof T]?.read) {
overrideFilters[key] = value;
} else {
domFilters[key] = value;
}
}
return { domFilters, overrideFilters };
}

static matchReadValue(readValue: string, filterValue: FilterValue, exact: boolean): boolean {
if (typeof filterValue === 'function') {
throw new Error(
`[SmartTable] Function filters are not supported for columns with columnOverrides.read. ` +
`Use a string, number, or RegExp filter instead.`
);
}
if (typeof filterValue === 'string' || typeof filterValue === 'number') {
const target = String(filterValue);
return exact ? readValue === target : readValue.includes(target);
}
if (filterValue instanceof RegExp) {
return filterValue.test(readValue);
}
return true;
}

private async matchesOverrideFilters(
rowLocator: Locator,
overrideFilters: Record<string, FilterValue>,
map: Map<string, number>,
exact: boolean
): Promise<boolean> {
for (const [colName, filterValue] of Object.entries(overrideFilters)) {
const colIndex = map.get(colName);
if (colIndex === undefined) continue;
const override = this.config.columnOverrides![colName as keyof T]!;
const cell = this.resolve(this.config.cellSelector, rowLocator).nth(colIndex);
const getCell = (name: string) => {
const idx = map.get(name);
if (idx === undefined) throw new Error(`Column "${name}" not found`);
return this.resolve(this.config.cellSelector, rowLocator).nth(idx);
};
const context = {
row: this.makeSmartRow(rowLocator, map, undefined),
columnName: colName,
columnIndex: colIndex,
getCell,
};
const readValue = String(await override.read!(cell, context));
if (!RowFinder.matchReadValue(readValue, filterValue, exact)) return false;
}
return true;
}
Comment thread
rickcedwhat marked this conversation as resolved.

public async findRow(
filters: Record<string, FilterValue>,
options: { exact?: boolean, maxPages?: number } = {}
Expand Down Expand Up @@ -68,13 +129,15 @@ export class RowFinder<T = any> {
const tracker = new ElementTracker('findRows');

try {
const { domFilters, overrideFilters } = this.splitFilters(filtersRecord);
const hasOverrideFilters = Object.keys(overrideFilters).length > 0;

const collectMatches = async () => {
let rowLocators = this.resolve(this.config.rowSelector, this.rootLocator);
// Only apply filters if we have them
if (Object.keys(filtersRecord).length > 0) {
if (Object.keys(domFilters).length > 0) {
rowLocators = this.filterEngine.applyFilters(
rowLocators,
filtersRecord,
domFilters,
map,
options?.exact ?? false,
this.rootLocator.page(),
Expand Down Expand Up @@ -115,6 +178,11 @@ export class RowFinder<T = any> {
continue; // 'skip'
}

if (hasOverrideFilters && !await this.matchesOverrideFilters(currentRows[idx], overrideFilters, map, options?.exact ?? false)) {
barrier?.markFinished();
continue;
}

allRows.push(smartRow);
added++;
}
Expand Down Expand Up @@ -181,37 +249,37 @@ export class RowFinder<T = any> {
}

const allRows = this.resolve(this.config.rowSelector, this.rootLocator);
const matchedRows = this.filterEngine.applyFilters(
allRows,
filters,
map,
options.exact || false,
this.rootLocator.page(),
this.rootLocator
);

const count = await matchedRows.count();
logDebug(this.config, 'verbose',`Page ${this.tableState.currentPageIndex}: Found ${count} matches.`);

if (count > 1) {
const sampleData: string[] = [];
try {
const firstFewRows = await matchedRows.all();
const sampleCount = Math.min(firstFewRows.length, 3);
for (let i = 0; i < sampleCount; i++) {
const rowData = await this.makeSmartRow(firstFewRows[i], map, 0, this.tableState.currentPageIndex).toJSON();
sampleData.push(JSON.stringify(rowData));
}
} catch (e) { }
const sampleMsg = sampleData.length > 0 ? `\nSample matching rows:\n${sampleData.map((d, i) => ` ${i + 1}. ${d}`).join('\n')}` : '';

throw new Error(
`Ambiguous Row: Found ${count} rows matching ${JSON.stringify(filters)} on page ${this.tableState.currentPageIndex}. ` +
`Expected exactly one match. Try adding more filters to make your query unique.${sampleMsg}`
const { domFilters, overrideFilters } = this.splitFilters(filters);
const hasOverrideFilters = Object.keys(overrideFilters).length > 0;

let matchedRows = allRows;
if (Object.keys(domFilters).length > 0) {
matchedRows = this.filterEngine.applyFilters(
allRows,
domFilters,
map,
options.exact || false,
this.rootLocator.page(),
this.rootLocator
);
}

if (count === 1) return matchedRows.first();
if (!hasOverrideFilters) {
const count = await matchedRows.count();
logDebug(this.config, 'verbose',`Page ${this.tableState.currentPageIndex}: Found ${count} matches.`);
if (count > 1) await this.throwIfAmbiguous(await matchedRows.all(), filters, map);
if (count === 1) return matchedRows.first();
} else {
const candidates = await matchedRows.all();
logDebug(this.config, 'verbose',`Page ${this.tableState.currentPageIndex}: ${candidates.length} DOM candidate(s), post-filtering with override columns`);
const results = await Promise.all(
candidates.map(c => this.matchesOverrideFilters(c, overrideFilters, map, options.exact || false))
);
const overrideMatches = candidates.filter((_, i) => results[i]);
logDebug(this.config, 'verbose',`Page ${this.tableState.currentPageIndex}: ${overrideMatches.length} match(es) after override filter`);
if (overrideMatches.length > 1) await this.throwIfAmbiguous(overrideMatches, filters, map);
if (overrideMatches.length === 1) return overrideMatches[0];
}

if (pagesScanned < effectiveMaxPages) {
logDebug(this.config, 'verbose',`Page ${this.tableState.currentPageIndex}: Not found. Attempting pagination...`);
Expand Down Expand Up @@ -251,6 +319,22 @@ export class RowFinder<T = any> {
* same selector/scope as everywhere else, so a string, function, or Locator rowSelector
* all behave identically. (#350)
*/
private async throwIfAmbiguous(rows: Locator[], filters: Record<string, FilterValue>, map: Map<string, number>): Promise<never> {
const sampleData: string[] = [];
try {
const sampleCount = Math.min(rows.length, 3);
for (let i = 0; i < sampleCount; i++) {
const rowData = await this.makeSmartRow(rows[i], map, 0, this.tableState.currentPageIndex).toJSON();
sampleData.push(JSON.stringify(rowData));
}
} catch (e) { }
const sampleMsg = sampleData.length > 0 ? `\nSample matching rows:\n${sampleData.map((d, i) => ` ${i + 1}. ${d}`).join('\n')}` : '';
throw new Error(
`Ambiguous Row: Found ${rows.length} rows matching ${JSON.stringify(filters)} on page ${this.tableState.currentPageIndex}. ` +
`Expected exactly one match. Try adding more filters to make your query unique.${sampleMsg}`
);
}

private async scanDomPosition(rowLocator: Locator): Promise<number | undefined> {
const targetHandle = await rowLocator.elementHandle();
if (!targetHandle) return undefined;
Expand Down
58 changes: 52 additions & 6 deletions src/useTable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,19 +331,62 @@ export const useTable = <T = any>(rootLocator: Locator, configOptions: TableConf
const pag = config.strategies.pagination;
const hasPagination = effectiveMaxPages > 1 && !!(pag?.goNext || pag?.goNextBulk);

const domFilters: Record<string, FilterValue> = {};
const overrideFilters: Record<string, FilterValue> = {};
if (hasFilters) {
for (const [key, value] of Object.entries(filtersRecord)) {
if (config.columnOverrides?.[key as keyof T]?.read) {
overrideFilters[key] = value;
} else {
domFilters[key] = value;
}
}
}
const hasOverrideFilters = Object.keys(overrideFilters).length > 0;

const resolveRows = () => {
let rows = resolve(config.rowSelector, rootLocator);
if (hasFilters) {
if (Object.keys(domFilters).length > 0) {
const map = tableMapper.getMapSync();
if (!map) throw new Error('Initialization Error: Table map not available. Call "await table.init()" first.');
rows = filterEngine.applyFilters(rows, filtersRecord, map, options?.exact ?? false, rootLocator.page(), rootLocator);
rows = filterEngine.applyFilters(rows, domFilters, map, options?.exact ?? false, rootLocator.page(), rootLocator);
}
return rows;
};

const countOverrideMatches = async (candidates: import('@playwright/test').Locator[], indices: number[]): Promise<number> => {
if (!hasOverrideFilters) return indices.length;
const map = tableMapper.getMapSync();
if (!map) throw new Error('Initialization Error: Table map not available. Call "await table.init()" first.');
const exact = options?.exact ?? false;
let count = 0;
for (const idx of indices) {
const row = candidates[idx];
let match = true;
for (const [colName, filterValue] of Object.entries(overrideFilters)) {
const colIndex = map.get(colName);
if (colIndex === undefined) continue;
const override = config.columnOverrides![colName as keyof T]!;
const cell = resolve(config.cellSelector, row).nth(colIndex);
const getCell = (name: string) => {
const ci = map.get(name);
if (ci === undefined) throw new Error(`Column "${name}" not found`);
return resolve(config.cellSelector, row).nth(ci);
};
const ctx = { row: _makeSmart(row, map, undefined), columnName: colName, columnIndex: colIndex, getCell };
const readValue = String(await override.read!(cell, ctx));
if (!RowFinder.matchReadValue(readValue, filterValue, exact)) { match = false; break; }
}
if (match) count++;
}
return count;
};

if (!hasPagination) {
log(`countRows: counting rows in current viewport (no pagination)${hasFilters ? ` filters=${safeStringify(filtersRecord)}` : ''}`);
return resolveRows().count();
if (!hasOverrideFilters) return resolveRows().count();
const all = await resolveRows().all();
return countOverrideMatches(all, all.map((_, i) => i));
}
Comment thread
rickcedwhat marked this conversation as resolved.

log(`countRows: paginating up to ${effectiveMaxPages} page(s)${hasFilters ? ` filters=${safeStringify(filtersRecord)}` : ''}`);
Expand All @@ -353,9 +396,12 @@ export const useTable = <T = any>(rootLocator: Locator, configOptions: TableConf
let paginationError: unknown;
try {
while (true) {
const newIndices = await tracker.getUnseenIndices(resolveRows());
total += newIndices.length;
log(`countRows: page ${pagesScanned} — ${newIndices.length} row(s) (running total: ${total})`);
const rowLocators = resolveRows();
const newIndices = await tracker.getUnseenIndices(rowLocators);
const candidates = await rowLocators.all();
const matched = await countOverrideMatches(candidates, newIndices);
total += matched;
log(`countRows: page ${pagesScanned} — ${matched} row(s) (running total: ${total})`);
if (pagesScanned >= effectiveMaxPages) break;
if (!await _advancePage(false)) break;
pagesScanned++;
Expand Down
76 changes: 76 additions & 0 deletions tests/override-filter.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { test, expect, type Page } from '@playwright/test';
import { useTable } from '../src/index';

const TABLE = `
<table id="t">
<thead><tr><th>Name</th><th>Link</th><th>Status</th></tr></thead>
<tbody>
<tr><td>Alpha</td><td><a href="/d/alpha">link</a></td><td>Active</td></tr>
<tr><td>Beta</td><td><a href="/d/beta">link</a></td><td>Active</td></tr>
<tr><td>Gamma</td><td><a href="/d/gamma">link</a></td><td>Inactive</td></tr>
</tbody>
</table>
`;

test.describe('findRow/findRows with columnOverride filters (#385)', () => {
const makeTable = (page: Page) =>
useTable(page.locator('#t'), {
columnOverrides: {
Link: {
read: async (cell) => {
const anchor = cell.locator('a');
if (await anchor.count() > 0) {
return await anchor.getAttribute('href') ?? cell.innerText();
}
return cell.innerText();
},
},
},
});

test('findRow filters on override-produced value', async ({ page }) => {
await page.setContent(TABLE);
const table = await makeTable(page).init();

const row = await table.findRow({ Link: '/d/beta' }, { exact: true });
const data = await row.toJSON() as Record<string, string>;
expect(data.Name).toBe('Beta');
});

test('findRows filters on override-produced value', async ({ page }) => {
await page.setContent(TABLE);
const table = await makeTable(page).init();

const rows = await table.findRows({ Link: '/d/beta' }, { exact: true });
expect(rows.length).toBe(1);
const data = await rows[0].toJSON() as Record<string, string>;
expect(data.Name).toBe('Beta');
});
Comment thread
rickcedwhat marked this conversation as resolved.

test('findRow combines DOM filter + override filter', async ({ page }) => {
await page.setContent(TABLE);
const table = await makeTable(page).init();

const row = await table.findRow({ Status: 'Active', Link: '/d/alpha' }, { exact: true });
const data = await row.toJSON() as Record<string, string>;
expect(data.Name).toBe('Alpha');
});

test('findRow throws Ambiguous when override filter matches multiple', async ({ page }) => {
await page.setContent(TABLE);
const table = await makeTable(page).init();

await expect(
table.findRow({ Link: '/d/' })
).rejects.toThrow(/Ambiguous Row/);
});

test('countRows with override filter counts only matching rows', async ({ page }) => {
await page.setContent(TABLE);
const table = await makeTable(page).init();

expect(await table.countRows({ Link: '/d/alpha' }, { exact: true })).toBe(1);
expect(await table.countRows({ Link: '/d/' })).toBe(3);
expect(await table.countRows({ Link: '/d/nonexistent' }, { exact: true })).toBe(0);
});
});
Loading