commit 21f18ab19f749d4a15096c3445121bc40e2dd55b Author: Ben Rohlfs Date: Tue Oct 13 23:03:08 2020 +0200 Fix 'Send' button being enabled even without the user making any changes This could happen, if a label has a default vote that 'Send' would apply to the change. For the purpose of 'dirty checking' such default votes should not be considered. Change-Id: I6108c8723e1eba853fe0c4241bac0f733b4e3a89 diff --git a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts index 245c65f..2e9189e 100644 --- a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts +++ b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts @@ -61,7 +61,7 @@ export class GrLabelScores extends GestureEventListeners( @property({type: Object}) _labelValues?: LabelValuesMap; - getLabelValues(): LabelNameToValuesMap { + getLabelValues(includeDefaults = true): LabelNameToValuesMap { const labels: LabelNameToValuesMap = {}; if (this.shadowRoot === null || !this.change) { return labels; @@ -106,8 +106,12 @@ export class GrLabelScores extends GestureEventListeners( prevValNum = prevVal; } + const defValNum = this._getDefaultValue(this.change.labels, label); + if (selectedVal !== prevValNum) { - labels[label] = selectedVal; + if (includeDefaults || !!prevValNum || selectedVal !== defValNum) { + labels[label] = selectedVal; + } } } return labels; @@ -126,6 +130,12 @@ export class GrLabelScores extends GestureEventListeners( return numberValue; } + _getDefaultValue(labels?: LabelNameToInfoMap, labelName?: string) { + if (!labelName || !labels?.[labelName]) return undefined; + const labelInfo = labels[labelName] as DetailedLabelInfo; + return labelInfo.default_value; + } + _getVoteForAccount( labels: LabelNameToInfoMap | undefined, labelName: string, diff --git a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.js b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.js index ffc17cd..ae639e1 100644 --- a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.js +++ b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.js @@ -99,6 +99,22 @@ suite('gr-label-scores tests', () => { }); }); + test('getLabelValues includeDefaults', async () => { + element.change = { + _number: '123', + labels: { + 'Code-Review': { + values: {'0': 'meh', '+1': 'good', '-1': 'bad'}, + default_value: 0, + }, + }, + }; + await flush(); + + assert.deepEqual(element.getLabelValues(true), {'Code-Review': 0}); + assert.deepEqual(element.getLabelValues(false), {}); + }); + test('_getVoteForAccount', () => { const labelName = 'Code-Review'; assert.strictEqual(element._getVoteForAccount( 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 50bf411..1afaa64 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 @@ -321,6 +321,11 @@ export class GrReplyDialog extends KeyboardShortcutMixin( @property({type: Boolean}) _reviewersMutated = false; + /** + * Signifies that the user has changed their vote on a label or (if they have + * not yet voted on a label) if a selected vote is different from the default + * vote. + */ @property({type: Boolean}) _labelsChanged = false; @@ -1368,7 +1373,7 @@ export class GrReplyDialog extends KeyboardShortcutMixin( _handleLabelsChanged() { this._labelsChanged = - Object.keys(this.$.labelScores.getLabelValues()).length !== 0; + Object.keys(this.$.labelScores.getLabelValues(false)).length !== 0; } _isState(knownLatestState?: LatestPatchState, value?: LatestPatchState) {