Surface errors in hashtag flow
Apply & Create buttons are also replaced with a single Cancel button.
https://imgur.com/a/peeCuau
Release-Notes: skip
Change-Id: I9d08e31121101f8df73c2944e3208a7a3e6796d5
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow.ts
index 97b64d8..cd4834c 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow.ts
@@ -12,6 +12,7 @@
import '../../shared/gr-button/gr-button';
import '../../shared/gr-autocomplete/gr-autocomplete';
import '@polymer/iron-dropdown/iron-dropdown';
+import '@polymer/iron-icon/iron-icon';
import {IronDropdownElement} from '@polymer/iron-dropdown/iron-dropdown';
import {getAppContext} from '../../../services/app-context';
import {notUndefined} from '../../../types/types';
@@ -103,6 +104,7 @@
.loadingOrError {
display: flex;
gap: var(--spacing-s);
+ align-items: baseline;
}
/* The basics of .loadingSpin are defined in spinnerStyles. */
@@ -111,6 +113,17 @@
position: relative;
top: 3px;
}
+ .error {
+ color: var(--deemphasized-text-color);
+ }
+ iron-icon {
+ color: var(--error-color);
+ /* Center with text by aligning it to the top and then pushing it down
+ to match the text */
+ vertical-align: top;
+ position: relative;
+ top: 7px;
+ }
`,
];
}
@@ -171,21 +184,35 @@
${this.renderLoadingOrError()}
</div>
<div class="buttons">
- <gr-button
- id="create-new-hashtag-button"
- flatten
- @click=${() =>
- this.applyHashtags('Creating hashtag...', true)}
- .disabled=${isCreateNewHashtagDisabled}
- >Create new hashtag</gr-button
- >
- <gr-button
- id="apply-hashtag-button"
- flatten
- @click=${() => this.applyHashtags('Applying hashtag...')}
- .disabled=${this.isApplyHashtagDisabled()}
- >Apply</gr-button
- >
+ ${when(
+ this.overallProgress !== ProgressStatus.FAILED,
+ () => html`
+ <gr-button
+ id="create-new-hashtag-button"
+ flatten
+ @click=${() =>
+ this.applyHashtags('Creating hashtag...', true)}
+ .disabled=${isCreateNewHashtagDisabled}
+ >Create new hashtag</gr-button
+ >
+ <gr-button
+ id="apply-hashtag-button"
+ flatten
+ @click=${() =>
+ this.applyHashtags('Applying hashtag...')}
+ .disabled=${this.isApplyHashtagDisabled()}
+ >Apply</gr-button
+ >
+ `,
+ () => html`
+ <gr-button
+ id="cancel-button"
+ flatten
+ @click=${this.closeDropdown}
+ >Cancel</gr-button
+ >
+ `
+ )}
</div>
</div>
</div>
@@ -225,15 +252,20 @@
}
private renderLoadingOrError() {
- if (this.overallProgress === ProgressStatus.RUNNING) {
- return html`
- <span class="loadingSpin"></span>
- <span class="loadingText">${this.loadingText}</span>
- `;
- } else if (this.errorText !== undefined) {
- return html`<div class="error">${this.errorText}</div>`;
+ switch (this.overallProgress) {
+ case ProgressStatus.RUNNING:
+ return html`
+ <span class="loadingSpin"></span>
+ <span class="loadingText">${this.loadingText}</span>
+ `;
+ case ProgressStatus.FAILED:
+ return html`
+ <iron-icon icon="gr-icons:error"></iron-icon>
+ <div class="error">${this.errorText}</div>
+ `;
+ default:
+ return nothing;
}
- return nothing;
}
private isApplyHashtagDisabled() {
@@ -256,12 +288,7 @@
}
private toggleDropdown() {
- if (this.isDropdownOpen) {
- this.closeDropdown();
- } else {
- this.reset();
- this.openDropdown();
- }
+ this.isDropdownOpen ? this.closeDropdown() : this.openDropdown();
}
private reset() {
@@ -277,6 +304,7 @@
}
private openDropdown() {
+ this.reset();
this.isDropdownOpen = true;
this.dropdown?.open();
}
@@ -316,11 +344,16 @@
this.loadingText = loadingText;
this.trackPromises(
this.getBulkActionsModel().addHashtags(allHashtagsToApply),
- alert
+ alert,
+ creatingHashtag ? 'Failed to create' : 'Failed to apply'
);
}
- private async trackPromises(promises: Promise<Hashtag[]>[], alert: string) {
+ private async trackPromises(
+ promises: Promise<Hashtag[]>[],
+ alert: string,
+ errorText: string
+ ) {
this.overallProgress = ProgressStatus.RUNNING;
const results = await allSettled(promises);
if (results.every(result => result.status === 'fulfilled')) {
@@ -332,7 +365,7 @@
this.dropdown?.notifyResize();
} else {
this.overallProgress = ProgressStatus.FAILED;
- // TODO: when some are rejected, show error and Cancel button
+ this.errorText = errorText;
}
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow_test.ts
index 176382d..b805046 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-hashtag-flow/gr-change-list-hashtag-flow_test.ts
@@ -16,6 +16,7 @@
import {
MockPromise,
mockPromise,
+ query,
queryAll,
queryAndAssert,
stubRestApi,
@@ -173,6 +174,13 @@
await element.updateComplete;
}
+ async function rejectPromises() {
+ setChangeHashtagPromises[0].reject(new Error('error'));
+ setChangeHashtagPromises[1].reject(new Error('error'));
+ setChangeHashtagPromises[2].reject(new Error('error'));
+ await element.updateComplete;
+ }
+
setup(async () => {
stubRestApi('getDetailedChangesWithActions').resolves(changes);
setChangeHashtagPromises = [];
@@ -329,6 +337,34 @@
);
});
+ test('shows error when apply hashtag fails', async () => {
+ // selects "hashtag1"
+ queryAll<HTMLButtonElement>(element, 'button.chip')[0].click();
+ await element.updateComplete;
+
+ queryAndAssert<GrButton>(element, '#apply-hashtag-button').click();
+ await element.updateComplete;
+
+ assert.equal(
+ queryAndAssert(element, '.loadingText').textContent,
+ 'Applying hashtag...'
+ );
+
+ await rejectPromises();
+ await element.updateComplete;
+ await waitUntil(() => query(element, '.error') !== undefined);
+
+ assert.equal(
+ queryAndAssert(element, '.error').textContent,
+ 'Failed to apply'
+ );
+ assert.equal(
+ queryAndAssert(element, 'gr-button#cancel-button').textContent,
+ 'Cancel'
+ );
+ assert.isUndefined(query(element, '.loadingText'));
+ });
+
test('apply multiple hashtag from selected change', async () => {
const alertStub = sinon.stub();
element.addEventListener('show-alert', alertStub);
@@ -482,11 +518,50 @@
queryAndAssert<IronDropdownElement>(element, 'iron-dropdown').opened
);
assert.equal(
- queryAll<HTMLSpanElement>(element, 'button.chip')[2].innerText,
+ queryAll<HTMLButtonElement>(element, 'button.chip')[2].innerText,
'foo'
);
});
+ test('shows error when create hashtag fails', async () => {
+ const getHashtagsStub = stubRestApi(
+ 'getChangesWithSimilarHashtag'
+ ).resolves([]);
+ const autocomplete = queryAndAssert<GrAutocomplete>(
+ element,
+ 'gr-autocomplete'
+ );
+ autocomplete.setFocus(true);
+ autocomplete.text = 'foo';
+ await element.updateComplete;
+ await waitUntilCalled(getHashtagsStub, 'getHashtagsStub');
+ assert.isTrue(
+ queryAndAssert<GrButton>(element, '#apply-hashtag-button').disabled
+ );
+
+ queryAndAssert<GrButton>(element, '#create-new-hashtag-button').click();
+ await element.updateComplete;
+
+ assert.equal(
+ queryAndAssert(element, '.loadingText').textContent,
+ 'Creating hashtag...'
+ );
+
+ await rejectPromises();
+ await element.updateComplete;
+ await waitUntil(() => query(element, '.error') !== undefined);
+
+ assert.equal(
+ queryAndAssert(element, '.error').textContent,
+ 'Failed to create'
+ );
+ assert.equal(
+ queryAndAssert(element, 'gr-button#cancel-button').textContent,
+ 'Cancel'
+ );
+ assert.isUndefined(query(element, '.loadingText'));
+ });
+
test('cannot apply existing hashtag already on selected changes', async () => {
const alertStub = sinon.stub();
element.addEventListener('show-alert', alertStub);
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow.ts
index 4477f4a..e286a74 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow.ts
@@ -327,12 +327,7 @@
}
private toggleDropdown() {
- if (this.isDropdownOpen) {
- this.closeDropdown();
- } else {
- this.reset();
- this.openDropdown();
- }
+ this.isDropdownOpen ? this.closeDropdown() : this.openDropdown();
}
private reset() {
@@ -348,6 +343,7 @@
}
private openDropdown() {
+ this.reset();
this.isDropdownOpen = true;
this.dropdown?.open();
}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow_test.ts
index 9d853de..c639701 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-topic-flow/gr-change-list-topic-flow_test.ts
@@ -388,6 +388,10 @@
queryAndAssert(element, '.error').textContent,
'Failed to remove topic'
);
+ assert.equal(
+ queryAndAssert(element, 'gr-button#cancel-button').textContent,
+ 'Cancel'
+ );
assert.isUndefined(query(element, '.loadingText'));
});
@@ -645,6 +649,10 @@
queryAndAssert(element, '.error').textContent,
'Failed to create topic'
);
+ assert.equal(
+ queryAndAssert(element, 'gr-button#cancel-button').textContent,
+ 'Cancel'
+ );
assert.isUndefined(query(element, '.loadingText'));
});
@@ -725,6 +733,10 @@
queryAndAssert(element, '.error').textContent,
'Failed to apply topic'
);
+ assert.equal(
+ queryAndAssert(element, 'gr-button#cancel-button').textContent,
+ 'Cancel'
+ );
assert.isUndefined(query(element, '.loadingText'));
});
});
