Skip to content

Commit ff685f0

Browse files
robertbrignullrobertbrignull
committed
Move all sorting to the modeling store, and then all views preserve the sorting
1 parent 948c1e2 commit ff685f0

File tree

9 files changed

+50
-87
lines changed

9 files changed

+50
-87
lines changed

‎extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-data-provider.ts‎

Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { INITIAL_HIDE_MODELED_METHODS_VALUE } from "../shared/hide-modeled-metho
1616
import { getModelingStatus } from "../shared/modeling-status";
1717
import { assertNever } from "../../common/helpers-pure";
1818
import type { ModeledMethod } from "../modeled-method";
19-
import { groupMethods, sortGroupNames, sortMethods } from "../shared/sorting";
19+
import { groupMethods, sortGroupNames } from "../shared/sorting";
2020
import type { Mode } from "../shared/mode";
2121
import { INITIAL_MODE } from "../shared/mode";
2222
import type { UrlValueResolvable } from "../../common/raw-result-types";
@@ -63,7 +63,6 @@ export class MethodsUsageDataProvider
6363
mode: Mode,
6464
modeledMethods: Readonly<Record<string, readonly ModeledMethod[]>>,
6565
modifiedMethodSignatures: ReadonlySet<string>,
66-
processedByAutoModelMethods: ReadonlySet<string>,
6766
): Promise<void> {
6867
if (
6968
this.methods !== methods ||
@@ -75,13 +74,7 @@ export class MethodsUsageDataProvider
7574
) {
7675
this.methods = methods;
7776
this.sortedTreeItems = createTreeItems(
78-
sortMethodsInGroups(
79-
methods,
80-
modeledMethods,
81-
mode,
82-
modifiedMethodSignatures,
83-
processedByAutoModelMethods,
84-
),
77+
sortMethodsInGroups(methods, mode),
8578
);
8679
this.databaseItem = databaseItem;
8780
this.sourceLocationPrefix =
@@ -253,27 +246,9 @@ function urlValueResolvablesAreEqual(
253246
return false;
254247
}
255248

256-
function sortMethodsInGroups(
257-
methods: readonly Method[],
258-
modeledMethods: Readonly<Record<string, readonly ModeledMethod[]>>,
259-
mode: Mode,
260-
modifiedMethodSignatures: ReadonlySet<string>,
261-
processedByAutoModelMethods: ReadonlySet<string>,
262-
): Method[] {
249+
function sortMethodsInGroups(methods: readonly Method[], mode: Mode): Method[] {
263250
const grouped = groupMethods(methods, mode);
264-
265-
const sortedGroupNames = sortGroupNames(grouped);
266-
267-
return sortedGroupNames.flatMap((groupName) => {
268-
const group = grouped[groupName];
269-
270-
return sortMethods(
271-
group,
272-
modeledMethods,
273-
modifiedMethodSignatures,
274-
processedByAutoModelMethods,
275-
);
276-
});
251+
return sortGroupNames(grouped).flatMap((groupName) => grouped[groupName]);
277252
}
278253

279254
function createTreeItems(methods: readonly Method[]): MethodTreeViewItem[] {

‎extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-panel.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ export class MethodsUsagePanel extends DisposableObject {
3939
mode: Mode,
4040
modeledMethods: Readonly<Record<string, readonly ModeledMethod[]>>,
4141
modifiedMethodSignatures: ReadonlySet<string>,
42-
processedByAutoModelMethods: ReadonlySet<string>,
4342
): Promise<void> {
4443
await this.dataProvider.setState(
4544
methods,
@@ -48,7 +47,6 @@ export class MethodsUsagePanel extends DisposableObject {
4847
mode,
4948
modeledMethods,
5049
modifiedMethodSignatures,
51-
processedByAutoModelMethods,
5250
);
5351
const numOfApis = hideModeledMethods
5452
? methods.filter((api) => !api.supported).length
@@ -122,7 +120,6 @@ export class MethodsUsagePanel extends DisposableObject {
122120
activeState.mode,
123121
activeState.modeledMethods,
124122
activeState.modifiedMethodSignatures,
125-
activeState.processedByAutoModelMethods,
126123
);
127124
}
128125
}

‎extensions/ql-vscode/src/model-editor/model-editor-view.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,8 @@ export class ModelEditorView extends AbstractWebview<
273273
Object.keys(modeledMethods),
274274
);
275275

276+
this.modelingStore.updateMethodSorting(this.databaseItem);
277+
276278
void telemetryListener?.sendUIInteraction(
277279
"model-editor-save-modeled-methods",
278280
);

‎extensions/ql-vscode/src/model-editor/modeling-store.ts‎

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type { ModeledMethod } from "./modeled-method";
66
import type { ModelingEvents } from "./modeling-events";
77
import { INITIAL_HIDE_MODELED_METHODS_VALUE } from "./shared/hide-modeled-methods";
88
import type { Mode } from "./shared/mode";
9+
import { sortMethods } from "./shared/sorting";
910

1011
interface InternalDbModelingState {
1112
databaseItem: DatabaseItem;
@@ -155,17 +156,25 @@ export class ModelingStore extends DisposableObject {
155156
}
156157

157158
public setMethods(dbItem: DatabaseItem, methods: Method[]) {
158-
const dbState = this.getState(dbItem);
159-
const dbUri = dbItem.databaseUri.toString();
160-
161-
dbState.methods = [...methods];
159+
this.changeMethods(dbItem, (state) => {
160+
state.methods = sortMethods(
161+
methods,
162+
state.modeledMethods,
163+
state.modifiedMethodSignatures,
164+
state.processedByAutoModelMethods,
165+
);
166+
});
167+
}
162168

163-
this.modelingEvents.fireMethodsChangedEvent(
164-
methods,
165-
dbUri,
166-
dbItem,
167-
dbUri === this.activeDb,
168-
);
169+
public updateMethodSorting(dbItem: DatabaseItem) {
170+
this.changeMethods(dbItem, (state) => {
171+
state.methods = sortMethods(
172+
state.methods,
173+
state.modeledMethods,
174+
state.modifiedMethodSignatures,
175+
state.processedByAutoModelMethods,
176+
);
177+
});
169178
}
170179

171180
public setHideModeledMethods(
@@ -374,6 +383,7 @@ export class ModelingStore extends DisposableObject {
374383
...processedByAutoModelMethods,
375384
]);
376385
});
386+
this.updateMethodSorting(dbItem);
377387
}
378388

379389
public updateModelEvaluationRun(
@@ -421,6 +431,22 @@ export class ModelingStore extends DisposableObject {
421431
return this.state.get(databaseItem.databaseUri.toString())!;
422432
}
423433

434+
private changeMethods(
435+
dbItem: DatabaseItem,
436+
updateState: (state: InternalDbModelingState) => void,
437+
) {
438+
const state = this.getState(dbItem);
439+
440+
updateState(state);
441+
442+
this.modelingEvents.fireMethodsChangedEvent(
443+
state.methods,
444+
dbItem.databaseUri.toString(),
445+
dbItem,
446+
dbItem.databaseUri.toString() === this.activeDb,
447+
);
448+
}
449+
424450
private changeModifiedMethods(
425451
dbItem: DatabaseItem,
426452
updateState: (state: InternalDbModelingState) => void,

‎extensions/ql-vscode/src/model-editor/shared/auto-model-candidates.ts‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { Method, MethodSignature } from "../method";
22
import type { ModeledMethod } from "../modeled-method";
33
import type { Mode } from "./mode";
4-
import { groupMethods, sortGroupNames, sortMethods } from "./sorting";
4+
import { groupMethods, sortGroupNames } from "./sorting";
55

66
/**
77
* Return the candidates that the model should be run on. This includes limiting the number of
@@ -44,10 +44,5 @@ export function getCandidates(
4444

4545
// Sort the same way as the UI so we send the first ones listed in the UI first
4646
const grouped = groupMethods(candidateMethods, mode);
47-
const sortedGroupNames = sortGroupNames(grouped);
48-
return sortedGroupNames.flatMap((name) =>
49-
// We can safely pass empty sets for `modifiedSignatures` and `processedByAutoModelMethods`
50-
// because we've filtered out all methods that are already modeled or have already been processed by auto-model.
51-
sortMethods(grouped[name], modeledMethodsBySignature, new Set(), new Set()),
52-
);
47+
return sortGroupNames(grouped).flatMap((name) => grouped[name]);
5348
}

‎extensions/ql-vscode/src/model-editor/shared/sorting.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ import type { ModeledMethod } from "../modeled-method";
44
import { Mode } from "./mode";
55
import { calculateModeledPercentage } from "./modeled-percentage";
66

7+
/**
8+
* Groups methods by library or package name.
9+
* Does not change the order of methods within a group.
10+
*/
711
export function groupMethods(
812
methods: readonly Method[],
913
mode: Mode,

‎extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import type { Method } from "../../model-editor/method";
33
import { canMethodBeModeled } from "../../model-editor/method";
44
import type { ModeledMethod } from "../../model-editor/modeled-method";
55
import { useMemo } from "react";
6-
import { sortMethods } from "../../model-editor/shared/sorting";
76
import { HiddenMethodsRow } from "./HiddenMethodsRow";
87
import type { ModelEditorViewState } from "../../model-editor/shared/view-state";
98
import { ScreenReaderOnly } from "../common/ScreenReaderOnly";
@@ -48,12 +47,7 @@ export const ModeledMethodDataGrid = ({
4847
] = useMemo(() => {
4948
const methodsWithModelability = [];
5049
let numHiddenMethods = 0;
51-
for (const method of sortMethods(
52-
methods,
53-
modeledMethodsMap,
54-
modifiedSignatures,
55-
processedByAutoModelMethods,
56-
)) {
50+
for (const method of methods) {
5751
const modeledMethods = modeledMethodsMap[method.signature] ?? [];
5852
const methodIsUnsaved = modifiedSignatures.has(method.signature);
5953
const methodCanBeModeled = canMethodBeModeled(
@@ -69,13 +63,7 @@ export const ModeledMethodDataGrid = ({
6963
}
7064
}
7165
return [methodsWithModelability, numHiddenMethods];
72-
}, [
73-
hideModeledMethods,
74-
methods,
75-
modeledMethodsMap,
76-
modifiedSignatures,
77-
processedByAutoModelMethods,
78-
]);
66+
}, [hideModeledMethods, methods, modeledMethodsMap, modifiedSignatures]);
7967

8068
const someMethodsAreVisible = methodsWithModelability.length > 0;
8169

‎extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/methods-usage/methods-usage-data-provider.test.ts‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ describe("MethodsUsageDataProvider", () => {
2727
const methods: Method[] = [];
2828
const modeledMethods: Record<string, ModeledMethod[]> = {};
2929
const modifiedMethodSignatures: Set<string> = new Set();
30-
const processedByAutoModelMethods: Set<string> = new Set();
3130
const dbItem = mockedObject<DatabaseItem>({
3231
getSourceLocationPrefix: () => "test",
3332
});
@@ -40,7 +39,6 @@ describe("MethodsUsageDataProvider", () => {
4039
mode,
4140
modeledMethods,
4241
modifiedMethodSignatures,
43-
processedByAutoModelMethods,
4442
);
4543

4644
const onDidChangeTreeDataListener = jest.fn();
@@ -53,7 +51,6 @@ describe("MethodsUsageDataProvider", () => {
5351
mode,
5452
modeledMethods,
5553
modifiedMethodSignatures,
56-
processedByAutoModelMethods,
5754
);
5855

5956
expect(onDidChangeTreeDataListener).not.toHaveBeenCalled();
@@ -69,7 +66,6 @@ describe("MethodsUsageDataProvider", () => {
6966
mode,
7067
modeledMethods,
7168
modifiedMethodSignatures,
72-
processedByAutoModelMethods,
7369
);
7470

7571
const onDidChangeTreeDataListener = jest.fn();
@@ -82,7 +78,6 @@ describe("MethodsUsageDataProvider", () => {
8278
mode,
8379
modeledMethods,
8480
modifiedMethodSignatures,
85-
processedByAutoModelMethods,
8681
);
8782

8883
expect(onDidChangeTreeDataListener).toHaveBeenCalledTimes(1);
@@ -100,7 +95,6 @@ describe("MethodsUsageDataProvider", () => {
10095
mode,
10196
modeledMethods,
10297
modifiedMethodSignatures,
103-
processedByAutoModelMethods,
10498
);
10599

106100
const onDidChangeTreeDataListener = jest.fn();
@@ -113,7 +107,6 @@ describe("MethodsUsageDataProvider", () => {
113107
mode,
114108
modeledMethods,
115109
modifiedMethodSignatures,
116-
processedByAutoModelMethods,
117110
);
118111

119112
expect(onDidChangeTreeDataListener).toHaveBeenCalledTimes(1);
@@ -127,7 +120,6 @@ describe("MethodsUsageDataProvider", () => {
127120
mode,
128121
modeledMethods,
129122
modifiedMethodSignatures,
130-
processedByAutoModelMethods,
131123
);
132124

133125
const onDidChangeTreeDataListener = jest.fn();
@@ -140,7 +132,6 @@ describe("MethodsUsageDataProvider", () => {
140132
mode,
141133
modeledMethods,
142134
modifiedMethodSignatures,
143-
processedByAutoModelMethods,
144135
);
145136

146137
expect(onDidChangeTreeDataListener).toHaveBeenCalledTimes(1);
@@ -156,7 +147,6 @@ describe("MethodsUsageDataProvider", () => {
156147
mode,
157148
modeledMethods,
158149
modifiedMethodSignatures,
159-
processedByAutoModelMethods,
160150
);
161151

162152
const onDidChangeTreeDataListener = jest.fn();
@@ -169,7 +159,6 @@ describe("MethodsUsageDataProvider", () => {
169159
mode,
170160
modeledMethods2,
171161
modifiedMethodSignatures,
172-
processedByAutoModelMethods,
173162
);
174163

175164
expect(onDidChangeTreeDataListener).toHaveBeenCalledTimes(1);
@@ -185,7 +174,6 @@ describe("MethodsUsageDataProvider", () => {
185174
mode,
186175
modeledMethods,
187176
modifiedMethodSignatures,
188-
processedByAutoModelMethods,
189177
);
190178

191179
const onDidChangeTreeDataListener = jest.fn();
@@ -198,7 +186,6 @@ describe("MethodsUsageDataProvider", () => {
198186
mode,
199187
modeledMethods,
200188
modifiedMethodSignatures2,
201-
processedByAutoModelMethods,
202189
);
203190

204191
expect(onDidChangeTreeDataListener).toHaveBeenCalledTimes(1);
@@ -217,7 +204,6 @@ describe("MethodsUsageDataProvider", () => {
217204
mode,
218205
modeledMethods,
219206
modifiedMethodSignatures,
220-
processedByAutoModelMethods,
221207
);
222208

223209
const onDidChangeTreeDataListener = jest.fn();
@@ -230,7 +216,6 @@ describe("MethodsUsageDataProvider", () => {
230216
mode,
231217
modeledMethods,
232218
modifiedMethodSignatures,
233-
processedByAutoModelMethods,
234219
);
235220

236221
expect(onDidChangeTreeDataListener).toHaveBeenCalledTimes(1);
@@ -270,7 +255,6 @@ describe("MethodsUsageDataProvider", () => {
270255
createSinkModeledMethod(),
271256
];
272257
const modifiedMethodSignatures: Set<string> = new Set();
273-
const processedByAutoModelMethods: Set<string> = new Set();
274258

275259
const dbItem = mockedObject<DatabaseItem>({
276260
getSourceLocationPrefix: () => "test",
@@ -307,7 +291,6 @@ describe("MethodsUsageDataProvider", () => {
307291
mode,
308292
modeledMethods,
309293
modifiedMethodSignatures,
310-
processedByAutoModelMethods,
311294
);
312295
expect(dataProvider.getChildren().length).toEqual(4);
313296
});
@@ -321,7 +304,6 @@ describe("MethodsUsageDataProvider", () => {
321304
mode,
322305
modeledMethods,
323306
modifiedMethodSignatures,
324-
processedByAutoModelMethods,
325307
);
326308
expect(dataProvider.getChildren().length).toEqual(3);
327309
});
@@ -418,7 +400,6 @@ describe("MethodsUsageDataProvider", () => {
418400
mode,
419401
modeledMethods,
420402
modifiedMethodSignatures,
421-
processedByAutoModelMethods,
422403
);
423404
expect(
424405
dataProvider

0 commit comments

Comments
 (0)