Always take reviewers from new change
Currently new change does not have complete reviewer info but
old change has.
Hence we were explicitly taking the reviewer info from old
change but this does not work for stacked changes where old
change has reviewers undefined.
Change 346700 that GrAccountLabel requests detailed account if
partial account is sent hence we can always rely on new change
data.
Given that we are always relying to new change data we don't
need special merging logic anymore.
Downside is that GrAccountLabel needs to request this information
but we are fine making the tradeoff to make the codebase simpler.
Google-bug-id: b/246912339
Release-Notes: skip
Change-Id: I365226b661f8b57e78dab16e5d788fc0682db2d0
diff --git a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
index b58fc3f..1d3802a 100644
--- a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
+++ b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model.ts
@@ -259,11 +259,7 @@
if (selectableChangeNums !== currentState.selectableChangeNums) return;
const allDetailedChanges: Map<NumericChangeId, ChangeInfo> = new Map();
for (const detailedChange of changeDetails ?? []) {
- const basicChange = basicChanges.get(detailedChange._number)!;
- allDetailedChanges.set(
- detailedChange._number,
- this.mergeOldAndDetailedChangeInfos(basicChange, detailedChange)
- );
+ allDetailedChanges.set(detailedChange._number, detailedChange);
}
this.setState({
...currentState,
@@ -272,17 +268,6 @@
});
}
- private mergeOldAndDetailedChangeInfos(
- originalChange: ChangeInfo,
- newData: ChangeInfo
- ) {
- return {
- ...originalChange,
- ...newData,
- reviewers: originalChange.reviewers,
- };
- }
-
private getNewReviewersToChange(
change: ChangeInfo,
state: ReviewerState,
diff --git a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts
index 7bc37d6..2192246 100644
--- a/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts
+++ b/polygerrit-ui/app/models/bulk-actions/bulk-actions-model_test.ts
@@ -14,10 +14,8 @@
NumericChangeId,
ChangeStatus,
HttpMethod,
- SubmitRequirementStatus,
AccountInfo,
ReviewerState,
- AccountId,
GroupInfo,
Hashtag,
} from '../../api/rest-api';
@@ -491,60 +489,6 @@
);
});
- test('sync retains keys from original change including reviewers', async () => {
- const c1: ChangeInfo = {
- ...createChange(),
- _number: 1 as NumericChangeId,
- submit_requirements: [
- {
- name: 'a',
- status: SubmitRequirementStatus.FORCED,
- submittability_expression_result: {
- expression: 'b',
- },
- },
- ],
- reviewers: {
- REVIEWER: [{_account_id: 1 as AccountId, display_name: 'MyName'}],
- },
- };
-
- stubRestApi('getDetailedChangesWithActions').callsFake(() => {
- const change: ChangeInfo = {
- ...createChange(),
- _number: 1 as NumericChangeId,
- actions: {abandon: {}},
- // detailed data will be missing names
- reviewers: {REVIEWER: [createAccountWithIdNameAndEmail()]},
- };
- assert.isNotOk(change.submit_requirements);
- return Promise.resolve([change]);
- });
-
- bulkActionsModel.sync([c1]);
-
- await waitUntilObserved(
- bulkActionsModel.loadingState$,
- s => s === LoadingState.LOADED
- );
-
- const changeAfterSync = bulkActionsModel
- .getState()
- .allChanges.get(1 as NumericChangeId);
- assert.deepEqual(changeAfterSync!.submit_requirements, [
- {
- name: 'a',
- status: SubmitRequirementStatus.FORCED,
- submittability_expression_result: {
- expression: 'b',
- },
- },
- ]);
- assert.deepEqual(changeAfterSync!.actions, {abandon: {}});
- // original reviewers are kept, which includes more details than loaded ones
- assert.deepEqual(changeAfterSync!.reviewers, c1.reviewers);
- });
-
test('sync ignores outdated fetch responses', async () => {
const c1 = createChange();
c1._number = 1 as NumericChangeId;
