Skip to content

Commit 39a5632

Browse files
CopilotgrigaspCopilotJonasDov
authored
Property widget: Downgrade missing IModelApp.userPreferences log from error to warning, deduplicate (#1718)
* Initial plan * fix: change useNullValueStorage log from error to warning, logged only once When `IModelApp.userPreferences` is not defined, the `IModelAppUserPreferencesStorage` now logs a warning instead of an error, and only logs it once per storage instance to avoid log spam. Closes #1714 * Add beachball change file for property-grid-react patch * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update packages/itwin/property-grid/src/property-grid-react/api/PreferencesStorage.ts --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Grigas <35135765+grigasp@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: JonasDov <100586436+JonasDov@users.noreply.github.com>
1 parent 9da9db5 commit 39a5632

3 files changed

Lines changed: 47 additions & 11 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Downgrade missing `IModelApp.userPreferences` log from error to warning and deduplicate",
4+
"packageName": "@itwin/property-grid-react",
5+
"email": "AzureDevOps@users.noreply.github.com",
6+
"dependentChangeType": "patch"
7+
}

packages/itwin/property-grid/src/property-grid-react/api/PreferencesStorage.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,14 @@ export interface PreferencesStorage {
2525
*/
2626
export class IModelAppUserPreferencesStorage implements PreferencesStorage {
2727
#nameSpace = PROPERTY_GRID_NAMESPACE;
28+
#warnedAboutMissingPreferences = false;
2829
constructor(nameSpace = PROPERTY_GRID_NAMESPACE) {
2930
this.#nameSpace = nameSpace;
3031
}
3132

3233
public async set(key: string, value: string): Promise<void> {
3334
if (!IModelApp.userPreferences) {
34-
Logger.logError(LOGGER_CATEGORY, `Cannot save user preference ${key} because 'IModelApp.userPreferences' not defined.`);
35+
this.logMissingPreferencesWarning(key, "save");
3536
return;
3637
}
3738

@@ -51,7 +52,7 @@ export class IModelAppUserPreferencesStorage implements PreferencesStorage {
5152

5253
public async get(key: string): Promise<string | undefined> {
5354
if (!IModelApp.userPreferences) {
54-
Logger.logError(LOGGER_CATEGORY, `Cannot get persisted user preference ${key} because 'IModelApp.userPreferences' not defined.`);
55+
this.logMissingPreferencesWarning(key, "get");
5556
return undefined;
5657
}
5758

@@ -68,4 +69,13 @@ export class IModelAppUserPreferencesStorage implements PreferencesStorage {
6869
}
6970
return undefined;
7071
}
72+
73+
private logMissingPreferencesWarning(key: string, action: "save" | "get") {
74+
if (this.#warnedAboutMissingPreferences) {
75+
return;
76+
}
77+
const subject = action === "get" ? "persisted user preference" : "user preference";
78+
Logger.logWarning(LOGGER_CATEGORY, `Cannot ${action} ${subject} ${key} because 'IModelApp.userPreferences' not defined.`);
79+
this.#warnedAboutMissingPreferences = true;
80+
}
7181
}

packages/itwin/property-grid/src/test/api/PreferencesStorage.test.ts

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,22 @@ describe("IModelAppUserPreferencesStorage", () => {
1818
};
1919

2020
let userPreferencesStub: ReturnType<typeof vi.spyOn>;
21-
let loggerStub: ReturnType<typeof vi.spyOn>;
21+
let logErrorStub: ReturnType<typeof vi.spyOn>;
22+
let logWarningStub: ReturnType<typeof vi.spyOn>;
2223
let storage: IModelAppUserPreferencesStorage;
2324

2425
beforeEach(() => {
2526
userPreferencesStub = vi.spyOn(IModelApp, "userPreferences", "get");
26-
loggerStub = vi.spyOn(Logger, "logError");
27+
logErrorStub = vi.spyOn(Logger, "logError");
28+
logWarningStub = vi.spyOn(Logger, "logWarning");
2729
userPreferencesStub.mockReturnValue(imodelUserPreferences);
2830
storage = new IModelAppUserPreferencesStorage();
2931
});
3032

3133
afterEach(() => {
3234
userPreferencesStub.mockReset();
33-
loggerStub.mockReset();
35+
logErrorStub.mockReset();
36+
logWarningStub.mockReset();
3437
imodelUserPreferences.get.mockReset();
3538
imodelUserPreferences.save.mockReset();
3639
});
@@ -46,14 +49,22 @@ describe("IModelAppUserPreferencesStorage", () => {
4649
throw new Error("Invalid Key");
4750
});
4851
await storage.set("test-key", "test-value");
49-
expect(loggerStub).toHaveBeenCalledWith("PropertyGrid", expect.stringContaining("Invalid Key"));
52+
expect(logErrorStub).toHaveBeenCalledWith("PropertyGrid", expect.stringContaining("Invalid Key"));
5053
});
5154

52-
it("logs error if `IModelApp.userPreferences` not defined", async () => {
55+
it("logs warning if `IModelApp.userPreferences` not defined", async () => {
5356
userPreferencesStub.mockReset();
5457
userPreferencesStub.mockReturnValue(undefined);
5558
await storage.set("test-key", "test-value");
56-
expect(loggerStub).toHaveBeenCalledWith("PropertyGrid", expect.stringContaining("'IModelApp.userPreferences' not defined"));
59+
expect(logWarningStub).toHaveBeenCalledWith("PropertyGrid", expect.stringContaining("'IModelApp.userPreferences' not defined"));
60+
});
61+
62+
it("logs warning only once if `IModelApp.userPreferences` not defined", async () => {
63+
userPreferencesStub.mockReset();
64+
userPreferencesStub.mockReturnValue(undefined);
65+
await storage.set("test-key", "test-value");
66+
await storage.set("test-key-2", "test-value-2");
67+
expect(logWarningStub).toHaveBeenCalledTimes(1);
5768
});
5869
});
5970

@@ -68,14 +79,22 @@ describe("IModelAppUserPreferencesStorage", () => {
6879
throw new Error("Invalid Key");
6980
});
7081
await storage.get("test-key");
71-
expect(loggerStub).toHaveBeenCalledWith("PropertyGrid", expect.stringContaining("Invalid Key"));
82+
expect(logErrorStub).toHaveBeenCalledWith("PropertyGrid", expect.stringContaining("Invalid Key"));
7283
});
7384

74-
it("logs error if `IModelApp.userPreferences` not defined", async () => {
85+
it("logs warning if `IModelApp.userPreferences` not defined", async () => {
7586
userPreferencesStub.mockReset();
7687
userPreferencesStub.mockReturnValue(undefined);
7788
expect(await storage.get("test-key")).toBeUndefined();
78-
expect(loggerStub).toHaveBeenCalledWith("PropertyGrid", expect.stringContaining("'IModelApp.userPreferences' not defined"));
89+
expect(logWarningStub).toHaveBeenCalledWith("PropertyGrid", expect.stringContaining("'IModelApp.userPreferences' not defined"));
90+
});
91+
92+
it("logs warning only once if `IModelApp.userPreferences` not defined", async () => {
93+
userPreferencesStub.mockReset();
94+
userPreferencesStub.mockReturnValue(undefined);
95+
await storage.get("test-key");
96+
await storage.get("test-key-2");
97+
expect(logWarningStub).toHaveBeenCalledTimes(1);
7998
});
8099
});
81100
});

0 commit comments

Comments
 (0)