Move userPrefs to model inside diff-view
Moving properties to the model will reflect any changes made in other
views(such as change view) to the preference object to the diff view
which is still retained in the background.
Change-Id: I1fa3eef29d62733ceb62f870f924170c411683f0
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
index a7702d1..b47c51c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector.ts
@@ -46,7 +46,7 @@
@property({type: Boolean})
showTooltipBelow = false;
- private readonly restApiService = appContext.restApiService;
+ private readonly userService = appContext.userService;
override connectedCallback() {
super.connectedCallback();
@@ -60,7 +60,7 @@
*/
setMode(newMode: DiffViewMode) {
if (this.saveOnChange && this.mode && this.mode !== newMode) {
- this.restApiService.savePreferences({diff_view: newMode});
+ this.userService.updatePreferences({diff_view: newMode});
}
this.mode = newMode;
let announcement;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts
index aa97394..8b06c75 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-mode-selector/gr-diff-mode-selector_test.ts
@@ -19,7 +19,7 @@
import './gr-diff-mode-selector';
import {GrDiffModeSelector} from './gr-diff-mode-selector';
import {DiffViewMode} from '../../../constants/constants';
-import {stubRestApi} from '../../../test/test-utils';
+import {stubUsers} from '../../../test/test-utils';
const basicFixture = fixtureFromElement('gr-diff-mode-selector');
@@ -47,7 +47,7 @@
});
test('setMode', () => {
- const saveStub = stubRestApi('savePreferences');
+ const saveStub = stubUsers('updatePreferences');
// Setting the mode initially does not save prefs.
element.saveOnChange = true;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 5b48345..c7a462e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -113,6 +113,7 @@
import {changeComments$} from '../../../services/comments/comments-model';
import {takeUntil} from 'rxjs/operators';
import {Subject} from 'rxjs';
+import {preferences$} from '../../../services/user/user-model';
const ERR_REVIEW_STATUS = 'Couldn’t change file review status.';
const LOADING_BLAME = 'Loading blame...';
@@ -361,11 +362,20 @@
this._getLoggedIn().then(loggedIn => {
this._loggedIn = loggedIn;
});
+ // TODO(brohlfs): This just ensures that the userService is instantiated at
+ // all. We need the service to manage the model, but we are not making any
+ // direct calls. Will need to find a better solution to this problem ...
+ assertIsDefined(appContext.userService);
+
changeComments$
.pipe(takeUntil(this.disconnected$))
.subscribe(changeComments => {
this._changeComments = changeComments;
});
+
+ preferences$.pipe(takeUntil(this.disconnected$)).subscribe(preferences => {
+ this._userPrefs = preferences;
+ });
this.addEventListener('open-fix-preview', e => this._onOpenFixPreview(e));
this.cursor.replaceDiffs([this.$.diffHost]);
this._onRenderHandler = (_: Event) => {
@@ -1148,12 +1158,6 @@
promises.push(this._getDiffPreferences());
- promises.push(
- this._getPreferences().then(prefs => {
- this._userPrefs = prefs;
- })
- );
-
if (!this._change) promises.push(this._getChangeDetail(this._changeNum));
if (!this._changeComments) this._loadComments(value.patchNum);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 2c4750a..0c7abc8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -1524,6 +1524,7 @@
assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
// User prefs but no change view state set.
+ element.changeViewState.diffMode = undefined;
element._userPrefs = {default_diff_view: 'UNIFIED_DIFF'};
assert.equal(element._getDiffViewMode(), 'UNIFIED_DIFF');
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
index 3533fd6..256e956 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
@@ -246,6 +246,7 @@
this.$.diffPrefs.loadData(),
];
+ // TODO(dhruvsri): move this to the service
promises.push(
this.restApiService.getPreferences().then(prefs => {
if (!prefs) {
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
index 8194d5b..1165f1e 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
@@ -273,7 +273,7 @@
stubRestApi('savePreferences').callsFake(prefs => {
assertMenusEqual(prefs.my, preferences.my);
assert.equal(prefs.publish_comments_on_push, true);
- return Promise.resolve(new Response());
+ return Promise.resolve(createDefaultPreferences());
});
// Save the change.
@@ -294,7 +294,7 @@
stubRestApi('savePreferences').callsFake(prefs => {
assert.equal(prefs.publish_comments_on_push, true);
- return Promise.resolve(new Response());
+ return Promise.resolve(createDefaultPreferences());
});
// Save the change.
@@ -315,7 +315,7 @@
stubRestApi('savePreferences').callsFake(prefs => {
assert.equal(prefs.work_in_progress_by_default, true);
- return Promise.resolve(new Response());
+ return Promise.resolve(createDefaultPreferences());
});
// Save the change.
@@ -348,7 +348,7 @@
stubRestApi('savePreferences').callsFake(prefs => {
assertMenusEqual(prefs.my, element._localMenu);
- return Promise.resolve(new Response());
+ return Promise.resolve(createDefaultPreferences());
});
await element._handleSaveMenu();
diff --git a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.ts b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.ts
index f808e38..8322682 100644
--- a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.ts
+++ b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands.ts
@@ -73,6 +73,8 @@
private readonly restApiService = appContext.restApiService;
+ private readonly userService = appContext.userService;
+
disconnected$ = new Subject();
override connectedCallback() {
@@ -106,7 +108,7 @@
if (scheme && scheme !== this.selectedScheme) {
this.set('selectedScheme', scheme);
if (this._loggedIn) {
- this.restApiService.savePreferences({
+ this.userService.updatePreferences({
download_scheme: this.selectedScheme,
});
}
diff --git a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.ts b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.ts
index c932a2e..ef712ac 100644
--- a/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-download-commands/gr-download-commands_test.ts
@@ -23,6 +23,7 @@
import {createPreferences} from '../../../test/test-data-generators';
import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
import {GrShellCommand} from '../gr-shell-command/gr-shell-command';
+import {createDefaultPreferences} from '../../../constants/constants';
const basicFixture = fixtureFromElement('gr-download-commands');
@@ -95,7 +96,7 @@
test('saves scheme to preferences', () => {
element._loggedIn = true;
const savePrefsStub = stubRestApi('savePreferences').returns(
- Promise.resolve(new Response())
+ Promise.resolve(createDefaultPreferences())
);
flush();
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
index ae2bda0..63576c2 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
@@ -682,19 +682,27 @@
});
}
- savePreferences(prefs: PreferencesInput): Promise<Response> {
+ savePreferences(
+ prefs: PreferencesInput
+ ): Promise<PreferencesInfo | undefined> {
// Note (Issue 5142): normalize the download scheme with lower case before
// saving.
if (prefs.download_scheme) {
prefs.download_scheme = prefs.download_scheme.toLowerCase();
}
- return this._restApiHelper.send({
- method: HttpMethod.PUT,
- url: '/accounts/self/preferences',
- body: prefs,
- reportUrlAsIs: true,
- });
+ return this._restApiHelper
+ .send({
+ method: HttpMethod.PUT,
+ url: '/accounts/self/preferences',
+ body: prefs,
+ reportUrlAsIs: true,
+ })
+ .then((response: Response) =>
+ this.getResponseObject(response).then(
+ obj => obj as unknown as PreferencesInfo
+ )
+ );
}
saveDiffPreferences(prefs: DiffPreferenceInput): Promise<Response> {
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
index 1aee75a..a60a1ef 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
@@ -386,7 +386,8 @@
});
test('savPreferences normalizes download scheme', () => {
- const sendStub = sinon.stub(element._restApiHelper, 'send');
+ const sendStub = sinon.stub(element._restApiHelper, 'send').returns(
+ Promise.resolve(new Response()));
element.savePreferences({download_scheme: 'HTTP'});
assert.isTrue(sendStub.called);
assert.equal(sendStub.lastCall.args[0].body.download_scheme, 'http');
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
index cd3fab3..1378211 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api.ts
@@ -209,7 +209,9 @@
errFn?: ErrorCallback
): Promise<ChangeInfo | null>;
- savePreferences(prefs: PreferencesInput): Promise<Response>;
+ savePreferences(
+ prefs: PreferencesInput
+ ): Promise<PreferencesInfo | undefined>;
getDiffPreferences(): Promise<DiffPreferencesInfo | undefined>;
diff --git a/polygerrit-ui/app/services/user/user-service.ts b/polygerrit-ui/app/services/user/user-service.ts
index 0612aca..125d20c 100644
--- a/polygerrit-ui/app/services/user/user-service.ts
+++ b/polygerrit-ui/app/services/user/user-service.ts
@@ -14,7 +14,11 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {AccountDetailInfo, PreferencesInfo} from '../../types/common';
+import {
+ AccountDetailInfo,
+ PreferencesInfo,
+ PreferencesInput,
+} from '../../types/common';
import {from, of} from 'rxjs';
import {account$, updateAccount, updatePreferences} from './user-model';
import {switchMap} from 'rxjs/operators';
@@ -39,4 +43,13 @@
updatePreferences(preferences ?? createDefaultPreferences());
});
}
+
+ updatePreferences(prefs: PreferencesInput) {
+ this.restApiService
+ .savePreferences(prefs)
+ .then((newPrefs: PreferencesInfo | undefined) => {
+ if (!newPrefs) return;
+ updatePreferences(newPrefs);
+ });
+ }
}
diff --git a/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts b/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
index 4be19fd..3bb0c34 100644
--- a/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
+++ b/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
@@ -83,6 +83,7 @@
import {
createDefaultDiffPrefs,
createDefaultEditPrefs,
+ createDefaultPreferences,
} from '../../constants/constants';
import {ParsedChangeInfo} from '../../types/types';
@@ -481,8 +482,8 @@
saveIncludedGroup(): Promise<GroupInfo | undefined> {
throw new Error('saveIncludedGroup() not implemented by RestApiMock.');
},
- savePreferences(): Promise<Response> {
- return Promise.resolve(new Response());
+ savePreferences(): Promise<PreferencesInfo> {
+ return Promise.resolve(createDefaultPreferences());
},
saveRepoConfig(): Promise<Response> {
return Promise.resolve(new Response());
diff --git a/polygerrit-ui/app/test/test-utils.ts b/polygerrit-ui/app/test/test-utils.ts
index 1cde372..39c30ad 100644
--- a/polygerrit-ui/app/test/test-utils.ts
+++ b/polygerrit-ui/app/test/test-utils.ts
@@ -24,6 +24,7 @@
import {AuthService} from '../services/gr-auth/gr-auth';
import {ReportingService} from '../services/gr-reporting/gr-reporting';
import {CommentsService} from '../services/comments/comments-service';
+import {UserService} from '../services/user/user-service';
export {query, queryAll, queryAndAssert} from '../utils/common-util';
export interface MockPromise extends Promise<unknown> {
@@ -111,6 +112,10 @@
return sinon.stub(appContext.commentsService, method);
}
+export function stubUsers<K extends keyof UserService>(method: K) {
+ return sinon.stub(appContext.userService, method);
+}
+
export function stubStorage<K extends keyof StorageService>(method: K) {
return sinon.stub(appContext.storageService, method);
}
