Skip to content

Commit 801fe3b

Browse files
committed
spec: clean up timer in GradebookExportManager
closes CNVS-34890 The timer in GradebookExportManager was getting cleaned up only when the export succeeded or failed and was continuing on in the test environment. This timer is now automatically cleaned up when the ActionMenu is unmounted test plan * Ensure the JS specs are no longer polluted with calls to the monitoring URL and other artifacts of this timer running * Ensure the Action menu's import/export functionality works as always. Change-Id: I89ee9ec48eb0c293c32857a673a9823b4da204af Reviewed-on: https://gerrit.instructure.com/101691 Tested-by: Jenkins Reviewed-by: Jeremy Neander <jneander@instructure.com> Reviewed-by: Spencer Olson <solson@instructure.com> QA-Review: Anju Reddy <areddy@instructure.com> Product-Review: Keith T. Garner <kgarner@instructure.com>
1 parent bfbaaec commit 801fe3b

3 files changed

Lines changed: 51 additions & 10 deletions

File tree

app/jsx/gradezilla/default_gradebook/components/ActionMenu.jsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ define([
6969
this.exportManager = new GradebookExportManager(this.props.gradebookExportUrl, this.props.currentUserId, existingExport);
7070
}
7171

72+
componentWillUnmount () {
73+
if (this.exportManager) this.exportManager.clearMonitor();
74+
}
75+
7276
getExistingExport () {
7377
if (!(this.props.lastExport && this.props.attachment)) return undefined;
7478
if (!(this.props.lastExport.progressId && this.props.attachment.id)) return undefined;

app/jsx/gradezilla/shared/GradebookExportManager.jsx

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,17 @@ define([
99
static DEFAULT_MONITORING_BASE_URL = '/api/v1/progress';
1010
static DEFAULT_ATTACHMENT_BASE_URL = '/api/v1/users';
1111

12+
static exportCompleted (workflowState) {
13+
return workflowState === 'completed';
14+
}
15+
16+
// Returns false if the workflowState is 'failed' or an unknown state
17+
static exportFailed (workflowState) {
18+
if (workflowState === 'failed') return true;
19+
20+
return !['completed', 'queued', 'running'].includes(workflowState);
21+
}
22+
1223
constructor (exportingUrl, currentUserId, existingExport, pollingInterval = GradebookExportManager.DEFAULT_POLLING_INTERVAL) {
1324
this.pollingInterval = pollingInterval;
1425

@@ -38,19 +49,26 @@ define([
3849
return `${this.attachmentBaseUrl}/${this.export.attachmentId}`;
3950
}
4051

52+
clearMonitor () {
53+
if (this.exportStatusPoll) {
54+
window.clearInterval(this.exportStatusPoll);
55+
this.exportStatusPoll = null;
56+
}
57+
}
58+
4159
monitorExport (resolve, reject) {
4260
if (!this.monitoringUrl()) {
4361
this.export = undefined;
4462

4563
reject(I18n.t('No way to monitor gradebook exports provided!'));
4664
}
4765

48-
const exportStatusPoll = window.setInterval(() => {
66+
this.exportStatusPoll = window.setInterval(() => {
4967
axios.get(this.monitoringUrl()).then((response) => {
5068
const workflowState = response.data.workflow_state;
5169

52-
if (workflowState === 'completed') {
53-
window.clearInterval(exportStatusPoll);
70+
if (GradebookExportManager.exportCompleted(workflowState)) {
71+
this.clearMonitor();
5472

5573
// Export is complete => let's get the attachment url
5674
axios.get(this.attachmentUrl()).then((attachmentResponse) => {
@@ -65,8 +83,8 @@ define([
6583
}).catch((error) => {
6684
reject(error);
6785
});
68-
} else if (workflowState === 'failed') {
69-
window.clearInterval(exportStatusPoll);
86+
} else if (GradebookExportManager.exportFailed(workflowState)) {
87+
this.clearMonitor();
7088

7189
reject(I18n.t('Error exporting gradebook: %{msg}', { msg: response.data.message }));
7290
}

spec/javascripts/jsx/gradezilla/shared/GradebookExportManagerSpec.jsx

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,12 +136,13 @@ define([
136136
teardown () {
137137
moxios.uninstall();
138138

139+
this.subject.clearMonitor();
139140
this.subject = undefined;
140141
}
141142
});
142143

143144
test('returns a rejected promise if the manager has no exportingUrl set', function () {
144-
this.subject = new GradebookExportManager(exportingUrl, currentUserId, undefined);
145+
this.subject = new GradebookExportManager(exportingUrl, currentUserId);
145146
this.subject.exportingUrl = undefined;
146147

147148
return this.subject.startExport().catch((reason) => {
@@ -163,7 +164,7 @@ define([
163164
attachmentId: 'newAttachmentId'
164165
};
165166

166-
this.subject = new GradebookExportManager(exportingUrl, currentUserId, undefined);
167+
this.subject = new GradebookExportManager(exportingUrl, currentUserId);
167168
this.subject.monitorExport = (resolve, _reject) => {
168169
resolve('success');
169170
};
@@ -175,7 +176,7 @@ define([
175176

176177
test('clears any new export and returns a rejected promise if no monitoring is possible', function () {
177178
this.stub(GradebookExportManager.prototype, 'monitoringUrl').returns(undefined);
178-
this.subject = new GradebookExportManager(exportingUrl, currentUserId, undefined);
179+
this.subject = new GradebookExportManager(exportingUrl, currentUserId);
179180

180181
return this.subject.startExport().catch((reason) => {
181182
equal(reason, 'No way to monitor gradebook exports provided!');
@@ -186,7 +187,7 @@ define([
186187
test('starts polling for progress and returns a rejected promise on progress failure', function () {
187188
const expectedMonitoringUrl = `${monitoringBase}/newProgressId`;
188189

189-
this.subject = new GradebookExportManager(exportingUrl, currentUserId, undefined);
190+
this.subject = new GradebookExportManager(exportingUrl, currentUserId);
190191

191192
moxios.stubRequest(expectedMonitoringUrl, {
192193
status: 200,
@@ -201,11 +202,29 @@ define([
201202
});
202203
});
203204

205+
test('starts polling for progress and returns a rejected promise on unknown progress status', function () {
206+
const expectedMonitoringUrl = `${monitoringBase}/newProgressId`;
207+
208+
this.subject = new GradebookExportManager(exportingUrl, currentUserId);
209+
210+
moxios.stubRequest(expectedMonitoringUrl, {
211+
status: 200,
212+
responseText: {
213+
workflow_state: 'discombobulated',
214+
message: 'Pattern buffer degradation'
215+
}
216+
});
217+
218+
return this.subject.startExport().catch((reason) => {
219+
equal(reason, 'Error exporting gradebook: Pattern buffer degradation');
220+
});
221+
});
222+
204223
test('starts polling for progress and returns a fulfilled promise on progress completion', function () {
205224
const expectedMonitoringUrl = `${monitoringBase}/newProgressId`;
206225
const expectedAttachmentUrl = `${attachmentBase}/newAttachmentId`;
207226

208-
this.subject = new GradebookExportManager(exportingUrl, currentUserId, undefined);
227+
this.subject = new GradebookExportManager(exportingUrl, currentUserId);
209228

210229
moxios.stubRequest(expectedMonitoringUrl, {
211230
status: 200,

0 commit comments

Comments
 (0)