Skip to content

Commit

Permalink
fix: sorting tables ignores case
Browse files Browse the repository at this point in the history
closes: #1122
Co-authored-by: Sebastian Leidig <sebastian.leidig@gmail.com>
  • Loading branch information
TheSlimvReal and sleidig authored Mar 23, 2022
1 parent 32befc0 commit b2279d2
Show file tree
Hide file tree
Showing 12 changed files with 207 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ export class PreviousSchoolsComponent
@Input() child: Child;
records = new Array<ChildSchoolRelation>();
columns: FormFieldConfig[] = [
{ id: "schoolId" },
{ id: "schoolClass" },
{ id: "start", visibleFrom: "md" },
{ id: "end", visibleFrom: "md" },
{ id: "schoolId" },
{ id: "schoolClass" },
{ id: "result" },
isActiveIndicator,
];
Expand Down
4 changes: 2 additions & 2 deletions src/app/core/config/config-fix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -561,8 +561,6 @@ export const defaultJsonConfig = {
"config": {
"single": true,
"columns": [
"schoolId",
"schoolClass",
{
id: "start",
visibleFrom: "sm",
Expand All @@ -571,6 +569,8 @@ export const defaultJsonConfig = {
id: "end",
visibleFrom: "sm",
},
"schoolId",
"schoolClass",
"result",
],
}
Expand Down
11 changes: 4 additions & 7 deletions src/app/core/entity-components/entity-list/filter-predicate.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
import { Entity } from "../../entity/model/entity";
import { entityListSortingAccessor } from "../entity-subrecord/entity-subrecord/sorting-accessor";
import { getReadableValue } from "../entity-subrecord/entity-subrecord/value-accessor";

export function entityFilterPredicate<T extends Entity>(
export function entityFilterPredicate<T extends Entity, P extends keyof T>(
data: T,
filter: string
): boolean {
const keys = Object.keys(data);
const keys = Object.keys(data) as P[];
return keys.some((property) =>
entityListSortingAccessor(data, property)
?.toString()
.toLowerCase()
.includes(filter)
getReadableValue(data, property)?.toString().toLowerCase().includes(filter)
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ describe("EntitySubrecordComponent", () => {
});

const sortedData = component.recordsDataSource
.sortData(component.recordsDataSource.data, component.sort)
._orderData(component.recordsDataSource.data)
.map((row) => row.record);
expect(sortedData).toEqual([first, second, third]);
});
Expand Down Expand Up @@ -130,7 +130,7 @@ describe("EntitySubrecordComponent", () => {

component.sort.sort({ id: "name", start: "asc", disableClear: false });
const sortedIds = component.recordsDataSource
.sortData(component.recordsDataSource.data, component.sort)
._orderData(component.recordsDataSource.data)
.map((c) => c.record.getId());

expect(sortedIds).toEqual(["0", "3", "1", "2"]);
Expand All @@ -143,16 +143,29 @@ describe("EntitySubrecordComponent", () => {
notes[2].category = { id: "2", label: "Z" };
notes[1].category = { id: "3", label: "C" };
component.records = notes;
component.ngOnChanges({ records: null });
component.ngOnChanges({ records: undefined });

component.sort.sort({ id: "category", start: "asc", disableClear: false });
const sortedIds = component.recordsDataSource
.sortData(component.recordsDataSource.data, component.sort)
._orderData(component.recordsDataSource.data)
.map((note) => note.record.getId());

expect(sortedIds).toEqual(["0", "3", "1", "2"]);
});

it("should sort strings ignoring case", () => {
const names = ["C", "b", "A"];
component.records = names.map((name) => Child.create(name));
component.ngOnChanges({ records: undefined });
component.sort.sort({ id: "name", start: "asc", disableClear: false });

const sortedNames = component.recordsDataSource
._orderData(component.recordsDataSource.data)
.map((row: TableRow<Child>) => row.record.name);

expect(sortedNames).toEqual(["A", "b", "C"]);
});

it("should log a warning when the column definition can not be initialized", () => {
const loggingService = TestBed.inject(LoggingService);
spyOn(loggingService, "warn");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy";
import { Entity } from "../../../entity/model/entity";
import { AlertService } from "../../../alerts/alert.service";
import { Subscription } from "rxjs";
import { entityListSortingAccessor } from "./sorting-accessor";
import { FormGroup } from "@angular/forms";
import { FormFieldConfig } from "../../entity-form/entity-form/FormConfig";
import { EntityFormService } from "../../entity-form/entity-form.service";
Expand All @@ -28,6 +27,7 @@ import {
EntityRemoveService,
RemoveResult,
} from "../../../entity/entity-remove.service";
import { tableSort } from "./table-sort";

export interface TableRow<T> {
record: T;
Expand Down Expand Up @@ -175,10 +175,11 @@ export class EntitySubrecordComponent<T extends Entity>

private initDefaultSort() {
this.recordsDataSource.sort = this.sort;
this.recordsDataSource.sortingDataAccessor = (
row: TableRow<T>,
id: string
) => entityListSortingAccessor(row.record, id);
this.recordsDataSource.sortData = (data, sort) =>
tableSort(data, {
active: sort.active as keyof T | "",
direction: sort.direction,
});
if (!this.sort || this.sort.active) {
// do not overwrite existing sort
return;
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { tableSort } from "./table-sort";
import moment from "moment";
import { ConfigurableEnumConfig } from "../../../configurable-enum/configurable-enum.interface";

describe("TableSort", () => {
it("should sort strings with partial numbers correctly", () => {
testSort(["PN1", "PN2", "PN12"]);
});

it("should sort dates correctly", () => {
testSort([
moment().subtract(1, "week").toDate(),
moment().subtract(3, "days").toDate(),
new Date(),
]);
});

it("should sort numbers correctly", () => {
testSort([1, 1.4, 3, Infinity]);
});

it("should sort a array with null and undefined values correctly", () => {
testSort(["1", 2, "three", undefined]);
});

it("should sort configurable enums based on their label", () => {
const values: ConfigurableEnumConfig = [
{ id: "first", label: "aAa" },
{ id: "second", label: "Bbb" },
{ id: "third", label: "CDE" },
];
testSort(values);
});

it("should allow to filter descending", () => {
testSort([null, 3, 2.5, 2, "1"], "desc");
});

it("should return input array if not sort direction is defined", () => {
const values = ["3", 1, 2, undefined, "ten"].map((val) => ({
record: { key: val },
}));
const result = tableSort([...values], { direction: "", active: "key" });
expect(result).toEqual(values);
});

it("should return input array if no active property is defined", () => {
const values = [2, 1, 3].map((val) => ({ record: { key: val } }));
const result = tableSort([...values], { direction: "asc", active: "" });
expect(result).toEqual(values);
});

function testSort(
sortedArray: any[],
direction: "asc" | "desc" | "" = "asc"
) {
const objectArray = sortedArray.map((val) => ({ record: { key: val } }));
for (let i = 0; i < sortedArray.length; i++) {
const shuffled = shuffle(objectArray);
const result = tableSort(shuffled, { direction, active: "key" });
const resultValues = result.map((row) => row.record.key);
expect(resultValues).toEqual(sortedArray);
}
}

function shuffle<OBJECT>(array: OBJECT[]): OBJECT[] {
const result = [...array];
for (let currentIndex = 0; currentIndex < array.length; currentIndex++) {
// Pick a remaining element...
const randomIndex = Math.floor(Math.random() * currentIndex);

// And swap it with the current element.
[result[currentIndex], result[randomIndex]] = [
result[randomIndex],
result[currentIndex],
];
}

return result;
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { getReadableValue } from "./value-accessor";
import { TableRow } from "./entity-subrecord.component";

/**
* Custom sort implementation for a MatTableDataSource<TableRow<T>>
* @param data The data of the data source
* @param direction direction "asc", "desc" or "" meaning none
* @param active the property of T for which it should be sorted
* @returns the sorted table rows
*/
export function tableSort<OBJECT, PROPERTY extends keyof OBJECT>(
data: TableRow<OBJECT>[],
{
direction,
active,
}: { direction: "asc" | "desc" | ""; active: PROPERTY | "" }
): TableRow<OBJECT>[] {
if (direction === "" || !active) {
return data;
}
data.sort((objA, objB) => {
const valueA = getComparableValue(objA.record, active);
const valueB = getComparableValue(objB.record, active);
return compareValues(valueA, valueB);
});
if (direction === "desc") {
data.reverse();
}
return data;
}

function getComparableValue<OBJECT, PROPERTY extends keyof OBJECT>(
obj: OBJECT,
key: PROPERTY
): OBJECT[PROPERTY] | string {
const value = getReadableValue(obj, key) as OBJECT[PROPERTY];
if (value instanceof Date) {
return value.getTime() + "";
} else if (typeof value === "number") {
return value + "";
} else {
return value;
}
}

function compareValues(a, b) {
if (a === b) {
return 0;
} else if (typeof a === "string" && typeof b === "string") {
return a.localeCompare(b, undefined, { numeric: true });
} else if (a > b || b === null || b === undefined) {
return -1;
} else if (a < b || a === null || a === undefined) {
return 1;
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import { entityListSortingAccessor } from "./sorting-accessor";
import { getReadableValue } from "./value-accessor";

describe("entityListSortingAccessor", () => {
function expectObjectToContain(obj: object, expected: any[], type: string) {
describe("getReadableValue", () => {
function expectObjectToContain<OBJECT, PROPERTY extends keyof OBJECT>(
obj: OBJECT,
expected: (OBJECT[PROPERTY] | "string")[],
type: string
) {
let index = 0;
for (const key of Object.keys(obj)) {
const accessed = entityListSortingAccessor(obj, key);
for (const key of Object.keys(obj) as PROPERTY[]) {
const accessed = getReadableValue(obj, key);
expect(accessed).toEqual(expected[index]);
expect(typeof accessed).toBe(type);
index += 1;
Expand All @@ -28,16 +32,6 @@ describe("entityListSortingAccessor", () => {
expectObjectToContain(obj, [1, 2.0, 10e3], "number");
});

it("should return numbers when a string is parsable", () => {
const obj = {
a: "1",
b: "2.0",
c: "10e3",
d: "0x1",
};
expectObjectToContain(obj, [1, 2.0, 10e3, 0x1], "number");
});

it("should return the label when the queried object has a 'label' key", () => {
const object = {
data: {
Expand All @@ -46,7 +40,7 @@ describe("entityListSortingAccessor", () => {
value2: "hello",
},
};
const accessed = entityListSortingAccessor(object, "data");
const accessed = getReadableValue(object, "data");
expect(typeof accessed).toBe("string");
expect(accessed).toBe("data label");
});
Expand All @@ -58,7 +52,7 @@ describe("entityListSortingAccessor", () => {
value2: "hello",
},
};
const accessed = entityListSortingAccessor(object, "data");
const accessed = getReadableValue(object, "data");
expect(typeof accessed).toBe("object");
expect(accessed).toEqual({
value1: 123,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* An enhanced sortingDataAccessor function that can be set for a MatTableDataSource
* in order to support sorting by ConfigurableEnum columns and other Entity specific values.
*
* @param data The object (table row); passed in by the data source
* @param key The active sorting header key; passed in by the data source
*/
export function getReadableValue<OBJECT, PROPERTY extends keyof OBJECT>(
data: OBJECT,
key: PROPERTY
): OBJECT[PROPERTY] | string {
if (isConfigurableEnum(data, key)) {
return (data[key] as any).label;
} else {
return data[key];
}
}

function isConfigurableEnum<OBJECT, PROPERTY extends keyof OBJECT>(
data: OBJECT,
key: PROPERTY
): boolean {
return typeof data[key] === "object" && data[key] && "label" in data[key];
}
4 changes: 2 additions & 2 deletions src/app/core/export/export-service/export.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -382,8 +382,8 @@ describe("ExportService", () => {
const resultRows = result.split(ExportService.SEPARATOR_ROW);
expect(resultRows).toEqual([
'"Name","Participation"',
'"some child","0.5"',
'"some child","1"',
'"some child","0.50"',
'"some child","1.00"',
]);
});

Expand Down
Loading

0 comments on commit b2279d2

Please sign in to comment.