This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-8817-28dbb8885713f3f92d1c1e2114d57f2d1a8e4a75 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 1fc75b1a2a961148e3b3479860627737d3fc1855 Author: Xinyuan Lin <[email protected]> AuthorDate: Fri Oct 2 20:46:53 2026 +0000 refactor(frontend): remove the unused dictionaryChangedSubject (#8817) ### What changes were proposed in this PR? Removes `dictionaryChangedSubject` from `UserConfigService`: the private field, its three `.next()` calls in `updateEntry` / `updateDict`, and the now-unused rxjs `Subject` import. **+8/−49 lines.** There is no behaviour change. The Subject was private with no getter, so nothing could subscribe to it. `updateEntry` and `updateDict` keep their guards, deletes and assignments; only the emissions go. In the spec, two cases that only counted emissions are deleted. Two others are kept, with their emission assertions replaced by behaviour checks, because they cover the surviving guards: a repeated `set()` still issues a PUT and keeps the entry, and deleting an absent key leaves the dictionary untouched. ### History | | | | --- | --- | | **Introduced by** | #1162 (2021-06-27), "Add UserDictionaryService (User Presets Step 2)". The field was private from the start, with no getter or `asObservable()` | | **Usage removed by** | **never used**. The only readers were spec cases reaching the private field through `(service as any)` | It has been unused for over five years. ### Any related issues, documentation, discussions? Closes #8816 ### How was this PR tested? No new tests. The two kept cases now pin the guards' behaviour instead of emission counts. From `frontend/`: - `npx ng test --watch=false --include='**/user-config.service.spec.ts' --include='**/preset.service.spec.ts'`: 2 files, 68 tests, all pass. `PresetService` is the only production consumer of `UserConfigService`. - `yarn format:ci`: clean. - `npx ng build`: success. To re-check: ``` git grep -n -i "dictionaryChanged" -- frontend # no hits ``` ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5.5) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- .../user/config/user-config.service.spec.ts | 51 +++------------------- .../service/user/config/user-config.service.ts | 6 +-- 2 files changed, 8 insertions(+), 49 deletions(-) diff --git a/frontend/src/app/common/service/user/config/user-config.service.spec.ts b/frontend/src/app/common/service/user/config/user-config.service.spec.ts index 085c9812ba..f43384b5ec 100644 --- a/frontend/src/app/common/service/user/config/user-config.service.spec.ts +++ b/frontend/src/app/common/service/user/config/user-config.service.spec.ts @@ -74,17 +74,6 @@ describe("UserConfigService", () => { expect(service.getDict()).toEqual(payload); }); - it("notifies dictionaryChanged subscribers when the dictionary is replaced", () => { - const next = vi.fn(); - const sub = (service as any).dictionaryChangedSubject.subscribe(next); - - service.fetchAll(); - httpMock.expectOne(endpoint).flush({ k: "v" }); - - expect(next).toHaveBeenCalledTimes(1); - sub.unsubscribe(); - }); - it("throws when the user is not logged in", () => { stubUserService.user = undefined; expect(() => service.fetchAll()).toThrowError("user not logged in"); @@ -105,21 +94,6 @@ describe("UserConfigService", () => { expect(service.getDict()).toEqual({ alpha: "one" }); }); - it("notifies dictionaryChanged subscribers only when the value actually changes", () => { - const next = vi.fn(); - const sub = (service as any).dictionaryChangedSubject.subscribe(next); - - service.fetchKey("alpha"); - httpMock.expectOne(`${endpoint}/alpha`).flush("one"); - expect(next).toHaveBeenCalledTimes(1); - - service.fetchKey("alpha"); - httpMock.expectOne(`${endpoint}/alpha`).flush("one"); - expect(next).toHaveBeenCalledTimes(1); - - sub.unsubscribe(); - }); - it("throws when the user is not logged in", () => { stubUserService.user = undefined; expect(() => service.fetchKey("alpha")).toThrowError("user not logged in"); @@ -142,18 +116,16 @@ describe("UserConfigService", () => { expect(service.getDict()).toEqual({ alpha: "one" }); }); - it("does not refire dictionaryChanged when setting the same value twice", () => { + it("issues a PUT and keeps the entry when setting the same value twice", () => { service.set("alpha", "one"); httpMock.expectOne(`${endpoint}/alpha`).flush(null); - const next = vi.fn(); - const sub = (service as any).dictionaryChangedSubject.subscribe(next); - service.set("alpha", "one"); - httpMock.expectOne(`${endpoint}/alpha`).flush(null); + const req = httpMock.expectOne(`${endpoint}/alpha`); + expect(req.request.method).toEqual("PUT"); + req.flush(null); - expect(next).not.toHaveBeenCalled(); - sub.unsubscribe(); + expect(service.getDict()).toEqual({ alpha: "one" }); }); it("throws when the user is not logged in", () => { @@ -210,18 +182,9 @@ describe("UserConfigService", () => { expect(service.getDict()).toEqual({}); }); - it("does not fire dictionaryChanged when told to delete a key that is absent", () => { - const next = vi.fn(); - const sub = (service as any).dictionaryChangedSubject.subscribe(next); - - (service as any).updateEntry("absent", undefined); - - // Asserting on the subject, not just on getDict(): a version that deleted - // and notified unconditionally would leave the dictionary looking identical - // while still waking every subscriber. - expect(next).not.toHaveBeenCalled(); + it("leaves the dictionary untouched when told to delete a key that is absent", () => { + expect(() => (service as any).updateEntry("absent", undefined)).not.toThrow(); expect(service.getDict()).toEqual({}); - sub.unsubscribe(); }); }); diff --git a/frontend/src/app/common/service/user/config/user-config.service.ts b/frontend/src/app/common/service/user/config/user-config.service.ts index abbc73b223..23a025c739 100644 --- a/frontend/src/app/common/service/user/config/user-config.service.ts +++ b/frontend/src/app/common/service/user/config/user-config.service.ts @@ -19,7 +19,7 @@ import { HttpClient } from "@angular/common/http"; import { Injectable } from "@angular/core"; -import { Observable, of, Subject } from "rxjs"; +import { Observable, of } from "rxjs"; import { AppSettings } from "src/app/common/app-setting"; import { UserService } from "../user.service"; import { shareReplay, tap } from "rxjs/operators"; @@ -34,7 +34,6 @@ export type UserConfig = { export class UserConfigService { public static readonly USER_DICTIONARY_ENDPOINT = "user/config"; - private dictionaryChangedSubject = new Subject<void>(); private localUserDictionary: UserConfig = {}; constructor( @@ -154,18 +153,15 @@ export class UserConfigService { if (value === undefined) { if (key in this.localUserDictionary) { delete this.localUserDictionary[key]; - this.dictionaryChangedSubject.next(); } } else { if (this.localUserDictionary[key] !== value) { this.localUserDictionary[key] = value; - this.dictionaryChangedSubject.next(); } } } private updateDict(newDict: UserConfig) { this.localUserDictionary = newDict; - this.dictionaryChangedSubject.next(); } }
