Remove new-change-summary feature flag from gr-change-view
Change-Id: I971bc62bacc61b1985adb5de8d4fd36da68f2f46
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 730fe09..d5667c3 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -163,7 +163,6 @@
fireDialogChange,
fireTitleChange,
} from '../../../utils/event-util';
-import {KnownExperimentId} from '../../../services/flags/flags';
import {GerritView} from '../../../services/router/router-model';
import {takeUntil} from 'rxjs/operators';
import {aPluginHasRegistered$} from '../../../services/checks/checks-model';
@@ -173,13 +172,7 @@
import {ChangeStates} from '../../shared/gr-change-status/gr-change-status';
import {getRevertCreatedChangeIds} from '../../../utils/message-util';
-const CHANGE_ID_ERROR = {
- MISMATCH: 'mismatch',
- MISSING: 'missing',
-};
-const CHANGE_ID_REGEX_PATTERN = /^(Change-Id:\s|Link:.*\/id\/)(I[0-9a-f]{8,40})/gm;
-
-const MIN_LINES_FOR_COMMIT_COLLAPSE = 30;
+const MIN_LINES_FOR_COMMIT_COLLAPSE = 17;
const REVIEWERS_REGEX = /^(R|CC)=/gm;
const MIN_CHECK_INTERVAL_SECS = 0;
@@ -255,8 +248,6 @@
private readonly reporting = appContext.reportingService;
- private readonly flagsService = appContext.flagsService;
-
private readonly jsAPI = appContext.jsApiService;
private readonly changeService = appContext.changeService;
@@ -360,8 +351,7 @@
type: Boolean,
computed:
'_computeHideEditCommitMessage(_loggedIn, ' +
- '_editingCommitMessage, _change, _editMode, _commitCollapsed, ' +
- '_commitCollapsible)',
+ '_editingCommitMessage, _change, _editMode)',
})
_hideEditCommitMessage?: boolean;
@@ -383,13 +373,6 @@
@property({type: Number})
_lineHeight?: number;
- @property({
- type: String,
- computed:
- '_computeChangeIdCommitMessageError(_latestCommitMessage, _change)',
- })
- _changeIdCommitMessageError?: string;
-
@property({type: Object})
_patchRange?: ChangeViewPatchRange;
@@ -529,9 +512,6 @@
@property({type: Boolean})
_showChecksTab = false;
- @property({type: Boolean})
- _isNewChangeSummaryUiEnabled = false;
-
@property({type: String})
_tabState?: TabState;
@@ -578,9 +558,6 @@
aPluginHasRegistered$.pipe(takeUntil(this.disconnected$)).subscribe(b => {
this._showChecksTab = b;
});
- this._isNewChangeSummaryUiEnabled = this.flagsService.isEnabled(
- KnownExperimentId.NEW_CHANGE_SUMMARY_UI
- );
}
constructor() {
@@ -870,11 +847,6 @@
});
}
- _handleEditCommitMessage() {
- this._editingCommitMessage = true;
- this.$.commitMessageEditor.focusTextarea();
- }
-
_handleCommitMessageSave(e: EditableContentSaveEvent) {
assertIsDefined(this._change, '_change');
if (!this._changeNum)
@@ -936,19 +908,13 @@
loggedIn: boolean,
editing: boolean,
change: ChangeInfo,
- editMode?: boolean,
- collapsed?: boolean,
- collapsible?: boolean
+ editMode?: boolean
) {
- const hideWhenCollapsed = this._isNewChangeSummaryUiEnabled
- ? false
- : collapsed && collapsible;
if (
!loggedIn ||
editing ||
(change && change.status === ChangeStatus.MERGED) ||
- editMode ||
- hideWhenCollapsed
+ editMode
) {
return true;
}
@@ -1558,53 +1524,6 @@
return GerritNav.getUrlForChange(change);
}
- _computeChangeIdClass(displayChangeId: string) {
- return displayChangeId === CHANGE_ID_ERROR.MISMATCH ? 'warning' : '';
- }
-
- _computeTitleAttributeWarning(displayChangeId: string) {
- if (displayChangeId === CHANGE_ID_ERROR.MISMATCH) {
- return 'Change-Id mismatch';
- } else if (displayChangeId === CHANGE_ID_ERROR.MISSING) {
- return 'No Change-Id in commit message';
- }
- return undefined;
- }
-
- _computeChangeIdCommitMessageError(
- commitMessage?: string,
- change?: ChangeInfo
- ) {
- if (change === undefined) {
- return undefined;
- }
-
- if (!commitMessage) {
- return CHANGE_ID_ERROR.MISSING;
- }
-
- // Find the last match in the commit message:
- let changeId;
- let changeIdArr;
-
- while ((changeIdArr = CHANGE_ID_REGEX_PATTERN.exec(commitMessage))) {
- changeId = changeIdArr[2];
- }
-
- if (changeId) {
- // A change-id is detected in the commit message.
-
- if (changeId === change.change_id) {
- // The change-id found matches the real change-id.
- return null;
- }
- // The change-id found does not match the change-id.
- return CHANGE_ID_ERROR.MISMATCH;
- }
- // There is no change-id in the commit message.
- return CHANGE_ID_ERROR.MISSING;
- }
-
_computeReplyButtonLabel(
changeRecord?: ElementPropertyDeepChange<
GrChangeView,
@@ -2387,13 +2306,6 @@
return `Change ${changeNum}`;
}
- _computeCommitMessageCollapsed(collapsed?: boolean, collapsible?: boolean) {
- if (this._isNewChangeSummaryUiEnabled) {
- return false;
- }
- return collapsible && collapsed;
- }
-
/**
* Returns the text to be copied when
* click the copy icon next to change subject
@@ -2406,21 +2318,11 @@
);
}
- _toggleCommitCollapsed() {
- this._commitCollapsed = !this._commitCollapsed;
- if (this._commitCollapsed) {
- window.scrollTo(0, 0);
- }
- }
-
_computeCommitCollapsible(commitMessage?: string) {
if (!commitMessage) {
return false;
}
- const MIN_LINES = this._isNewChangeSummaryUiEnabled
- ? 17
- : MIN_LINES_FOR_COMMIT_COLLAPSE;
- return commitMessage.split('\n').length >= MIN_LINES;
+ return commitMessage.split('\n').length >= MIN_LINES_FOR_COMMIT_COLLAPSE;
}
_startUpdateCheckTimer() {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index 211810c..ac43c59 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -91,11 +91,6 @@
background-color: var(--view-background-color);
box-shadow: var(--elevation-level-1);
}
- .changeId {
- color: var(--deemphasized-text-color);
- font-family: var(--font-family);
- margin-top: var(--spacing-l);
- }
.changeMetadata {
/* Limit meta section to half of the screen at max */
max-width: 50%;
@@ -115,18 +110,8 @@
#commitMessageEditor {
/* Account for border and padding and rounding errors. */
min-width: calc(72ch + 2px + 2 * var(--spacing-m) + 0.4px);
- --collapsed-max-height: 36em;
- }
- .new-change-summary-true #commitMessageEditor {
--collapsed-max-height: 300px;
}
- .editCommitMessage {
- margin-top: var(--spacing-l);
-
- --gr-button: {
- padding: 5px 0px;
- }
- }
.changeStatuses,
.commitActions,
.statusText {
@@ -156,9 +141,6 @@
.mobile {
display: none;
}
- .warning {
- color: var(--error-text-color);
- }
hr {
border: 0;
border-top: 1px solid var(--border-color);
@@ -175,13 +157,6 @@
margin: var(--spacing-l) 0;
padding: 0 var(--spacing-l);
}
- .collapseToggleContainer {
- display: flex;
- margin-bottom: 8px;
- }
- .collapseToggleContainer gr-button {
- display: block;
- }
.showOnEdit {
display: none;
}
@@ -223,7 +198,7 @@
padding-top: var(--spacing-l);
width: 100%;
}
- gr-change-summary.new-change-summary-true {
+ gr-change-summary {
/* temporary for old checks status */
margin-bottom: var(--spacing-m);
}
@@ -438,10 +413,7 @@
>[[_replyButtonLabel]]</gr-button
>
</div>
- <div
- id="commitMessage"
- class$="commitMessage new-change-summary-[[_isNewChangeSummaryUiEnabled]]"
- >
+ <div id="commitMessage" class="commitMessage">
<gr-editable-content
id="commitMessageEditor"
editing="{{_editingCommitMessage}}"
@@ -450,7 +422,6 @@
hide-edit-commit-message="[[_hideEditCommitMessage]]"
commit-collapsible="[[_commitCollapsible]]"
remove-zero-width-space=""
- collapsed$="[[_computeCommitMessageCollapsed(_commitCollapsed, _commitCollapsible)]]"
>
<gr-linked-text
pre=""
@@ -459,46 +430,7 @@
remove-zero-width-space=""
></gr-linked-text>
</gr-editable-content>
- <template is="dom-if" if="[[!_isNewChangeSummaryUiEnabled]]">
- <gr-button
- link=""
- class="editCommitMessage"
- title="Edit commit message"
- on-click="_handleEditCommitMessage"
- hidden$="[[_hideEditCommitMessage]]"
- >Edit</gr-button
- >
- <div
- class="changeId"
- hidden$="[[!_changeIdCommitMessageError]]"
- >
- <hr />
- Change-Id:
- <span
- class$="[[_computeChangeIdClass(_changeIdCommitMessageError)]]"
- title$="[[_computeTitleAttributeWarning(_changeIdCommitMessageError)]]"
- >
- [[_change.change_id]]
- </span>
- </div>
- </template>
</div>
- <template is="dom-if" if="[[!_isNewChangeSummaryUiEnabled]]">
- <div
- id="commitCollapseToggle"
- class="collapseToggleContainer"
- hidden$="[[!_commitCollapsible]]"
- >
- <gr-button
- link=""
- id="commitCollapseToggleButton"
- class="collapseToggleButton"
- on-click="_toggleCommitCollapsed"
- >
- [[_computeCollapseText(_commitCollapsed)]]
- </gr-button>
- </div>
- </template>
<gr-change-summary
change-comments="[[_changeComments]]"
comment-threads="[[_commentThreads]]"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index dc8367b..63a0d91 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -55,7 +55,6 @@
createUserConfig,
TEST_NUMERIC_CHANGE_ID,
TEST_PROJECT_NAME,
- getCurrentRevision,
createEditRevision,
createAccountWithIdNameAndEmail,
createChangeViewChange,
@@ -1768,104 +1767,6 @@
assert.equal(putStub.lastCall.args[1], '\n\n\n\n\n\n\n\n');
});
- test('_computeChangeIdCommitMessageError', () => {
- let commitMessage = 'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282483';
- let change: ChangeInfo = {
- ...createChangeViewChange(),
- change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282483' as ChangeId,
- };
- assert.equal(
- element._computeChangeIdCommitMessageError(commitMessage, change),
- null
- );
-
- change = {
- ...createChangeViewChange(),
- change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282484' as ChangeId,
- };
- assert.equal(
- element._computeChangeIdCommitMessageError(commitMessage, change),
- 'mismatch'
- );
-
- commitMessage = 'This is the greatest change.';
- assert.equal(
- element._computeChangeIdCommitMessageError(commitMessage, change),
- 'missing'
- );
- });
-
- test('multiple change Ids in commit message picks last', () => {
- const commitMessage = [
- 'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282484',
- 'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282483',
- ].join('\n');
- let change: ChangeInfo = {
- ...createChangeViewChange(),
- change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282483' as ChangeId,
- };
- assert.equal(
- element._computeChangeIdCommitMessageError(commitMessage, change),
- null
- );
- change = {
- ...createChangeViewChange(),
- change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282484' as ChangeId,
- };
- assert.equal(
- element._computeChangeIdCommitMessageError(commitMessage, change),
- 'mismatch'
- );
- });
-
- test('does not count change Id that starts mid line', () => {
- const commitMessage = [
- 'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282484',
- 'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282483',
- ].join(' and ');
- let change: ChangeInfo = {
- ...createChangeViewChange(),
- change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282484' as ChangeId,
- };
- assert.equal(
- element._computeChangeIdCommitMessageError(commitMessage, change),
- null
- );
- change = {
- ...createChangeViewChange(),
- change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282483' as ChangeId,
- };
- assert.equal(
- element._computeChangeIdCommitMessageError(commitMessage, change),
- 'mismatch'
- );
- });
-
- test('_computeTitleAttributeWarning', () => {
- let changeIdCommitMessageError = 'missing';
- assert.equal(
- element._computeTitleAttributeWarning(changeIdCommitMessageError),
- 'No Change-Id in commit message'
- );
-
- changeIdCommitMessageError = 'mismatch';
- assert.equal(
- element._computeTitleAttributeWarning(changeIdCommitMessageError),
- 'Change-Id mismatch'
- );
- });
-
- test('_computeChangeIdClass', () => {
- let changeIdCommitMessageError = 'missing';
- assert.equal(element._computeChangeIdClass(changeIdCommitMessageError), '');
-
- changeIdCommitMessageError = 'mismatch';
- assert.equal(
- element._computeChangeIdClass(changeIdCommitMessageError),
- 'warning'
- );
- });
-
test('topic is coalesced to null', done => {
sinon.stub(element, '_changeChanged');
stubRestApi('getChangeDetail').returns(
@@ -2219,57 +2120,6 @@
});
});
- suite('commit message expand/collapse', () => {
- setup(() => {
- element._change = {
- ...createChangeViewChange(),
- revisions: createRevisions(1),
- messages: createChangeMessages(1),
- };
- element._change.labels = {};
- stubRestApi('getChangeDetail').callsFake(() =>
- Promise.resolve({
- ...createChangeViewChange(),
- // new patchset was uploaded
- revisions: createRevisions(2),
- current_revision: getCurrentRevision(2),
- messages: createChangeMessages(1),
- })
- );
- });
-
- test('commitCollapseToggle hidden for short commit message', () => {
- element._latestCommitMessage = '';
- flush();
- const commitCollapseToggle = element.shadowRoot!.querySelector(
- '#commitCollapseToggle'
- );
- assert.isTrue(commitCollapseToggle?.hasAttribute('hidden'));
- });
-
- test('commitCollapseToggle shown for long commit message', () => {
- element._latestCommitMessage = _.times(31, String).join('\n');
- const commitCollapseToggle = element.shadowRoot!.querySelector(
- '#commitCollapseToggle'
- );
- assert.isFalse(commitCollapseToggle?.hasAttribute('hidden'));
- });
-
- test('commitCollapseToggle functions', () => {
- element._latestCommitMessage = _.times(35, String).join('\n');
- assert.isTrue(element._commitCollapsed);
- assert.isTrue(element._commitCollapsible);
- assert.isTrue(element.$.commitMessageEditor.hasAttribute('collapsed'));
- const commitCollapseToggleButton = element.shadowRoot!.querySelector(
- '#commitCollapseToggleButton'
- )!;
- tap(commitCollapseToggleButton);
- assert.isFalse(element._commitCollapsed);
- assert.isTrue(element._commitCollapsible);
- assert.isFalse(element.$.commitMessageEditor.hasAttribute('collapsed'));
- });
- });
-
test('header class computation', () => {
assert.equal(element._computeHeaderClass(), 'header');
assert.equal(element._computeHeaderClass(true), 'header editMode');
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index 6f94b8d..92a5d55 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -28,7 +28,6 @@
// be used by plugins. The new Checks UI will show up, if a plugin registers
// with the new Checks plugin API.
CI_REBOOT_CHECKS = 'UiFeature__ci_reboot_checks',
- NEW_CHANGE_SUMMARY_UI = 'UiFeature__new_change_summary_ui',
NEW_IMAGE_DIFF_UI = 'UiFeature__new_image_diff_ui',
COMMENT_CONTEXT = 'UiFeature__comment_context',
TOKEN_HIGHLIGHTING = 'UiFeature__token_highlighting',
