Skip to content

Commit f547efc

Browse files
authored
Fix issues in the json facet support (#2630)
1 parent 523d891 commit f547efc

File tree

5 files changed

+116
-19
lines changed

5 files changed

+116
-19
lines changed

src/components/faceting/facet-choice-picker.tsx

+14-12
Original file line numberDiff line numberDiff line change
@@ -121,16 +121,17 @@ const FacetChoicePicker = ({
121121

122122
const listContainer = useRef<HTMLDivElement>(null);
123123

124-
// populate facetReference and columnName that are used throughout the component
125-
let facetReference: any, columnName: string;
124+
// populate facetReference and baseColumn that are used throughout the component
125+
let facetReference: any, baseColumn: any;
126126
if (facetColumn.isEntityMode) {
127127
facetReference = facetColumn.sourceReference.contextualize.compactSelect;
128-
columnName = facetColumn.column.name;
128+
baseColumn = facetColumn.column;
129129
} else {
130130
facetReference = facetColumn.scalarValuesReference;
131131
// the first column will be the value column
132-
columnName = facetReference.columns[0].name;
132+
baseColumn = facetReference.columns[0];
133133
}
134+
const baseColumnIsJSON = baseColumn.type.name === 'json' || baseColumn.type.name === 'jsonb';
134135

135136
// make sure to add the search term
136137
if (searchTerm) {
@@ -338,7 +339,7 @@ const FacetChoicePicker = ({
338339
}
339340

340341
// filter and tuple uniqueId might be different
341-
const value = getFilterUniqueId(tuple, columnName);
342+
const value = getFilterUniqueId(tuple);
342343

343344
const i = updatedRows.findIndex(function (row) {
344345
// ermrestjs always returns a string for uniqueId, but internally we don't
@@ -396,15 +397,16 @@ const FacetChoicePicker = ({
396397
}
397398

398399
/**
399-
* Given tuple and the columnName that should be used, return
400+
* Given the tuple object, return the filter's uniqueId
400401
* the filter's uniqueId (in case of entityPicker, it might be different from the tuple's uniqueId)
401402
* @param {Object} tuple the tuple object
402-
* @param {string} columnName name of column (in scalar it is 'value')
403403
* @return {string} filter's uniqueId
404404
*/
405-
const getFilterUniqueId = (tuple: any, columnName: string) => {
406-
if (tuple.data && columnName in tuple.data) {
407-
return tuple.data[columnName];
405+
const getFilterUniqueId = (tuple: any) => {
406+
if (tuple.data && baseColumn.name in tuple.data) {
407+
// if the column is JSON, we need to stringify it otherwise it will print [object Object]
408+
// NOTE this is the exact same logic as ermrestjs
409+
return baseColumnIsJSON ? JSON.stringify(tuple.data[baseColumn.name], undefined, 0) : tuple.data[baseColumn.name];
408410
}
409411
return tuple.uniqueId;
410412
}
@@ -507,7 +509,7 @@ const FacetChoicePicker = ({
507509
// create the list of choice filters
508510
let hasNull = false;
509511
const filters = selectedRows.map(function (t: any) {
510-
const val = getFilterUniqueId(t, columnName);
512+
const val = getFilterUniqueId(t);
511513
hasNull = hasNull || (val === null);
512514
return val;
513515
});
@@ -534,7 +536,7 @@ const FacetChoicePicker = ({
534536

535537
selectedRows.forEach((row: any) => {
536538
// filter and tuple uniqueId might be different
537-
const value = getFilterUniqueId(row, columnName);
539+
const value = getFilterUniqueId(row);
538540

539541
updatedRows.push({
540542
selected: true,

test/e2e/data_setup/data/faceting/main.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -116,14 +116,14 @@
116116
{
117117
"id": "16", "fk_to_f1": "2", "fk_to_f2": "3",
118118
"int_col": 8, "float_col": 8.8, "date_col": "2008-08-08", "timestamp_col": "2008-08-08T08:08:08-07:00", "text_col": "eight", "shorttext_col": "eight", "longtext_col": "eight", "markdown_col": "**eight**",
119-
"boolean_col": false, "jsonb_col": {"key": "eight"}, "jsonb_sort_col": "08",
119+
"boolean_col": false, "jsonb_col": 8, "jsonb_sort_col": "08",
120120
"col_w_long_values": "10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000016",
121121
"col_w_column_order": "06", "col_w_column_order_column_order" : "08", "col_w_column_order_false": "02", "col_w_column_order_false_facet_order": "06"
122122
},
123123
{
124124
"id": "17", "fk_to_f1": "2", "fk_to_f2": "3",
125125
"int_col": 9, "float_col": 9.9, "date_col": "2009-09-09", "timestamp_col": "2009-09-09T09:09:09-07:00", "text_col": "one", "shorttext_col": "one", "longtext_col": "nine", "markdown_col": "**nine**",
126-
"boolean_col": false, "jsonb_col": {"key": "nine"}, "jsonb_sort_col": "09",
126+
"boolean_col": false, "jsonb_col": "nine", "jsonb_sort_col": "09",
127127
"col_w_long_values": "10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000017",
128128
"col_w_column_order": "05", "col_w_column_order_column_order" : "09", "col_w_column_order_false": "02", "col_w_column_order_false_facet_order": "06"
129129
},

test/e2e/data_setup/schema/recordset/faceting.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@
214214
{"source": "longtext_col", "comment": "A lengthy comment for the facet of the longtext_col. This should be displyed properly in the facet.", "hide_not_null_choice": true, "hide_null_choice": true},
215215
{"source": "markdown_col", "hide_null_choice": true, "hide_not_null_choice": true},
216216
{"source": "boolean_col", "hide_null_choice": true},
217-
{"source": "jsonb_col", "order": [{"num_occurrences": true, "descending": true}, "jsonb_sort_col"], "hide_null_choice": true},
217+
{"source": "jsonb_col", "order": [{"num_occurrences": true, "descending": true}, "jsonb_sort_col"]},
218218
{"sourcekey": "outbound_to_f1", "markdown_name": "**F1**", "hide_not_null_choice": true},
219219
{"source": [{"outbound": ["faceting", "main_fk2"]}, "id"], "open": true, "comment": "open facet", "hide_null_choice": true},
220220
{"source": [{"inbound": ["faceting", "main_f3_assoc_fk1"]}, {"outbound": ["faceting", "main_f3_assoc_fk2"]}, "term"]},

test/e2e/specs/delete-prohibited/recordset/individual-facet.spec.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -197,14 +197,14 @@ const testParams: {
197197
index: 9,
198198
name: 'jsonb_col',
199199
type: 'choice',
200-
option: 4,
200+
option: 5,
201201
filter: 'jsonb_col{\n"key": "four"\n}',
202202
numRows: 1,
203203
options: [
204-
'All records with value', '{\n"key": "one"\n}', '{\n"key": "two"\n}',
204+
'All records with value', 'No value',
205+
'{\n"key": "one"\n}', '{\n"key": "two"\n}',
205206
'{\n"key": "three"\n}', '{\n"key": "four"\n}', '{\n"key": "five"\n}',
206-
'{\n"key": "six"\n}', '{\n"key": "seven"\n}', '{\n"key": "eight"\n}',
207-
'{\n"key": "nine"\n}', '{\n"key": "ten"\n}'
207+
'{\n"key": "six"\n}', '{\n"key": "seven"\n}', '8', '"nine"', '{\n"key": "ten"\n}'
208208
]
209209
},
210210
{

test/e2e/specs/delete-prohibited/recordset/misc-facet.spec.ts

+95
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,55 @@ const testParams = {
8989
}
9090
}
9191
},
92+
json_support: {
93+
facetIdx: 9,
94+
totalNumOptions: 12,
95+
numRows: 25,
96+
tests: [
97+
{
98+
description: 'not-null',
99+
option: 0,
100+
filter: 'jsonb_colAll records with value',
101+
numRows: 20,
102+
},
103+
{
104+
description: 'null',
105+
option: 1,
106+
filter: 'jsonb_colNo value ',
107+
numRows: 10,
108+
},
109+
{
110+
description: 'json object',
111+
option: 2,
112+
filter: '"one"',
113+
numRows: 5,
114+
modal: {
115+
numRows: 12,
116+
checkedOption: 0
117+
}
118+
},
119+
{
120+
description: 'number',
121+
option: 9,
122+
filter: 'jsonb_col8',
123+
numRows: 1,
124+
modal: {
125+
numRows: 12,
126+
checkedOption: 7
127+
}
128+
},
129+
{
130+
description: 'string literal',
131+
option: 10,
132+
filter: 'jsonb_col"nine"',
133+
numRows: 1,
134+
modal: {
135+
numRows: 12,
136+
checkedOption: 8
137+
}
138+
}
139+
]
140+
},
92141
hide_row_count: {
93142
hidden: {
94143
facetIdx: 11,
@@ -481,6 +530,52 @@ test.describe('Other facet features', () => {
481530
});
482531
});
483532

533+
test.describe('json/jsonb support', () => {
534+
testParams.json_support.tests.forEach((params) => {
535+
test(`${params.description}`, async ({ page, baseURL }, testInfo) => {
536+
const facet = RecordsetLocators.getFacetById(page, testParams.json_support.facetIdx);
537+
const numRowsWhenCleared = testParams.json_support.numRows;
538+
539+
const checkPageStatusAfterSelection = async () => {
540+
await expect.soft(RecordsetLocators.getRows(page)).toHaveCount(params.numRows);
541+
await expect.soft(RecordsetLocators.getCheckedFacetOptions(facet)).toHaveCount(1);
542+
await expect.soft(RecordsetLocators.getFacetFilters(page).nth(0)).toContainText(params.filter);
543+
}
544+
545+
await test.step('should load recordset page, clear all filters, and open the json facet', async () => {
546+
await page.goto(generateChaiseURL(APP_NAMES.RECORDSET, testParams.schema_name, testParams.table_name, testInfo, baseURL));
547+
await RecordsetLocators.waitForRecordsetPageReady(page);
548+
await testClearAllFilters(page, numRowsWhenCleared);
549+
550+
// 4 open facets because 3 are already open on page load
551+
await openFacet(page, facet, testParams.json_support.facetIdx, testParams.json_support.totalNumOptions, 4);
552+
});
553+
554+
await test.step('choosing the option should work properly.', async () => {
555+
await testSelectFacetOption(page, facet, params.option, params.numRows, 1);
556+
await checkPageStatusAfterSelection();
557+
});
558+
559+
await test.step('refreshing the page should keep the selected filter.', async () => {
560+
await page.reload();
561+
await RecordsetLocators.waitForRecordsetPageReady(page);
562+
await checkPageStatusAfterSelection();
563+
});
564+
565+
if ('modal' in params) {
566+
await test.step('the modal should show the selected option.', async () => {
567+
const modal = ModalLocators.getRecordsetSearchPopup(page);
568+
await testShowMoreClick(facet, modal, params.modal!.numRows, 1);
569+
await expect.soft(RecordsetLocators.getCheckboxInputs(modal).nth(params.modal!.checkedOption)).toBeChecked();
570+
await ModalLocators.getSubmitButton(modal).click();
571+
await expect.soft(modal).not.toBeVisible();
572+
await checkPageStatusAfterSelection();
573+
});
574+
}
575+
});
576+
});
577+
});
578+
484579
test('regarding the logic to show only certain number of selected items', async ({ page, baseURL }, testInfo) => {
485580
const params = testParams.hide_selected_items;
486581
const facet1 = RecordsetLocators.getFacetById(page, params.firstFacet.index);

0 commit comments

Comments
 (0)