Skip to content
This repository was archived by the owner on Jan 28, 2020. It is now read-only.

Implemented import status #810

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
235 changes: 235 additions & 0 deletions ui/jstests/listing/test_import_status.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,235 @@
define(['QUnit', 'jquery', 'import_status', 'react',
'test_utils'],
function (QUnit, $, ImportStatus, React, TestUtils) {
'use strict';

var waitForAjax = TestUtils.waitForAjax;

var TASK1_SUCCESS = {
"id": "task1",
"status": "success",
"task_type": "import_course",
"task_info": {
"repo_slug": "repo"
}
};
var OTHER_REPO_FAILURE = {
"id": "task2",
"status": "failure",
"task_type": "import_course",
"task_info": {
"repo_slug": "otherrepo"
}
};
var NOT_IMPORT_SUCCESS = {
"id": "task4",
"status": "success",
"task_type": "reindex"
};
var TASK3_PROCESSING = {
"id": "task3",
"status": "processing",
"task_type": "import_course",
"task_info": {
"repo_slug": "repo"
}
};
var TASK3_SUCCESS = {
"id": "task3",
"status": "success",
"task_type": "import_course",
"task_info": {
"repo_slug": "repo"
}
};
var TASK3_FAILURE = {
"id": "task3",
"status": "failure",
"task_type": "import_course",
"task_info": {
"repo_slug": "repo"
},
"result": {
"error": "Task 3 failed"
}
};

var makeCollectionResult = function(items) {
return {
"count": items.length,
"next": null,
"previous": null,
"results": items
};
};

QUnit.module('Test import status', {
beforeEach: function () {
TestUtils.setup();

TestUtils.initMockjax({
url: '/api/v1/tasks/',
type: 'GET',
responseText: makeCollectionResult([
OTHER_REPO_FAILURE,
NOT_IMPORT_SUCCESS,
TASK3_PROCESSING
])
});

TestUtils.initMockjax({
url: '/api/v1/tasks/task3/',
type: 'DELETE'
});
},
afterEach: function() {
TestUtils.cleanup();
}
});

QUnit.test('Assert that existing task is deleted',
function(assert) {
var done = assert.async();
var refreshCount = 0;
var refreshFromAPI = function() {
refreshCount++;
};
TestUtils.replaceMockjax({
url: '/api/v1/tasks/',
type: 'GET',
responseText: makeCollectionResult([
TASK1_SUCCESS
])
});
TestUtils.initMockjax({
url: '/api/v1/tasks/task1/',
type: 'DELETE'
});

var afterMount = function(component) {
waitForAjax(2, function () {
// If we get 2 AJAX calls one of them should be a DELETE.
assert.deepEqual(component.state, {
"importTasks": {
task1: TASK1_SUCCESS
},
"hasRefreshed": false
});

done();
});
};

React.addons.TestUtils.renderIntoDocument(
<ImportStatus
repoSlug="repo"
refreshFromAPI={refreshFromAPI}
ref={afterMount}
/>
);
}
);

QUnit.test('Assert successful import status',
function(assert) {
var done = assert.async();

var refreshCount = 0;
var refreshFromAPI = function() {
refreshCount++;
};

var afterMount = function(component) {
waitForAjax(1, function() {
assert.deepEqual(component.state, {
"importTasks": {
"task3": TASK3_PROCESSING
},
"hasRefreshed": true
});

TestUtils.replaceMockjax({
url: '/api/v1/tasks/',
type: 'GET',
responseText: makeCollectionResult([
OTHER_REPO_FAILURE,
NOT_IMPORT_SUCCESS,
TASK3_SUCCESS
])
});

waitForAjax(2, function() {
// One ajax call to get tasks, one to delete.
assert.deepEqual(component.state, {
importTasks: {
task3: TASK3_SUCCESS
},
hasRefreshed: true
});
done();
});
});
};

React.addons.TestUtils.renderIntoDocument(
<ImportStatus
repoSlug="repo"
refreshFromAPI={refreshFromAPI}
ref={afterMount}
/>
);
}
);

QUnit.test('Assert import status of failure',
function(assert) {
var done = assert.async();

var refreshCount = 0;
var refreshFromAPI = function() {
refreshCount++;
};

var afterMount = function(component) {
waitForAjax(1, function() {
assert.deepEqual(component.state, {
"importTasks": {
"task3": TASK3_PROCESSING
},
"hasRefreshed": true
});

TestUtils.replaceMockjax({
url: '/api/v1/tasks/',
type: 'GET',
responseText: makeCollectionResult([
OTHER_REPO_FAILURE,
NOT_IMPORT_SUCCESS,
TASK3_FAILURE
])
});

waitForAjax(2, function() {
// One ajax call to get tasks, one to delete.
assert.deepEqual(component.state, {
importTasks: {
task3: TASK3_FAILURE
},
hasRefreshed: true
});
done();
});
});
};

React.addons.TestUtils.renderIntoDocument(
<ImportStatus
repoSlug="repo"
refreshFromAPI={refreshFromAPI}
ref={afterMount}
/>
);
}
);
}
);
130 changes: 130 additions & 0 deletions ui/static/ui/js/listing/import_status.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
define("import_status", ["React", "lodash", "jquery", "utils",
"react_spinner", "status_box"],
function (React, _, $, Utils, ReactSpinner, StatusBox) {
'use strict';

return React.createClass({
getInitialState: function () {
return {
importTasks: {},
hasRefreshed: false
};
},
updateImportStatus: function () {
var thiz = this;

Utils.getCollection("/api/v1/tasks/").then(function (tasks) {
// Only deal with tasks on this repo that deal with imports.

tasks = _.filter(tasks, function(task) {
return (task.task_type === 'import_course' &&
task.task_info.repo_slug === thiz.props.repoSlug);
});
var tasksMap = {};
_.each(tasks, function (task) {
tasksMap[task.id] = task;
});
thiz.setState({importTasks: tasksMap});

var notProcessing = _.filter(tasks, function (task) {
return task.status === 'success' || task.status === 'failure';
});

var deferred = $.Deferred();

// Delete one after another to avoid race conditions with
// Django session.
_.each(notProcessing, function (task) {
deferred.then(function () {
return $.ajax({
method: 'DELETE',
url: '/api/v1/tasks/' + task.id + '/'
});
});
});
deferred.resolve();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if some call fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code above attaches handlers for the success case of deferred but doesn't actually resolve or reject the initial promise until this line. We need to resolve the initial promise so that it can start evaluating the other success callbacks. To illustrate:

deferred -> task1 -> task2 -> task3

deferred is always resolved. It doesn't do anything but we need to attach the task chain to something we have control of. Then task1 is evaluted. It returns a promise that's either rejected or resolved depending on how the AJAX request went. If task1 succeeded task2's success handler is then executed.

If task1 failed it would try to execute task2's failure handler, or task3, or whatever the next in line with a failure handler is. None of them have failure handlers so we just stop executing the chain in the event of a AJAX failure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, got it


var numProcessing = _.filter(tasks, function (task) {
return task.status === 'processing';
}).length;
if (numProcessing > 0) {
thiz.setState({hasRefreshed: true});
setTimeout(thiz.updateImportStatus, 3000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I would make this timeout a bit more visible, maybe adding a comment or moving it into a variable with a meaningful name.

} else {
if (thiz.state.hasRefreshed) {
// There were tasks and they're finished now so we should refresh.
thiz.props.refreshFromAPI();
}
}
});
},
componentDidMount: function () {
this.updateImportStatus();
},
render: function () {
var numSuccessful = _.filter(this.state.importTasks, function (task) {
return task.status === 'success';
}).length;
var numProcessing = _.filter(this.state.importTasks, function (task) {
return task.status === 'processing';
}).length;

var importWord = function (n) {
if (n === 1) {
return "import";
} else {
return "imports";
}
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra minor nit: you can just append the "s" to the word

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would work here but string concatenation for plurals should be avoided in general since it complicates localization

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree:


var successMessage;
if (numSuccessful !== 0) {
successMessage = <StatusBox
message={numSuccessful + ' ' +
importWord(numSuccessful) + ' finished'} />;
}
var progressMessage;
if (numProcessing !== 0) {
// Use a table for vertical centering
progressMessage = <table>
<tr>
<td>
{numProcessing + ' ' + importWord(numProcessing) + ' processing'}
</td>
<td>
<ReactSpinner
position="relative"
speed={2}
scale={0.3}
/>
</td>
</tr>
</table>;
}

var failed = _.filter(this.state.importTasks, function (task) {
return task.status === 'failure';
});
var errorMessages = [];
_.each(failed, function (task) {
errorMessages.push(
<p key={errorMessages.length}>
{"Import failed: " + task.result.error}
</p>
);
});

var errorMessage;
if (errorMessages.length > 0) {
errorMessage = <StatusBox message={{error: errorMessages}} />;
}

return <span>
{successMessage}
{errorMessage}
{progressMessage}
</span>;
}
});
}
);
12 changes: 9 additions & 3 deletions ui/static/ui/js/listing/listing.jsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
define('listing',
['csrf', 'react', 'jquery', 'lodash', 'uri', 'history', 'manage_taxonomies',
'learning_resources', 'static_assets', 'utils',
'lr_exports', 'listing_resources', 'xml_panel',
'lr_exports', 'listing_resources', 'xml_panel', 'import_status',
'bootstrap', 'icheck'],
function (CSRF, React, $, _, URI, History,
ManageTaxonomies, LearningResources, StaticAssets,
Utils, Exports, ListingResources, XmlPanel) {
Utils, Exports, ListingResources, XmlPanel, ImportStatus) {
'use strict';

var EMAIL_EXTENSION = '@mit.edu';
Expand Down Expand Up @@ -184,7 +184,13 @@ define('listing',
facetCounts: this.state.facetCounts,
ref: "listingResources"
};
return React.createElement(ListingResources.ListingPage, options);
return React.DOM.div(null,
React.createElement(ImportStatus, {
repoSlug: this.props.repoSlug,
refreshFromAPI: this.refreshFromAPI
}),
React.createElement(ListingResources.ListingPage, options)
);
},
/**
* Clears exports on page. Assumes DELETE to clear on server already
Expand Down
Loading