Merge "Move userPrefs to model inside diff-view"
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);
}
