Remove the list of change titles from revert submission.
The changes can be located in different repos/branches, some of which
might have restricted visibility. Including all subjects can leak
information.
Google-Bug-Id: b/204391110
Release-Notes: skip
Change-Id: I299bb6bae9785ef55269e78accff039c3dd2e62f
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index d5e1049..a78ba1c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -1364,7 +1364,11 @@
return;
}
assertIsDefined(this.confirmRevertDialog, 'confirmRevertDialog');
- this.confirmRevertDialog.populate(change, this.commitMessage, changes);
+ this.confirmRevertDialog.populate(
+ change,
+ this.commitMessage,
+ changes.length
+ );
this.showActionDialog(this.confirmRevertDialog);
});
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
index a1021a0..bdcdc31 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
@@ -1582,13 +1582,8 @@
'Revert submission 199 0' +
'\n\n' +
'Reason for revert: <INSERT REASONING HERE>' +
- '\n' +
- 'Reverted Changes:' +
- '\n' +
- '1234567890:random' +
- '\n' +
- '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
- '\n';
+ '\n\n' +
+ 'Reverted changes: /q/submissionid:199+0\n';
assert.equal(confirmRevertDialog.message, expectedMsg);
const radioInputs = queryAll<HTMLInputElement>(
confirmRevertDialog,
@@ -1648,13 +1643,8 @@
'Revert submission 199 0' +
'\n\n' +
'Reason for revert: <INSERT REASONING HERE>' +
- '\n' +
- 'Reverted Changes:' +
- '\n' +
- '1234567890:random' +
- '\n' +
- '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
- '\n';
+ '\n\n' +
+ 'Reverted changes: /q/submissionid:199+0\n';
const singleChangeMsg =
'Revert "random commit message"\n\nThis reverts ' +
'commit 2000.\n\nReason' +
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
index b238d77..6b37284 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
@@ -14,9 +14,9 @@
import {BindValueChangeEvent} from '../../../types/events';
import {resolve} from '../../../models/dependency';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
+import {createSearchUrl} from '../../../models/views/search';
const ERR_COMMIT_NOT_FOUND = 'Unable to find the commit hash of this change.';
-const CHANGE_SUBJECT_LIMIT = 50;
const INSERT_REASON_STRING = '<INSERT REASONING HERE>';
// TODO(dhruvsri): clean up repeated definitions after moving to js modules
@@ -58,8 +58,9 @@
@state()
private showRevertSubmission = false;
+ // Value supplied by populate(). Non-private for access in tests.
@state()
- private changesCount?: number;
+ changesCount?: number;
@state()
showErrorMessage = false;
@@ -189,15 +190,15 @@
);
}
- populate(change: ChangeInfo, commitMessage: string, changes: ChangeInfo[]) {
- this.changesCount = changes.length;
+ populate(change: ChangeInfo, commitMessage: string, changesCount: number) {
+ this.changesCount = changesCount;
// The option to revert a single change is always available
this.populateRevertSingleChangeMessage(
change,
commitMessage,
change.current_revision
);
- this.populateRevertSubmissionMessage(change, changes, commitMessage);
+ this.populateRevertSubmissionMessage(change, commitMessage);
}
populateRevertSingleChangeMessage(
@@ -225,12 +226,6 @@
this.originalRevertMessages[this.revertType] = this.message;
}
- private getTrimmedChangeSubject(subject: string) {
- if (!subject) return '';
- if (subject.length < CHANGE_SUBJECT_LIMIT) return subject;
- return subject.substring(0, CHANGE_SUBJECT_LIMIT) + '...';
- }
-
private modifyRevertSubmissionMsg(
change: ChangeInfo,
msg: string,
@@ -243,30 +238,22 @@
);
}
- populateRevertSubmissionMessage(
- change: ChangeInfo,
- changes: ChangeInfo[],
- commitMessage: string
- ) {
+ populateRevertSubmissionMessage(change: ChangeInfo, commitMessage: string) {
// Follow the same convention of the revert
const commitHash = change.current_revision;
if (!commitHash) {
fireAlert(this, ERR_COMMIT_NOT_FOUND);
return;
}
- if (!changes || changes.length <= 1) return;
- const revertTitle = `Revert submission ${change.submission_id}`;
- let message =
- revertTitle +
+ if (this.changesCount! <= 1) return;
+ const message =
+ `Revert submission ${change.submission_id}` +
'\n\n' +
'Reason for revert: <INSERT ' +
- 'REASONING HERE>\n';
- message += 'Reverted Changes:\n';
- changes.forEach(change => {
- message +=
- `${change.change_id.substring(0, 10)}:` +
- `${this.getTrimmedChangeSubject(change.subject)}\n`;
- });
+ 'REASONING HERE>\n\n' +
+ 'Reverted changes: ' +
+ createSearchUrl({query: `submissionid:${change.submission_id}`}) +
+ '\n';
this.message = this.modifyRevertSubmissionMsg(
change,
message,
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
index 59416da..38309f2 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
@@ -6,7 +6,7 @@
import {fixture, html, assert} from '@open-wc/testing';
import '../../../test/common-test-setup';
import {createChange} from '../../../test/test-data-generators';
-import {CommitId} from '../../../types/common';
+import {ChangeSubmissionId, CommitId} from '../../../types/common';
import {EventType} from '../../../types/events';
import './gr-confirm-revert-dialog';
import {GrConfirmRevertDialog} from './gr-confirm-revert-dialog';
@@ -111,4 +111,22 @@
'Reason for revert: <INSERT REASONING HERE>\n';
assert.equal(element.message, expected);
});
+
+ test('revert submission', () => {
+ element.changesCount = 3;
+ element.populateRevertSubmissionMessage(
+ {
+ ...createChange(),
+ submission_id: '5545' as ChangeSubmissionId,
+ current_revision: 'abcd123' as CommitId,
+ },
+ 'one line commit\n\nChange-Id: abcdefg\n'
+ );
+
+ const expected =
+ 'Revert submission 5545\n\n' +
+ 'Reason for revert: <INSERT REASONING HERE>\n\n' +
+ 'Reverted changes: /q/submissionid:5545\n';
+ assert.equal(element.message, expected);
+ });
});
