Allow to create a change instead of updating project config directly.
In the [1] the new config is added for disabling project update without
creating a change. This change makes required UI changes to support this
functionality:
1. "Save for review" button is added to the project config page.
2. The "Save" buttons are automatically disabled if updating project
config without creating a change is not allowed.
[1] https://gerrit-review.git.corp.google.com/c/gerrit/+/425807
Release-Notes: "Save for review" button was added into the project config page.
Google-Bug-Id: b/330832714
Change-Id: I63d02112999c91a66d3e8fdce52724161385aeff
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
index 615528c..ff87849 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
@@ -74,6 +74,8 @@
// private but used in test
@state() canUpload?: boolean = false; // restAPI can return undefined
+ @state() disableSaveWithoutReview = true;
+
// private but used in test
@state() inheritFromFilter?: RepoName;
@@ -238,7 +240,7 @@
? 'invisible'
: ''}
primary
- ?disabled=${!this.modified}
+ ?disabled=${!this.modified || this.disableSaveWithoutReview}
@click=${this.handleSave}
>Save</gr-button
>
@@ -343,6 +345,7 @@
this.groups = res.groups;
this.weblinks = res.config_web_links || [];
this.canUpload = res.can_upload;
+ this.disableSaveWithoutReview = !!res.require_change_for_config_update;
this.ownerOf = res.owner_of || [];
return toSortedPermissionsArray(this.local);
});
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.ts b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.ts
index 467857d..495de52 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.ts
@@ -157,12 +157,13 @@
Edit
</gr-button>
<gr-button
- aria-disabled="false"
+ aria-disabled="true"
+ disabled=""
class="invisible"
id="saveBtn"
primary=""
role="button"
- tabindex="0"
+ tabindex="-1"
>
Save
</gr-button>
@@ -1445,9 +1446,18 @@
);
element.repo = 'test-repo' as RepoName;
+ await element.updateComplete;
sinon.stub(element, 'computeAddAndRemove').returns(repoAccessInput);
-
+ assert.equal(
+ queryAndAssert<GrButton>(element, '#saveBtn').hasAttribute('disabled'),
+ true
+ );
element.modified = true;
+ await element.updateComplete;
+ assert.equal(
+ queryAndAssert<GrButton>(element, '#saveBtn').hasAttribute('disabled'),
+ false
+ );
queryAndAssert<GrButton>(element, '#saveBtn').click();
await element.updateComplete;
assert.equal(
@@ -1460,6 +1470,58 @@
assert.isTrue(setUrlStub.notCalled);
});
+ test('saveBtn remains disabled when require_change_for_config_update is set', async () => {
+ const repoAccessInput = {
+ add: {
+ 'refs/*': {
+ permissions: {
+ owner: {
+ rules: {
+ 123: {action: 'DENY', modified: true},
+ },
+ },
+ },
+ },
+ },
+ remove: {
+ 'refs/*': {
+ permissions: {
+ owner: {
+ rules: {
+ 123: {},
+ },
+ },
+ },
+ },
+ },
+ };
+ stubRestApi('getRepoAccessRights').returns(
+ Promise.resolve(
+ JSON.parse(
+ JSON.stringify({
+ ...accessRes,
+ require_change_for_config_update: true,
+ })
+ )
+ )
+ );
+
+ element.repo = 'test-repo' as RepoName;
+ await element.updateComplete;
+ sinon.stub(element, 'computeAddAndRemove').returns(repoAccessInput);
+ assert.equal(
+ queryAndAssert<GrButton>(element, '#saveBtn').hasAttribute('disabled'),
+ true
+ );
+ element.modified = true;
+ await element.updateComplete;
+ assert.equal(element.disableSaveWithoutReview, true);
+ assert.equal(
+ queryAndAssert<GrButton>(element, '#saveBtn').hasAttribute('disabled'),
+ true
+ );
+ });
+
test('handleSaveForReview', async () => {
const repoAccessInput = {
add: {
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
index 2251321..6e1aba7 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
@@ -11,6 +11,7 @@
import '../../shared/gr-select/gr-select';
import '../../shared/gr-suggestion-textarea/gr-suggestion-textarea';
import '../gr-repo-plugin-config/gr-repo-plugin-config';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {
ConfigInfo,
RepoName,
@@ -39,11 +40,14 @@
import {deepClone} from '../../../utils/deep-util';
import {LitElement, PropertyValues, css, html, nothing} from 'lit';
import {customElement, property, state} from 'lit/decorators.js';
+import {createChangeUrl} from '../../../models/views/change';
import {when} from 'lit/directives/when.js';
import {subscribe} from '../../lit/subscription-controller';
import {createSearchUrl} from '../../../models/views/search';
import {userModelToken} from '../../../models/user/user-model';
import {resolve} from '../../../models/dependency';
+import {GrButton} from '../../shared/gr-button/gr-button';
+import {KnownExperimentId} from '../../../services/flags/flags';
const STATES = {
active: {value: RepoState.ACTIVE, label: 'Active'},
@@ -101,6 +105,11 @@
// private but used in test
@state() readOnly = true;
+ @state() showSaveForReviewButton = false;
+
+ // private but used in test
+ @state() disableSaveWithoutReview = true;
+
@state() private states = Object.values(STATES);
@state() private originalConfig?: ConfigInfo;
@@ -114,10 +123,14 @@
@state() private pluginConfigChanged = false;
+ private readonly flagsService = getAppContext().flagsService;
+
private readonly getUserModel = resolve(this, userModelToken);
private readonly restApiService = getAppContext().restApiService;
+ private readonly getNavigation = resolve(this, navigationToken);
+
constructor() {
super();
subscribe(
@@ -195,10 +208,20 @@
${this.renderDescription()} ${this.renderRepoOptions()}
${this.renderPluginConfig()}
<gr-button
- ?disabled=${this.readOnly || !configChanged}
+ id="saveBtn"
+ ?disabled=${this.readOnly ||
+ this.disableSaveWithoutReview ||
+ !configChanged}
@click=${this.handleSaveRepoConfig}
>Save changes</gr-button
>
+ <gr-button
+ id="saveReviewBtn"
+ ?disabled=${this.readOnly || !configChanged}
+ ?hidden=${!this.showSaveForReviewButton}
+ @click=${this.handleSaveRepoConfigForReview}
+ >Save for review</gr-button
+ >
</fieldset>
<gr-endpoint-decorator name="repo-config">
<gr-endpoint-param
@@ -781,6 +804,11 @@
// If the user is not an owner, is_owner is not a property.
this.readOnly = !access[repo]?.is_owner;
+ this.showSaveForReviewButton = this.flagsService.isEnabled(
+ KnownExperimentId.SAVE_PROJECT_CONFIG_FOR_REVIEW
+ );
+ this.disableSaveWithoutReview =
+ !!access[repo]?.require_change_for_config_update;
});
}
})
@@ -925,6 +953,32 @@
return;
}
+ async handleSaveRepoConfigForReview(e: Event) {
+ if (!this.repoConfig || !this.repo)
+ return Promise.reject(new Error('undefined repoConfig or repo'));
+ const button = e && (e.target as GrButton);
+ if (button) {
+ button.loading = true;
+ }
+
+ return this.restApiService
+ .saveRepoConfigForReview(
+ this.repo,
+ this.formatRepoConfigForSave(this.repoConfig)
+ )
+ .then(change => {
+ // Don't navigate on server error.
+ if (change) {
+ this.getNavigation().setUrl(createChangeUrl({change}));
+ }
+ })
+ .finally(() => {
+ if (button) {
+ button.loading = false;
+ }
+ });
+ }
+
private isEdited(
original?: InheritedBooleanInfo | MaxObjectSizeLimitInfo,
repo?: InheritedBooleanInfo | MaxObjectSizeLimitInfo
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.ts b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.ts
index 0d30933..a4e0f09 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo_test.ts
@@ -6,6 +6,8 @@
import '../../../test/common-test-setup';
import './gr-repo';
import {GrRepo} from './gr-repo';
+import {createChange} from '../../../test/test-data-generators';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {mockPromise} from '../../../test/test-utils';
import {
addListenerForTest,
@@ -17,6 +19,7 @@
createInheritedBoolean,
createServerInfo,
} from '../../../test/test-data-generators';
+import {testResolver} from '../../../test/common-test-setup';
import {
ConfigInfo,
GitRef,
@@ -25,6 +28,7 @@
InheritedBooleanInfo,
MaxObjectSizeLimitInfo,
PluginParameterToConfigParameterInfoMap,
+ ProjectAccessInfo,
RepoAccessGroups,
RepoAccessInfoMap,
RepoName,
@@ -32,6 +36,7 @@
import {
ConfigParameterInfoType,
InheritedBooleanInfoConfiguredValue,
+ PermissionAction,
RepoState,
SubmitType,
} from '../../../constants/constants';
@@ -45,6 +50,9 @@
import {GrSuggestionTextarea} from '../../shared/gr-suggestion-textarea/gr-suggestion-textarea';
import {IronInputElement} from '@polymer/iron-input/iron-input';
import {fixture, html, assert} from '@open-wc/testing';
+import {getAppContext} from '../../../services/app-context';
+import {KnownExperimentId} from '../../../services/flags/flags';
+import {ChangeInfo} from '../../../api/rest-api';
suite('gr-repo tests', () => {
let element: GrRepo;
@@ -375,6 +383,7 @@
</section>
</fieldset>
<gr-button
+ id="saveBtn"
aria-disabled="true"
disabled=""
role="button"
@@ -382,6 +391,16 @@
>
Save changes
</gr-button>
+ <gr-button
+ id="saveReviewBtn"
+ aria-disabled="true"
+ disabled=""
+ hidden=""
+ role="button"
+ tabindex="-1"
+ >
+ Save for review
+ </gr-button>
</fieldset>
<gr-endpoint-decorator name="repo-config">
<gr-endpoint-param name="repoName"> </gr-endpoint-param>
@@ -633,31 +652,35 @@
});
suite('admin', () => {
+ const testRepoAccess: ProjectAccessInfo = {
+ revision: 'xxxx',
+ local: {
+ 'refs/*': {
+ permissions: {
+ owner: {
+ rules: {xxx: {action: PermissionAction.ALLOW, force: false}},
+ },
+ },
+ },
+ },
+ is_owner: true,
+ owner_of: ['refs/*'] as GitRef[],
+ groups: {
+ xxxx: {
+ id: 'xxxx' as GroupId,
+ url: 'test',
+ name: 'test' as GroupName,
+ },
+ } as RepoAccessGroups,
+ config_web_links: [{name: 'gitiles', url: 'test'}],
+ };
+ let getRepoAccessStub: sinon.SinonStub;
setup(() => {
element.repo = REPO as RepoName;
loggedInStub.returns(Promise.resolve(true));
- stubRestApi('getRepoAccess').callsFake(() =>
+ getRepoAccessStub = stubRestApi('getRepoAccess').callsFake(() =>
Promise.resolve({
- 'test-repo': {
- revision: 'xxxx',
- local: {
- 'refs/*': {
- permissions: {
- owner: {rules: {xxx: {action: 'ALLOW', force: false}}},
- },
- },
- },
- is_owner: true,
- owner_of: ['refs/*'] as GitRef[],
- groups: {
- xxxx: {
- id: 'xxxx' as GroupId,
- url: 'test',
- name: 'test' as GroupName,
- },
- } as RepoAccessGroups,
- config_web_links: [{name: 'gitiles', url: 'test'}],
- },
+ 'test-repo': testRepoAccess,
} as RepoAccessInfoMap)
);
});
@@ -720,7 +743,7 @@
await element.loadRepo();
- const button = queryAll<GrButton>(element, 'gr-button')[2];
+ const button = queryAndAssert<GrButton>(element, 'gr-button#saveBtn');
assert.isTrue(button.hasAttribute('disabled'));
assert.isFalse(
queryAndAssert<HTMLHeadingElement>(
@@ -800,5 +823,76 @@
saveStub.lastCall.calledWithExactly(REPO as RepoName, configInputObj)
);
});
+
+ test('saveReviewBtn visible when experiment is enabled', async () => {
+ const flagsService = getAppContext().flagsService;
+ sinon
+ .stub(flagsService, 'isEnabled')
+ .callsFake(
+ id => id === KnownExperimentId.SAVE_PROJECT_CONFIG_FOR_REVIEW
+ );
+ await element.loadRepo();
+ await element.updateComplete;
+ const button = queryAndAssert<GrButton>(
+ element,
+ 'gr-button#saveReviewBtn'
+ );
+ assert.isFalse(button.hasAttribute('hidden'));
+ });
+
+ test('saveBtn remains disabled when require_change_for_config_update is set', async () => {
+ const flagsService = getAppContext().flagsService;
+ sinon
+ .stub(flagsService, 'isEnabled')
+ .callsFake(
+ id => id === KnownExperimentId.SAVE_PROJECT_CONFIG_FOR_REVIEW
+ );
+ getRepoAccessStub.callsFake(() =>
+ Promise.resolve({
+ 'test-repo': {
+ ...testRepoAccess,
+ require_change_for_config_update: true,
+ },
+ } as RepoAccessInfoMap)
+ );
+ await element.loadRepo();
+ await element.updateComplete;
+ const button = queryAndAssert<GrButton>(element, 'gr-button#saveBtn');
+ assert.isTrue(button.hasAttribute('disabled'));
+ });
+
+ test('saveReviewBtn', async () => {
+ const flagsService = getAppContext().flagsService;
+ sinon
+ .stub(flagsService, 'isEnabled')
+ .callsFake(
+ id => id === KnownExperimentId.SAVE_PROJECT_CONFIG_FOR_REVIEW
+ );
+ let resolver: (value: ChangeInfo | PromiseLike<ChangeInfo>) => void;
+ const saveForReviewStub = stubRestApi('saveRepoConfigForReview').returns(
+ new Promise(r => (resolver = r))
+ );
+ resolver!(createChange());
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
+
+ await element.loadRepo();
+ await element.updateComplete;
+ const input = queryAndAssert<GrSuggestionTextarea>(
+ element,
+ '#descriptionInput'
+ );
+ input.text = 'New description';
+ await input.updateComplete;
+ await element.updateComplete;
+ const button = queryAndAssert<GrButton>(element, 'gr-button#saveBtn');
+ assert.isFalse(button.hasAttribute('disabled'));
+ queryAndAssert<GrButton>(element, 'gr-button#saveReviewBtn').click();
+ await element.updateComplete;
+ assert.isTrue(saveForReviewStub.called);
+ assert.isTrue(setUrlStub.called);
+ assert.isTrue(
+ setUrlStub.lastCall.args?.[0]?.includes(`${createChange()._number}`)
+ );
+ });
});
});
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index ee1d813..7f3f9f5 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -24,4 +24,5 @@
REVISION_PARENTS_DATA = 'UiFeature__revision_parents_data',
COMMENT_AUTOCOMPLETION = 'UiFeature__comment_autocompletion_enabled',
GR_TEXTAREA = 'UiFeature__gr_textarea_enabled',
+ SAVE_PROJECT_CONFIG_FOR_REVIEW = 'UiFeature__save_project_config_for_review',
}
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
index 7a12e7a..b7eb7ac 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
@@ -348,6 +348,22 @@
});
}
+ saveRepoConfigForReview(
+ repo: RepoName,
+ config: ConfigInput
+ ): Promise<ChangeInfo | undefined> {
+ const url = `/projects/${encodeURIComponent(repo)}/config:review`;
+ return this._restApiHelper.fetchJSON({
+ fetchOptions: getFetchOptions({
+ method: HttpMethod.PUT,
+ body: config,
+ }),
+ url,
+ anonymizedUrl: '/projects/*/config',
+ reportServerError: true,
+ }) as unknown as Promise<ChangeInfo | undefined>;
+ }
+
runRepoGC(repo: RepoName): Promise<Response> {
const encodeName = encodeURIComponent(repo);
return this._restApiHelper.fetch({
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 814e97a..66a60a6 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
@@ -645,6 +645,10 @@
deleteRepoTags(repo: RepoName, ref: GitRef): Promise<Response>;
deleteRepoBranches(repo: RepoName, ref: GitRef): Promise<Response>;
saveRepoConfig(repo: RepoName, config: ConfigInput): Promise<Response>;
+ saveRepoConfigForReview(
+ repo: RepoName,
+ config: ConfigInput
+ ): Promise<ChangeInfo | undefined>;
getRelatedChanges(
changeNum: NumericChangeId,
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 570b50a..c1fe4ef 100644
--- a/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
+++ b/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
@@ -519,6 +519,9 @@
saveRepoConfig(): Promise<Response> {
return Promise.resolve(new Response());
},
+ saveRepoConfigForReview(): Promise<ChangeInfo | undefined> {
+ throw new Error('saveRepoConfigForReview() not implemented by mock.');
+ },
saveWatchedProjects(): Promise<ProjectWatchInfo[] | undefined> {
return Promise.resolve([]);
},
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index 072a872..720899f 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -986,6 +986,7 @@
can_add?: boolean;
can_add_tags?: boolean;
config_visible?: boolean;
+ require_change_for_config_update?: boolean;
groups: RepoAccessGroups;
config_web_links: WebLinkInfo[];
}
