Merge changes from topic "kamilm-checks-plugin-email-composition"
* changes:
Migrate RevertedSender to composition based classes
Migrate RestoreSender to composition based classes
Migrate ReplacePatchSetSender to composition based classes
diff --git a/plugins/reviewnotes b/plugins/reviewnotes
index 158435d..9321303 160000
--- a/plugins/reviewnotes
+++ b/plugins/reviewnotes
@@ -1 +1 @@
-Subproject commit 158435d2aa9f8729e4e78835969e54701af9203b
+Subproject commit 9321303265fcab2ff7f764a444f8c23915747638
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts
index 893a997..b30619e 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts
@@ -61,7 +61,7 @@
override connectedCallback() {
super.connectedCallback();
this.getCreateGroupCapability();
- fireTitleChange(this, 'Groups');
+ fireTitleChange('Groups');
}
static override get styles() {
diff --git a/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.ts b/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.ts
index e0c0d30..f4a7bf3 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.ts
@@ -40,7 +40,7 @@
override connectedCallback() {
super.connectedCallback();
- fireTitleChange(this, 'Audit Log');
+ fireTitleChange('Audit Log');
}
static override get styles() {
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
index f8d9934..cffde7a 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
@@ -129,7 +129,7 @@
super.connectedCallback();
this.loadGroupDetails();
- fireTitleChange(this, 'Members');
+ fireTitleChange('Members');
}
static override get styles() {
diff --git a/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts b/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
index 1ec83efa8..223a700 100644
--- a/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
@@ -342,7 +342,7 @@
this.groupConfig = config;
this.originalOptionsVisibleToAll = config?.options?.visible_to_all;
- fireTitleChange(this, config.name);
+ fireTitleChange(config.name);
await Promise.all(promises);
this.loading = false;
diff --git a/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.ts b/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.ts
index a2de840..cb17de7 100644
--- a/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.ts
+++ b/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.ts
@@ -47,7 +47,7 @@
override connectedCallback() {
super.connectedCallback();
- fireTitleChange(this, 'Plugins');
+ fireTitleChange('Plugins');
}
static override get styles() {
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts
index 11cfaab..553de0e 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-commands/gr-repo-commands.ts
@@ -91,7 +91,7 @@
override connectedCallback() {
super.connectedCallback();
- fireTitleChange(this, 'Repo Commands');
+ fireTitleChange('Repo Commands');
}
static override get styles() {
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
index 4837f1a..5318500 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
@@ -59,7 +59,7 @@
override async connectedCallback() {
super.connectedCallback();
await this.getCreateRepoCapability();
- fireTitleChange(this, 'Repos');
+ fireTitleChange('Repos');
this.maybeOpenCreateModal(this.params);
}
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 3599224..4bbd533 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
@@ -134,7 +134,7 @@
override connectedCallback() {
super.connectedCallback();
- fireTitleChange(this, `${this.repo}`);
+ fireTitleChange(`${this.repo}`);
}
static override get styles() {
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow.ts
index df63780..2ca7f36 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-abandon-flow/gr-change-list-bulk-abandon-flow.ts
@@ -147,7 +147,7 @@
private handleClose() {
this.actionModal.close();
fireAlert(this, 'Reloading page..');
- fireReload(this, true);
+ fireReload(this);
}
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
index db82523..6d7045a 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-bulk-vote-flow/gr-change-list-bulk-vote-flow.ts
@@ -308,7 +308,7 @@
this.actionModal.close();
if (getOverallStatus(this.progressByChange) === ProgressStatus.NOT_STARTED)
return;
- fireReload(this, true);
+ fireReload(this);
}
private async handleConfirm() {
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
index 8afc283..07a27c6 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
@@ -32,12 +32,6 @@
@customElement('gr-change-list-view')
export class GrChangeListView extends LitElement {
- /**
- * Fired when the title of the page should change.
- *
- * @event title-change
- */
-
@query('#prevArrow') protected prevArrow?: HTMLAnchorElement;
@query('#nextArrow') protected nextArrow?: HTMLAnchorElement;
@@ -248,7 +242,7 @@
override updated(changedProperties: PropertyValues) {
if (changedProperties.has('query')) {
- fireTitleChange(this, this.query);
+ fireTitleChange(this.query);
}
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
index 70f1755..cb6d047 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
@@ -68,12 +68,6 @@
@customElement('gr-dashboard-view')
export class GrDashboardView extends LitElement {
- /**
- * Fired when the title of the page should change.
- *
- * @event title-change
- */
-
@query('#confirmDeleteDialog') protected confirmDeleteDialog?: GrDialog;
@query('#commandsDialog') protected commandsDialog?: GrCreateCommandsDialog;
@@ -404,7 +398,7 @@
return dashboardPromise
.then(res => {
if (res && res.title) {
- fireTitleChange(this, res.title);
+ fireTitleChange(res.title);
}
return this.fetchDashboardChanges(res, checkForNewUser);
})
@@ -413,7 +407,7 @@
this.reporting.dashboardDisplayed();
})
.catch(err => {
- fireTitleChange(this, title || this.computeTitle(user));
+ fireTitleChange(title || this.computeTitle(user));
this.reporting.error('Dashboard reload', err);
})
.finally(() => {
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 0b96ce7..4eee3cb 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
@@ -81,7 +81,6 @@
fireAlert,
fireError,
fireNoBubbleNoCompose,
- fireReload,
} from '../../../utils/event-util';
import {
getApprovalInfo,
@@ -393,9 +392,6 @@
@property({type: Boolean})
disableEdit = false;
- @property({type: Boolean})
- _hasKnownChainState = false;
-
// private but used in test
@state() _hideQuickApproveAction = false;
@@ -411,9 +407,6 @@
@property({type: String})
commitNum?: CommitId;
- @property({type: Boolean})
- hasParent?: boolean;
-
@state() latestPatchNum?: PatchSetNumber;
@property({type: String})
@@ -678,14 +671,12 @@
<gr-confirm-rebase-dialog
id="confirmRebase"
class="confirmDialog"
- .changeNumber=${this.change?._number}
@confirm-rebase=${this.handleRebaseConfirm}
@cancel=${this.handleConfirmDialogCancel}
.disableActions=${this.inProgressActionKeys.has(
RevisionActions.REBASE
)}
.branch=${this.change?.branch}
- .hasParent=${this.hasParent}
.rebaseOnCurrent=${this.revisionRebaseAction
? !!this.revisionRebaseAction.enabled
: null}
@@ -810,10 +801,6 @@
}
override willUpdate(changedProperties: PropertyValues) {
- if (changedProperties.has('hasParent')) {
- this.computeChainState();
- }
-
if (changedProperties.has('change')) {
this.reload();
this.actions = this.change?.actions ?? {};
@@ -1543,22 +1530,10 @@
return key === '/' ? key : `/${key}`;
}
- /**
- * _hasKnownChainState set to true true if hasParent is defined (can be
- * either true or false). set to false otherwise.
- *
- * private but used in test
- */
- computeChainState() {
- this._hasKnownChainState = true;
- }
-
- // private but used in test
- calculateDisabled(action: UIActionInfo) {
- if (action.__key === 'rebase') {
- // Rebase button is only disabled when change has no parent(s).
- return this._hasKnownChainState === false;
- }
+ private calculateDisabled(action: UIActionInfo) {
+ // TODO(b/270972983): Remove this special casing once the backend is more
+ // aggressive about setting`enabled:true`.
+ if (action.__key === 'rebase') return false;
return !action.enabled;
}
@@ -1893,7 +1868,7 @@
// Hide rebase dialog only if the action succeeds
this.actionsModal?.close();
this.hideAllDialogs();
- fireReload(this, true);
+ this.getChangeModel().navigateToChangeResetReload();
break;
case ChangeActions.REVERT_SUBMISSION: {
const revertSubmistionInfo = obj as unknown as RevertSubmissionInfo;
@@ -1909,7 +1884,7 @@
break;
}
default:
- fireReload(this, true);
+ this.getChangeModel().navigateToChangeResetReload();
break;
}
});
@@ -1977,7 +1952,7 @@
'Cannot set label: a newer patch has been ' +
'uploaded to this change.',
action: 'Reload',
- callback: () => fireReload(this, true),
+ callback: () => this.getChangeModel().navigateToChangeResetReload(),
});
// Because this is not a network error, call the cleanup function
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 277eddd..946191b 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
@@ -40,7 +40,7 @@
TopicName,
} from '../../../types/common';
import {ActionType} from '../../../api/change-actions';
-import {SinonFakeTimers} from 'sinon';
+import {SinonFakeTimers, SinonStubbedMember} from 'sinon';
import {IronAutogrowTextareaElement} from '@polymer/iron-autogrow-textarea';
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrDialog} from '../../shared/gr-dialog/gr-dialog';
@@ -56,10 +56,17 @@
import {testResolver} from '../../../test/common-test-setup';
import {storageServiceToken} from '../../../services/storage/gr-storage_impl';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
+import {
+ ChangeModel,
+ changeModelToken,
+} from '../../../models/change/change-model';
// TODO(dhruvsri): remove use of _populateRevertMessage as it's private
suite('gr-change-actions tests', () => {
let element: GrChangeActions;
+ let navigateResetStub: SinonStubbedMember<
+ ChangeModel['navigateToChangeResetReload']
+ >;
suite('basic tests', () => {
setup(async () => {
@@ -139,6 +146,10 @@
_account_id: 123 as AccountId,
};
stubRestApi('getRepoBranches').returns(Promise.resolve([]));
+ navigateResetStub = sinon.stub(
+ testResolver(changeModelToken),
+ 'navigateToChangeResetReload'
+ );
await element.updateComplete;
await element.reload();
@@ -177,14 +188,13 @@
title="Rebase onto tip of branch or parent change"
>
<gr-button
- aria-disabled="true"
+ aria-disabled="false"
class="rebase"
data-action-key="rebase"
data-label="Rebase"
- disabled=""
link=""
role="button"
- tabindex="-1"
+ tabindex="0"
>
<gr-icon icon="rebase"> </gr-icon>
Rebase
@@ -570,34 +580,6 @@
assert.equal(fireActionStub.callCount, 0);
});
- test('chain state', async () => {
- assert.equal(element._hasKnownChainState, false);
- element.hasParent = true;
- await element.updateComplete;
- assert.equal(element._hasKnownChainState, true);
- });
-
- test('calculateDisabled', () => {
- const action = {
- __key: 'rebase',
- enabled: true,
- __type: ActionType.CHANGE,
- label: 'l',
- };
- element._hasKnownChainState = false;
- assert.equal(element.calculateDisabled(action), true);
-
- action.__key = 'delete';
- assert.equal(element.calculateDisabled(action), false);
-
- action.__key = 'rebase';
- element._hasKnownChainState = true;
- assert.equal(element.calculateDisabled(action), false);
-
- action.enabled = false;
- assert.equal(element.calculateDisabled(action), false);
- });
-
test('rebase change', async () => {
const fireActionStub = sinon.stub(element, 'fireAction');
const fetchChangesStub = sinon
@@ -606,7 +588,6 @@
'fetchRecentChanges'
)
.returns(Promise.resolve([]));
- element._hasKnownChainState = true;
await element.updateComplete;
queryAndAssert<GrButton>(
element,
@@ -642,13 +623,11 @@
});
test('rebase change fires reload event', async () => {
- const eventStub = sinon.stub(element, 'dispatchEvent');
await element.handleResponse(
{__key: 'rebase', __type: ActionType.CHANGE, label: 'l'},
new Response()
);
- assert.isTrue(eventStub.called);
- assert.equal(eventStub.lastCall.args[0].type, 'reload');
+ assert.isTrue(navigateResetStub.called);
});
test("rebase dialog gets recent changes each time it's opened", async () => {
@@ -658,7 +637,6 @@
'fetchRecentChanges'
)
.returns(Promise.resolve([]));
- element._hasKnownChainState = true;
await element.updateComplete;
const rebaseButton = queryAndAssert<GrButton>(
element,
@@ -683,7 +661,6 @@
});
test('two dialogs are not shown at the same time', async () => {
- element._hasKnownChainState = true;
await element.updateComplete;
queryAndAssert<GrButton>(
element,
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
index 54752b0..b7851a6 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
@@ -126,6 +126,7 @@
@property({type: Object}) revision?: RevisionInfo | EditRevisionInfo;
+ // TODO: Just use `revision.commit` instead.
@property({type: Object}) commitInfo?: CommitInfoWithRequiredCommit;
@property({type: Object}) serverConfig?: ServerInfo;
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 9dee851..a64dcd2 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
@@ -34,27 +34,16 @@
import {ChangeStarToggleStarDetail} from '../../shared/gr-change-star/gr-change-star';
import {GrEditConstants} from '../../edit/gr-edit-constants';
import {pluralize} from '../../../utils/string-util';
-import {whenVisible} from '../../../utils/dom-util';
+import {untilRendered, whenVisible} from '../../../utils/dom-util';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
-import {RevisionInfo as RevisionInfoClass} from '../../shared/revision-info/revision-info';
-import {
- ChangeStatus,
- DefaultBase,
- Tab,
- DiffViewMode,
-} from '../../../constants/constants';
+import {ChangeStatus, Tab, DiffViewMode} from '../../../constants/constants';
import {getAppContext} from '../../../services/app-context';
import {
computeAllPatchSets,
computeLatestPatchNum,
- findEdit,
- findEditParentRevision,
PatchSet,
} from '../../../utils/patch-set-util';
import {
- changeIsAbandoned,
- changeIsMerged,
- changeIsOpen,
changeStatuses,
isInvolved,
roleDetails,
@@ -63,37 +52,27 @@
import {GrApplyFixDialog} from '../../diff/gr-apply-fix-dialog/gr-apply-fix-dialog';
import {GrFileListHeader} from '../gr-file-list-header/gr-file-list-header';
import {GrEditableContent} from '../../shared/gr-editable-content/gr-editable-content';
-import {GrRelatedChangesList} from '../gr-related-changes-list/gr-related-changes-list';
import {GrChangeStar} from '../../shared/gr-change-star/gr-change-star';
import {GrChangeActions} from '../gr-change-actions/gr-change-actions';
import {
AccountDetailInfo,
ActionNameToActionInfoMap,
BasePatchSetNum,
- ChangeId,
ChangeInfo,
CommentThread,
- CommitId,
- CommitInfo,
ConfigInfo,
DetailedLabelInfo,
EDIT,
LabelNameToInfoMap,
NumericChangeId,
PARENT,
- PatchRange,
PatchSetNum,
- PatchSetNumber,
- PreferencesInfo,
QuickLabelInfo,
- RelatedChangeAndCommitInfo,
- RelatedChangesInfo,
RevisionInfo,
RevisionPatchSetNum,
ServerInfo,
UrlEncodedCommentId,
isRobot,
- ChangeStates,
} from '../../../types/common';
import {FocusTarget, GrReplyDialog} from '../gr-reply-dialog/gr-reply-dialog';
import {GrIncludedInDialog} from '../gr-included-in-dialog/gr-included-in-dialog';
@@ -127,16 +106,15 @@
fireDialogChange,
fire,
fireReload,
- fireTitleChange,
} from '../../../utils/event-util';
import {
debounce,
DelayedTask,
throttleWrap,
until,
+ waitUntil,
} from '../../../utils/async-util';
-import {Interaction, Timing} from '../../../constants/reporting';
-import {getRevertCreatedChangeIds} from '../../../utils/message-util';
+import {Interaction} from '../../../constants/reporting';
import {
getAddedByReason,
getRemovedByReason,
@@ -152,7 +130,7 @@
import {resolve} from '../../../models/dependency';
import {checksModelToken} from '../../../models/checks/checks-model';
import {changeModelToken} from '../../../models/change/change-model';
-import {css, html, LitElement, nothing, PropertyValues} from 'lit';
+import {css, html, LitElement, nothing} from 'lit';
import {a11yStyles} from '../../../styles/gr-a11y-styles';
import {paperStyles} from '../../../styles/gr-paper-styles';
import {sharedStyles} from '../../../styles/shared-styles';
@@ -162,7 +140,6 @@
import {FilesExpandedState} from '../gr-file-list-constants';
import {subscribe} from '../../lit/subscription-controller';
import {configModelToken} from '../../../models/config/config-model';
-import {filesModelToken} from '../../../models/change/files-model';
import {getBaseUrl, prependOrigin} from '../../../utils/url-util';
import {CopyLink, GrCopyLinks} from '../gr-copy-links/gr-copy-links';
import {
@@ -176,6 +153,7 @@
import {userModelToken} from '../../../models/user/user-model';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {modalStyles} from '../../../styles/gr-modal-styles';
+import {relatedChangesModelToken} from '../../../models/change/related-changes-model';
const MIN_LINES_FOR_COMMIT_COLLAPSE = 18;
@@ -199,17 +177,9 @@
// Making the tab names more unique in case a plugin adds one with same name
const ROBOT_COMMENTS_LIMIT = 10;
-export type ChangeViewPatchRange = Partial<PatchRange>;
-
@customElement('gr-change-view')
export class GrChangeView extends LitElement {
/**
- * Fired when the title of the page should change.
- *
- * @event title-change
- */
-
- /**
* Fired if an error occurs when fetching the change data.
*
* @event page-error
@@ -265,35 +235,18 @@
@query('gr-copy-links') private copyLinksDropdown?: GrCopyLinks;
- private _viewState?: ChangeViewState;
-
- @property({type: Object})
- get viewState() {
- return this._viewState;
- }
-
- set viewState(viewState: ChangeViewState | undefined) {
- if (this._viewState === viewState) return;
- const oldViewState = this._viewState;
- this._viewState = viewState;
- this.viewStateChanged();
- this.requestUpdate('viewState', oldViewState);
- }
+ @state()
+ viewState?: ChangeViewState;
@property({type: String})
backPage?: string;
@state()
- private hasParent?: boolean;
-
- // Private but used in tests.
- @state()
commentThreads?: CommentThread[];
// Don't use, use serverConfig instead.
private _serverConfig?: ServerInfo;
- // Private but used in tests.
@state()
get serverConfig() {
return this._serverConfig;
@@ -310,10 +263,6 @@
@state()
private account?: AccountDetailInfo;
- // Private but used in tests.
- @state()
- prefs?: PreferencesInfo;
-
canStartReview() {
return !!(
this.change &&
@@ -341,38 +290,19 @@
// Private but used in tests.
@state()
- commitInfo?: CommitInfo;
-
- // Private but used in tests.
- @state()
changeNum?: NumericChangeId;
@state()
private editingCommitMessage = false;
@state()
- private latestCommitMessage: string | null = '';
+ private latestCommitMessage = '';
- // Use patchRange getter/setter.
- private _patchRange?: ChangeViewPatchRange;
+ @state() basePatchNum: BasePatchSetNum = PARENT;
- // Private but used in tests.
- @state()
- get patchRange() {
- return this._patchRange;
- }
+ @state() patchNum?: RevisionPatchSetNum;
- set patchRange(patchRange: ChangeViewPatchRange | undefined) {
- if (this._patchRange === patchRange) return;
- const oldPatchRange = this._patchRange;
- this._patchRange = patchRange;
- this.patchNumChanged();
- this.requestUpdate('patchRange', oldPatchRange);
- }
-
- // Private but used in tests.
- @state()
- selectedRevision?: RevisionInfo | EditRevisionInfo;
+ @state() revision?: RevisionInfo | EditRevisionInfo;
/**
* <gr-change-actions> populates this via two-way data binding.
@@ -400,30 +330,14 @@
// Private but used in tests.
@state()
- initialLoadComplete = false;
-
- // Private but used in tests.
- @state()
replyDisabled = true;
- // Private but used in tests.
- @state()
- changeStatuses: ChangeStates[] = [];
-
@state()
private updateCheckTimerHandle?: number | null;
// Private but used in tests.
- getEditMode() {
- if (!this.patchRange || !this.viewState) {
- return false;
- }
-
- if (this.viewState.edit) {
- return true;
- }
-
- return this.patchRange.patchNum === EDIT;
+ getEditMode(): boolean {
+ return !!this.viewState?.edit || this.patchNum === EDIT;
}
isSubmitEnabled(): boolean {
@@ -434,9 +348,7 @@
);
}
- // Private but used in tests.
- @state()
- mergeable: boolean | null = null;
+ @state() mergeable?: boolean;
/**
* Plugins can provide (multiple) tabs. For each plugin tab we render an
@@ -503,7 +415,7 @@
private tabState?: TabState;
@state()
- private revertedChange?: ChangeInfo;
+ private revertingChange?: ChangeInfo;
// Private but used in tests.
@state()
@@ -530,10 +442,13 @@
private readonly getConfigModel = resolve(this, configModelToken);
- private readonly getFilesModel = resolve(this, filesModelToken);
-
private readonly getViewModel = resolve(this, changeViewModelToken);
+ private readonly getRelatedChangesModel = resolve(
+ this,
+ relatedChangesModelToken
+ );
+
private readonly getShortcutsService = resolve(this, shortcutsServiceToken);
private scrollTask?: DelayedTask;
@@ -581,14 +496,7 @@
this.handleCommitMessageCancel()
);
this.addEventListener('open-fix-preview', e => this.onOpenFixPreview(e));
-
this.addEventListener('show-tab', e => this.setActiveTab(e));
- this.addEventListener('reload', e => {
- this.loadData(
- /* isLocationChange= */ false,
- /* clearPatchset= */ e.detail && e.detail.clearPatchset
- );
- });
}
private setupShortcuts() {
@@ -596,7 +504,7 @@
this.shortcutsController.addAbstract(Shortcut.EMOJI_DROPDOWN, () => {}); // docOnly
this.shortcutsController.addAbstract(Shortcut.MENTIONS_DROPDOWN, () => {}); // docOnly
this.shortcutsController.addAbstract(Shortcut.REFRESH_CHANGE, () =>
- fireReload(this, true)
+ this.getChangeModel().navigateToChangeResetReload()
);
this.shortcutsController.addAbstract(Shortcut.OPEN_REPLY_DIALOG, () =>
this.handleOpenReplyDialog()
@@ -666,7 +574,21 @@
subscribe(
this,
() => this.getViewModel().tab$,
- t => (this.activeTab = t ?? Tab.FILES)
+ t => (this.activeTab = t)
+ );
+ subscribe(
+ this,
+ () => this.getViewModel().commentId$,
+ commentId => (this.scrollCommentId = commentId)
+ );
+ subscribe(
+ this,
+ () => this.getViewModel().openReplyDialog$,
+ openReplyDialog => {
+ // Here we are relying on `this.loggedIn` being set *before*
+ // `openReplyDialog`, but that is fine for this feature.
+ if (openReplyDialog && this.loggedIn) this.handleOpenReplyDialog();
+ }
);
subscribe(
this,
@@ -687,6 +609,11 @@
() => this.getViewModel().childView$,
childView => {
this.isViewCurrent = childView === ChangeChildView.OVERVIEW;
+ // When coming back from ChangeChildView.DIFF we want to restore the
+ // scroll position to what it was before leaving the OVERVIEW page.
+ if (this.isViewCurrent) {
+ document.documentElement.scrollTop = this.scrollPosition ?? 0;
+ }
}
);
subscribe(
@@ -728,6 +655,49 @@
);
subscribe(
this,
+ () => this.getChangeModel().changeNum$,
+ changeNum => {
+ // The change view is tied to a specific change number, so don't update
+ // changeNum to undefined and only set it once.
+ if (changeNum && !this.changeNum) this.changeNum = changeNum;
+ }
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().patchNum$,
+ patchNum => (this.patchNum = patchNum)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().basePatchNum$,
+ basePatchNum => (this.basePatchNum = basePatchNum)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().mergeable$,
+ mergeable => (this.mergeable = mergeable)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().revision$,
+ revision => (this.revision = revision)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().changeLoadingStatus$,
+ status => (this.loading = status !== LoadingStatus.LOADED)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().latestRevision$,
+ revision => {
+ this.latestCommitMessage = this.prepareCommitMsgForLinkify(
+ revision?.commit?.message ?? ''
+ );
+ }
+ );
+ subscribe(
+ this,
() => this.getUserModel().account$,
account => {
this.account = account;
@@ -755,6 +725,13 @@
this.projectConfig = config;
}
);
+ subscribe(
+ this,
+ () => this.getRelatedChangesModel().revertingChange$,
+ revertingChange => {
+ this.revertingChange = revertingChange;
+ }
+ );
}
override connectedCallback() {
@@ -769,6 +746,8 @@
}
override firstUpdated() {
+ this.maybeScrollToMessage(window.location.hash);
+ this.maybeShowRevertDialog();
// _onTabSizingChanged is called when iron-items-changed event is fired
// from iron-selectable but that is called before the element is present
// in view which whereas the method requires paper tabs already be visible
@@ -807,8 +786,7 @@
new Error('Mismatch of headers and content.')
);
}
- })
- .then(() => this.initActiveTab());
+ });
this.throttledToggleChangeStar = throttleWrap<KeyboardEvent>(_ =>
this.handleToggleChangeStar()
@@ -830,16 +808,6 @@
super.disconnectedCallback();
}
- protected override willUpdate(changedProperties: PropertyValues): void {
- if (
- changedProperties.has('change') ||
- changedProperties.has('mergeable') ||
- changedProperties.has('currentRevisionActions')
- ) {
- this.changeStatuses = this.computeChangeStatusChips();
- }
- }
-
static override get styles() {
return [
a11yStyles,
@@ -1050,7 +1018,7 @@
.relatedChanges {
padding: 0;
}
- #relatedChanges {
+ .relatedChanges gr-related-changes-list {
padding-top: var(--spacing-l);
}
#commitAndRelated {
@@ -1219,12 +1187,14 @@
}
private renderHeaderTitle() {
- const resolveWeblinks = this.commitInfo?.resolve_conflicts_web_links ?? [];
+ const changeStatuses = this.computeChangeStatusChips();
+ const resolveWeblinks =
+ this.revision?.commit?.resolve_conflicts_web_links ?? [];
return html` <div class="headerTitle">
<div class="changeStatuses">
- ${this.changeStatuses.map(
+ ${changeStatuses.map(
status => html` <gr-change-status
- .revertedChange=${this.revertedChange}
+ .revertedChange=${this.revertingChange}
.status=${status}
.resolveWeblinks=${resolveWeblinks}
></gr-change-status>`
@@ -1323,11 +1293,10 @@
id="actions"
.change=${this.change}
.disableEdit=${false}
- .hasParent=${this.hasParent}
.account=${this.account}
.changeNum=${this.changeNum}
.changeStatus=${this.change?.status}
- .commitNum=${this.commitInfo?.commit}
+ .commitNum=${this.revision?.commit?.commit}
.commitMessage=${this.latestCommitMessage}
.editMode=${this.getEditMode()}
.privateByDefault=${this.projectConfig?.private_by_default}
@@ -1353,10 +1322,10 @@
<gr-change-metadata
id="metadata"
.change=${this.change}
- .revertedChange=${this.revertedChange}
+ .revertedChange=${this.revertingChange}
.account=${this.account}
- .revision=${this.selectedRevision}
- .commitInfo=${this.commitInfo}
+ .revision=${this.revision}
+ .commitInfo=${this.revision?.commit}
.serverConfig=${this.serverConfig}
.parentIsCurrent=${this.isParentCurrent()}
.repoConfig=${this.projectConfig}
@@ -1396,7 +1365,7 @@
remove-zero-width-space=""
>
<gr-formatted-text
- .content=${this.latestCommitMessage ?? ''}
+ .content=${this.latestCommitMessage}
.markdown=${false}
></gr-formatted-text>
</gr-editable-content>
@@ -1406,18 +1375,12 @@
<gr-endpoint-decorator name="commit-container">
<gr-endpoint-param name="change" .value=${this.change}>
</gr-endpoint-param>
- <gr-endpoint-param
- name="revision"
- .value=${this.selectedRevision}
- >
+ <gr-endpoint-param name="revision" .value=${this.revision}>
</gr-endpoint-param>
</gr-endpoint-decorator>
</div>
<div class="relatedChanges">
- <gr-related-changes-list
- id="relatedChanges"
- .mergeable=${this.mergeable}
- ></gr-related-changes-list>
+ <gr-related-changes-list></gr-related-changes-list>
</div>
<div class="emptySpace"></div>
</div>
@@ -1460,10 +1423,7 @@
<gr-endpoint-decorator name=${tabHeader}>
<gr-endpoint-param name="change" .value=${this.change}>
</gr-endpoint-param>
- <gr-endpoint-param
- name="revision"
- .value=${this.selectedRevision}
- >
+ <gr-endpoint-param name="revision" .value=${this.revision}>
</gr-endpoint-param>
</gr-endpoint-decorator>
</paper-tab>
@@ -1499,7 +1459,7 @@
.account=${this.account}
.change=${this.change}
.changeNum=${this.changeNum}
- .commitInfo=${this.commitInfo}
+ .commitInfo=${this.revision?.commit}
.changeUrl=${this.computeChangeUrl()}
.editMode=${this.getEditMode()}
.loggedIn=${this.loggedIn}
@@ -1594,7 +1554,7 @@
<gr-endpoint-decorator .name=${pluginTabContentEndpoint}>
<gr-endpoint-param name="change" .value=${this.change}>
</gr-endpoint-param>
- <gr-endpoint-param name="revision" .value=${this.selectedRevision}></gr-endpoint-param>
+ <gr-endpoint-param name="revision" .value=${this.revision}></gr-endpoint-param>
</gr-endpoint-param>
</gr-endpoint-decorator>
`;
@@ -1605,7 +1565,7 @@
<gr-endpoint-decorator name="change-view-integration">
<gr-endpoint-param name="change" .value=${this.change}>
</gr-endpoint-param>
- <gr-endpoint-param name="revision" .value=${this.selectedRevision}>
+ <gr-endpoint-param name="revision" .value=${this.revision}>
</gr-endpoint-param>
</gr-endpoint-decorator>
@@ -1619,7 +1579,7 @@
<gr-messages-list
.labels=${this.change?.labels}
.messages=${this.change?.messages}
- .reviewerUpdates=${this.change?.reviewer_updates}
+ .reviewerUpdates=${this.change?.reviewer_updates ?? []}
@message-anchor-tap=${this.handleMessageAnchorTap}
></gr-messages-list>
</section>
@@ -1629,11 +1589,12 @@
override updated() {
const tabs = [...queryAll<HTMLElement>(this.tabs!, 'paper-tab')];
const tabIndex = tabs.findIndex(t => t.dataset['name'] === this.activeTab);
- assert(tabIndex !== -1, `tab ${this.activeTab} not found`);
- if (this.tabs!.selected !== tabIndex) {
+ if (tabIndex !== -1 && this.tabs!.selected !== tabIndex) {
this.tabs!.selected = tabIndex;
}
+ this.reportChangeDisplayed();
+ this.reportFullyLoaded();
}
private readonly handleScroll = () => {
@@ -1720,7 +1681,8 @@
}
private handleContentChanged(e: ValueChangedEvent) {
- this.latestCommitMessage = e.detail.value;
+ // optimistic update
+ this.latestCommitMessage = e.detail.value ?? '';
}
// Private but used in tests.
@@ -1747,9 +1709,8 @@
return;
}
- this.latestCommitMessage = this.prepareCommitMsgForLinkify(message);
this.editingCommitMessage = false;
- fireReload(this, true);
+ this.getChangeModel().navigateToChangeResetReload();
})
.catch(() => {
assertIsDefined(this.commitMessageEditor);
@@ -1762,19 +1723,12 @@
}
private computeChangeStatusChips() {
- if (!this.change) {
- return [];
- }
-
- // Show no chips until mergeability is loaded.
- if (this.mergeable === null) {
- return [];
- }
+ if (!this.change || this.mergeable === undefined) return [];
const options = {
- includeDerived: true,
- mergeable: !!this.mergeable,
+ mergeable: this.mergeable,
submitEnabled: !!this.isSubmitEnabled(),
+ revertingChangeStatus: this.revertingChange?.status,
};
return changeStatuses(this.change as ChangeInfo, options);
}
@@ -1942,16 +1896,9 @@
// Private but used in tests.
handleReplySent() {
- this.addEventListener(
- 'change-details-loaded',
- () => {
- this.reporting.timeEnd(Timing.SEND_REPLY);
- },
- {once: true}
- );
assertIsDefined(this.replyModal);
this.replyModal.close();
- fireReload(this);
+ this.getChangeModel().navigateToChangeResetReload();
}
private handleReplyCancel() {
@@ -2002,158 +1949,14 @@
}
// Private but used in tests.
- hasPatchRangeChanged(viewState: ChangeViewState) {
- if (!this.patchRange) return false;
- if (this.patchRange.basePatchNum !== viewState.basePatchNum) return true;
- return this.hasPatchNumChanged(viewState);
- }
-
- // Private but used in tests.
- hasPatchNumChanged(viewState: ChangeViewState) {
- if (!this.patchRange) return false;
- if (viewState.patchNum !== undefined) {
- return this.patchRange.patchNum !== viewState.patchNum;
- } else {
- // value.patchNum === undefined specifies the latest patchset
- return (
- this.patchRange.patchNum !== computeLatestPatchNum(this.allPatchSets)
- );
- }
- }
-
- // Private but used in tests.
- viewStateChanged() {
- if (!this.viewState) return;
- if (this.isChangeObsolete()) return;
-
- if (this.viewState.basePatchNum === undefined)
- this.viewState.basePatchNum = PARENT;
-
- const patchChanged = this.hasPatchRangeChanged(this.viewState);
-
- this.patchRange = {
- patchNum: this.viewState.patchNum,
- basePatchNum: this.viewState.basePatchNum,
- };
- this.scrollCommentId = this.viewState.commentId;
-
- const patchKnown =
- !this.patchRange.patchNum ||
- (this.allPatchSets ?? []).some(
- ps => ps.num === this.patchRange!.patchNum
- );
- // _allPatchsets does not know value.patchNum so force a reload.
- const forceReload = this.viewState.forceReload || !patchKnown;
-
- // If changeNum is defined that means the change has already been
- // rendered once before so a full reload is not required.
- if (this.changeNum !== undefined && !forceReload) {
- if (!this.patchRange.patchNum) {
- this.patchRange = {
- ...this.patchRange,
- patchNum: computeLatestPatchNum(this.allPatchSets),
- };
- }
- if (patchChanged) {
- // We need to collapse all diffs when viewState changes so that a non
- // existing diff is not requested. See Issue 125270 for more details.
- this.fileList?.resetFileState();
- this.fileList?.collapseAllDiffs();
- this.reloadPatchNumDependentResources();
- }
-
- // If there is no change in patchset or changeNum, such as when user goes
- // to the diff view and then comes back to change page then there is no
- // need to reload anything and we render the change view component as is.
- document.documentElement.scrollTop = this.scrollPosition ?? 0;
- this.reporting.reportInteraction('change-view-re-rendered');
- this.updateTitle(this.change);
- // We still need to check if post load tasks need to be done such as when
- // user wants to open the reply dialog when in the diff page, the change
- // page should open the reply dialog
- this.performPostLoadTasks();
- return;
- }
-
- // We need to collapse all diffs when viewState changes so that a non
- // existing diff is not requested. See Issue 125270 for more details.
- this.updateComplete.then(() => {
- assertIsDefined(this.fileList);
- this.fileList?.collapseAllDiffs();
- this.fileList?.resetFileState();
- });
-
- // If the change was loaded before, then we are firing a 'reload' event
- // instead of calling `loadData()` directly for two reasons:
- // 1. We want to avoid code such as `this.initialLoadComplete = false` that
- // is only relevant for the initial load of a change.
- // 2. We have to somehow trigger the change-model reloading. Otherwise
- // this.change is not updated.
- if (this.changeNum) {
- if (!this._patchRange?.patchNum) {
- this._patchRange = {
- basePatchNum: PARENT,
- patchNum: computeLatestPatchNum(this.allPatchSets),
- };
- }
- fireReload(this);
- return;
- }
-
- this.initialLoadComplete = false;
- this.changeNum = this.viewState.changeNum;
- this.loadData(true).then(() => {
- this.performPostLoadTasks();
- });
-
- this.getPluginLoader()
- .awaitPluginsLoaded()
- .then(() => {
- this.initActiveTab();
- });
- }
-
- private initActiveTab() {
- let tab = Tab.FILES;
- if (this.viewState?.tab) {
- tab = this.viewState?.tab as Tab;
- } else if (this.viewState?.commentId) {
- tab = Tab.COMMENT_THREADS;
- }
- this.setActiveTab(new CustomEvent('show-tab', {detail: {tab}}));
- }
-
- // Private but used in tests.
- sendShowChangeEvent() {
- assertIsDefined(this.patchRange, 'patchRange');
- this.getPluginLoader().jsApiService.handleShowChange({
- change: this.change,
- patchNum: this.patchRange.patchNum,
- info: {mergeable: this.mergeable},
- });
- }
-
- private performPostLoadTasks() {
- this.maybeShowReplyDialog();
- this.maybeShowRevertDialog();
-
- this.sendShowChangeEvent();
-
- this.updateComplete.then(() => {
- this.maybeScrollToMessage(window.location.hash);
- this.initialLoadComplete = true;
- });
- }
-
- // Private but used in tests.
handleMessageAnchorTap(e: CustomEvent<{id: string}>) {
assertIsDefined(this.change, 'change');
- assertIsDefined(this.patchRange, 'patchRange');
+ assertIsDefined(this.patchNum, 'patchNum');
const hash = PREFIX + e.detail.id;
const url = createChangeUrl({
change: this.change,
- patchNum: this.patchRange.patchNum,
- basePatchNum: this.patchRange.basePatchNum,
+ patchNum: this.patchNum,
+ basePatchNum: this.basePatchNum,
edit: this.getEditMode(),
messageHash: hash,
});
@@ -2161,9 +1964,10 @@
}
// Private but used in tests.
- maybeScrollToMessage(hash: string) {
- if (hash.startsWith(PREFIX) && this.messagesList) {
- this.messagesList.scrollToMessage(hash.substr(PREFIX.length));
+ async maybeScrollToMessage(hash: string) {
+ if (hash.startsWith(PREFIX)) {
+ await waitUntil(() => !!this.messagesList);
+ this.messagesList!.scrollToMessage(hash.substr(PREFIX.length));
}
}
@@ -2186,39 +1990,18 @@
}
// Private but used in tests.
- maybeShowRevertDialog() {
- this.getPluginLoader()
- .awaitPluginsLoaded()
- .then(() => {
- if (
- !this.loggedIn ||
- !this.change ||
- this.change.status !== ChangeStatus.MERGED
- ) {
- // Do not display dialog if not logged-in or the change is not
- // merged.
- return;
- }
- if (this._getUrlParameter('revert')) {
- assertIsDefined(this.actions);
- this.actions.showRevertDialog();
- }
- });
- }
+ async maybeShowRevertDialog() {
+ if (!this._getUrlParameter('revert')) return;
- private maybeShowReplyDialog() {
- if (!this.loggedIn) return;
- if (this.viewState?.openReplyDialog) {
- this.openReplyDialog(FocusTarget.ANY);
+ await this.getPluginLoader().awaitPluginsLoaded();
+ await waitUntil(() => !!this.actions);
+ await waitUntil(() => !!this.change);
+
+ if (this.change?.status === ChangeStatus.MERGED && this.loggedIn) {
+ this.actions!.showRevertDialog();
}
}
- private updateTitle(change?: ChangeInfo | ParsedChangeInfo) {
- if (!change) return;
- const title = `${change.subject} (${change._number})`;
- fireTitleChange(this, title);
- }
-
// Private but used in tests.
changeChanged(oldChange: ParsedChangeInfo | undefined) {
this.allPatchSets = computeAllPatchSets(this.change);
@@ -2232,57 +2015,11 @@
this.currentRobotCommentsPatchSet =
this.change.revisions[this.change.current_revision]._number;
}
- if (!this.change || !this.patchRange || !this.allPatchSets) {
- return;
- }
-
- // We get the parent first so we keep the original value for basePatchNum
- // and not the updated value.
- const parent = this.getBasePatchNum();
-
- this.patchRange = {
- ...this.patchRange,
- basePatchNum: parent,
- patchNum:
- this.patchRange.patchNum || computeLatestPatchNum(this.allPatchSets),
- };
- this.updateTitle(this.change);
}
/**
- * Gets base patch number, if it is a parent try and decide from
- * preference whether to default to `auto merge`, `Parent 1` or `PARENT`.
- * Private but used in tests.
+ * This is the URL equivalent of changeModel.navigateToChangeResetReload().
*/
- getBasePatchNum() {
- if (
- this.patchRange &&
- this.patchRange.basePatchNum &&
- this.patchRange.basePatchNum !== PARENT
- ) {
- return this.patchRange.basePatchNum;
- }
-
- const revisionInfo = this.getRevisionInfo();
- if (!revisionInfo) return PARENT;
-
- // TODO: It is a bit unclear why `1` is used here instead of
- // `patchRange.patchNum`. Maybe that is a bug? Maybe if one patchset
- // is a merge commit, then all patchsets are merge commits??
- const isMerge = revisionInfo.isMergeCommit(1 as PatchSetNumber);
- const preferFirst =
- this.prefs &&
- this.prefs.default_base_for_merges === DefaultBase.FIRST_PARENT;
-
- // Verified via reportExecution that -1 is returned(1-5 times per day)
- // changeChanged does set this.patchRange?.patchNum so it's still unclear
- // how it is undefined.
- if (isMerge && preferFirst && !this.patchRange?.patchNum) {
- return -1 as BasePatchSetNum;
- }
- return PARENT;
- }
-
private computeChangeUrl(forceReload?: boolean) {
if (!this.change) return undefined;
return createChangeUrl({
@@ -2358,13 +2095,13 @@
// Private but used in tests.
handleDiffAgainstBase() {
assertIsDefined(this.change, 'change');
- assertIsDefined(this.patchRange, 'patchRange');
- if (this.patchRange.basePatchNum === PARENT) {
+ assertIsDefined(this.patchNum, 'patchNum');
+ if (this.basePatchNum === PARENT) {
fireAlert(this, 'Base is already selected.');
return;
}
this.getNavigation().setUrl(
- createChangeUrl({change: this.change, patchNum: this.patchRange.patchNum})
+ createChangeUrl({change: this.change, patchNum: this.patchNum})
);
}
@@ -2372,16 +2109,16 @@
handleDiffBaseAgainstLeft() {
if (this.viewState?.childView !== ChangeChildView.OVERVIEW) return;
assertIsDefined(this.change, 'change');
- assertIsDefined(this.patchRange, 'patchRange');
+ assertIsDefined(this.patchNum, 'patchNum');
- if (this.patchRange.basePatchNum === PARENT) {
+ if (this.basePatchNum === PARENT) {
fireAlert(this, 'Left is already base.');
return;
}
this.getNavigation().setUrl(
createChangeUrl({
change: this.change,
- patchNum: this.patchRange.basePatchNum as RevisionPatchSetNum,
+ patchNum: this.basePatchNum as RevisionPatchSetNum,
})
);
}
@@ -2389,9 +2126,9 @@
// Private but used in tests.
handleDiffAgainstLatest() {
assertIsDefined(this.change, 'change');
- assertIsDefined(this.patchRange, 'patchRange');
+ assertIsDefined(this.patchNum, 'patchNum');
const latestPatchNum = computeLatestPatchNum(this.allPatchSets);
- if (this.patchRange.patchNum === latestPatchNum) {
+ if (this.patchNum === latestPatchNum) {
fireAlert(this, 'Latest is already selected.');
return;
}
@@ -2399,7 +2136,7 @@
createChangeUrl({
change: this.change,
patchNum: latestPatchNum,
- basePatchNum: this.patchRange.basePatchNum,
+ basePatchNum: this.basePatchNum,
})
);
}
@@ -2407,9 +2144,9 @@
// Private but used in tests.
handleDiffRightAgainstLatest() {
assertIsDefined(this.change, 'change');
- assertIsDefined(this.patchRange, 'patchRange');
+ assertIsDefined(this.patchNum, 'patchNum');
const latestPatchNum = computeLatestPatchNum(this.allPatchSets);
- if (this.patchRange.patchNum === latestPatchNum) {
+ if (this.patchNum === latestPatchNum) {
fireAlert(this, 'Right is already latest.');
return;
}
@@ -2417,7 +2154,7 @@
createChangeUrl({
change: this.change,
patchNum: latestPatchNum,
- basePatchNum: this.patchRange.patchNum as BasePatchSetNum,
+ basePatchNum: this.patchNum as BasePatchSetNum,
})
);
}
@@ -2425,12 +2162,9 @@
// Private but used in tests.
handleDiffBaseAgainstLatest() {
assertIsDefined(this.change, 'change');
- assertIsDefined(this.patchRange, 'patchRange');
+ assertIsDefined(this.patchNum, 'patchNum');
const latestPatchNum = computeLatestPatchNum(this.allPatchSets);
- if (
- this.patchRange.patchNum === latestPatchNum &&
- this.patchRange.basePatchNum === PARENT
- ) {
+ if (this.patchNum === latestPatchNum && this.basePatchNum === PARENT) {
fireAlert(this, 'Already diffing base against latest.');
return;
}
@@ -2524,179 +2258,11 @@
// TODO(wyatta) switch linkify sequence, see issue 5526.
// This is a zero-with space. It is added to prevent the linkify library
// from including R= or CC= as part of the email address.
+ // TODO: Is this comment referring to the ba-linkify library that we are
+ // not using anymore? If so, then remove this hack.
return msg.replace(REVIEWERS_REGEX, '$1=\u200B');
}
- /**
- * Utility function to make the necessary modifications to a change in the
- * case an edit exists.
- * Private but used in tests.
- */
- processEdit(change: ParsedChangeInfo) {
- const revisions = Object.values(change.revisions || {});
- const editRev = findEdit(revisions);
- const editParentRev = findEditParentRevision(revisions);
- if (
- !editRev &&
- this.patchRange?.patchNum === EDIT &&
- changeIsOpen(change)
- ) {
- fireAlert(this, 'Change edit not found. Please create a change edit.');
- fireReload(this, true);
- return;
- }
-
- if (
- !editRev &&
- (changeIsMerged(change) || changeIsAbandoned(change)) &&
- this.getEditMode()
- ) {
- fireAlert(
- this,
- 'Change edits cannot be created if change is merged or abandoned. Redirecting to non edit mode.'
- );
- fireReload(this, true);
- return;
- }
-
- if (!editRev) return;
- assertIsDefined(this.patchRange, 'patchRange');
- assertIsDefined(editRev.commit.commit, 'editRev.commit.commit');
- assertIsDefined(editParentRev, 'editParentRev');
-
- const latestPsNum = computeLatestPatchNum(computeAllPatchSets(change));
- // If the change was loaded without a specific patchset, then this normally
- // means that the *latest* patchset should be loaded. But if there is an
- // active edit, then automatically switch to that edit as the current
- // patchset.
- // TODO: This goes together with `change.current_revision` being set, which
- // is under change-model control. `patchRange.patchNum` should eventually
- // also be model managed, so we can reconcile these two code snippets into
- // one location.
- if (!this.viewModelPatchNum && latestPsNum === editParentRev._number) {
- this.patchRange = {...this.patchRange, patchNum: EDIT};
- // The file list is not reactive (yet) with regards to patch range
- // changes, so we have to actively trigger it.
- this.reloadPatchNumDependentResources();
- }
- }
-
- computeRevertSubmitted(change?: ChangeInfo | ParsedChangeInfo) {
- if (!change?.messages) return;
- Promise.all(
- getRevertCreatedChangeIds(change.messages).map(changeId =>
- this.restApiService.getChange(changeId)
- )
- ).then(changes => {
- // if a change is deleted then getChanges returns null for that changeId
- changes = changes.filter(
- change => change && change.status !== ChangeStatus.ABANDONED
- );
- if (!changes.length) return;
- const submittedRevert = changes.find(
- change => change?.status === ChangeStatus.MERGED
- );
- if (!this.changeStatuses) return;
- // Protect against `computeRevertSubmitted()` being called twice.
- // TODO: Convert this to be rxjs based, so computeRevertSubmitted() is not
- // actively called, but instead we can subscribe to something.
- if (this.changeStatuses.includes(ChangeStates.REVERT_SUBMITTED)) return;
- if (this.changeStatuses.includes(ChangeStates.REVERT_CREATED)) return;
- if (submittedRevert) {
- this.revertedChange = submittedRevert;
- this.changeStatuses = this.changeStatuses.concat([
- ChangeStates.REVERT_SUBMITTED,
- ]);
- } else {
- if (changes[0]) this.revertedChange = changes[0];
- this.changeStatuses = this.changeStatuses.concat([
- ChangeStates.REVERT_CREATED,
- ]);
- }
- });
- }
-
- private async untilModelLoaded() {
- // NOTE: Wait until this page is connected before determining whether the
- // model is loaded. This can happen when viewState changes when setting up
- // this view. It's unclear whether this issue is related to Polymer
- // specifically.
- if (!this.isConnected) {
- await until(this.connected$, connected => connected);
- }
- await until(
- this.getChangeModel().changeLoadingStatus$,
- status => status === LoadingStatus.LOADED
- );
- }
-
- /**
- * Process edits
- * Check if a revert of this change has been submitted
- * Calculate selected revision
- */
- // private but used in tests
- async performPostChangeLoadTasks() {
- assertIsDefined(this.changeNum, 'changeNum');
-
- const prefCompletes = this.restApiService.getPreferences();
- await this.untilModelLoaded();
-
- this.prefs = await prefCompletes;
-
- if (!this.change) return false;
-
- this.processEdit(this.change);
- // Issue 4190: Coalesce missing topics to null.
- // TODO(TS): code needs second thought,
- // it might be that nulls were assigned to trigger some bindings
- if (!this.change.topic) {
- this.change.topic = null as unknown as undefined;
- }
- if (!this.change.reviewer_updates) {
- this.change.reviewer_updates = null as unknown as undefined;
- }
- const latestRevisionSha = this.getLatestRevisionSHA(this.change);
- if (!latestRevisionSha)
- throw new Error('Could not find latest Revision Sha');
- const currentRevision = this.change.revisions[latestRevisionSha];
- if (currentRevision.commit && currentRevision.commit.message) {
- this.latestCommitMessage = this.prepareCommitMsgForLinkify(
- currentRevision.commit.message
- );
- } else {
- this.latestCommitMessage = null;
- }
-
- this.computeRevertSubmitted(this.change);
- if (
- !this.patchRange ||
- !this.patchRange.patchNum ||
- this.patchRange.patchNum === currentRevision._number
- ) {
- // CommitInfo.commit is optional, and may need patching.
- if (currentRevision.commit && !currentRevision.commit.commit) {
- currentRevision.commit.commit = latestRevisionSha as CommitId;
- }
- this.commitInfo = currentRevision.commit;
- this.selectedRevision = currentRevision;
- // TODO: Fetch and process files.
- } else {
- if (!this.change?.revisions || !this.patchRange) return false;
- this.selectedRevision = Object.values(this.change.revisions).find(
- revision => {
- // edit patchset is a special one
- const thePatchNum = this.patchRange!.patchNum;
- if (thePatchNum === EDIT) {
- return revision._number === thePatchNum;
- }
- return revision._number === Number(`${thePatchNum}`);
- }
- );
- }
- return true;
- }
-
private isParentCurrent() {
const revisionActions = this.currentRevisionActions;
if (revisionActions && revisionActions.rebase) {
@@ -2706,226 +2272,35 @@
}
}
- // Private but used in tests.
- getLatestCommitMessage() {
- assertIsDefined(this.changeNum, 'changeNum');
- const lastpatchNum = computeLatestPatchNum(this.allPatchSets);
- if (lastpatchNum === undefined)
- throw new Error('missing lastPatchNum property');
- return this.restApiService
- .getChangeCommitInfo(this.changeNum, lastpatchNum)
- .then(commitInfo => {
- if (!commitInfo) return;
- this.latestCommitMessage = this.prepareCommitMsgForLinkify(
- commitInfo.message
- );
- });
+ private async reportChangeDisplayed() {
+ await waitUntil(() => !!this.metadata);
+ await untilRendered(this.metadata!);
+ await waitUntil(() => !!this.fileList);
+ await untilRendered(this.fileList!);
+ await waitUntil(() => !!this.messagesList);
+ await untilRendered(this.messagesList!);
+ // We are ending the timer after each change view update, because ending a
+ // timer that was not started is a no-op. :-)
+ if (this.change && this.isConnected && !this.isChangeObsolete()) {
+ this.reporting.changeDisplayed(roleDetails(this.change, this.account));
+ }
}
- // Private but used in tests.
- getLatestRevisionSHA(change: ChangeInfo | ParsedChangeInfo) {
- if (change.current_revision) return change.current_revision;
- // current_revision may not be present in the case where the latest rev is
- // a draft and the user doesn’t have permission to view that rev.
- let latestRev = null;
- let latestPatchNum = -1 as PatchSetNum;
- for (const [rev, revInfo] of Object.entries(change.revisions ?? {})) {
- if (revInfo._number > latestPatchNum) {
- latestRev = rev;
- latestPatchNum = revInfo._number;
- }
+ private async reportFullyLoaded() {
+ await waitUntil(() => !!this.metadata);
+ await untilRendered(this.metadata!);
+ await waitUntil(() => !!this.fileList);
+ await untilRendered(this.fileList!);
+ await waitUntil(() => !!this.messagesList);
+ await untilRendered(this.messagesList!);
+ await waitUntil(() => this.mergeable !== undefined);
+ await until(this.getCommentsModel().comments$, c => c !== undefined);
+ await until(this.getCommentsModel().drafts$, c => c !== undefined);
+ // We are ending the timer after each change view update, because ending a
+ // timer that was not started is a no-op. :-)
+ if (this.change && this.isConnected && !this.isChangeObsolete()) {
+ this.reporting.changeFullyLoaded();
}
- return latestRev;
- }
-
- // visible for testing
- loadAndSetCommitInfo() {
- assertIsDefined(this.changeNum, 'changeNum');
- assertIsDefined(this.patchRange?.patchNum, 'patchRange.patchNum');
- return this.restApiService
- .getChangeCommitInfo(this.changeNum, this.patchRange.patchNum)
- .then(commitInfo => {
- this.commitInfo = commitInfo;
- });
- }
-
- /**
- * Reload the change.
- *
- * @param isLocationChange Reloads the related changes
- * when true and ends reporting events that started on location change.
- * @param clearPatchset Reloads the change ignoring any patchset
- * choice made.
- * @return A promise that resolves when the core data has loaded.
- * Some non-core data loading may still be in-flight when the core data
- * promise resolves.
- */
- loadData(isLocationChange?: boolean, clearPatchset?: boolean) {
- if (this.isChangeObsolete()) return Promise.resolve();
- if (clearPatchset && this.change) {
- this.getNavigation().setUrl(
- createChangeUrl({change: this.change, forceReload: true})
- );
- return Promise.resolve();
- }
- this.loading = true;
- this.reporting.time(Timing.CHANGE_RELOAD);
- this.reporting.time(Timing.CHANGE_DATA);
-
- // Array to house all promises related to data requests.
- const allDataPromises: Promise<unknown>[] = [];
-
- // Resolves when the change detail and the edit patch set (if available)
- // are loaded.
- const detailCompletes = this.untilModelLoaded();
- allDataPromises.push(detailCompletes);
-
- // Resolves when the loading flag is set to false, meaning that some
- // change content may start appearing.
- const loadingFlagSet = detailCompletes.then(() => {
- this.loading = false;
- this.performPostChangeLoadTasks();
- });
-
- let coreDataPromise;
-
- // If the patch number is specified
- if (this.patchRange && this.patchRange.patchNum) {
- // Because a specific patchset is specified, reload the resources that
- // are keyed by patch number or patch range.
- const patchResourcesLoaded = this.reloadPatchNumDependentResources();
- allDataPromises.push(patchResourcesLoaded);
-
- // Promise resolves when the change detail and patch dependent resources
- // have loaded.
- coreDataPromise = Promise.all([patchResourcesLoaded, loadingFlagSet]);
- } else {
- const latestCommitMessageLoaded = loadingFlagSet.then(() => {
- // If the latest commit message is known, there is nothing to do.
- if (this.latestCommitMessage) {
- return Promise.resolve();
- }
- return this.getLatestCommitMessage();
- });
- allDataPromises.push(latestCommitMessageLoaded);
-
- coreDataPromise = loadingFlagSet;
- }
- const mergeabilityLoaded = coreDataPromise.then(() =>
- this.getMergeability()
- );
- allDataPromises.push(mergeabilityLoaded);
-
- coreDataPromise.then(() => {
- fire(this, 'change-details-loaded', {});
- this.reporting.timeEnd(Timing.CHANGE_RELOAD);
- if (isLocationChange) {
- this.reporting.changeDisplayed(roleDetails(this.change, this.account));
- }
- });
-
- if (isLocationChange) {
- this.editingCommitMessage = false;
- }
- const relatedChangesLoaded = coreDataPromise.then(() => {
- let relatedChangesPromise:
- | Promise<RelatedChangesInfo | undefined>
- | undefined;
- const patchNum = computeLatestPatchNum(this.allPatchSets);
- if (this.change && patchNum) {
- relatedChangesPromise = this.restApiService
- .getRelatedChanges(this.change._number, patchNum)
- .then(response => {
- if (this.change && response) {
- this.hasParent = this.calculateHasParent(
- this.change.change_id,
- response.changes
- );
- }
- return response;
- });
- }
- return this.getRelatedChangesList()?.reload(relatedChangesPromise);
- });
- allDataPromises.push(relatedChangesLoaded);
- allDataPromises.push(this.filesLoaded());
-
- Promise.all(allDataPromises).then(() => {
- // Loading of commments data is no longer part of this reporting
- this.reporting.timeEnd(Timing.CHANGE_DATA);
- if (isLocationChange) {
- this.reporting.changeFullyLoaded();
- }
- });
-
- return coreDataPromise;
- }
-
- private async filesLoaded() {
- if (!this.isConnected) await until(this.connected$, connected => connected);
- await until(this.getFilesModel().files$, f => f.length > 0);
- }
-
- /**
- * Determines whether or not the given change has a parent change. If there
- * is a relation chain, and the change id is not the last item of the
- * relation chain, there is a parent.
- *
- * Private but used in tests.
- */
- calculateHasParent(
- currentChangeId: ChangeId,
- relatedChanges: RelatedChangeAndCommitInfo[]
- ) {
- return (
- relatedChanges.length > 0 &&
- relatedChanges[relatedChanges.length - 1].change_id !== currentChangeId
- );
- }
-
- /**
- * Kicks off requests for resources that rely on the patch range
- * (`this.patchRange`) being defined.
- */
- reloadPatchNumDependentResources() {
- return this.loadAndSetCommitInfo();
- }
-
- // Private but used in tests
- getMergeability(): Promise<void> {
- if (!this.change) {
- this.mergeable = null;
- return Promise.resolve();
- }
- // If the change is closed, it is not mergeable. Note: already merged
- // changes are obviously not mergeable, but the mergeability API will not
- // answer for abandoned changes.
- if (
- this.change.status === ChangeStatus.MERGED ||
- this.change.status === ChangeStatus.ABANDONED
- ) {
- this.mergeable = false;
- return Promise.resolve();
- }
-
- if (!this.changeNum) {
- return Promise.reject(new Error('missing required changeNum property'));
- }
-
- // If mergeable bit was already returned in detail REST endpoint, use it.
- if (this.change.mergeable !== undefined) {
- this.mergeable = this.change.mergeable;
- return Promise.resolve();
- }
-
- this.mergeable = null;
- return this.restApiService
- .getMergeable(this.changeNum)
- .then(mergableInfo => {
- if (mergableInfo) {
- this.mergeable = mergableInfo.mergeable;
- }
- });
}
/**
@@ -2941,13 +2316,11 @@
);
}
- private computeCommitCollapsible() {
- if (!this.latestCommitMessage) {
- return false;
- }
+ private computeCommitCollapsible(): boolean {
return (
+ !!this.latestCommitMessage &&
this.latestCommitMessage.split('\n').length >=
- MIN_LINES_FOR_COMMIT_COLLAPSE
+ MIN_LINES_FOR_COMMIT_COLLAPSE
);
}
@@ -3008,7 +2381,7 @@
dismissOnNavigation: true,
showDismiss: true,
action: 'Reload',
- callback: () => fireReload(this, true),
+ callback: () => this.getChangeModel().navigateToChangeResetReload(),
});
});
}, this.serverConfig.change.update_delay * 1000);
@@ -3047,7 +2420,7 @@
);
if (!controls) throw new Error('Missing edit controls');
assertIsDefined(this.change, 'change');
- assertIsDefined(this.patchRange, 'patchRange');
+ assertIsDefined(this.patchNum, 'patchNum');
const path = e.detail.path;
switch (e.detail.action) {
@@ -3055,12 +2428,12 @@
controls.openDeleteDialog(path);
break;
case GrEditConstants.Actions.OPEN.id:
- assertIsDefined(this.patchRange.patchNum, 'patchset number');
+ assertIsDefined(this.patchNum, 'patchset number');
this.getNavigation().setUrl(
createEditUrl({
changeNum: this.change._number,
repo: this.change.project,
- patchNum: this.patchRange.patchNum,
+ patchNum: this.patchNum,
editView: {path},
})
);
@@ -3074,21 +2447,6 @@
}
}
- private patchNumChanged() {
- if (!this.selectedRevision || !this.patchRange?.patchNum) {
- return;
- }
- assertIsDefined(this.change, 'change');
-
- if (this.patchRange.patchNum === this.selectedRevision._number) {
- return;
- }
- if (!this.change.revisions) return;
- this.selectedRevision = Object.values(this.change.revisions).find(
- revision => revision._number === this.patchRange!.patchNum
- );
- }
-
/**
* If an edit exists already, load it. Otherwise, toggle edit mode via the
* navigation API.
@@ -3118,11 +2476,11 @@
private handleStopEditTap() {
assertIsDefined(this.change, 'change');
- assertIsDefined(this.patchRange, 'patchRange');
+ assertIsDefined(this.patchNum, 'patchNum');
this.getNavigation().setUrl(
createChangeUrl({
change: this.change,
- patchNum: this.patchRange.patchNum,
+ patchNum: this.patchNum,
forceReload: true,
})
);
@@ -3152,17 +2510,6 @@
fire(this, 'hide-alert', {});
}
- private getRevisionInfo(): RevisionInfoClass | undefined {
- if (this.change === undefined) return undefined;
- return new RevisionInfoClass(this.change);
- }
-
- getRelatedChangesList() {
- return this.shadowRoot!.querySelector<GrRelatedChangesList>(
- '#relatedChanges'
- );
- }
-
createTitle(shortcutName: Shortcut, section: ShortcutSection) {
return this.getShortcutsService().createTitle(shortcutName, section);
}
@@ -3177,7 +2524,6 @@
declare global {
interface HTMLElementEventMap {
'toggle-star': CustomEvent<ChangeStarToggleStarDetail>;
- 'change-details-loaded': CustomEvent<{}>;
}
interface HTMLElementTagNameMap {
'gr-change-view': GrChangeView;
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 21b163f..5331558 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
@@ -10,9 +10,7 @@
import {
ChangeStatus,
CommentSide,
- DefaultBase,
DiffViewMode,
- MessageTag,
createDefaultPreferences,
Tab,
} from '../../../constants/constants';
@@ -25,64 +23,43 @@
queryAndAssert,
stubFlags,
stubRestApi,
- waitEventLoop,
waitUntil,
waitUntilVisible,
} from '../../../test/test-utils';
import {
createChangeViewState,
- createApproval,
- createChange,
createChangeMessages,
- createCommit,
- createMergeable,
- createPreferences,
createRevision,
createRevisions,
createServerInfo,
createUserConfig,
TEST_NUMERIC_CHANGE_ID,
TEST_PROJECT_NAME,
- createEditRevision,
- createAccountWithIdNameAndEmail,
createChangeViewChange,
- createRelatedChangeAndCommitInfo,
createAccountDetailWithId,
createParsedChange,
} from '../../../test/test-data-generators';
import {GrChangeView} from './gr-change-view';
import {
AccountId,
- ApprovalInfo,
BasePatchSetNum,
- ChangeId,
- ChangeInfo,
CommitId,
EDIT,
NumericChangeId,
PARENT,
- RelatedChangeAndCommitInfo,
- ReviewInputTag,
- RevisionInfo,
RevisionPatchSetNum,
RobotId,
RobotCommentInfo,
Timestamp,
UrlEncodedCommentId,
- DetailedLabelInfo,
RepoName,
- QuickLabelInfo,
- PatchSetNumber,
CommentThread,
- ChangeStates,
SavingState,
} from '../../../types/common';
import {GrEditControls} from '../../edit/gr-edit-controls/gr-edit-controls';
-import {SinonFakeTimers, SinonStubbedMember} from 'sinon';
-import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
+import {SinonFakeTimers} from 'sinon';
import {GerritView} from '../../../services/router/router-model';
import {ParsedChangeInfo} from '../../../types/types';
-import {GrRelatedChangesList} from '../gr-related-changes-list/gr-related-changes-list';
import {
ChangeModel,
changeModelToken,
@@ -92,9 +69,7 @@
import {GrChangeStar} from '../../shared/gr-change-star/gr-change-star';
import {GrThreadList} from '../gr-thread-list/gr-thread-list';
import {assertIsDefined} from '../../../utils/common-util';
-import {DEFAULT_NUM_FILES_SHOWN} from '../gr-file-list/gr-file-list';
import {fixture, html, assert} from '@open-wc/testing';
-import {deepClone} from '../../../utils/deep-util';
import {Modifier} from '../../../utils/dom-util';
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrCopyLinks} from '../gr-copy-links/gr-copy-links';
@@ -103,6 +78,7 @@
import {testResolver} from '../../../test/common-test-setup';
import {UserModel, userModelToken} from '../../../models/user/user-model';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
+import {commentsModelToken} from '../../../models/comments/comments-model';
suite('gr-change-view tests', () => {
let element: GrChangeView;
@@ -457,8 +433,7 @@
</gr-endpoint-decorator>
</div>
<div class="relatedChanges">
- <gr-related-changes-list id="relatedChanges">
- </gr-related-changes-list>
+ <gr-related-changes-list></gr-related-changes-list>
</div>
<div class="emptySpace"></div>
</div>
@@ -544,10 +519,7 @@
test('handleMessageAnchorTap', async () => {
element.changeNum = 1 as NumericChangeId;
- element.patchRange = {
- basePatchNum: PARENT,
- patchNum: 1 as RevisionPatchSetNum,
- };
+ element.patchNum = 1 as RevisionPatchSetNum;
element.change = createChangeViewChange();
await element.updateComplete;
const replaceStateStub = sinon.stub(history, 'replaceState');
@@ -563,10 +535,8 @@
...createChangeViewChange(),
revisions: createRevisions(10),
};
- element.patchRange = {
- patchNum: 3 as RevisionPatchSetNum,
- basePatchNum: 1 as BasePatchSetNum,
- };
+ element.basePatchNum = 1 as BasePatchSetNum;
+ element.patchNum = 3 as RevisionPatchSetNum;
element.handleDiffAgainstBase();
assert.isTrue(setUrlStub.called);
assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/3');
@@ -577,10 +547,8 @@
...createChangeViewChange(),
revisions: createRevisions(10),
};
- element.patchRange = {
- basePatchNum: 1 as BasePatchSetNum,
- patchNum: 3 as RevisionPatchSetNum,
- };
+ element.basePatchNum = 1 as BasePatchSetNum;
+ element.patchNum = 3 as RevisionPatchSetNum;
element.handleDiffAgainstLatest();
assert.isTrue(setUrlStub.called);
assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/1..10');
@@ -591,10 +559,8 @@
...createChangeViewChange(),
revisions: createRevisions(10),
};
- element.patchRange = {
- patchNum: 3 as RevisionPatchSetNum,
- basePatchNum: 1 as BasePatchSetNum,
- };
+ element.basePatchNum = 1 as BasePatchSetNum;
+ element.patchNum = 3 as RevisionPatchSetNum;
element.handleDiffBaseAgainstLeft();
assert.isTrue(setUrlStub.called);
assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/1');
@@ -605,10 +571,8 @@
...createChangeViewChange(),
revisions: createRevisions(10),
};
- element.patchRange = {
- basePatchNum: 1 as BasePatchSetNum,
- patchNum: 3 as RevisionPatchSetNum,
- };
+ element.basePatchNum = 1 as BasePatchSetNum;
+ element.patchNum = 3 as RevisionPatchSetNum;
element.handleDiffRightAgainstLatest();
assert.isTrue(setUrlStub.called);
assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/3..10');
@@ -619,10 +583,8 @@
...createChangeViewChange(),
revisions: createRevisions(10),
};
- element.patchRange = {
- basePatchNum: 1 as BasePatchSetNum,
- patchNum: 3 as RevisionPatchSetNum,
- };
+ element.basePatchNum = 1 as BasePatchSetNum;
+ element.patchNum = 3 as RevisionPatchSetNum;
element.handleDiffBaseAgainstLatest();
assert.isTrue(setUrlStub.called);
assert.equal(setUrlStub.lastCall.firstArg, '/c/test-project/+/42/10');
@@ -640,10 +602,8 @@
const removeFromAttentionSetStub = stubRestApi(
'removeFromAttentionSet'
).returns(Promise.resolve(new Response()));
- element.patchRange = {
- basePatchNum: 1 as BasePatchSetNum,
- patchNum: 3 as RevisionPatchSetNum,
- };
+ element.basePatchNum = 1 as BasePatchSetNum;
+ element.patchNum = 3 as RevisionPatchSetNum;
await element.updateComplete;
assert.isNotOk(element.change.attention_set);
element.handleToggleAttentionSet();
@@ -698,19 +658,6 @@
assert.equal(element.activeTab, 'change-view-tab-header-url');
});
- test('param change should switch primary tab correctly', async () => {
- assert.equal(element.activeTab, Tab.FILES);
- // view is required
- element.changeNum = undefined;
- element.viewState = {
- ...createChangeViewState(),
- ...element.viewState,
- tab: Tab.COMMENT_THREADS,
- };
- await element.updateComplete;
- assert.equal(element.activeTab, Tab.COMMENT_THREADS);
- });
-
test('invalid param change should not switch primary tab', async () => {
assert.equal(element.activeTab, Tab.FILES);
// view is required
@@ -815,8 +762,6 @@
stubRestApi('getChangeDetail').returns(
Promise.resolve(createParsedChange())
);
- sinon.stub(element, 'performPostChangeLoadTasks');
- sinon.stub(element, 'getMergeability');
const change = {
...createChangeViewChange(),
revisions: createRevisions(1),
@@ -946,10 +891,8 @@
suite('thread list and change log tabs', () => {
setup(() => {
element.changeNum = TEST_NUMERIC_CHANGE_ID;
- element.patchRange = {
- basePatchNum: PARENT,
- patchNum: 1 as RevisionPatchSetNum,
- };
+ element.basePatchNum = PARENT;
+ element.patchNum = 1 as RevisionPatchSetNum;
element.change = {
...createChangeViewChange(),
revisions: {
@@ -969,12 +912,6 @@
},
},
};
- const relatedChanges = element.shadowRoot!.querySelector(
- '#relatedChanges'
- ) as GrRelatedChangesList;
- sinon.stub(relatedChanges, 'reload');
- sinon.stub(element, 'loadData').returns(Promise.resolve());
- sinon.spy(element, 'viewStateChanged');
element.viewState = createChangeViewState();
});
});
@@ -1154,183 +1091,6 @@
);
});
- test('changeStatuses', async () => {
- element.loading = false;
- element.change = {
- ...createChangeViewChange(),
- revisions: {
- rev2: createRevision(2),
- rev1: createRevision(1),
- rev13: createRevision(13),
- rev3: createRevision(3),
- },
- current_revision: 'rev3' as CommitId,
- status: ChangeStatus.MERGED,
- labels: {
- test: {
- all: [],
- default_value: 0,
- values: {},
- approved: {},
- },
- },
- };
- element.mergeable = true;
- await element.updateComplete;
- const expectedStatuses = [ChangeStates.MERGED];
- assert.deepEqual(element.changeStatuses, expectedStatuses);
- const statusChips =
- element.shadowRoot!.querySelectorAll('gr-change-status');
- assert.equal(statusChips.length, 1);
- });
-
- suite('ChangeStatus revert', () => {
- test('do not show any chip if no revert created', async () => {
- const change = {
- ...createParsedChange(),
- messages: createChangeMessages(2),
- };
- const getChangeStub = stubRestApi('getChange');
- getChangeStub.onFirstCall().returns(
- Promise.resolve({
- ...createChange(),
- })
- );
- getChangeStub.onSecondCall().returns(
- Promise.resolve({
- ...createChange(),
- })
- );
- element.change = change;
- element.mergeable = true;
- element.currentRevisionActions = {submit: {enabled: true}};
- assert.isTrue(element.isSubmitEnabled());
- await element.updateComplete;
- element.computeRevertSubmitted(element.change);
- await element.updateComplete;
- assert.isFalse(
- element.changeStatuses?.includes(ChangeStates.REVERT_SUBMITTED)
- );
- assert.isFalse(
- element.changeStatuses?.includes(ChangeStates.REVERT_CREATED)
- );
- });
-
- test('do not show any chip if all reverts are abandoned', async () => {
- const change = {
- ...createParsedChange(),
- messages: createChangeMessages(2),
- };
- change.messages[0].message = 'Created a revert of this change as 12345';
- change.messages[0].tag = MessageTag.TAG_REVERT as ReviewInputTag;
-
- change.messages[1].message = 'Created a revert of this change as 23456';
- change.messages[1].tag = MessageTag.TAG_REVERT as ReviewInputTag;
-
- const getChangeStub = stubRestApi('getChange');
- getChangeStub.onFirstCall().returns(
- Promise.resolve({
- ...createChange(),
- status: ChangeStatus.ABANDONED,
- })
- );
- getChangeStub.onSecondCall().returns(
- Promise.resolve({
- ...createChange(),
- status: ChangeStatus.ABANDONED,
- })
- );
- element.change = change;
- element.mergeable = true;
- element.currentRevisionActions = {submit: {enabled: true}};
- assert.isTrue(element.isSubmitEnabled());
- await element.updateComplete;
- element.computeRevertSubmitted(element.change);
- await element.updateComplete;
- assert.isFalse(
- element.changeStatuses?.includes(ChangeStates.REVERT_SUBMITTED)
- );
- assert.isFalse(
- element.changeStatuses?.includes(ChangeStates.REVERT_CREATED)
- );
- });
-
- test('show revert created if no revert is merged', async () => {
- const change = {
- ...createParsedChange(),
- messages: createChangeMessages(2),
- };
- change.messages[0].message = 'Created a revert of this change as 12345';
- change.messages[0].tag = MessageTag.TAG_REVERT as ReviewInputTag;
-
- change.messages[1].message = 'Created a revert of this change as 23456';
- change.messages[1].tag = MessageTag.TAG_REVERT as ReviewInputTag;
-
- const getChangeStub = stubRestApi('getChange');
- getChangeStub.onFirstCall().returns(
- Promise.resolve({
- ...createChange(),
- })
- );
- getChangeStub.onSecondCall().returns(
- Promise.resolve({
- ...createChange(),
- })
- );
- element.change = change;
- element.mergeable = true;
- element.currentRevisionActions = {submit: {enabled: true}};
- assert.isTrue(element.isSubmitEnabled());
- await element.updateComplete;
- element.computeRevertSubmitted(element.change);
- // Wait for promises to settle.
- await waitEventLoop();
- await element.updateComplete;
- assert.isFalse(
- element.changeStatuses?.includes(ChangeStates.REVERT_SUBMITTED)
- );
- assert.isTrue(
- element.changeStatuses?.includes(ChangeStates.REVERT_CREATED)
- );
- });
-
- test('show revert submitted if revert is merged', async () => {
- const change = {
- ...createParsedChange(),
- messages: createChangeMessages(2),
- };
- change.messages[0].message = 'Created a revert of this change as 12345';
- change.messages[0].tag = MessageTag.TAG_REVERT as ReviewInputTag;
- const getChangeStub = stubRestApi('getChange');
- getChangeStub.onFirstCall().returns(
- Promise.resolve({
- ...createChange(),
- status: ChangeStatus.MERGED,
- })
- );
- getChangeStub.onSecondCall().returns(
- Promise.resolve({
- ...createChange(),
- })
- );
- element.change = change;
- element.mergeable = true;
- element.currentRevisionActions = {submit: {enabled: true}};
- assert.isTrue(element.isSubmitEnabled());
- await element.updateComplete;
- element.computeRevertSubmitted(element.change);
- // Wait for promises to settle.
- await waitEventLoop();
- await element.updateComplete;
- assert.isFalse(
- element.changeStatuses?.includes(ChangeStates.REVERT_CREATED)
- );
- assert.isTrue(
- element.changeStatuses?.includes(ChangeStates.REVERT_SUBMITTED)
- );
- });
- });
-
test('diff preferences open when open-diff-prefs is fired', async () => {
await element.updateComplete;
assertIsDefined(element.fileList);
@@ -1368,66 +1128,6 @@
assert.isTrue(element.isSubmitEnabled());
});
- test('reload is called when an approved label is removed', async () => {
- const vote: ApprovalInfo = {
- ...createApproval(),
- _account_id: 1 as AccountId,
- name: 'bojack',
- value: 1,
- };
- element.changeNum = TEST_NUMERIC_CHANGE_ID;
- element.patchRange = {
- basePatchNum: PARENT,
- patchNum: 1 as RevisionPatchSetNum,
- };
- const change = {
- ...createParsedChange(),
- owner: createAccountWithIdNameAndEmail(),
- revisions: {
- rev2: createRevision(2),
- rev1: createRevision(1),
- rev13: createRevision(13),
- rev3: createRevision(3),
- },
- current_revision: 'rev3' as CommitId,
- status: ChangeStatus.NEW,
- labels: {
- test: {
- all: [vote],
- default_value: 0,
- values: {},
- approved: {},
- },
- },
- };
- element.change = change;
- await element.updateComplete;
- const reloadStub = sinon.stub(element, 'loadData');
- const newChange = {...element.change};
- (newChange.labels!.test! as DetailedLabelInfo).all = [];
- element.change = deepClone(newChange);
- await element.updateComplete;
- assert.isFalse(reloadStub.called);
-
- assert.isDefined(element.change);
- const testLabels: DetailedLabelInfo & QuickLabelInfo =
- newChange.labels!.test;
- assertIsDefined(testLabels);
- testLabels.all!.push(vote);
- testLabels.all!.push(vote);
- testLabels.approved = vote;
- element.change = deepClone(newChange);
- await element.updateComplete;
- assert.isFalse(reloadStub.called);
-
- assert.isDefined(element.change);
- (newChange.labels!.test! as DetailedLabelInfo).all = [];
- element.change = deepClone(newChange);
- await element.updateComplete;
- assert.isTrue(reloadStub.called);
- assert.isTrue(reloadStub.calledOnce);
- });
-
test('reply button has updated count when there are drafts', () => {
const getLabel = (canReview: boolean) => {
element.change!.actions!.ready = {enabled: canReview};
@@ -1448,134 +1148,6 @@
assert.equal(getLabel(true), 'Start Review (3)');
});
- test('change num change', async () => {
- const change = {
- ...createChangeViewChange(),
- labels: {},
- } as ParsedChangeInfo;
- element.changeNum = undefined;
- element.patchRange = {
- basePatchNum: PARENT,
- patchNum: 2 as RevisionPatchSetNum,
- };
- element.change = change;
- assertIsDefined(element.fileList);
- assert.equal(element.fileList.numFilesShown, DEFAULT_NUM_FILES_SHOWN);
- element.fileList.numFilesShown = 150;
- element.fileList.selectedIndex = 15;
- await element.updateComplete;
-
- element.changeNum = 2 as NumericChangeId;
- element.viewState = {
- ...createChangeViewState(),
- changeNum: 2 as NumericChangeId,
- };
- await element.updateComplete;
- assert.equal(element.fileList.numFilesShown, DEFAULT_NUM_FILES_SHOWN);
- assert.equal(element.fileList.selectedIndex, 0);
- });
-
- test('don’t reload entire page when patchRange changes', async () => {
- const reloadStub = sinon
- .stub(element, 'loadData')
- .callsFake(() => Promise.resolve());
- const reloadPatchDependentStub = sinon
- .stub(element, 'reloadPatchNumDependentResources')
- .callsFake(() => Promise.resolve());
- assertIsDefined(element.fileList);
- await element.fileList.updateComplete;
- const collapseStub = sinon.stub(element.fileList, 'collapseAllDiffs');
- const value: ChangeViewState = {
- ...createChangeViewState(),
- view: GerritView.CHANGE,
- patchNum: 1 as RevisionPatchSetNum,
- };
- element.changeNum = undefined;
- element.viewState = value;
- await element.updateComplete;
- assert.isTrue(reloadStub.calledOnce);
-
- element.initialLoadComplete = true;
- element.fileList.selectedIndex = 15;
- element.change = {
- ...createChangeViewChange(),
- revisions: {
- rev1: createRevision(1),
- rev2: createRevision(2),
- },
- };
-
- value.basePatchNum = 1 as BasePatchSetNum;
- value.patchNum = 2 as RevisionPatchSetNum;
- element.viewState = {...value};
- await element.updateComplete;
- await waitEventLoop();
- assert.equal(element.fileList.selectedIndex, 0);
- assert.isFalse(reloadStub.calledTwice);
- assert.isTrue(reloadPatchDependentStub.calledOnce);
- assert.isTrue(collapseStub.calledTwice);
- });
-
- test('do not reload entire page when patchRange doesnt change', async () => {
- assertIsDefined(element.fileList);
- const reloadStub = sinon
- .stub(element, 'loadData')
- .callsFake(() => Promise.resolve());
- const collapseStub = sinon.stub(element.fileList, 'collapseAllDiffs');
- const value: ChangeViewState = createChangeViewState();
- element.viewState = value;
- // change already loaded
- assert.isOk(element.changeNum);
- await element.updateComplete;
- assert.isFalse(reloadStub.calledOnce);
- element.initialLoadComplete = true;
- element.viewState = {...value};
- await element.updateComplete;
- assert.isFalse(reloadStub.calledTwice);
- assert.isFalse(collapseStub.calledTwice);
- });
-
- test('forceReload updates the change', async () => {
- assertIsDefined(element.fileList);
- const getChangeStub = stubRestApi('getChangeDetail').returns(
- Promise.resolve(createParsedChange())
- );
- const loadDataStub = sinon
- .stub(element, 'loadData')
- .callsFake(() => Promise.resolve());
- const collapseStub = sinon.stub(element.fileList, 'collapseAllDiffs');
- element.viewState = {...createChangeViewState(), forceReload: true};
- await element.updateComplete;
- assert.isTrue(getChangeStub.called);
- assert.isTrue(loadDataStub.called);
- assert.isTrue(collapseStub.called);
- // patchNum is set by changeChanged, so this verifies that change was set.
- assert.isOk(element.patchRange?.patchNum);
- });
-
- test('related changes are updated when loadData is called', async () => {
- await element.updateComplete;
- const relatedChanges = element.shadowRoot!.querySelector(
- '#relatedChanges'
- ) as GrRelatedChangesList;
- const reloadStub = sinon.stub(relatedChanges, 'reload');
- stubRestApi('getMergeable').returns(
- Promise.resolve({...createMergeable(), mergeable: true})
- );
-
- element.viewState = createChangeViewState();
- changeModel.setState({
- loadingStatus: LoadingStatus.LOADED,
- change: {
- ...createChangeViewChange(),
- },
- });
-
- await element.loadData(true);
- assert.isFalse(setUrlStub.called);
- assert.isTrue(reloadStub.called);
- });
-
test('computeCopyTextForTitle', () => {
element.change = {
...createChangeViewChange(),
@@ -1593,26 +1165,6 @@
);
});
- test('get latest revision', () => {
- let change: ChangeInfo = {
- ...createChange(),
- revisions: {
- rev1: createRevision(1),
- rev3: createRevision(3),
- },
- current_revision: 'rev3' as CommitId,
- };
- assert.equal(element.getLatestRevisionSHA(change), 'rev3');
- change = {
- ...createChange(),
- revisions: {
- rev1: createRevision(1),
- },
- current_revision: undefined,
- };
- assert.equal(element.getLatestRevisionSHA(change), 'rev1');
- });
-
test('show commit message edit button', () => {
const change = createParsedChange();
const mergedChanged: ParsedChangeInfo = {
@@ -1635,6 +1187,7 @@
});
test('handleCommitMessageSave trims trailing whitespace', async () => {
+ element.changeNum = TEST_NUMERIC_CHANGE_ID;
element.change = createChangeViewChange();
// Response code is 500, because we want to avoid window reloading
const putStub = stubRestApi('putChangeCommitMessage').returns(
@@ -1655,85 +1208,6 @@
assert.equal(putStub.lastCall.args[1], '\n\n\n\n\n\n\n\n');
});
- test('topic is coalesced to null', async () => {
- sinon.stub(element, 'changeChanged');
- changeModel.setState({
- loadingStatus: LoadingStatus.LOADED,
- change: {
- ...createChangeViewChange(),
- labels: {},
- current_revision: 'foo' as CommitId,
- revisions: {foo: createRevision()},
- },
- });
-
- await element.performPostChangeLoadTasks();
- assert.isNull(element.change!.topic);
- });
-
- test('commit sha is populated from getChangeDetail', async () => {
- changeModel.setState({
- loadingStatus: LoadingStatus.LOADED,
- change: {
- ...createChangeViewChange(),
- labels: {},
- current_revision: 'foo' as CommitId,
- revisions: {foo: createRevision()},
- },
- });
-
- await element.performPostChangeLoadTasks();
- assert.equal('foo', element.commitInfo!.commit);
- });
-
- test('getBasePatchNum', async () => {
- element.change = {
- ...createChangeViewChange(),
- revisions: {
- '98da160735fb81604b4c40e93c368f380539dd0e': createRevision(),
- },
- };
- element.patchRange = {
- basePatchNum: PARENT,
- };
- await element.updateComplete;
- assert.equal(element.getBasePatchNum(), PARENT);
-
- element.prefs = {
- ...createPreferences(),
- default_base_for_merges: DefaultBase.FIRST_PARENT,
- };
-
- element.change = {
- ...createChangeViewChange(),
- revisions: {
- '98da160735fb81604b4c40e93c368f380539dd0e': {
- ...createRevision(1),
- commit: {
- ...createCommit(),
- parents: [
- {
- commit: '6e12bdf1176eb4ab24d8491ba3b6d0704409cde8' as CommitId,
- subject: 'test',
- },
- {
- commit: '22f7db4754b5d9816fc581f3d9a6c0ef8429c841' as CommitId,
- subject: 'test3',
- },
- ],
- },
- },
- },
- };
- await element.updateComplete;
- assert.equal(element.getBasePatchNum(), -1 as BasePatchSetNum);
-
- element.patchRange.basePatchNum = PARENT;
- element.patchRange.patchNum = 1 as RevisionPatchSetNum;
- await element.updateComplete;
- assert.equal(element.getBasePatchNum(), PARENT);
- });
-
test('openReplyDialog called with `ANY` when coming from tap event', async () => {
await element.updateComplete;
assertIsDefined(element.replyBtn);
@@ -1787,10 +1261,8 @@
.stub(testResolver(pluginLoaderToken), 'awaitPluginsLoaded')
.callsFake(() => Promise.resolve());
- element.patchRange = {
- basePatchNum: PARENT,
- patchNum: 2 as RevisionPatchSetNum,
- };
+ element.basePatchNum = PARENT;
+ element.patchNum = 2 as RevisionPatchSetNum;
element.change = {
...createChangeViewChange(),
revisions: {
@@ -1854,14 +1326,18 @@
});
test('maybeScrollToMessage', async () => {
+ element.change = {
+ ...createChangeViewChange(),
+ messages: createChangeMessages(1),
+ };
await element.updateComplete;
const scrollStub = sinon.stub(element.messagesList!, 'scrollToMessage');
- element.maybeScrollToMessage('');
+ await element.maybeScrollToMessage('');
assert.isFalse(scrollStub.called);
- element.maybeScrollToMessage('message');
+ await element.maybeScrollToMessage('message');
assert.isFalse(scrollStub.called);
- element.maybeScrollToMessage('#message-TEST');
+ await element.maybeScrollToMessage('#message-TEST');
assert.isTrue(scrollStub.called);
assert.equal(scrollStub.lastCall.args[0], 'TEST');
});
@@ -1869,6 +1345,8 @@
test('computeEditMode', async () => {
const callCompute = async (viewState: ChangeViewState) => {
element.viewState = viewState;
+ element.patchNum = viewState.patchNum;
+ element.basePatchNum = viewState.basePatchNum ?? PARENT;
await element.updateComplete;
return element.getEditMode();
};
@@ -1896,46 +1374,8 @@
);
});
- test('processEdit', () => {
- element.patchRange = {};
- const change: ParsedChangeInfo = {
- ...createChangeViewChange(),
- current_revision: 'foo' as CommitId,
- revisions: {
- foo: {...createRevision()},
- },
- };
-
- // With no edit, nothing happens.
- element.processEdit(change);
- assert.equal(element.patchRange.patchNum, undefined);
-
- change.revisions['bar'] = {
- _number: EDIT,
- basePatchNum: 1 as BasePatchSetNum,
- commit: {
- ...createCommit(),
- commit: 'bar' as CommitId,
- },
- fetch: {},
- };
-
- // When edit is set, but not patchNum, then switch to edit ps.
- element.processEdit(change);
- assert.equal(element.patchRange.patchNum, EDIT);
-
- // When edit is set, but patchNum as well, then keep patchNum.
- element.patchRange.patchNum = 5 as RevisionPatchSetNum;
- element.viewModelPatchNum = 5 as RevisionPatchSetNum;
- element.processEdit(change);
- assert.equal(element.patchRange.patchNum, 5 as RevisionPatchSetNum);
- });
-
test('file-action-tap handling', async () => {
- element.patchRange = {
- basePatchNum: PARENT,
- patchNum: 1 as RevisionPatchSetNum,
- };
+ element.patchNum = 1 as RevisionPatchSetNum;
element.change = {
...createChangeViewChange(),
};
@@ -2006,109 +1446,6 @@
assert.isTrue(setUrlStub.called);
});
- test('selectedRevision updates when patchNum is changed', async () => {
- const revision1: RevisionInfo = createRevision(1);
- const revision2: RevisionInfo = createRevision(2);
- changeModel.setState({
- loadingStatus: LoadingStatus.LOADED,
- change: {
- ...createChangeViewChange(),
- revisions: {
- aaa: revision1,
- bbb: revision2,
- },
- labels: {},
- actions: {},
- current_revision: 'bbb' as CommitId,
- },
- });
- userModel.setPreferences(createPreferences());
-
- element.patchRange = {patchNum: 2 as RevisionPatchSetNum};
- await element.performPostChangeLoadTasks();
- assert.strictEqual(element.selectedRevision, revision2);
-
- element.patchRange = {patchNum: 1 as RevisionPatchSetNum};
- await element.updateComplete;
- assert.strictEqual(element.selectedRevision, revision1);
- });
-
- test('selectedRevision is assigned when patchNum is edit', async () => {
- const revision1 = createRevision(1);
- const revision2 = createRevision(2);
- const revision3 = createEditRevision();
- changeModel.setState({
- loadingStatus: LoadingStatus.LOADED,
- change: {
- ...createChangeViewChange(),
- revisions: {
- aaa: revision1,
- bbb: revision2,
- ccc: revision3,
- },
- labels: {},
- actions: {},
- current_revision: 'ccc' as CommitId,
- },
- });
- stubRestApi('getPreferences').returns(Promise.resolve(createPreferences()));
-
- element.patchRange = {patchNum: EDIT};
- await element.performPostChangeLoadTasks();
- assert.strictEqual(element.selectedRevision, revision3);
- });
-
- test('sendShowChangeEvent', () => {
- const change = {...createChangeViewChange(), labels: {}};
- element.change = {...change};
- element.patchRange = {patchNum: 4 as RevisionPatchSetNum};
- element.mergeable = true;
- const showStub = sinon.stub(
- testResolver(pluginLoaderToken).jsApiService,
- 'handleShowChange'
- );
- element.sendShowChangeEvent();
- assert.isTrue(showStub.calledOnce);
- assert.deepEqual(showStub.lastCall.args[0], {
- change,
- patchNum: 4 as PatchSetNumber,
- info: {mergeable: true},
- });
- });
-
- test('patch range changed', () => {
- element.patchRange = undefined;
- element.change = createChangeViewChange();
- element.change.revisions = createRevisions(4);
- element.change.current_revision = '1' as CommitId;
- element.change = {...element.change};
-
- const viewState = createChangeViewState();
-
- assert.isFalse(element.hasPatchRangeChanged(viewState));
- assert.isFalse(element.hasPatchNumChanged(viewState));
-
- viewState.basePatchNum = PARENT;
- // undefined means navigate to latest patchset
- viewState.patchNum = undefined;
-
- element.patchRange = {
- patchNum: 2 as RevisionPatchSetNum,
- basePatchNum: PARENT,
- };
-
- assert.isTrue(element.hasPatchRangeChanged(viewState));
- assert.isTrue(element.hasPatchNumChanged(viewState));
-
- element.patchRange = {
- patchNum: 4 as RevisionPatchSetNum,
- basePatchNum: PARENT,
- };
-
- assert.isFalse(element.hasPatchRangeChanged(viewState));
- assert.isFalse(element.hasPatchNumChanged(viewState));
- });
-
suite('handleEditTap', () => {
let fireEdit: () => void;
@@ -2157,7 +1494,7 @@
const newChange = {...element.change};
newChange.revisions.rev2 = createRevision(2);
element.change = newChange;
- element.patchRange = {patchNum: 2 as RevisionPatchSetNum};
+ element.patchNum = 2 as RevisionPatchSetNum;
await element.updateComplete;
fireEdit();
@@ -2178,7 +1515,7 @@
assertIsDefined(element.actions);
sinon.stub(element.metadata, 'computeLabelNames');
- element.patchRange = {patchNum: 1 as RevisionPatchSetNum};
+ element.patchNum = 1 as RevisionPatchSetNum;
element.actions.dispatchEvent(
new CustomEvent('stop-edit-tap', {bubbles: false})
);
@@ -2193,7 +1530,7 @@
suite('plugin endpoints', () => {
test('endpoint params', async () => {
element.change = {...createChangeViewChange(), labels: {}};
- element.selectedRevision = createRevision();
+ element.revision = createRevision();
const promise = mockPromise();
window.Gerrit.install(
promise.resolve,
@@ -2207,43 +1544,7 @@
.getLastAttached();
assert.strictEqual((hookEl as any).plugin, plugin);
assert.strictEqual((hookEl as any).change, element.change);
- assert.strictEqual((hookEl as any).revision, element.selectedRevision);
- });
- });
-
- suite('getMergeability', () => {
- let getMergeableStub: SinonStubbedMember<RestApiService['getMergeable']>;
- setup(() => {
- element.change = {...createChangeViewChange(), labels: {}};
- getMergeableStub = stubRestApi('getMergeable').returns(
- Promise.resolve({...createMergeable(), mergeable: true})
- );
- });
-
- test('merged change', () => {
- element.mergeable = null;
- element.change!.status = ChangeStatus.MERGED;
- return element.getMergeability().then(() => {
- assert.isFalse(element.mergeable);
- assert.isFalse(getMergeableStub.called);
- });
- });
-
- test('abandoned change', () => {
- element.mergeable = null;
- element.change!.status = ChangeStatus.ABANDONED;
- return element.getMergeability().then(() => {
- assert.isFalse(element.mergeable);
- assert.isFalse(getMergeableStub.called);
- });
- });
-
- test('open change', () => {
- element.mergeable = null;
- return element.getMergeability().then(() => {
- assert.isTrue(element.mergeable);
- assert.isTrue(getMergeableStub.called);
- });
+ assert.strictEqual((hookEl as any).revision, element.revision);
});
});
@@ -2265,18 +1566,8 @@
suite('gr-reporting tests', () => {
setup(() => {
- element.patchRange = {
- basePatchNum: PARENT,
- patchNum: 1 as RevisionPatchSetNum,
- };
- sinon
- .stub(element, 'performPostChangeLoadTasks')
- .returns(Promise.resolve(false));
- sinon.stub(element, 'getMergeability').returns(Promise.resolve());
- sinon.stub(element, 'getLatestCommitMessage').returns(Promise.resolve());
- sinon
- .stub(element, 'reloadPatchNumDependentResources')
- .returns(Promise.resolve());
+ element.basePatchNum = PARENT;
+ element.patchNum = 1 as RevisionPatchSetNum;
});
test("don't report changeDisplayed on reply", async () => {
@@ -2294,7 +1585,8 @@
assert.isFalse(changeFullyLoadedStub.called);
});
- test('report changeDisplayed on viewStateChanged', async () => {
+ test('report changeDisplayed and changeFullyLoaded', async () => {
+ const commentsModel = testResolver(commentsModelToken);
stubRestApi('getChangeOrEditFiles').resolves({
'a-file.js': {},
});
@@ -2322,32 +1614,23 @@
revisions: {foo: createRevision()},
},
});
- await element.updateComplete;
- await waitEventLoop();
+
+ await waitUntil(() => changeDisplayStub.called);
assert.isTrue(changeDisplayStub.called);
+ assert.isFalse(changeFullyLoadedStub.called);
+
+ element.mergeable = true;
+ commentsModel.setState({
+ comments: {},
+ drafts: {},
+ discardedDrafts: [],
+ });
+
+ await waitUntil(() => changeFullyLoadedStub.called);
assert.isTrue(changeFullyLoadedStub.called);
});
});
- test('calculateHasParent', () => {
- const changeId = '123' as ChangeId;
- const relatedChanges: RelatedChangeAndCommitInfo[] = [];
-
- assert.equal(element.calculateHasParent(changeId, relatedChanges), false);
-
- relatedChanges.push({
- ...createRelatedChangeAndCommitInfo(),
- change_id: '123' as ChangeId,
- });
- assert.equal(element.calculateHasParent(changeId, relatedChanges), false);
-
- relatedChanges.push({
- ...createRelatedChangeAndCommitInfo(),
- change_id: '234' as ChangeId,
- });
- assert.equal(element.calculateHasParent(changeId, relatedChanges), true);
- });
-
test('renders sha in copy links', async () => {
stubFlags('isEnabled').returns(true);
const sha = '123' as CommitId;
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
index 166d895..6ad416e 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
@@ -28,8 +28,9 @@
import {fireNoBubbleNoCompose} from '../../../utils/event-util';
import {resolve} from '../../../models/dependency';
import {changeModelToken} from '../../../models/change/change-model';
-import {subscribe} from '../../lit/subscription-controller';
import {userModelToken} from '../../../models/user/user-model';
+import {relatedChangesModelToken} from '../../../models/change/related-changes-model';
+import {subscribe} from '../../lit/subscription-controller';
export interface RebaseChange {
name: string;
@@ -63,12 +64,6 @@
@property({type: String})
branch?: BranchName;
- @property({type: Number})
- changeNumber?: NumericChangeId;
-
- @property({type: Boolean})
- hasParent?: boolean;
-
@property({type: Boolean})
rebaseOnCurrent?: boolean;
@@ -76,6 +71,12 @@
disableActions = false;
@state()
+ changeNum?: NumericChangeId;
+
+ @state()
+ hasParent?: boolean;
+
+ @state()
text = '';
@state()
@@ -91,16 +92,16 @@
allowConflicts = false;
@query('#rebaseOnParentInput')
- private rebaseOnParentInput!: HTMLInputElement;
+ private rebaseOnParentInput?: HTMLInputElement;
@query('#rebaseOnTipInput')
- private rebaseOnTipInput!: HTMLInputElement;
+ private rebaseOnTipInput?: HTMLInputElement;
@query('#rebaseOnOtherInput')
- rebaseOnOtherInput!: HTMLInputElement;
+ rebaseOnOtherInput?: HTMLInputElement;
@query('#rebaseAllowConflicts')
- private rebaseAllowConflicts!: HTMLInputElement;
+ private rebaseAllowConflicts?: HTMLInputElement;
@query('#rebaseChain')
private rebaseChain?: HTMLInputElement;
@@ -120,6 +121,11 @@
private readonly getUserModel = resolve(this, userModelToken);
+ private readonly getRelatedChangesModel = resolve(
+ this,
+ relatedChangesModelToken
+ );
+
constructor() {
super();
this.query = input => this.getChangeSuggestions(input);
@@ -133,6 +139,16 @@
() => this.getChangeModel().latestUploader$,
x => (this.uploader = x)
);
+ subscribe(
+ this,
+ () => this.getChangeModel().changeNum$,
+ x => (this.changeNum = x)
+ );
+ subscribe(
+ this,
+ () => this.getRelatedChangesModel().hasParent$,
+ x => (this.hasParent = x)
+ );
}
override willUpdate(changedProperties: PropertyValues): void {
@@ -199,6 +215,9 @@
Rebase on parent change
</label>
</div>
+ <div class="message" ?hidden=${this.hasParent !== undefined}>
+ Still loading parent information ...
+ </div>
<div
id="parentUpToDateMsg"
class="message"
@@ -361,8 +380,7 @@
): AutocompleteSuggestion[] {
return changes
.filter(
- change =>
- change.name.includes(input) && change.value !== this.changeNumber
+ change => change.name.includes(input) && change.value !== this.changeNum
)
.map(
change =>
@@ -393,10 +411,10 @@
* should be rebased on top of its current parent.
*/
getSelectedBase() {
- if (this.rebaseOnParentInput.checked) {
+ if (this.rebaseOnParentInput?.checked) {
return null;
}
- if (this.rebaseOnTipInput.checked) {
+ if (this.rebaseOnTipInput?.checked) {
return '';
}
if (!this.text) {
@@ -412,7 +430,7 @@
e.stopPropagation();
const detail: ConfirmRebaseEventDetail = {
base: this.getSelectedBase(),
- allowConflicts: this.rebaseAllowConflicts.checked,
+ allowConflicts: !!this.rebaseAllowConflicts?.checked,
rebaseChain: !!this.rebaseChain?.checked,
onBehalfOfUploader: this.rebaseOnBehalfOfUploader(),
};
@@ -437,7 +455,7 @@
}
private handleEnterChangeNumberClick() {
- this.rebaseOnOtherInput.checked = true;
+ if (this.rebaseOnOtherInput) this.rebaseOnOtherInput.checked = true;
}
/**
@@ -451,11 +469,11 @@
}
if (this.displayParentOption()) {
- this.rebaseOnParentInput.checked = true;
+ if (this.rebaseOnParentInput) this.rebaseOnParentInput.checked = true;
} else if (this.displayTipOption()) {
- this.rebaseOnTipInput.checked = true;
+ if (this.rebaseOnTipInput) this.rebaseOnTipInput.checked = true;
} else {
- this.rebaseOnOtherInput.checked = true;
+ if (this.rebaseOnOtherInput) this.rebaseOnOtherInput.checked = true;
}
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.ts
index 8d907c9..24f8a34 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.ts
@@ -44,6 +44,7 @@
test('render', async () => {
element.branch = 'test' as BranchName;
+ element.hasParent = false;
await element.updateComplete;
assert.shadowDom.equal(
element,
@@ -60,6 +61,9 @@
Rebase on parent change
</label>
</div>
+ <div class="message" hidden="">
+ Still loading parent information ...
+ </div>
<div class="message" hidden="" id="parentUpToDateMsg">
This change is up to date with its parent.
</div>
@@ -239,7 +243,7 @@
element.hasParent = false;
await element.updateComplete;
- assert.isTrue(element.rebaseOnOtherInput.checked);
+ assert.isTrue(element.rebaseOnOtherInput?.checked);
assert.isTrue(
queryAndAssert(element, '#rebaseOnParent').hasAttribute('hidden')
);
@@ -360,7 +364,7 @@
assert.equal(element.filterChanges('awesome', recentChanges).length, 3);
assert.equal(element.filterChanges('third', recentChanges).length, 1);
- element.changeNumber = 123 as NumericChangeId;
+ element.changeNum = 123 as NumericChangeId;
await element.updateComplete;
assert.equal(element.filterChanges('123', recentChanges).length, 0);
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index c206d57..62693ea 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -826,6 +826,13 @@
override willUpdate(changedProperties: PropertyValues): void {
if (
+ changedProperties.has('patchNum') ||
+ changedProperties.has('basePatchNum')
+ ) {
+ this.resetFileState();
+ this.collapseAllDiffs();
+ }
+ if (
changedProperties.has('diffPrefs') ||
changedProperties.has('diffViewMode')
) {
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
index 9a069a4..ac087cef 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.ts
@@ -46,6 +46,7 @@
shortcutsServiceToken,
} from '../../../services/shortcuts/shortcuts-service';
import {GrFormattedText} from '../../shared/gr-formatted-text/gr-formatted-text';
+import {waitUntil} from '../../../utils/async-util';
/**
* The content of the enum is also used in the UI for the button text.
@@ -439,6 +440,9 @@
}
async scrollToMessage(messageID: string) {
+ await waitUntil(() => this.messages.length > 0);
+ await this.updateComplete;
+
const selector = `[data-message-id="${messageID}"]`;
const el = this.shadowRoot!.querySelector(selector) as
| GrMessage
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.ts b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.ts
index 0217aa8..df04f68 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.ts
@@ -195,9 +195,9 @@
}
});
- test('expand/collapse from external keypress', () => {
+ test('expand/collapse from external keypress', async () => {
// Start with one expanded message. -> not all collapsed
- element.scrollToMessage(messages[1].id);
+ await element.scrollToMessage(messages[1].id);
assert.isFalse(
[...getMessages()].filter(m => m.message?.expanded).length === 0
);
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
index 05acb29..6bdabec 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
@@ -11,31 +11,26 @@
import '../../shared/gr-icon/gr-icon';
import {classMap} from 'lit/directives/class-map.js';
import {LitElement, css, html, TemplateResult} from 'lit';
-import {customElement, property, state} from 'lit/decorators.js';
+import {customElement, state} from 'lit/decorators.js';
import {sharedStyles} from '../../../styles/shared-styles';
import {
ChangeInfo,
CommitId,
PatchSetNumber,
RelatedChangeAndCommitInfo,
- RelatedChangesInfo,
RevisionPatchSetNum,
SubmittedTogetherInfo,
} from '../../../types/common';
-import {getAppContext} from '../../../services/app-context';
import {ParsedChangeInfo} from '../../../types/types';
import {truncatePath} from '../../../utils/path-list-util';
import {pluralize} from '../../../utils/string-util';
-import {
- changeIsOpen,
- getChangeNumber,
- getRevisionKey,
-} from '../../../utils/change-util';
+import {getChangeNumber, getRevisionKey} from '../../../utils/change-util';
import {DEFALT_NUM_CHANGES_WHEN_COLLAPSED} from './gr-related-collapse';
import {createChangeUrl} from '../../../models/views/change';
import {subscribe} from '../../lit/subscription-controller';
import {resolve} from '../../../models/dependency';
import {changeModelToken} from '../../../models/change/change-model';
+import {relatedChangesModelToken} from '../../../models/change/related-changes-model';
export interface ChangeMarkersInList {
showCurrentChangeArrow: boolean;
@@ -54,9 +49,6 @@
@customElement('gr-related-changes-list')
export class GrRelatedChangesList extends LitElement {
- @property({type: Boolean})
- mergeable?: boolean;
-
@state()
change?: ParsedChangeInfo;
@@ -81,10 +73,13 @@
@state()
sameTopicChanges: ChangeInfo[] = [];
- private readonly restApiService = getAppContext().restApiService;
-
private readonly getChangeModel = resolve(this, changeModelToken);
+ private readonly getRelatedChangesModel = resolve(
+ this,
+ relatedChangesModelToken
+ );
+
constructor() {
super();
subscribe(
@@ -97,6 +92,31 @@
() => this.getChangeModel().latestPatchNum$,
x => (this.latestPatchNum = x)
);
+ subscribe(
+ this,
+ () => this.getRelatedChangesModel().relatedChanges$,
+ x => (this.relatedChanges = x ?? [])
+ );
+ subscribe(
+ this,
+ () => this.getRelatedChangesModel().submittedTogether$,
+ x => (this.submittedTogether = x)
+ );
+ subscribe(
+ this,
+ () => this.getRelatedChangesModel().cherryPicks$,
+ x => (this.cherryPickChanges = x ?? [])
+ );
+ subscribe(
+ this,
+ () => this.getRelatedChangesModel().conflictingChanges$,
+ x => (this.conflictingChanges = x ?? [])
+ );
+ subscribe(
+ this,
+ () => this.getRelatedChangesModel().sameTopicChanges$,
+ x => (this.sameTopicChanges = x ?? [])
+ );
}
static override get styles() {
@@ -586,72 +606,6 @@
return html`<span class="marker space"></span>`;
}
- reload(getRelatedChanges?: Promise<RelatedChangesInfo | undefined>) {
- const change = this.change;
- if (!change) return Promise.reject(new Error('change missing'));
- if (!this.latestPatchNum)
- return Promise.reject(new Error('latestPatchNum missing'));
- if (!getRelatedChanges) {
- getRelatedChanges = this.restApiService.getRelatedChanges(
- change._number,
- this.latestPatchNum
- );
- }
- const promises: Array<Promise<void>> = [
- getRelatedChanges.then(response => {
- if (!response) {
- throw new Error('getRelatedChanges returned undefined response');
- }
- this.relatedChanges = response?.changes ?? [];
- }),
- this.restApiService
- .getChangesSubmittedTogether(change._number)
- .then(response => {
- this.submittedTogether = response;
- }),
- this.restApiService
- .getChangeCherryPicks(change.project, change.change_id, change._number)
- .then(response => {
- this.cherryPickChanges = response || [];
- }),
- ];
-
- // Get conflicts if change is open and is mergeable.
- // Mergeable is output of restApiServict.getMergeable from gr-change-view
- if (changeIsOpen(change) && this.mergeable) {
- promises.push(
- this.restApiService
- .getChangeConflicts(change._number)
- .then(response => {
- this.conflictingChanges = response ?? [];
- })
- );
- }
- if (change.topic) {
- const changeTopic = change.topic;
- promises.push(
- this.restApiService.getConfig().then(config => {
- if (config && !config.change.submit_whole_topic) {
- return this.restApiService
- .getChangesWithSameTopic(changeTopic, {
- openChangesOnly: true,
- changeToExclude: change._number,
- })
- .then(response => {
- if (changeTopic === this.change?.topic) {
- this.sameTopicChanges = response ?? [];
- }
- });
- }
- this.sameTopicChanges = [];
- return Promise.resolve();
- })
- );
- }
-
- return Promise.all(promises);
- }
-
/**
* Do the given objects describe the same change? Compares the changes by
* their numbers.
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
index 571b5b5..24a8217 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list_test.ts
@@ -4,10 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {fixture, html, assert} from '@open-wc/testing';
-import {SinonStubbedMember} from 'sinon';
import {PluginApi} from '../../../api/plugin';
-import {ChangeStatus} from '../../../constants/constants';
-import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
import '../../../test/common-test-setup';
import {testResolver} from '../../../test/common-test-setup';
import {
@@ -19,12 +16,7 @@
createRevision,
createSubmittedTogetherInfo,
} from '../../../test/test-data-generators';
-import {
- query,
- queryAndAssert,
- stubRestApi,
- waitEventLoop,
-} from '../../../test/test-utils';
+import {query, queryAndAssert, waitEventLoop} from '../../../test/test-utils';
import {
ChangeId,
ChangeInfo,
@@ -196,16 +188,10 @@
});
test('render', async () => {
- stubRestApi('getRelatedChanges').returns(
- Promise.resolve(relatedChangeInfo)
- );
- stubRestApi('getChangesSubmittedTogether').returns(
- Promise.resolve(submittedTogether)
- );
- stubRestApi('getChangeCherryPicks').returns(
- Promise.resolve([createChange()])
- );
- await element.reload();
+ element.relatedChanges = relatedChangeInfo.changes;
+ element.submittedTogether = submittedTogether;
+ element.cherryPickChanges = [createChange()];
+ await element.updateComplete;
assert.shadowDom.equal(
element,
@@ -262,10 +248,9 @@
});
test('first list', async () => {
- stubRestApi('getRelatedChanges').returns(
- Promise.resolve(relatedChangeInfo)
- );
- await element.reload();
+ element.relatedChanges = relatedChangeInfo.changes;
+ await element.updateComplete;
+
const section = queryAndAssert<HTMLElement>(element, '#relatedChanges');
const relatedChanges = queryAndAssert<GrRelatedCollapse>(
section,
@@ -275,13 +260,10 @@
});
test('first empty second non-empty', async () => {
- stubRestApi('getRelatedChanges').returns(
- Promise.resolve(createRelatedChangesInfo())
- );
- stubRestApi('getChangesSubmittedTogether').returns(
- Promise.resolve(submittedTogether)
- );
- await element.reload();
+ element.relatedChanges = createRelatedChangesInfo().changes;
+ element.submittedTogether = submittedTogether;
+ await element.updateComplete;
+
const relatedChanges = query<HTMLElement>(element, '#relatedChanges');
assert.notExists(relatedChanges);
const submittedTogetherSection = queryAndAssert<GrRelatedCollapse>(
@@ -292,16 +274,10 @@
});
test('first non-empty second empty third non-empty', async () => {
- stubRestApi('getRelatedChanges').returns(
- Promise.resolve(relatedChangeInfo)
- );
- stubRestApi('getChangesSubmittedTogether').returns(
- Promise.resolve(createSubmittedTogetherInfo())
- );
- stubRestApi('getChangeCherryPicks').returns(
- Promise.resolve([createChange()])
- );
- await element.reload();
+ element.relatedChanges = relatedChangeInfo.changes;
+ element.submittedTogether = createSubmittedTogetherInfo();
+ element.cherryPickChanges = [createChange()];
+ await element.updateComplete;
const relatedChanges = queryAndAssert<GrRelatedCollapse>(
queryAndAssert<HTMLElement>(element, '#relatedChanges'),
@@ -364,67 +340,6 @@
assert.equal(getChangeNumber(change2), 1);
});
- suite('get conflicts tests', () => {
- let element: GrRelatedChangesList;
- let conflictsStub: SinonStubbedMember<RestApiService['getChangeConflicts']>;
-
- setup(async () => {
- element = await fixture(
- html`<gr-related-changes-list></gr-related-changes-list>`
- );
- conflictsStub = stubRestApi('getChangeConflicts').returns(
- Promise.resolve(undefined)
- );
- });
-
- test('request conflicts if open and mergeable', () => {
- element.latestPatchNum = 7 as PatchSetNumber;
- element.change = {
- ...createParsedChange(),
- change_id: '123' as ChangeId,
- status: ChangeStatus.NEW,
- };
- element.mergeable = true;
- element.reload();
- assert.isTrue(conflictsStub.called);
- });
-
- test('does not request conflicts if closed and mergeable', () => {
- element.latestPatchNum = 7 as PatchSetNumber;
- element.change = {
- ...createParsedChange(),
- change_id: '123' as ChangeId,
- status: ChangeStatus.NEW,
- };
- element.reload();
- assert.isFalse(conflictsStub.called);
- });
-
- test('does not request conflicts if open and not mergeable', () => {
- element.latestPatchNum = 7 as PatchSetNumber;
- element.change = {
- ...createParsedChange(),
- change_id: '123' as ChangeId,
- status: ChangeStatus.NEW,
- };
- element.mergeable = false;
- element.reload();
- assert.isFalse(conflictsStub.called);
- });
-
- test('doesnt request conflicts if closed and not mergeable', () => {
- element.latestPatchNum = 7 as PatchSetNumber;
- element.change = {
- ...createParsedChange(),
- change_id: '123' as ChangeId,
- status: ChangeStatus.NEW,
- };
- element.mergeable = false;
- element.reload();
- assert.isFalse(conflictsStub.called);
- });
- });
-
test('connected revisions', () => {
const change: ParsedChangeInfo = {
...createParsedChange(),
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index 2483a51..f41236b 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -88,7 +88,6 @@
fire,
fireNoBubble,
fireIronAnnounce,
- fireReload,
fireServerError,
} from '../../../utils/event-util';
import {ErrorCallback} from '../../../api/rest';
@@ -1338,6 +1337,7 @@
// visible for testing
async send(includeComments: boolean, startReview: boolean) {
+ // The change model will end this timing when the change was reloaded.
this.reporting.time(Timing.SEND_REPLY);
const labels = this.getLabelScores().getLabelValues();
@@ -1929,7 +1929,7 @@
}
_reload() {
- fireReload(this, true);
+ this.getChangeModel().navigateToChangeResetReload();
this.cancel();
}
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
index 2766114..98f65c0 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
@@ -327,7 +327,7 @@
review
)
.then(() => {
- fireReload(this, true);
+ fireReload(this);
});
}
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 304f9ea..aa6bb7a4 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -1326,10 +1326,10 @@
};
const queryMap = new URLSearchParams(ctx.querystring);
- if (queryMap.has('forceReload')) state.forceReload = true;
if (queryMap.has('openReplyDialog')) state.openReplyDialog = true;
const tab = queryMap.get('tab');
+ if (queryMap.has('forceReload')) state.forceReload = true;
if (tab) state.tab = tab;
const checksPatchset = Number(queryMap.get('checksPatchset'));
if (Number.isInteger(checksPatchset) && checksPatchset > 0) {
@@ -1422,6 +1422,8 @@
view: GerritView.CHANGE,
childView: ChangeChildView.OVERVIEW,
};
+ const queryMap = new URLSearchParams(ctx.querystring);
+ if (queryMap.has('forceReload')) state.forceReload = true;
assertIsDefined(state.repo);
this.reporting.setRepoName(state.repo);
this.reporting.setChangeId(changeNum);
@@ -1443,6 +1445,8 @@
childView: ChangeChildView.DIFF,
diffView: {path: ctx.params[8]},
};
+ const queryMap = new URLSearchParams(ctx.querystring);
+ if (queryMap.has('forceReload')) state.forceReload = true;
const address = this.parseLineAddress(ctx.hash);
if (address) {
state.diffView!.leftSide = address.leftSide;
@@ -1493,6 +1497,8 @@
childView: ChangeChildView.EDIT,
editView: {path: ctx.params[3], lineNum: Number(ctx.hash)},
};
+ const queryMap = new URLSearchParams(ctx.querystring);
+ if (queryMap.has('forceReload')) state.forceReload = true;
this.normalizePatchRangeParams(state);
// Note that router model view must be updated before view models.
this.setState(state);
@@ -1516,14 +1522,7 @@
};
const tab = queryMap.get('tab');
if (tab) state.tab = tab;
- if (queryMap.has('forceReload')) {
- state.forceReload = true;
- history.replaceState(
- null,
- '',
- location.href.replace(/[?&]forceReload=true/, '')
- );
- }
+ if (queryMap.has('forceReload')) state.forceReload = true;
this.normalizePatchRangeParams(state);
// Note that router model view must be updated before view models.
this.setState(state);
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 4687fc1..bdb634b 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
@@ -59,7 +59,7 @@
import {CommentSide, DiffViewMode, Side} from '../../../constants/constants';
import {GrApplyFixDialog} from '../gr-apply-fix-dialog/gr-apply-fix-dialog';
import {OpenFixPreviewEvent, ValueChangedEvent} from '../../../types/events';
-import {fireAlert, fire, fireTitleChange} from '../../../utils/event-util';
+import {fireAlert, fire} from '../../../utils/event-util';
import {assertIsDefined, queryAndAssert} from '../../../utils/common-util';
import {toggleClass, whenVisible} from '../../../utils/dom-util';
import {CursorMoveResult} from '../../../api/core';
@@ -115,12 +115,6 @@
@customElement('gr-diff-view')
export class GrDiffView extends LitElement {
/**
- * Fired when the title of the page should change.
- *
- * @event title-change
- */
-
- /**
* Fired when user tries to navigate away while comments are pending save.
*
* @event show-alert
@@ -187,21 +181,7 @@
@state()
files: Files = {sortedPaths: [], changeFilesByPath: {}};
- // Private but used in tests
- // Use path getter/setter.
- _path?: string;
-
- get path() {
- return this._path;
- }
-
- set path(path: string | undefined) {
- if (this._path === path) return;
- const oldPath = this._path;
- this._path = path;
- this.pathChanged();
- this.requestUpdate('path', oldPath);
- }
+ @state() path?: string;
/** Allows us to react when the user switches to the DIFF view. */
// Private but used in tests.
@@ -1404,12 +1384,6 @@
};
}
- private pathChanged() {
- if (this.path) {
- fireTitleChange(this, computeTruncatedPath(this.path));
- }
- }
-
// Private but used in tests
formatFilesForDropdown(): DropdownItem[] {
if (!this.files) return [];
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
index e1dfd1f..3082979 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
@@ -14,7 +14,6 @@
getParentIndex,
getRevisionByPatchNum,
isMergeParent,
- sortRevisions,
PatchSet,
convertToPatchSetNum,
} from '../../../utils/patch-set-util';
@@ -159,7 +158,7 @@
subscribe(
this,
() => this.getChangeModel().revisions$,
- x => (this.sortedRevisions = sortRevisions(Object.values(x || {})))
+ x => (this.sortedRevisions = x)
);
subscribe(
this,
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts
index 6dbdca6..a8bc84c 100644
--- a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts
+++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search.ts
@@ -45,7 +45,7 @@
override connectedCallback() {
super.connectedCallback();
- fireTitleChange(this, 'Documentation Search');
+ fireTitleChange('Documentation Search');
}
static override get styles() {
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
index 0fc754c..5786112 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
@@ -18,7 +18,7 @@
GrAutocomplete,
} from '../../shared/gr-autocomplete/gr-autocomplete';
import {getAppContext} from '../../../services/app-context';
-import {fireAlert, fireReload} from '../../../utils/event-util';
+import {fireAlert} from '../../../utils/event-util';
import {
assertIsDefined,
query as queryUtil,
@@ -34,6 +34,7 @@
import {modalStyles} from '../../../styles/gr-modal-styles';
import {whenVisible} from '../../../utils/dom-util';
import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
+import {changeModelToken} from '../../../models/change/change-model';
@customElement('gr-edit-controls')
export class GrEditControls extends LitElement {
@@ -77,6 +78,8 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getChangeModel = resolve(this, changeModelToken);
+
private readonly getNavigation = resolve(this, navigationToken);
static override get styles() {
@@ -451,7 +454,7 @@
return;
}
this.closeDialog(this.openDialog);
- fireReload(this, true);
+ this.getChangeModel().navigateToChangeResetReload();
});
}
@@ -471,7 +474,7 @@
return;
}
this.closeDialog(dialog);
- fireReload(this, true);
+ this.getChangeModel().navigateToChangeResetReload();
});
};
@@ -489,7 +492,7 @@
return;
}
this.closeDialog(dialog);
- fireReload(this, true);
+ this.getChangeModel().navigateToChangeResetReload();
});
};
@@ -507,7 +510,7 @@
return;
}
this.closeDialog(dialog);
- fireReload(this, true);
+ this.getChangeModel().navigateToChangeResetReload();
});
};
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
index 0729f21..0e6778a 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
@@ -26,8 +26,12 @@
import {fixture, html, assert} from '@open-wc/testing';
import {GrButton} from '../../shared/gr-button/gr-button';
import '../../shared/gr-dialog/gr-dialog';
-import {waitForEventOnce} from '../../../utils/event-util';
import {testResolver} from '../../../test/common-test-setup';
+import {
+ ChangeModel,
+ changeModelToken,
+} from '../../../models/change/change-model';
+import {SinonStubbedMember} from 'sinon';
suite('gr-edit-controls tests', () => {
let element: GrEditControls;
@@ -36,6 +40,9 @@
let closeDialogSpy: sinon.SinonSpy;
let hideDialogStub: sinon.SinonStub;
let queryStub: sinon.SinonStub;
+ let navigateResetStub: SinonStubbedMember<
+ ChangeModel['navigateToChangeResetReload']
+ >;
setup(async () => {
element = await fixture<GrEditControls>(html`
@@ -47,6 +54,10 @@
closeDialogSpy = sinon.spy(element, 'closeDialog');
hideDialogStub = sinon.stub(element, 'hideAllDialogs');
queryStub = stubRestApi('queryChangeFiles').returns(Promise.resolve([]));
+ navigateResetStub = sinon.stub(
+ testResolver(changeModelToken),
+ 'navigateToChangeResetReload'
+ );
await element.updateComplete;
});
@@ -298,7 +309,7 @@
assert.isTrue(deleteStub.called);
await deleteStub.lastCall.returnValue;
assert.equal(element.path, '');
- assert.equal(eventStub.firstCall.args[0].type, 'reload');
+ assert.equal(navigateResetStub.callCount, 1);
assert.isTrue(closeDialogSpy.called);
});
@@ -397,7 +408,7 @@
await renameStub.lastCall.returnValue;
assert.equal(element.path, '');
- assert.equal(eventStub.firstCall.args[0].type, 'reload');
+ assert.equal(navigateResetStub.callCount, 1);
assert.isTrue(closeDialogSpy.called);
});
@@ -485,7 +496,7 @@
assert.equal(restoreStub.lastCall.args[1], 'src/test.cpp');
return restoreStub.lastCall.returnValue.then(() => {
assert.equal(element.path, '');
- assert.equal(eventStub.firstCall.args[0].type, 'reload');
+ assert.equal(navigateResetStub.callCount, 1);
assert.isTrue(closeDialogSpy.called);
});
});
@@ -553,7 +564,8 @@
assert.equal(fileStub.lastCall.args[0], 1);
assert.equal(fileStub.lastCall.args[1], 'test.php');
assert.equal(fileStub.lastCall.args[2], 'base64');
- await waitForEventOnce(element, 'reload');
+ await waitUntil(() => navigateResetStub.called);
+ assert.equal(navigateResetStub.callCount, 1);
});
});
diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
index 878c5e9..f37a3d9 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.ts
@@ -9,7 +9,6 @@
import '../../shared/gr-editable-label/gr-editable-label';
import '../gr-default-editor/gr-default-editor';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
-import {computeTruncatedPath} from '../../../utils/path-list-util';
import {
EditPreferencesInfo,
Base64FileContent,
@@ -17,11 +16,7 @@
} from '../../../types/common';
import {ParsedChangeInfo} from '../../../types/types';
import {HttpMethod, NotifyType} from '../../../constants/constants';
-import {
- fireAlert,
- fireTitleChange,
- fireReload,
-} from '../../../utils/event-util';
+import {fireAlert} from '../../../utils/event-util';
import {getAppContext} from '../../../services/app-context';
import {ErrorCallback} from '../../../api/rest';
import {assertIsDefined} from '../../../utils/common-util';
@@ -57,12 +52,6 @@
@customElement('gr-editor-view')
export class GrEditorView extends LitElement {
/**
- * Fired when the title of the page should change.
- *
- * @event title-change
- */
-
- /**
* Fired to notify the user of
*
* @event show-alert
@@ -333,17 +322,6 @@
viewStateChanged() {
if (this.viewState?.childView !== ChangeChildView.EDIT) return;
- // NOTE: This may be called before attachment (e.g. while parentElement is
- // null). Fire title-change in an async so that, if attachment to the DOM
- // has been queued, the event can bubble up to the handler in gr-app.
- setTimeout(() => {
- if (!this.viewState) return;
- const title = `Editing ${computeTruncatedPath(
- this.viewState.editView?.path
- )}`;
- fireTitleChange(this, title);
- });
-
const promises = [];
promises.push(this.getFileData());
return Promise.all(promises);
@@ -512,13 +490,7 @@
)
.then(() => {
assertIsDefined(this.change, 'change');
- // TODO: `forceReload: true` does not seem to work as expected: The patchset is not
- // updated. Thus we are also calling `fireReload()` here. That can probably be
- // cleaned up once the change-view was migrated to fully relying on the change model.
- fireReload(this);
- this.getNavigation().setUrl(
- createChangeUrl({change: this.change, forceReload: true})
- );
+ this.getChangeModel().navigateToChangeResetReload();
});
});
};
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index 98f1f25..58d7105 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -177,7 +177,7 @@
document.addEventListener('page-error', e => {
this.handlePageError(e);
});
- this.addEventListener('title-change', e => {
+ document.addEventListener('title-change', e => {
this.handleTitleChange(e);
});
this.addEventListener('dialog-change', e => {
diff --git a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts
index 2f835f7..a2b61e0 100644
--- a/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts
+++ b/polygerrit-ui/app/elements/settings/gr-cla-view/gr-cla-view.ts
@@ -53,7 +53,7 @@
super.connectedCallback();
this.loadData();
- fireTitleChange(this, 'New Contributor Agreement');
+ fireTitleChange('New Contributor Agreement');
}
static override get styles() {
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 2695ea7..24a18c9 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
@@ -81,12 +81,6 @@
@customElement('gr-settings-view')
export class GrSettingsView extends LitElement {
/**
- * Fired when the title of the page should change.
- *
- * @event title-change
- */
-
- /**
* Fired with email confirmation text, or when the page reloads.
*
* @event show-alert
@@ -261,7 +255,7 @@
// Polymer 2: anchor tag won't work on shadow DOM
// we need to manually calling scrollIntoView when hash changed
document.addEventListener('location-change', this.handleLocationChange);
- fireTitleChange(this, 'Settings');
+ fireTitleChange('Settings');
}
override firstUpdated() {
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 a0d7fab..0bcb09c 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
@@ -544,10 +544,8 @@
test('calls the title-change event', async () => {
const titleChangedStub = sinon.stub();
-
- // Create a new view.
const newElement = document.createElement('gr-settings-view');
- newElement.addEventListener('title-change', titleChangedStub);
+ document.addEventListener('title-change', titleChangedStub);
const div = await fixture(html`<div></div>`);
div.appendChild(newElement);
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account-contents.ts b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account-contents.ts
index 14c2c15..394015e 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account-contents.ts
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account-contents.ts
@@ -43,7 +43,7 @@
import {configModelToken} from '../../../models/config/config-model';
import {createSearchUrl} from '../../../models/views/search';
import {createDashboardUrl} from '../../../models/views/dashboard';
-import {fire} from '../../../utils/event-util';
+import {fire, fireReload} from '../../../utils/event-util';
import {userModelToken} from '../../../models/user/user-model';
@customElement('gr-hovercard-account-contents')
@@ -445,7 +445,7 @@
this.getReviewerState(this.change!)
);
}
- fire(this, 'reload', {clearPatchset: true});
+ fireReload(this);
});
}
@@ -464,7 +464,7 @@
if (!response || !response.ok) {
throw new Error('something went wrong when removing user');
}
- fire(this, 'reload', {clearPatchset: true});
+ fireReload(this);
return response;
});
}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts
index ae93977..4977ec5 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts
@@ -56,7 +56,6 @@
<gr-change-actions></gr-change-actions>
`);
element.change = {} as ChangeViewChangeInfo;
- element._hasKnownChainState = false;
window.Gerrit.install(
p => {
plugin = p;
diff --git a/polygerrit-ui/app/models/change/change-model.ts b/polygerrit-ui/app/models/change/change-model.ts
index b6b9de7..6d111aa 100644
--- a/polygerrit-ui/app/models/change/change-model.ts
+++ b/polygerrit-ui/app/models/change/change-model.ts
@@ -5,6 +5,7 @@
*/
import {
BasePatchSetNum,
+ ChangeInfo,
EditInfo,
EDIT,
PARENT,
@@ -13,25 +14,20 @@
PreferencesInfo,
RevisionPatchSetNum,
PatchSetNumber,
+ CommitId,
} from '../../types/common';
-import {DefaultBase} from '../../constants/constants';
-import {combineLatest, from, fromEvent, Observable, forkJoin, of} from 'rxjs';
-import {
- map,
- filter,
- withLatestFrom,
- startWith,
- switchMap,
-} from 'rxjs/operators';
+import {ChangeStatus, DefaultBase} from '../../constants/constants';
+import {combineLatest, from, Observable, forkJoin, of} from 'rxjs';
+import {map, filter, withLatestFrom, switchMap} from 'rxjs/operators';
import {
computeAllPatchSets,
computeLatestPatchNum,
computeLatestPatchNumWithEdit,
+ findEdit,
+ sortRevisions,
} from '../../utils/patch-set-util';
-import {ParsedChangeInfo} from '../../types/types';
-import {fireAlert} from '../../utils/event-util';
-
-import {ChangeInfo} from '../../types/common';
+import {isDefined, ParsedChangeInfo} from '../../types/types';
+import {fireAlert, fireTitleChange} from '../../utils/event-util';
import {RestApiService} from '../../services/gr-rest-api/gr-rest-api';
import {select} from '../../utils/observable-util';
import {assertIsDefined} from '../../utils/common-util';
@@ -40,12 +36,18 @@
import {define} from '../dependency';
import {isOwner} from '../../utils/change-util';
import {
+ ChangeChildView,
ChangeViewModel,
createChangeUrl,
createDiffUrl,
createEditUrl,
} from '../views/change';
import {NavigationService} from '../../elements/core/gr-navigation/gr-navigation';
+import {getRevertCreatedChangeIds} from '../../utils/message-util';
+import {computeTruncatedPath} from '../../utils/path-list-util';
+import {PluginLoader} from '../../elements/shared/gr-js-api-interface/gr-plugin-loader';
+import {ReportingService} from '../../services/gr-reporting/gr-reporting';
+import {Timing} from '../../constants/reporting';
export enum LoadingStatus {
NOT_LOADED = 'NOT_LOADED',
@@ -69,6 +71,39 @@
* Undefined means it's still loading and empty set means no files reviewed.
*/
reviewedFiles?: string[];
+ /**
+ * Either filled from `change.mergeable`, or from a dedicated REST API call.
+ * Is initially `undefined`, such that you can identify whether this
+ * information has already been loaded once for this change or not. Will never
+ * go back to `undefined` after being set for a change.
+ */
+ mergeable?: boolean;
+}
+
+/**
+ * `change.revisions` is a dictionary mapping the revision sha to RevisionInfo,
+ * but the info object itself does not contain the sha, which is a problem when
+ * working with just the info objects.
+ *
+ * So we are iterating over the map here and are assigning the sha map key to
+ * the property `revision.commit.commit`.
+ *
+ * As usual we are treating data objects as immutable, so we are doind a lot of
+ * cloning here.
+ */
+export function updateRevisionsWithCommitShas(changeInput?: ParsedChangeInfo) {
+ if (!changeInput?.revisions) return changeInput;
+ const changeOutput = {...changeInput, revisions: {...changeInput.revisions}};
+ for (const sha of Object.keys(changeOutput.revisions)) {
+ const revision = changeOutput.revisions[sha];
+ if (revision?.commit && !revision.commit.commit) {
+ changeOutput.revisions[sha] = {
+ ...revision,
+ commit: {...revision.commit, commit: sha as CommitId},
+ };
+ }
+ }
+ return changeOutput;
}
/**
@@ -168,20 +203,36 @@
changeState => changeState.loadingStatus
);
+ public readonly loading$ = select(
+ this.changeLoadingStatus$,
+ status =>
+ status === LoadingStatus.LOADING || status === LoadingStatus.RELOADING
+ );
+
public readonly reviewedFiles$ = select(
this.state$,
changeState => changeState?.reviewedFiles
);
+ public readonly mergeable$ = select(
+ this.state$,
+ changeState => changeState.mergeable
+ );
+
public readonly changeNum$ = select(this.change$, change => change?._number);
+ public readonly changeId$ = select(this.change$, change => change?.change_id);
+
public readonly repo$ = select(this.change$, change => change?.project);
+ public readonly topic$ = select(this.change$, change => change?.topic);
+
+ public readonly status$ = select(this.change$, change => change?.status);
+
public readonly labels$ = select(this.change$, change => change?.labels);
- public readonly revisions$ = select(
- this.change$,
- change => change?.revisions
+ public readonly revisions$ = select(this.change$, change =>
+ sortRevisions(Object.values(change?.revisions || {}))
);
public readonly patchsets$ = select(this.change$, change =>
@@ -264,49 +315,56 @@
computeBase(viewModelBasePatchNum, patchNum, change, preferences)
);
+ private selectRevision(
+ revisionNum$: Observable<RevisionPatchSetNum | undefined>
+ ) {
+ return select(
+ combineLatest([this.revisions$, revisionNum$]),
+ ([revisions, patchNum]) => {
+ if (!revisions || !patchNum) return undefined;
+ return Object.values(revisions).find(
+ revision => revision._number === patchNum
+ );
+ }
+ );
+ }
+
+ public readonly revision$ = this.selectRevision(this.patchNum$);
+
+ public readonly latestRevision$ = this.selectRevision(this.latestPatchNum$);
+
public readonly isOwner$: Observable<boolean> = select(
combineLatest([this.change$, this.userModel.account$]),
([change, account]) => isOwner(change, account)
);
- // For usage in `combineLatest` we need `startWith` such that reload$ has an
- // initial value.
- readonly reload$: Observable<unknown> = fromEvent(document, 'reload').pipe(
- startWith(undefined)
+ public readonly messages$ = select(this.change$, change => change?.messages);
+
+ public readonly revertingChangeIds$ = select(this.messages$, messages =>
+ getRevertCreatedChangeIds(messages ?? [])
);
constructor(
private readonly navigation: NavigationService,
private readonly viewModel: ChangeViewModel,
private readonly restApiService: RestApiService,
- private readonly userModel: UserModel
+ private readonly userModel: UserModel,
+ private readonly pluginLoader: PluginLoader,
+ private readonly reporting: ReportingService
) {
super(initialState);
this.subscriptions = [
- combineLatest([this.viewModel.changeNum$, this.reload$])
- .pipe(
- map(([changeNum, _]) => changeNum),
- switchMap(changeNum => {
- if (changeNum !== undefined) this.updateStateLoading(changeNum);
- const change = from(this.restApiService.getChangeDetail(changeNum));
- const edit = from(this.restApiService.getChangeEdit(changeNum));
- return forkJoin([change, edit]);
- }),
- withLatestFrom(this.viewModel.patchNum$),
- map(([[change, edit], patchNum]) =>
- updateChangeWithEdit(change, edit, patchNum)
- )
- )
- .subscribe(change => {
- // The change service is currently a singleton, so we have to be
- // careful to avoid situations where the application state is
- // partially set for the old change where the user is coming from,
- // and partially for the new change where the user is navigating to.
- // So setting the change explicitly to undefined when the user
- // moves away from diff and change pages (changeNum === undefined)
- // helps with that.
- this.updateStateChange(change ?? undefined);
- }),
+ this.loadChange(),
+ this.loadMergeable(),
+ this.loadReviewedFiles(),
+ this.setOverviewTitle(),
+ this.setDiffTitle(),
+ this.setEditTitle(),
+ this.reportChangeReload(),
+ this.reportSendReply(),
+ this.fireShowChange(),
+ this.refuseEditForOpenChange(),
+ this.refuseEditForClosedChange(),
this.change$.subscribe(change => (this.change = change)),
this.patchNum$.subscribe(patchNum => (this.patchNum = patchNum)),
this.basePatchNum$.subscribe(
@@ -315,20 +373,215 @@
this.latestPatchNum$.subscribe(
latestPatchNum => (this.latestPatchNum = latestPatchNum)
),
- combineLatest([this.patchNum$, this.changeNum$, this.userModel.loggedIn$])
- .pipe(
- switchMap(([patchNum, changeNum, loggedIn]) => {
- if (!changeNum || !patchNum || !loggedIn) {
- this.updateStateReviewedFiles([]);
- return of(undefined);
- }
- return from(this.fetchReviewedFiles(patchNum, changeNum));
- })
- )
- .subscribe(),
];
}
+ private reportSendReply() {
+ return this.changeLoadingStatus$.subscribe(loadingStatus => {
+ // We are ending the timer on each change load, because ending a timer
+ // that was not started is a no-op. :-)
+ if (loadingStatus === LoadingStatus.LOADED) {
+ this.reporting.timeEnd(Timing.SEND_REPLY);
+ }
+ });
+ }
+
+ private reportChangeReload() {
+ return this.changeLoadingStatus$.subscribe(loadingStatus => {
+ if (
+ loadingStatus === LoadingStatus.LOADING ||
+ loadingStatus === LoadingStatus.RELOADING
+ ) {
+ this.reporting.time(Timing.CHANGE_RELOAD);
+ }
+ if (
+ loadingStatus === LoadingStatus.LOADED ||
+ loadingStatus === LoadingStatus.NOT_LOADED
+ ) {
+ this.reporting.timeEnd(Timing.CHANGE_RELOAD);
+ }
+ });
+ }
+
+ private fireShowChange() {
+ return combineLatest([
+ this.viewModel.childView$,
+ this.change$,
+ this.patchNum$,
+ this.mergeable$,
+ ])
+ .pipe(
+ filter(
+ ([childView, change, patchNum, mergeable]) =>
+ childView === ChangeChildView.OVERVIEW &&
+ !!change &&
+ !!patchNum &&
+ mergeable !== undefined
+ )
+ )
+ .subscribe(([_, change, patchNum, mergeable]) => {
+ this.pluginLoader.jsApiService.handleShowChange({
+ change,
+ patchNum,
+ // `?? null` is for the TypeScript compiler only. We have a
+ // `mergeable !== undefined` filter above, so this cannot happen.
+ // It would be nice to change `ShowChangeDetail` to accept `undefined`
+ // instaed of `null`, but that would be a Plugin API change ...
+ info: {mergeable: mergeable ?? null},
+ });
+ });
+ }
+
+ private refuseEditForOpenChange() {
+ return combineLatest([this.revisions$, this.patchNum$, this.status$])
+ .pipe(
+ filter(
+ ([revisions, patchNum, status]) =>
+ status === ChangeStatus.NEW &&
+ revisions.length > 0 &&
+ patchNum === EDIT
+ )
+ )
+ .subscribe(([revisions]) => {
+ const editRev = findEdit(revisions);
+ if (!editRev) {
+ const msg = 'Change edit not found. Please create a change edit.';
+ fireAlert(document, msg);
+ this.navigateToChangeResetReload();
+ }
+ });
+ }
+
+ private refuseEditForClosedChange() {
+ return combineLatest([
+ this.revisions$,
+ this.viewModel.edit$,
+ this.patchNum$,
+ this.status$,
+ ])
+ .pipe(
+ filter(
+ ([revisions, edit, patchNum, status]) =>
+ (status === ChangeStatus.ABANDONED ||
+ status === ChangeStatus.MERGED) &&
+ revisions.length > 0 &&
+ (patchNum === EDIT || edit)
+ )
+ )
+ .subscribe(([revisions]) => {
+ const editRev = findEdit(revisions);
+ if (!editRev) {
+ const msg =
+ 'Change edits cannot be created if change is merged ' +
+ 'or abandoned. Redirecting to non edit mode.';
+ fireAlert(document, msg);
+ this.navigateToChangeResetReload();
+ }
+ });
+ }
+
+ private setOverviewTitle() {
+ return combineLatest([this.viewModel.childView$, this.change$])
+ .pipe(
+ filter(([childView, _]) => childView === ChangeChildView.OVERVIEW),
+ map(([_, change]) => change),
+ filter(isDefined)
+ )
+ .subscribe(change => {
+ const title = `${change.subject} (${change._number})`;
+ fireTitleChange(title);
+ });
+ }
+
+ private setDiffTitle() {
+ return combineLatest([this.viewModel.childView$, this.viewModel.diffPath$])
+ .pipe(
+ filter(([childView, _]) => childView === ChangeChildView.DIFF),
+ map(([_, diffPath]) => diffPath),
+ filter(isDefined)
+ )
+ .subscribe(diffPath => {
+ const title = computeTruncatedPath(diffPath);
+ fireTitleChange(title);
+ });
+ }
+
+ private setEditTitle() {
+ return combineLatest([this.viewModel.childView$, this.viewModel.editPath$])
+ .pipe(
+ filter(([childView, _]) => childView === ChangeChildView.EDIT),
+ map(([_, editPath]) => editPath),
+ filter(isDefined)
+ )
+ .subscribe(editPath => {
+ const title = `Editing ${computeTruncatedPath(editPath)}`;
+ fireTitleChange(title);
+ });
+ }
+
+ private loadReviewedFiles() {
+ return combineLatest([
+ this.patchNum$,
+ this.changeNum$,
+ this.userModel.loggedIn$,
+ ])
+ .pipe(
+ switchMap(([patchNum, changeNum, loggedIn]) => {
+ if (!changeNum || !patchNum || !loggedIn) {
+ this.updateStateReviewedFiles([]);
+ return of(undefined);
+ }
+ return from(this.fetchReviewedFiles(patchNum, changeNum));
+ })
+ )
+ .subscribe();
+ }
+
+ private loadMergeable() {
+ return this.change$
+ .pipe(
+ switchMap(change => {
+ if (change?._number === undefined) return of(undefined);
+ if (change.mergeable !== undefined) return of(change.mergeable);
+ if (change.status === ChangeStatus.MERGED) return of(false);
+ if (change.status === ChangeStatus.ABANDONED) return of(false);
+ return from(
+ this.restApiService
+ .getMergeable(change._number)
+ .then(mergableInfo => mergableInfo?.mergeable ?? false)
+ );
+ })
+ )
+ .subscribe(mergeable => this.updateState({mergeable}));
+ }
+
+ private loadChange() {
+ return this.viewModel.changeNum$
+ .pipe(
+ switchMap(changeNum => {
+ if (changeNum !== undefined) this.updateStateLoading(changeNum);
+ const change = from(this.restApiService.getChangeDetail(changeNum));
+ const edit = from(this.restApiService.getChangeEdit(changeNum));
+ return forkJoin([change, edit]);
+ }),
+ withLatestFrom(this.viewModel.patchNum$),
+ map(([[change, edit], patchNum]) =>
+ updateChangeWithEdit(change, edit, patchNum)
+ ),
+ map(updateRevisionsWithCommitShas)
+ )
+ .subscribe(change => {
+ // The change service is currently a singleton, so we have to be
+ // careful to avoid situations where the application state is
+ // partially set for the old change where the user is coming from,
+ // and partially for the new change where the user is navigating to.
+ // So setting the change explicitly to undefined when the user
+ // moves away from diff and change pages (changeNum === undefined)
+ // helps with that.
+ this.updateStateChange(change ?? undefined);
+ });
+ }
+
updateStateReviewedFiles(reviewedFiles: string[]) {
this.updateState({reviewedFiles});
}
@@ -430,12 +683,27 @@
});
}
+ // Mainly used for navigating from DIFF to OVERVIEW.
navigateToChange(openReplyDialog = false) {
const url = this.changeUrl(openReplyDialog);
if (!url) return;
this.navigation.setUrl(url);
}
+ /**
+ * Wipes all URL parameters and other view state and goes to the change
+ * overview page, forcing a reload.
+ *
+ * This will also wipe the `patchNum`, so will always go to the latest
+ * patchset.
+ */
+ navigateToChangeResetReload() {
+ if (!this.change) return;
+ const url = createChangeUrl({change: this.change, forceReload: true});
+ if (!url) return;
+ this.navigation.setUrl(url);
+ }
+
editUrl(editView: {path: string; lineNum?: number}) {
if (!this.change) return;
return createEditUrl({
diff --git a/polygerrit-ui/app/models/change/change-model_test.ts b/polygerrit-ui/app/models/change/change-model_test.ts
index c11c15b..e5d2e36 100644
--- a/polygerrit-ui/app/models/change/change-model_test.ts
+++ b/polygerrit-ui/app/models/change/change-model_test.ts
@@ -7,10 +7,12 @@
import {ChangeStatus} from '../../constants/constants';
import '../../test/common-test-setup';
import {
+ TEST_NUMERIC_CHANGE_ID,
createChange,
createChangeMessageInfo,
createChangeViewState,
createEditInfo,
+ createMergeable,
createParsedChange,
createRevision,
} from '../../test/test-data-generators';
@@ -29,13 +31,34 @@
} from '../../types/common';
import {ParsedChangeInfo} from '../../types/types';
import {getAppContext} from '../../services/app-context';
-import {ChangeState, LoadingStatus, updateChangeWithEdit} from './change-model';
+import {
+ ChangeState,
+ LoadingStatus,
+ updateChangeWithEdit,
+ updateRevisionsWithCommitShas,
+} from './change-model';
import {ChangeModel} from './change-model';
import {assert} from '@open-wc/testing';
import {testResolver} from '../../test/common-test-setup';
import {userModelToken} from '../user/user-model';
-import {changeViewModelToken} from '../views/change';
+import {
+ ChangeChildView,
+ ChangeViewModel,
+ changeViewModelToken,
+} from '../views/change';
import {navigationToken} from '../../elements/core/gr-navigation/gr-navigation';
+import {SinonStub} from 'sinon';
+import {pluginLoaderToken} from '../../elements/shared/gr-js-api-interface/gr-plugin-loader';
+import {ShowChangeDetail} from '../../elements/shared/gr-js-api-interface/gr-js-api-types';
+
+suite('updateRevisionsWithCommitShas() tests', () => {
+ test('undefined edit', async () => {
+ const change = createParsedChange();
+ const updated = updateRevisionsWithCommitShas(change);
+ assert.equal(change?.revisions?.['abc'].commit?.commit, undefined);
+ assert.equal(updated?.revisions?.['abc'].commit?.commit, 'abc' as CommitId);
+ });
+});
suite('updateChangeWithEdit() tests', () => {
test('undefined change', async () => {
@@ -69,6 +92,7 @@
});
suite('change model tests', () => {
+ let changeViewModel: ChangeViewModel;
let changeModel: ChangeModel;
let knownChange: ParsedChangeInfo;
const testCompleted = new Subject<void>();
@@ -84,25 +108,20 @@
}
setup(() => {
+ changeViewModel = testResolver(changeViewModelToken);
changeModel = new ChangeModel(
testResolver(navigationToken),
- testResolver(changeViewModelToken),
+ changeViewModel,
getAppContext().restApiService,
- testResolver(userModelToken)
+ testResolver(userModelToken),
+ testResolver(pluginLoaderToken),
+ getAppContext().reportingService
);
knownChange = {
...createChange(),
revisions: {
- sha1: {
- ...createRevision(1),
- description: 'patch 1',
- _number: 1 as PatchSetNumber,
- },
- sha2: {
- ...createRevision(2),
- description: 'patch 2',
- _number: 2 as PatchSetNumber,
- },
+ sha1: {...createRevision(1), description: 'patch 1'},
+ sha2: {...createRevision(2), description: 'patch 2'},
},
status: ChangeStatus.NEW,
current_revision: 'abc' as CommitId,
@@ -115,6 +134,114 @@
changeModel.finalize();
});
+ suite('mergeability', async () => {
+ let getMergeableStub: SinonStub;
+ let mergeableApiResponse = false;
+
+ setup(() => {
+ getMergeableStub = stubRestApi('getMergeable').callsFake(() =>
+ Promise.resolve(createMergeable(mergeableApiResponse))
+ );
+ });
+
+ test('mergeability initially undefined', async () => {
+ waitUntilObserved(
+ changeModel.mergeable$,
+ mergeable => mergeable === undefined
+ );
+ assert.isFalse(getMergeableStub.called);
+ });
+
+ test('mergeability true from change', async () => {
+ changeModel.updateStateChange({...knownChange, mergeable: true});
+
+ waitUntilObserved(
+ changeModel.mergeable$,
+ mergeable => mergeable === true
+ );
+ assert.isFalse(getMergeableStub.called);
+ });
+
+ test('mergeability false from change', async () => {
+ changeModel.updateStateChange({...knownChange, mergeable: false});
+
+ waitUntilObserved(
+ changeModel.mergeable$,
+ mergeable => mergeable === true
+ );
+ assert.isFalse(getMergeableStub.called);
+ });
+
+ test('mergeability false for MERGED change', async () => {
+ changeModel.updateStateChange({
+ ...knownChange,
+ status: ChangeStatus.MERGED,
+ });
+
+ waitUntilObserved(
+ changeModel.mergeable$,
+ mergeable => mergeable === false
+ );
+ assert.isFalse(getMergeableStub.called);
+ });
+
+ test('mergeability false for ABANDONED change', async () => {
+ changeModel.updateStateChange({
+ ...knownChange,
+ status: ChangeStatus.ABANDONED,
+ });
+
+ waitUntilObserved(
+ changeModel.mergeable$,
+ mergeable => mergeable === false
+ );
+ assert.isFalse(getMergeableStub.called);
+ });
+
+ test('mergeability true from API', async () => {
+ mergeableApiResponse = true;
+ changeModel.updateStateChange(knownChange);
+
+ waitUntilObserved(
+ changeModel.mergeable$,
+ mergeable => mergeable === true
+ );
+ assert.isTrue(getMergeableStub.calledOnce);
+ });
+
+ test('mergeability false from API', async () => {
+ mergeableApiResponse = false;
+ changeModel.updateStateChange(knownChange);
+
+ waitUntilObserved(
+ changeModel.mergeable$,
+ mergeable => mergeable === false
+ );
+ assert.isTrue(getMergeableStub.calledOnce);
+ });
+ });
+
+ test('fireShowChange', async () => {
+ const pluginLoader = testResolver(pluginLoaderToken);
+ const jsApiService = pluginLoader.jsApiService;
+ const showChangeStub = sinon.stub(jsApiService, 'handleShowChange');
+
+ changeViewModel.updateState({
+ childView: ChangeChildView.OVERVIEW,
+ patchNum: 1 as PatchSetNumber,
+ });
+ changeModel.updateState({
+ change: createParsedChange(),
+ mergeable: true,
+ });
+
+ assert.isTrue(showChangeStub.calledOnce);
+ const detail: ShowChangeDetail = showChangeStub.lastCall.firstArg;
+ assert.equal(detail.change?._number, createParsedChange()._number);
+ assert.equal(detail.patchNum, 1 as PatchSetNumber);
+ assert.equal(detail.info.mergeable, true);
+ });
+
test('load a change', async () => {
const promise = mockPromise<ParsedChangeInfo | undefined>();
const stub = stubRestApi('getChangeDetail').callsFake(() => promise);
@@ -132,7 +259,7 @@
promise.resolve(knownChange);
state = await waitForLoadingStatus(LoadingStatus.LOADED);
assert.equal(stub.callCount, 1);
- assert.equal(state?.change, knownChange);
+ assert.deepEqual(state?.change, updateRevisionsWithCommitShas(knownChange));
});
test('reload a change', async () => {
@@ -143,17 +270,20 @@
testResolver(changeViewModelToken).setState(createChangeViewState());
promise.resolve(knownChange);
state = await waitForLoadingStatus(LoadingStatus.LOADED);
+ assert.equal(stub.callCount, 1);
// Reloading same change
document.dispatchEvent(new CustomEvent('reload'));
state = await waitForLoadingStatus(LoadingStatus.RELOADING);
- assert.equal(stub.callCount, 2);
- assert.equal(state?.change, knownChange);
+ assert.equal(stub.callCount, 3);
+ assert.equal(stub.getCall(1).firstArg, undefined);
+ assert.equal(stub.getCall(2).firstArg, TEST_NUMERIC_CHANGE_ID);
+ assert.deepEqual(state?.change, updateRevisionsWithCommitShas(knownChange));
promise.resolve(knownChange);
state = await waitForLoadingStatus(LoadingStatus.LOADED);
- assert.equal(stub.callCount, 2);
- assert.equal(state?.change, knownChange);
+ assert.equal(stub.callCount, 3);
+ assert.deepEqual(state?.change, updateRevisionsWithCommitShas(knownChange));
});
test('navigating to another change', async () => {
@@ -183,7 +313,7 @@
promise.resolve(otherChange);
state = await waitForLoadingStatus(LoadingStatus.LOADED);
assert.equal(stub.callCount, 2);
- assert.equal(state?.change, otherChange);
+ assert.deepEqual(state?.change, updateRevisionsWithCommitShas(otherChange));
});
test('navigating to dashboard', async () => {
@@ -211,7 +341,7 @@
testResolver(changeViewModelToken).setState(createChangeViewState());
state = await waitForLoadingStatus(LoadingStatus.LOADED);
assert.equal(stub.callCount, 3);
- assert.equal(state?.change, knownChange);
+ assert.deepEqual(state?.change, updateRevisionsWithCommitShas(knownChange));
});
test('changeModel.fetchChangeUpdates on latest', async () => {
@@ -294,4 +424,28 @@
changeModel.updateStateChange(createParsedChange());
assert.equal(spy.callCount, 2);
});
+
+ test('revision$ selector latest', async () => {
+ changeViewModel.updateState({patchNum: undefined});
+ changeModel.updateState({change: knownChange});
+ await waitUntilObserved(changeModel.revision$, x => x?._number === 2);
+ });
+
+ test('revision$ selector 1', async () => {
+ changeViewModel.updateState({patchNum: 1 as PatchSetNumber});
+ changeModel.updateState({change: knownChange});
+ await waitUntilObserved(changeModel.revision$, x => x?._number === 1);
+ });
+
+ test('latestRevision$ selector latest', async () => {
+ changeViewModel.updateState({patchNum: undefined});
+ changeModel.updateState({change: knownChange});
+ await waitUntilObserved(changeModel.latestRevision$, x => x?._number === 2);
+ });
+
+ test('latestRevision$ selector 1', async () => {
+ changeViewModel.updateState({patchNum: 1 as PatchSetNumber});
+ changeModel.updateState({change: knownChange});
+ await waitUntilObserved(changeModel.latestRevision$, x => x?._number === 2);
+ });
});
diff --git a/polygerrit-ui/app/models/change/files-model.ts b/polygerrit-ui/app/models/change/files-model.ts
index b7ff16f..c01b718 100644
--- a/polygerrit-ui/app/models/change/files-model.ts
+++ b/polygerrit-ui/app/models/change/files-model.ts
@@ -22,6 +22,8 @@
import {define} from '../dependency';
import {ChangeModel} from './change-model';
import {CommentsModel} from '../comments/comments-model';
+import {Timing} from '../../constants/reporting';
+import {ReportingService} from '../../services/gr-reporting/gr-reporting';
export type FileNameToNormalizedFileInfoMap = {
[name: string]: NormalizedFileInfo;
@@ -141,10 +143,13 @@
constructor(
readonly changeModel: ChangeModel,
readonly commentsModel: CommentsModel,
- readonly restApiService: RestApiService
+ readonly restApiService: RestApiService,
+ private readonly reporting: ReportingService
) {
super(initialState);
this.subscriptions = [
+ this.reportChangeDataStart(),
+ this.reportChangeDataEnd(),
this.subscribeToFiles(
(psLeft, psRight) => {
return {basePatchNum: psLeft, patchNum: psRight};
@@ -176,6 +181,26 @@
];
}
+ private reportChangeDataStart() {
+ return combineLatest([this.changeModel.loading$]).subscribe(
+ ([changeLoading]) => {
+ if (changeLoading) {
+ this.reporting.time(Timing.CHANGE_DATA);
+ }
+ }
+ );
+ }
+
+ private reportChangeDataEnd() {
+ return combineLatest([this.changeModel.loading$, this.files$]).subscribe(
+ ([changeLoading, files]) => {
+ if (!changeLoading && files.length > 0) {
+ this.reporting.timeEnd(Timing.CHANGE_DATA);
+ }
+ }
+ );
+ }
+
private subscribeToFiles(
rangeChooser: (
basePatchNum: BasePatchSetNum,
@@ -184,13 +209,12 @@
filesToState: (files: NormalizedFileInfo[]) => Partial<FilesState>
) {
return combineLatest([
- this.changeModel.reload$,
this.changeModel.changeNum$,
this.changeModel.basePatchNum$,
this.changeModel.patchNum$,
])
.pipe(
- switchMap(([_, changeNum, basePatchNum, patchNum]) => {
+ switchMap(([changeNum, basePatchNum, patchNum]) => {
if (!changeNum || !patchNum) return of({});
const range = rangeChooser(basePatchNum, patchNum);
if (!range) return of({});
diff --git a/polygerrit-ui/app/models/change/related-changes-model.ts b/polygerrit-ui/app/models/change/related-changes-model.ts
new file mode 100644
index 0000000..9ae4295
--- /dev/null
+++ b/polygerrit-ui/app/models/change/related-changes-model.ts
@@ -0,0 +1,242 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {
+ ChangeInfo,
+ RelatedChangeAndCommitInfo,
+ SubmittedTogetherInfo,
+} from '../../types/common';
+import {RestApiService} from '../../services/gr-rest-api/gr-rest-api';
+import {select} from '../../utils/observable-util';
+import {Model} from '../model';
+import {define} from '../dependency';
+import {ChangeModel} from './change-model';
+import {combineLatest, forkJoin, from, of} from 'rxjs';
+import {map, switchMap} from 'rxjs/operators';
+import {ConfigModel} from '../config/config-model';
+import {ChangeStatus} from '../../api/rest-api';
+import {isDefined} from '../../types/types';
+
+export interface RelatedChangesState {
+ /** `undefined` means "not yet loaded". */
+ relatedChanges?: RelatedChangeAndCommitInfo[];
+ submittedTogether?: SubmittedTogetherInfo;
+ cherryPicks?: ChangeInfo[];
+ conflictingChanges?: ChangeInfo[];
+ sameTopicChanges?: ChangeInfo[];
+ revertingChanges: ChangeInfo[];
+}
+
+const initialState: RelatedChangesState = {
+ relatedChanges: undefined,
+ submittedTogether: undefined,
+ cherryPicks: undefined,
+ conflictingChanges: undefined,
+ sameTopicChanges: undefined,
+ revertingChanges: [],
+};
+
+export const relatedChangesModelToken = define<RelatedChangesModel>(
+ 'related-changes-model'
+);
+
+export class RelatedChangesModel extends Model<RelatedChangesState> {
+ public readonly relatedChanges$ = select(
+ this.state$,
+ state => state.relatedChanges
+ );
+
+ public readonly submittedTogether$ = select(
+ this.state$,
+ state => state.submittedTogether
+ );
+
+ public readonly cherryPicks$ = select(
+ this.state$,
+ state => state.cherryPicks
+ );
+
+ public readonly conflictingChanges$ = select(
+ this.state$,
+ state => state.conflictingChanges
+ );
+
+ public readonly sameTopicChanges$ = select(
+ this.state$,
+ state => state.sameTopicChanges
+ );
+
+ /**
+ * Emits all changes that have reverted the current change, based on
+ * information from parsed change messages. Abandoned changes are not
+ * included.
+ */
+ public readonly revertingChanges$ = select(
+ this.state$,
+ state => state.revertingChanges
+ );
+
+ /**
+ * Emits one reverting change (if there is any) from revertingChanges$.
+ * It prefers MERGED changes. Otherwise the choice is random.
+ */
+ public readonly revertingChange$ = select(
+ this.revertingChanges$,
+ revertingChanges => {
+ if (revertingChanges.length === 0) return undefined;
+ const submittedRevert = revertingChanges.find(
+ c => c.status === ChangeStatus.MERGED
+ );
+ if (submittedRevert) return submittedRevert;
+ return revertingChanges[0];
+ }
+ );
+
+ /**
+ * Determines whether the change has a parent change. If there
+ * is a relation chain, and the change id is not the last item of the
+ * relation chain, then there is a parent.
+ */
+ public readonly hasParent$ = select(
+ combineLatest([this.changeModel.change$, this.relatedChanges$]),
+ ([change, relatedChanges]) => {
+ if (!change) return undefined;
+ if (relatedChanges === undefined) return undefined;
+ if (relatedChanges.length === 0) return false;
+ const lastChangeId = relatedChanges[relatedChanges.length - 1].change_id;
+ return lastChangeId !== change.change_id;
+ }
+ );
+
+ constructor(
+ readonly changeModel: ChangeModel,
+ readonly configModel: ConfigModel,
+ readonly restApiService: RestApiService
+ ) {
+ super(initialState);
+ this.subscriptions = [
+ this.loadRelatedChanges(),
+ this.loadSubmittedTogether(),
+ this.loadCherryPicks(),
+ this.loadConflictingChanges(),
+ this.loadSameTopicChanges(),
+ this.loadRevertingChanges(),
+ ];
+ }
+
+ private loadRelatedChanges() {
+ return combineLatest([
+ this.changeModel.changeNum$,
+ this.changeModel.latestPatchNum$,
+ ])
+ .pipe(
+ switchMap(([changeNum, latestPatchNum]) => {
+ if (!changeNum || !latestPatchNum) return of(undefined);
+ return from(
+ this.restApiService
+ .getRelatedChanges(changeNum, latestPatchNum)
+ .then(info => info?.changes ?? [])
+ );
+ })
+ )
+ .subscribe(relatedChanges => {
+ this.updateState({relatedChanges});
+ });
+ }
+
+ private loadSubmittedTogether() {
+ return this.changeModel.changeNum$
+ .pipe(
+ switchMap(changeNum => {
+ if (!changeNum) return of(undefined);
+ return from(
+ this.restApiService.getChangesSubmittedTogether(changeNum)
+ );
+ })
+ )
+ .subscribe(submittedTogether => {
+ this.updateState({submittedTogether});
+ });
+ }
+
+ private loadCherryPicks() {
+ return combineLatest([
+ this.changeModel.changeNum$,
+ this.changeModel.changeId$,
+ this.changeModel.repo$,
+ ])
+ .pipe(
+ switchMap(([changeNum, changeId, repo]) => {
+ if (!changeNum || !changeId || !repo) return of(undefined);
+ return from(
+ this.restApiService.getChangeCherryPicks(repo, changeId, changeNum)
+ );
+ })
+ )
+ .subscribe(cherryPicks => {
+ this.updateState({cherryPicks});
+ });
+ }
+
+ private loadConflictingChanges() {
+ return combineLatest([
+ this.changeModel.changeNum$,
+ this.changeModel.status$,
+ this.changeModel.mergeable$,
+ ])
+ .pipe(
+ switchMap(([changeNum, status, mergeable]) => {
+ if (!changeNum || !status || !mergeable) return of(undefined);
+ if (status !== ChangeStatus.NEW) return of(undefined);
+ return from(this.restApiService.getChangeConflicts(changeNum));
+ })
+ )
+ .subscribe(conflictingChanges => {
+ this.updateState({conflictingChanges});
+ });
+ }
+
+ private loadSameTopicChanges() {
+ return combineLatest([
+ this.changeModel.changeNum$,
+ this.changeModel.topic$,
+ this.configModel.serverConfig$,
+ ])
+ .pipe(
+ switchMap(([changeNum, topic, config]) => {
+ if (!changeNum || !topic || !config) return of(undefined);
+ if (config.change.submit_whole_topic) return of(undefined);
+ return from(
+ this.restApiService.getChangesWithSameTopic(topic, {
+ openChangesOnly: true,
+ changeToExclude: changeNum,
+ })
+ );
+ })
+ )
+ .subscribe(sameTopicChanges => {
+ this.updateState({sameTopicChanges});
+ });
+ }
+
+ private loadRevertingChanges() {
+ return this.changeModel.revertingChangeIds$
+ .pipe(
+ switchMap(changeIds => {
+ if (!changeIds?.length) return of([]);
+ return forkJoin(
+ changeIds.map(changeId =>
+ from(this.restApiService.getChange(changeId))
+ )
+ );
+ }),
+ map(changes => changes.filter(isDefined)),
+ map(changes => changes.filter(c => c.status !== ChangeStatus.ABANDONED))
+ )
+ .subscribe(revertingChanges => {
+ this.updateState({revertingChanges});
+ });
+ }
+}
diff --git a/polygerrit-ui/app/models/change/related-changes-model_test.ts b/polygerrit-ui/app/models/change/related-changes-model_test.ts
new file mode 100644
index 0000000..295f284
--- /dev/null
+++ b/polygerrit-ui/app/models/change/related-changes-model_test.ts
@@ -0,0 +1,286 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../../test/common-test-setup';
+import {getAppContext} from '../../services/app-context';
+import {ChangeModel, changeModelToken} from '../change/change-model';
+import {assert} from '@open-wc/testing';
+import {testResolver} from '../../test/common-test-setup';
+import {RelatedChangesModel} from './related-changes-model';
+import {configModelToken} from '../config/config-model';
+import {SinonStub} from 'sinon';
+import {
+ ChangeInfo,
+ RelatedChangesInfo,
+ SubmittedTogetherInfo,
+} from '../../types/common';
+import {stubRestApi, waitUntilObserved} from '../../test/test-utils';
+import {
+ createParsedChange,
+ createRelatedChangesInfo,
+ createRelatedChangeAndCommitInfo,
+ createChange,
+ createChangeMessage,
+} from '../../test/test-data-generators';
+import {ChangeStatus, ReviewInputTag, TopicName} from '../../api/rest-api';
+import {MessageTag} from '../../constants/constants';
+
+suite('related-changes-model tests', () => {
+ let model: RelatedChangesModel;
+ let changeModel: ChangeModel;
+
+ setup(async () => {
+ changeModel = testResolver(changeModelToken);
+ model = new RelatedChangesModel(
+ changeModel,
+ testResolver(configModelToken),
+ getAppContext().restApiService
+ );
+ await waitUntilObserved(changeModel.change$, c => c === undefined);
+ });
+
+ teardown(() => {
+ model.finalize();
+ });
+
+ test('register and fetch', async () => {
+ assert.equal('', '');
+ });
+
+ suite('related changes and hasParent', async () => {
+ let getRelatedChangesStub: SinonStub;
+ let getRelatedChangesResponse: RelatedChangesInfo;
+ let hasParent: boolean | undefined;
+
+ setup(() => {
+ getRelatedChangesStub = stubRestApi('getRelatedChanges').callsFake(() =>
+ Promise.resolve(getRelatedChangesResponse)
+ );
+ model.hasParent$.subscribe(x => (hasParent = x));
+ });
+
+ test('relatedChanges initially undefined', async () => {
+ await waitUntilObserved(
+ model.relatedChanges$,
+ relatedChanges => relatedChanges === undefined
+ );
+ assert.isFalse(getRelatedChangesStub.called);
+ assert.isUndefined(hasParent);
+ });
+
+ test('relatedChanges loading empty', async () => {
+ changeModel.updateStateChange({...createParsedChange()});
+
+ await waitUntilObserved(
+ model.relatedChanges$,
+ relatedChanges => relatedChanges?.length === 0
+ );
+ assert.isTrue(getRelatedChangesStub.calledOnce);
+ assert.isFalse(hasParent);
+ });
+
+ test('relatedChanges loading one change', async () => {
+ getRelatedChangesResponse = {
+ ...createRelatedChangesInfo(),
+ changes: [createRelatedChangeAndCommitInfo()],
+ };
+ changeModel.updateStateChange({...createParsedChange()});
+
+ await waitUntilObserved(
+ model.relatedChanges$,
+ relatedChanges => relatedChanges?.length === 1
+ );
+ assert.isTrue(getRelatedChangesStub.calledOnce);
+ assert.isTrue(hasParent);
+ });
+ });
+
+ suite('loadSubmittedTogether', async () => {
+ let getChangesSubmittedTogetherStub: SinonStub;
+ let getChangesSubmittedTogetherResponse: SubmittedTogetherInfo;
+
+ setup(() => {
+ getChangesSubmittedTogetherStub = stubRestApi(
+ 'getChangesSubmittedTogether'
+ ).callsFake(() => Promise.resolve(getChangesSubmittedTogetherResponse));
+ });
+
+ test('submittedTogether initially undefined', async () => {
+ await waitUntilObserved(
+ model.submittedTogether$,
+ submittedTogether => submittedTogether === undefined
+ );
+ assert.isFalse(getChangesSubmittedTogetherStub.called);
+ });
+
+ test('submittedTogether emits', async () => {
+ getChangesSubmittedTogetherResponse = {
+ changes: [createChange()],
+ non_visible_changes: 0,
+ };
+ changeModel.updateStateChange({...createParsedChange()});
+
+ await waitUntilObserved(
+ model.submittedTogether$,
+ submittedTogether => submittedTogether?.changes?.length === 1
+ );
+ assert.isTrue(getChangesSubmittedTogetherStub.calledOnce);
+ });
+ });
+
+ suite('loadCherryPicks', async () => {
+ let getChangeCherryPicksStub: SinonStub;
+ let getChangeCherryPicksResponse: ChangeInfo[];
+
+ setup(() => {
+ getChangeCherryPicksStub = stubRestApi('getChangeCherryPicks').callsFake(
+ () => Promise.resolve(getChangeCherryPicksResponse)
+ );
+ });
+
+ test('cherryPicks initially undefined', async () => {
+ await waitUntilObserved(
+ model.cherryPicks$,
+ cherryPicks => cherryPicks === undefined
+ );
+ assert.isFalse(getChangeCherryPicksStub.called);
+ });
+
+ test('cherryPicks emits', async () => {
+ getChangeCherryPicksResponse = [createChange()];
+ changeModel.updateStateChange({...createParsedChange()});
+
+ await waitUntilObserved(
+ model.cherryPicks$,
+ cherryPicks => cherryPicks?.length === 1
+ );
+ assert.isTrue(getChangeCherryPicksStub.calledOnce);
+ });
+ });
+
+ suite('loadConflictingChanges', async () => {
+ let getChangeConflictsStub: SinonStub;
+ let getChangeConflictsResponse: ChangeInfo[];
+
+ setup(() => {
+ getChangeConflictsStub = stubRestApi('getChangeConflicts').callsFake(() =>
+ Promise.resolve(getChangeConflictsResponse)
+ );
+ });
+
+ test('conflictingChanges initially undefined', async () => {
+ await waitUntilObserved(
+ model.conflictingChanges$,
+ conflictingChanges => conflictingChanges === undefined
+ );
+ assert.isFalse(getChangeConflictsStub.called);
+ });
+
+ test('conflictingChanges not loaded for merged changes', async () => {
+ getChangeConflictsResponse = [createChange()];
+ changeModel.updateStateChange({
+ ...createParsedChange(),
+ mergeable: true,
+ status: ChangeStatus.MERGED,
+ });
+
+ await waitUntilObserved(
+ model.conflictingChanges$,
+ conflictingChanges => conflictingChanges === undefined
+ );
+ assert.isFalse(getChangeConflictsStub.called);
+ });
+
+ test('conflictingChanges emits', async () => {
+ getChangeConflictsResponse = [createChange()];
+ changeModel.updateStateChange({...createParsedChange(), mergeable: true});
+
+ await waitUntilObserved(
+ model.conflictingChanges$,
+ conflictingChanges => conflictingChanges?.length === 1
+ );
+ assert.isTrue(getChangeConflictsStub.calledOnce);
+ });
+ });
+
+ suite('loadSameTopicChanges', async () => {
+ let getChangesWithSameTopicStub: SinonStub;
+ let getChangesWithSameTopicResponse: ChangeInfo[];
+
+ setup(() => {
+ getChangesWithSameTopicStub = stubRestApi(
+ 'getChangesWithSameTopic'
+ ).callsFake(() => Promise.resolve(getChangesWithSameTopicResponse));
+ });
+
+ test('sameTopicChanges initially undefined', async () => {
+ await waitUntilObserved(
+ model.sameTopicChanges$,
+ sameTopicChanges => sameTopicChanges === undefined
+ );
+ assert.isFalse(getChangesWithSameTopicStub.called);
+ });
+
+ test('sameTopicChanges emits', async () => {
+ getChangesWithSameTopicResponse = [createChange()];
+ changeModel.updateStateChange({
+ ...createParsedChange(),
+ topic: 'test-topic' as TopicName,
+ });
+
+ await waitUntilObserved(
+ model.sameTopicChanges$,
+ sameTopicChanges => sameTopicChanges?.length === 1
+ );
+ assert.isTrue(getChangesWithSameTopicStub.calledOnce);
+ });
+ });
+
+ suite('loadRevertingChanges', async () => {
+ let getChangeStub: SinonStub;
+
+ setup(() => {
+ getChangeStub = stubRestApi('getChange').callsFake(() =>
+ Promise.resolve(createChange())
+ );
+ });
+
+ test('revertingChanges initially empty', async () => {
+ await waitUntilObserved(
+ model.revertingChanges$,
+ revertingChanges => revertingChanges.length === 0
+ );
+ assert.isFalse(getChangeStub.called);
+ });
+
+ test('revertingChanges empty when change does not contain a revert message', async () => {
+ changeModel.updateStateChange(createParsedChange());
+ await waitUntilObserved(
+ model.revertingChanges$,
+ revertingChanges => revertingChanges.length === 0
+ );
+ assert.isFalse(getChangeStub.called);
+ });
+
+ test('revertingChanges emits', async () => {
+ changeModel.updateStateChange({
+ ...createParsedChange(),
+ messages: [
+ {
+ ...createChangeMessage(),
+ message: 'Created a revert of this change as 123',
+ tag: MessageTag.TAG_REVERT as ReviewInputTag,
+ },
+ ],
+ });
+
+ await waitUntilObserved(
+ model.revertingChanges$,
+ revertingChanges => revertingChanges?.length === 1
+ );
+ assert.isTrue(getChangeStub.calledOnce);
+ });
+ });
+});
diff --git a/polygerrit-ui/app/models/checks/checks-model.ts b/polygerrit-ui/app/models/checks/checks-model.ts
index 2aa4aa4..729d46f 100644
--- a/polygerrit-ui/app/models/checks/checks-model.ts
+++ b/polygerrit-ui/app/models/checks/checks-model.ts
@@ -195,8 +195,6 @@
private readonly documentVisibilityChange$ = new BehaviorSubject(undefined);
- private readonly reloadListener: () => void;
-
private readonly visibilityChangeListener: () => void;
public checksSelectedPatchsetNumber$ = select(
@@ -414,8 +412,6 @@
'visibilitychange',
this.visibilityChangeListener
);
- this.reloadListener = () => this.reloadAll();
- document.addEventListener('reload', this.reloadListener);
}
private reportStats(state: {[name: string]: ChecksProviderState}) {
@@ -459,7 +455,6 @@
}
override finalize() {
- document.removeEventListener('reload', this.reloadListener);
document.removeEventListener(
'visibilitychange',
this.visibilityChangeListener
diff --git a/polygerrit-ui/app/models/comments/comments-model.ts b/polygerrit-ui/app/models/comments/comments-model.ts
index 5f40393..ac832a1 100644
--- a/polygerrit-ui/app/models/comments/comments-model.ts
+++ b/polygerrit-ui/app/models/comments/comments-model.ts
@@ -424,8 +424,6 @@
private patchNum?: PatchSetNum;
- private readonly reloadListener: () => void;
-
private drafts: {[path: string]: DraftInfo[]} = {};
private draftToastTask?: DelayedTask;
@@ -495,16 +493,6 @@
this.reloadAllPortedComments();
})
);
- this.reloadListener = () => {
- this.reloadAllComments();
- this.reloadAllPortedComments();
- };
- document.addEventListener('reload', this.reloadListener);
- }
-
- override finalize() {
- document.removeEventListener('reload', this.reloadListener);
- super.finalize();
}
// Note that this does *not* reload ported comments.
diff --git a/polygerrit-ui/app/models/views/change.ts b/polygerrit-ui/app/models/views/change.ts
index d2a0dd8..713d401 100644
--- a/polygerrit-ui/app/models/views/change.ts
+++ b/polygerrit-ui/app/models/views/change.ts
@@ -69,9 +69,16 @@
/** for scrolling a Change Log message into view in gr-change-view */
messageHash?: string;
- /** for logging where the user came from */
+ /**
+ * For logging where the user came from. This is handled by the router, so
+ * this is not inspected by the model.
+ */
usp?: string;
- /** triggers all change related data to be reloaded */
+ /**
+ * Triggers all change related data to be reloaded. This is implemented by
+ * intercepting change view state updates and `forceReload` causing the view
+ * state to be wiped clean as `undefined` in an intermediate update.
+ */
forceReload?: boolean;
/** triggers opening the reply dialog */
openReplyDialog?: boolean;
@@ -261,6 +268,20 @@
state => state?.basePatchNum
);
+ public readonly openReplyDialog$ = select(
+ this.state$,
+ state => state?.openReplyDialog
+ );
+
+ public readonly commentId$ = select(this.state$, state => state?.commentId);
+
+ public readonly edit$ = select(this.state$, state => !!state?.edit);
+
+ public readonly editPath$ = select(
+ this.state$,
+ state => state?.editView?.path
+ );
+
public readonly diffPath$ = select(
this.state$,
state => state?.diffView?.path
@@ -278,7 +299,11 @@
public readonly childView$ = select(this.state$, state => state?.childView);
- public readonly tab$ = select(this.state$, state => state?.tab);
+ public readonly tab$ = select(this.state$, state => {
+ if (state?.commentId) return Tab.COMMENT_THREADS;
+ if (state?.tab) return state.tab;
+ return Tab.FILES;
+ });
public readonly checksPatchset$ = select(
this.state$,
@@ -310,6 +335,39 @@
});
}
});
+ document.addEventListener('reload', this.reload);
+ }
+
+ override finalize(): void {
+ document.removeEventListener('reload', this.reload);
+ }
+
+ /**
+ * Calling this is the same as firing the 'reload' event. This is also the
+ * same as adding `forceReload` parameter in the URL. See below.
+ */
+ reload = () => {
+ const state = this.getState();
+ if (state !== undefined) this.forceLoad(state);
+ };
+
+ /**
+ * This is the destination of where the `reload()` method, the `reload` event
+ * and the `forceReload` URL parameter all end up.
+ */
+ private forceLoad(state: ChangeViewState) {
+ this.setState(undefined);
+ // We have to do this in a timeout, because we need the `undefined` value to
+ // be processed by all observers first and thus have the "reset" completed.
+ setTimeout(() => this.setState({...state, forceReload: undefined}));
+ }
+
+ override setState(state: ChangeViewState | undefined): void {
+ if (state?.forceReload) {
+ this.forceLoad(state);
+ } else {
+ super.setState(state);
+ }
}
toggleSelectedCheckRun(checkName: string) {
diff --git a/polygerrit-ui/app/services/app-context-init.ts b/polygerrit-ui/app/services/app-context-init.ts
index 3b9d3bb..8589ae3 100644
--- a/polygerrit-ui/app/services/app-context-init.ts
+++ b/polygerrit-ui/app/services/app-context-init.ts
@@ -68,6 +68,10 @@
ServiceWorkerInstaller,
serviceWorkerInstallerToken,
} from './service-worker-installer';
+import {
+ RelatedChangesModel,
+ relatedChangesModelToken,
+} from '../models/change/related-changes-model';
/**
* The AppContext lazy initializator for all services
@@ -151,7 +155,9 @@
resolver(navigationToken),
resolver(changeViewModelToken),
appContext.restApiService,
- resolver(userModelToken)
+ resolver(userModelToken),
+ resolver(pluginLoaderToken),
+ appContext.reportingService
),
],
[
@@ -172,7 +178,8 @@
new FilesModel(
resolver(changeModelToken),
resolver(commentsModelToken),
- appContext.restApiService
+ appContext.restApiService,
+ appContext.reportingService
),
],
[
@@ -181,6 +188,15 @@
new ConfigModel(resolver(changeModelToken), appContext.restApiService),
],
[
+ relatedChangesModelToken,
+ () =>
+ new RelatedChangesModel(
+ resolver(changeModelToken),
+ resolver(configModelToken),
+ appContext.restApiService
+ ),
+ ],
+ [
pluginLoaderToken,
() =>
new PluginLoader(
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 b60ace5..f8e4160 100644
--- a/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
+++ b/polygerrit-ui/app/test/mocks/gr-rest-api_mock.ts
@@ -67,6 +67,7 @@
createChange,
createCommit,
createConfig,
+ createMergeable,
createPreferences,
createServerInfo,
createSubmittedTogetherInfo,
@@ -368,7 +369,7 @@
return Promise.resolve(true);
},
getMergeable(): Promise<MergeableInfo | undefined> {
- throw new Error('getMergeable() not implemented by RestApiMock.');
+ return Promise.resolve(createMergeable());
},
getPlugins(): Promise<{[p: string]: PluginInfo} | undefined> {
return Promise.resolve({});
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index a92e932..dce8e4b 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -672,10 +672,10 @@
};
}
-export function createMergeable(): MergeableInfo {
+export function createMergeable(mergeable = false): MergeableInfo {
return {
submit_type: SubmitType.MERGE_IF_NECESSARY,
- mergeable: false,
+ mergeable,
};
}
diff --git a/polygerrit-ui/app/test/test-utils.ts b/polygerrit-ui/app/test/test-utils.ts
index d897423..6e20dd4 100644
--- a/polygerrit-ui/app/test/test-utils.ts
+++ b/polygerrit-ui/app/test/test-utils.ts
@@ -16,8 +16,9 @@
import {assert} from '@open-wc/testing';
import {Route, ViewState} from '../models/views/base';
import {PageContext} from '../elements/core/gr-router/gr-page';
+import {waitUntil} from '../utils/async-util';
export {query, queryAll, queryAndAssert} from '../utils/common-util';
-export {mockPromise} from '../utils/async-util';
+export {mockPromise, waitUntil} from '../utils/async-util';
export type {MockPromise} from '../utils/async-util';
export function isHidden(el: Element | undefined | null) {
@@ -149,32 +150,6 @@
return queryAndAssert<E>(el, selector);
}
-export async function waitUntil(
- predicate: (() => boolean) | (() => Promise<boolean>),
- message = 'The waitUntil() predicate is still false after 1000 ms.',
- timeout_ms = 1000
-): Promise<void> {
- const start = Date.now();
- let sleep = 0;
- if (await predicate()) return Promise.resolve();
- const error = new Error(message);
- return new Promise((resolve, reject) => {
- const waiter = async () => {
- if (await predicate()) {
- resolve();
- return;
- }
- if (Date.now() - start >= timeout_ms) {
- reject(error);
- return;
- }
- setTimeout(waiter, sleep);
- sleep = sleep === 0 ? 1 : sleep * 4;
- };
- waiter();
- });
-}
-
export async function waitUntilVisible(element: Element): Promise<void> {
return new Promise(resolve => {
whenVisible(element, () => resolve());
diff --git a/polygerrit-ui/app/types/events.ts b/polygerrit-ui/app/types/events.ts
index 4bf0c78..922d779 100644
--- a/polygerrit-ui/app/types/events.ts
+++ b/polygerrit-ui/app/types/events.ts
@@ -52,14 +52,13 @@
'open-fix-preview': OpenFixPreviewEvent;
'reply-to-comment': ReplyToCommentEvent;
// prettier-ignore
- 'reload': ReloadEvent;
+ 'reload': CustomEvent<{}>;
'remove-reviewer': RemoveReviewerEvent;
'show-alert': ShowAlertEvent;
'show-error': ShowErrorEvent;
'show-tab': SwitchTabEvent;
'show-secondary-tab': SwitchTabEvent;
'tap-item': TapItemEvent;
- 'title-change': TitleChangeEvent;
}
}
@@ -69,11 +68,12 @@
'network-error': NetworkErrorEvent;
'page-error': PageErrorEvent;
// prettier-ignore
- 'reload': ReloadEvent;
+ 'reload': CustomEvent<{}>;
'server-error': ServerErrorEvent;
'show-alert': ShowAlertEvent;
'show-error': ShowErrorEvent;
'auth-error': AuthErrorEvent;
+ 'title-change': TitleChangeEvent;
}
}
@@ -186,11 +186,6 @@
}
export type PageErrorEvent = CustomEvent<PageErrorEventDetail>;
-export interface ReloadEventDetail {
- clearPatchset: boolean;
-}
-export type ReloadEvent = CustomEvent<ReloadEventDetail>;
-
export interface RemoveAccountEventDetail {
account: AccountInfo;
}
diff --git a/polygerrit-ui/app/utils/async-util.ts b/polygerrit-ui/app/utils/async-util.ts
index 2dde07a..32a6867 100644
--- a/polygerrit-ui/app/utils/async-util.ts
+++ b/polygerrit-ui/app/utils/async-util.ts
@@ -371,3 +371,29 @@
setTimeout(resolve, timeoutMs);
});
}
+
+export async function waitUntil(
+ predicate: (() => boolean) | (() => Promise<boolean>),
+ message = 'The waitUntil() predicate is still false after 1000 ms.',
+ timeout_ms = 1000
+): Promise<void> {
+ if (await predicate()) return Promise.resolve();
+ const start = Date.now();
+ let sleep = 10;
+ const error = new Error(message);
+ return new Promise((resolve, reject) => {
+ const waiter = async () => {
+ if (await predicate()) {
+ resolve();
+ return;
+ }
+ if (Date.now() - start >= timeout_ms) {
+ reject(error);
+ return;
+ }
+ setTimeout(waiter, sleep);
+ sleep *= 2;
+ };
+ waiter();
+ });
+}
diff --git a/polygerrit-ui/app/utils/change-util.ts b/polygerrit-ui/app/utils/change-util.ts
index 5dda8d6..4490afa 100644
--- a/polygerrit-ui/app/utils/change-util.ts
+++ b/polygerrit-ui/app/utils/change-util.ts
@@ -20,6 +20,8 @@
interface ChangeStatusesOptions {
mergeable: boolean; // This can be wrong! See WARNING above
submitEnabled: boolean; // This can be wrong! See WARNING above
+ /** Is there a reverting change and if so, what status has it? */
+ revertingChangeStatus?: ChangeStatus;
}
export const ChangeDiffType = {
@@ -158,6 +160,12 @@
): ChangeStates[] {
const states = [];
if (change.status === ChangeStatus.MERGED) {
+ if (options?.revertingChangeStatus === ChangeStatus.MERGED) {
+ return [ChangeStates.MERGED, ChangeStates.REVERT_SUBMITTED];
+ }
+ if (options?.revertingChangeStatus !== undefined) {
+ return [ChangeStates.MERGED, ChangeStates.REVERT_CREATED];
+ }
return [ChangeStates.MERGED];
}
if (change.status === ChangeStatus.ABANDONED) {
diff --git a/polygerrit-ui/app/utils/change-util_test.ts b/polygerrit-ui/app/utils/change-util_test.ts
index ccaa1db..f768145 100644
--- a/polygerrit-ui/app/utils/change-util_test.ts
+++ b/polygerrit-ui/app/utils/change-util_test.ts
@@ -129,6 +129,32 @@
assert.deepEqual(changeStatuses(change), [ChangeStates.MERGED]);
});
+ test('Merged and Reverted status', () => {
+ const change = {
+ ...createChange(),
+ revisions: createRevisions(1),
+ current_revision: 'rev1' as CommitId,
+ status: ChangeStatus.MERGED,
+ };
+ assert.deepEqual(changeStatuses(change), [ChangeStates.MERGED]);
+ assert.deepEqual(
+ changeStatuses(change, {
+ revertingChangeStatus: ChangeStatus.NEW,
+ mergeable: true,
+ submitEnabled: true,
+ }),
+ [ChangeStates.MERGED, ChangeStates.REVERT_CREATED]
+ );
+ assert.deepEqual(
+ changeStatuses(change, {
+ revertingChangeStatus: ChangeStatus.MERGED,
+ mergeable: true,
+ submitEnabled: true,
+ }),
+ [ChangeStates.MERGED, ChangeStates.REVERT_SUBMITTED]
+ );
+ });
+
test('Abandoned status', () => {
const change = {
...createChange(),
diff --git a/polygerrit-ui/app/utils/event-util.ts b/polygerrit-ui/app/utils/event-util.ts
index 3704557..af545e7 100644
--- a/polygerrit-ui/app/utils/event-util.ts
+++ b/polygerrit-ui/app/utils/event-util.ts
@@ -95,8 +95,8 @@
fire(document, 'network-error', {error});
}
-export function fireTitleChange(target: EventTarget, title: string) {
- fire(target, 'title-change', {title});
+export function fireTitleChange(title: string) {
+ fire(document, 'title-change', {title});
}
// TODO(milutin) - remove once new gr-dialog will do it out of the box
@@ -122,8 +122,8 @@
fire(target, 'show-tab', detail);
}
-export function fireReload(target: EventTarget, clearPatchset?: boolean) {
- fire(target, 'reload', {clearPatchset: !!clearPatchset});
+export function fireReload(target: EventTarget) {
+ fire(target, 'reload', {});
}
export function waitForEventOnce<K extends keyof HTMLElementEventMap>(
diff --git a/polygerrit-ui/app/utils/message-util_test.ts b/polygerrit-ui/app/utils/message-util_test.ts
new file mode 100644
index 0000000..22a5e4d
--- /dev/null
+++ b/polygerrit-ui/app/utils/message-util_test.ts
@@ -0,0 +1,37 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {getRevertCreatedChangeIds} from './message-util';
+import {assert} from '@open-wc/testing';
+import {MessageTag} from '../constants/constants';
+import {ChangeId, ReviewInputTag} from '../api/rest-api';
+import {createChangeMessage} from '../test/test-data-generators';
+
+suite('message-util tests', () => {
+ test('getRevertCreatedChangeIds', () => {
+ const messages = [
+ {
+ ...createChangeMessage(),
+ message: 'Created a revert of this change as 123',
+ tag: MessageTag.TAG_REVERT as ReviewInputTag,
+ },
+ {
+ ...createChangeMessage(),
+ message: 'Created a revert of this change as xyz',
+ tag: MessageTag.TAG_REVERT as ReviewInputTag,
+ },
+ {
+ ...createChangeMessage(),
+ message: 'Created a revert of this change as abc',
+ tag: undefined,
+ },
+ ];
+
+ assert.deepEqual(getRevertCreatedChangeIds(messages), [
+ '123' as ChangeId,
+ 'xyz' as ChangeId,
+ ]);
+ });
+});
diff --git a/polygerrit-ui/app/utils/patch-set-util.ts b/polygerrit-ui/app/utils/patch-set-util.ts
index 7e16ad9..7f3b6eb 100644
--- a/polygerrit-ui/app/utils/patch-set-util.ts
+++ b/polygerrit-ui/app/utils/patch-set-util.ts
@@ -35,11 +35,6 @@
wip?: boolean;
}
-interface PatchRange {
- patchNum?: RevisionPatchSetNum;
- basePatchNum?: BasePatchSetNum;
-}
-
/**
* Whether the given patch is a numbered parent of a merge (i.e. a negative
* number).
@@ -294,10 +289,6 @@
return allPatchSets[0].num === EDIT;
}
-export function hasEditPatchsetLoaded(patchRange: PatchRange) {
- return patchRange.patchNum === EDIT;
-}
-
/**
* @param revisions A sorted array of revisions.
*
