Merge "Remove assert check for revert_submission submission"
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index 0389f39..20249df 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.restapi.change;
import static com.google.common.base.MoreObjects.firstNonNull;
+import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
import static com.google.gerrit.server.permissions.ChangePermission.REVERT;
import static com.google.gerrit.server.permissions.RefPermission.CREATE_CHANGE;
import static com.google.gerrit.server.project.ProjectCache.illegalState;
@@ -40,6 +41,7 @@
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
@@ -64,6 +66,7 @@
import com.google.gerrit.server.project.ContributorAgreementsChecker;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.restapi.change.CherryPickChange.Result;
@@ -98,7 +101,8 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
-public class RevertSubmission implements RestModifyView<ChangeResource, RevertInput> {
+public class RevertSubmission
+ implements RestModifyView<ChangeResource, RevertInput>, UiAction<ChangeResource> {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final Provider<InternalChangeQuery> queryProvider;
@@ -513,6 +517,35 @@
}
}
+ @Override
+ public Description getDescription(ChangeResource rsrc) {
+ Change change = rsrc.getChange();
+ boolean projectStatePermitsWrite = false;
+ try {
+ projectStatePermitsWrite =
+ projectCache.get(rsrc.getProject()).map(ProjectState::statePermitsWrite).orElse(false);
+ } catch (StorageException e) {
+ logger.atSevere().withCause(e).log(
+ "Failed to check if project state permits write: %s", rsrc.getProject());
+ }
+ return new UiAction.Description()
+ .setLabel("Revert submission")
+ .setTitle(
+ "Revert this change and all changes that have been submitted together with this change")
+ .setVisible(
+ and(
+ and(
+ change.isMerged()
+ && change.getSubmissionId() != null
+ && isChangePartOfSubmission(change.getSubmissionId())
+ && projectStatePermitsWrite,
+ permissionBackend
+ .user(rsrc.getUser())
+ .ref(change.getDest())
+ .testCond(CREATE_CHANGE)),
+ permissionBackend.user(rsrc.getUser()).change(rsrc.getNotes()).testCond(REVERT)));
+ }
+
/**
* @param submissionId the submission id of the change.
* @return True if the submission has more than one change, false otherwise.
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
index 4b33664..e35f758 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
@@ -62,12 +62,26 @@
}
@Test
- public void mergedChangeActionHasRevert() throws Exception {
+ public void changeActionOneMergedChangeHasOnlyNormalRevert() throws Exception {
String changeId = createChangeWithTopic().getChangeId();
gApi.changes().id(changeId).current().review(ReviewInput.approve());
gApi.changes().id(changeId).current().submit();
Map<String, ActionInfo> actions = getChangeActions(changeId);
assertThat(actions).containsKey("revert");
+ assertThat(actions).doesNotContainKey("revert_submission");
+ }
+
+ @Test
+ public void changeActionTwoMergedChangesHaveReverts() throws Exception {
+ String changeId1 = createChangeWithTopic().getChangeId();
+ String changeId2 = createChangeWithTopic().getChangeId();
+ gApi.changes().id(changeId1).current().review(ReviewInput.approve());
+ gApi.changes().id(changeId2).current().review(ReviewInput.approve());
+ gApi.changes().id(changeId2).current().submit();
+ Map<String, ActionInfo> actions1 = getChangeActions(changeId1);
+ assertThatMap(actions1).keys().containsAtLeast("revert", "revert_submission");
+ Map<String, ActionInfo> actions2 = getChangeActions(changeId2);
+ assertThatMap(actions2).keys().containsAtLeast("revert", "revert_submission");
}
@Test
diff --git a/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls.ts b/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls.ts
index 60b1a1a..41a448e 100644
--- a/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls.ts
+++ b/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls.ts
@@ -29,7 +29,14 @@
import {fire} from '../../../utils/event-util';
import {DiffInfo} from '../../../types/diff';
import {assertIsDefined} from '../../../utils/common-util';
-import {css, customElement, html, LitElement, property} from 'lit-element';
+import {
+ css,
+ customElement,
+ html,
+ LitElement,
+ property,
+ TemplateResult,
+} from 'lit-element';
import {
ContextButtonType,
@@ -205,7 +212,7 @@
private createContextButton(
type: ContextButtonType,
linesToExpand: number,
- tooltipText?: string
+ tooltip?: TemplateResult
) {
let text = '';
let groups: GrDiffGroup[] = []; // The groups that replace this one if tapped.
@@ -269,11 +276,6 @@
groups
);
- const tooltip = tooltipText
- ? html`<paper-tooltip offset="10"
- ><div class="breadcrumbTooltip">${tooltipText}</div></paper-tooltip
- >`
- : undefined;
const button = html` <gr-button
class="${classes}"
link="true"
@@ -396,6 +398,24 @@
return undefined;
}
+ private createBlockButtonTooltip(
+ buttonType: ContextButtonType,
+ syntaxPath: SyntaxBlock[],
+ linesToExpand: number
+ ) {
+ // Create breadcrumb string:
+ // myNamepace > MyClass > myMethod1 > aLocalFunctionInsideMethod1 > (anonymous)
+ const tooltipText = syntaxPath.length
+ ? syntaxPath.map(b => b.name || '(anonymous)').join(' > ')
+ : `${linesToExpand} common lines`;
+
+ const position =
+ buttonType === ContextButtonType.BLOCK_ABOVE ? 'top' : 'bottom';
+ return html`<paper-tooltip offset="10" position="${position}"
+ ><div class="breadcrumbTooltip">${tooltipText}</div></paper-tooltip
+ >`;
+ }
+
private createBlockButton(
buttonType: ContextButtonType,
numLines: number,
@@ -408,14 +428,8 @@
syntaxTree
);
let linesToExpand = numLines;
- let tooltipText = `${linesToExpand} common lines`;
if (outlineSyntaxPath.length) {
const {range} = outlineSyntaxPath[outlineSyntaxPath.length - 1];
- // Create breadcrumb string:
- // myNamepace > MyClass > myMethod1 > aLocalFunctionInsideMethod1 > (anonymous)
- tooltipText = outlineSyntaxPath
- .map(b => b.name || '(anonymous)')
- .join(' > ');
const targetLine =
buttonType === ContextButtonType.BLOCK_ABOVE
? range.end_line
@@ -425,7 +439,12 @@
linesToExpand = distanceToTargetLine;
}
}
- return this.createContextButton(buttonType, linesToExpand, tooltipText);
+ const tooltip = this.createBlockButtonTooltip(
+ buttonType,
+ outlineSyntaxPath,
+ linesToExpand
+ );
+ return this.createContextButton(buttonType, linesToExpand, tooltip);
}
private contextRange() {
diff --git a/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls_test.ts b/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls_test.ts
index d866c1a..f59b19d 100644
--- a/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls_test.ts
@@ -366,12 +366,21 @@
const blockExpansionButtons = element.shadowRoot!.querySelectorAll(
'.blockExpansion gr-button'
);
- // querySelector('.breadcrumbTooltip')!.textContent!.trim()
+ const tooltipAbove = blockExpansionButtons[0].querySelector(
+ 'paper-tooltip'
+ )!;
+ const tooltipBelow = blockExpansionButtons[1].querySelector(
+ 'paper-tooltip'
+ )!;
assert.equal(
- blockExpansionButtons[0]
- .querySelector('.breadcrumbTooltip')!
- .textContent?.trim(),
+ tooltipAbove.querySelector('.breadcrumbTooltip')!.textContent?.trim(),
'20 common lines'
);
+ assert.equal(
+ tooltipBelow.querySelector('.breadcrumbTooltip')!.textContent?.trim(),
+ '20 common lines'
+ );
+ assert.equal(tooltipAbove!.getAttribute('position'), 'top');
+ assert.equal(tooltipBelow!.getAttribute('position'), 'bottom');
});
});