Show negated passing atoms as blocking submit requirements A submit requirement that is unsatisfied because of a veto vote (-label:Code-Review=MIN) is shown with a red "cancel" icon, while one that is merely missing votes is shown with the neutral "block" icon. Other requirements can be blocked in the same way, by something that is present but must be absent. For example, a requirement like "-subject:WIP" or "-prefixsubject:Edit" cannot be satisfied by adding votes; the author must change the commit. These were shown with the neutral icon, as if waiting for reviewers. Treat an unsatisfied requirement as blocking when one of its passing atoms is negated in the submittability expression, either with a unary minus or with NOT. The veto vote case is a special case of this rule, and the existing check for it is kept. Note that this also applies to the default No-Unresolved-Comments requirement (-has:unresolved), which is now shown as blocking while there are unresolved comments. These also require action from the author or the commenter rather than from additional voters. Release-Notes: Unsatisfied submit requirements with a negated passing atom are shown as blocking Change-Id: Ie5b938b3d376d736a010e58baf5c25ab09d56c85
diff --git a/polygerrit-ui/app/utils/label-util.ts b/polygerrit-ui/app/utils/label-util.ts index 0e13c44..9e377ba 100644 --- a/polygerrit-ui/app/utils/label-util.ts +++ b/polygerrit-ui/app/utils/label-util.ts
@@ -8,6 +8,7 @@ isQuickLabelInfo, LabelNameToValuesMap, LabelValueToDescriptionMap, + SubmitRequirementExpressionInfo, SubmitRequirementResultInfo, SubmitRequirementStatus, } from '../api/rest-api'; @@ -22,6 +23,10 @@ } from '../types/common'; import {ParsedChangeInfo} from '../types/types'; import {assertNever, hasOwnProperty, unique} from './common-util'; +import { + atomizeExpression, + SubmitRequirementExpressionAtomStatus, +} from './submit-requirement-util'; export interface Label { name: string; @@ -573,14 +578,44 @@ ); } +/** + * Returns true if the requirement is unsatisfied because something is present + * that must be absent, e.g. a veto vote (`-label:Code-Review=MIN`) or a + * forbidden commit subject (`-subject:WIP`). Such a requirement cannot be + * satisfied by adding votes. + */ export function isBlockingCondition( requirement: SubmitRequirementResultInfo ): boolean { if (requirement.status !== SubmitRequirementStatus.UNSATISFIED) return false; - return !!requirement.submittability_expression_result.passing_atoms?.some( - atom => atom.match(/^label[0-9]*:[\w-]+=MIN$/) - ); + const result = requirement.submittability_expression_result; + if ( + result.passing_atoms?.some(atom => atom.match(/^label[0-9]*:[\w-]+=MIN$/)) + ) { + return true; + } + return hasPassingNegatedAtom(result); +} + +function hasPassingNegatedAtom(expression: SubmitRequirementExpressionInfo) { + if (!expression.passing_atoms?.length) return false; + const parts = atomizeExpression(expression); + return parts.some((part, i) => { + if (!part.isAtom) return false; + // atomizeExpression() includes a unary minus in the atom and negates its + // status, so a passing atom that is negated is reported as failing. + if (part.value.startsWith('-')) { + return part.atomStatus === SubmitRequirementExpressionAtomStatus.FAILING; + } + const previous = i > 0 ? parts[i - 1] : undefined; + return ( + part.atomStatus === SubmitRequirementExpressionAtomStatus.PASSING && + !!previous && + !previous.isAtom && + /(^|[\s(])NOT\s+$/.test(previous.value) + ); + }); } const LABEL_PREFIX_INVALID_PROLOG = 'Invalid-Prolog-Rules-Label-Name--';
diff --git a/polygerrit-ui/app/utils/label-util_test.ts b/polygerrit-ui/app/utils/label-util_test.ts index 45b3802..8a5ce8e 100644 --- a/polygerrit-ui/app/utils/label-util_test.ts +++ b/polygerrit-ui/app/utils/label-util_test.ts
@@ -912,6 +912,86 @@ }; assert.isFalse(isBlockingCondition(requirement)); }); + + test('true for passing negated non-label atom', () => { + const requirement: SubmitRequirementResultInfo = { + name: 'Commit-Subject', + status: SubmitRequirementStatus.UNSATISFIED, + is_legacy: false, + submittability_expression_result: { + expression: '-prefixsubject:Edit', + fulfilled: false, + status: SubmitRequirementExpressionInfoStatus.FAIL, + passing_atoms: ['prefixsubject:Edit'], + failing_atoms: [], + }, + }; + assert.isTrue(isBlockingCondition(requirement)); + }); + + test('true for passing atom negated with NOT', () => { + const requirement: SubmitRequirementResultInfo = { + name: 'Commit-Subject', + status: SubmitRequirementStatus.UNSATISFIED, + is_legacy: false, + submittability_expression_result: { + expression: 'label:Verified=MAX AND NOT prefixsubject:Edit', + fulfilled: false, + status: SubmitRequirementExpressionInfoStatus.FAIL, + passing_atoms: ['label:Verified=MAX', 'prefixsubject:Edit'], + failing_atoms: [], + }, + }; + assert.isTrue(isBlockingCondition(requirement)); + }); + + test('false for failing non-negated non-label atom', () => { + const requirement: SubmitRequirementResultInfo = { + name: 'Commit-Subject', + status: SubmitRequirementStatus.UNSATISFIED, + is_legacy: false, + submittability_expression_result: { + expression: 'prefixsubject:Fix', + fulfilled: false, + status: SubmitRequirementExpressionInfoStatus.FAIL, + passing_atoms: [], + failing_atoms: ['prefixsubject:Fix'], + }, + }; + assert.isFalse(isBlockingCondition(requirement)); + }); + + test('false for passing non-negated atom', () => { + const requirement: SubmitRequirementResultInfo = { + name: 'Commit-Subject', + status: SubmitRequirementStatus.UNSATISFIED, + is_legacy: false, + submittability_expression_result: { + expression: 'label:Verified=MAX AND prefixsubject:Fix', + fulfilled: false, + status: SubmitRequirementExpressionInfoStatus.FAIL, + passing_atoms: ['prefixsubject:Fix'], + failing_atoms: ['label:Verified=MAX'], + }, + }; + assert.isFalse(isBlockingCondition(requirement)); + }); + + test('false when satisfied', () => { + const requirement: SubmitRequirementResultInfo = { + name: 'Commit-Subject', + status: SubmitRequirementStatus.SATISFIED, + is_legacy: false, + submittability_expression_result: { + expression: '-prefixsubject:Edit', + fulfilled: false, + status: SubmitRequirementExpressionInfoStatus.FAIL, + passing_atoms: ['prefixsubject:Edit'], + failing_atoms: [], + }, + }; + assert.isFalse(isBlockingCondition(requirement)); + }); }); suite('valueString', () => {