Skip to content

Commit 1678935

Browse files
authored
Merge pull request #448 from AllenInstitute/bugfix/wrong-values-in-group
Fix SQL for groupings and csv downloads
2 parents d06863b + eee8730 commit 1678935

File tree

4 files changed

+45
-7
lines changed

4 files changed

+45
-7
lines changed

packages/core/entity/FileSet/index.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ export default class FileSet {
211211

212212
Object.entries(filtersGroupedByAnnotation).forEach(([_, appliedFilters]) => {
213213
sqlBuilder.where(
214-
appliedFilters.map((filter) => filter.toSQLWhereString()).join(") OR (")
214+
appliedFilters.map((filter) => filter.toSQLWhereString()).join(" OR ")
215215
);
216216
});
217217

packages/core/entity/FileSet/test/FileSet.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ describe("FileSet", () => {
127127
'WHERE (REGEXP_MATCHES("scientist"'
128128
);
129129
expect(fileSet.toQuerySQLBuilder().from(mockDatasource).toString()).to.contain(
130-
'OR (REGEXP_MATCHES("scientist"'
130+
'OR REGEXP_MATCHES("scientist"'
131131
);
132132
});
133133
});

packages/core/services/FileService/DatabaseFileService/index.ts

+3-5
Original file line numberDiff line numberDiff line change
@@ -169,11 +169,9 @@ export default class DatabaseFileService implements FileService {
169169
public static applyFiltersAndSorting(subQuery: SQLBuilder, selection: Selection): void {
170170
if (!isEmpty(selection.filters)) {
171171
subQuery.where(
172-
Object.entries(selection.filters)
173-
.flatMap(([column, values]) =>
174-
values.map((v) => SQLBuilder.regexMatchValueInList(column, v))
175-
)
176-
.join(") OR (")
172+
Object.entries(selection.filters).flatMap(([column, values]) =>
173+
values.map((v) => SQLBuilder.regexMatchValueInList(column, v)).join(" OR ")
174+
)
177175
);
178176
}
179177
if (selection.sort) {

packages/core/services/FileService/DatabaseFileService/test/DatabaseFileService.test.ts

+40
Original file line numberDiff line numberDiff line change
@@ -154,5 +154,45 @@ describe("DatabaseFileService", () => {
154154
// Assert
155155
expect(modifiedSQL).to.include("OFFSET 0 LIMIT 3");
156156
});
157+
158+
it("correctly applies AND vs OR clauses", () => {
159+
const selectionsWithOR = [
160+
{
161+
indexRanges: [{ start: 0, end: 2 }], // File range
162+
filters: {
163+
Structure: ["structure1", "structure2"],
164+
},
165+
},
166+
];
167+
const selectionsWithAND = [
168+
{
169+
indexRanges: [{ start: 0, end: 2 }], // File range
170+
filters: {
171+
"Cell Line": ["AICS-01"],
172+
Structure: ["structure1"],
173+
},
174+
},
175+
];
176+
// Make a separate SQLBuilder for comparison
177+
const sqlBuilderAND = new SQLBuilder().select("*").from("mock_source");
178+
179+
// Act
180+
DatabaseFileService.applySelectionFilters(sqlBuilder, selectionsWithOR, [
181+
"mock_source",
182+
]);
183+
const modifiedSQLWithOR = normalizeSQL(sqlBuilder.toSQL());
184+
DatabaseFileService.applySelectionFilters(sqlBuilderAND, selectionsWithAND, [
185+
"mock_source",
186+
]);
187+
const modifiedSQLWithAND = normalizeSQL(sqlBuilderAND.toSQL());
188+
189+
// Assert
190+
// Uses OR within single filter type
191+
expect(modifiedSQLWithOR).to.include("OR");
192+
expect(modifiedSQLWithOR).not.to.include("AND");
193+
// Uses AND between different filter types
194+
expect(modifiedSQLWithAND).to.include("AND");
195+
expect(modifiedSQLWithAND).not.to.include("OR");
196+
});
157197
});
158198
});

0 commit comments

Comments
 (0)