Revert incorrect handling of FAILURE status (fetch job statuses) #99
Conversation
ross-spencer
changed the title
AIPscan/static/js/tasks.js
Outdated
| $.ajax({ | ||
| type: 'GET', | ||
| url: '/aggregator/package_list_task_status/' + taskId, | ||
| datatype: "json", | ||
| success: function(data) { | ||
| state = data['state'] | ||
| if (state != status_pending && state != status_progress && state != status_failure) { | ||
| state = data['state']; |
There was a problem hiding this comment.
You may want to initialize state with let state = .... Otherwise I think loose mode JS will make this a global variable and strict mode will throw an error.
There was a problem hiding this comment.
Ah, yep. Language switching failings.
There was a problem hiding this comment.
Yeah no worries, that happens!
There was a problem hiding this comment.
LGTM! Code changes make sense and I'm seeing the expected UI behaviour on my end.
tasks.py could benefit from some linting (I see missing semi-colons in places not touched by your changes in these commits, for instance) but I think touching that up might be out of scope for this PR, so I'm approving as-is.
FAILURE handling was added incorrectly to package_list_task_status. This commit reverts those changes to make the console output work as it did previously. This also quietens down the completed tasks alerts as well.
ross-spencer
force-pushed
the
dev/issue-73-correct-console-issues
branch
from
ec0f739 to
39dbf88
Compare
|
Thanks @tw4l! Glad to have this fixed. I've logged two extra issues around JavaScript and also related to your suggestions for |
FAILURE handling was added incorrectly to package_list_task_status.
This commit reverts those changes to make the console output work
as it did previously. This also quietens down the completed tasks
alerts as well.
Resolves #72
Resolves #73
@peterVG first, apologies for my speaking out of term the other day. I had diagnosed this wrong. Looking again tonight I have discovered a mis-reading of the original JavaScript functions you had created when I changed them here: e59056e#diff-606a3136c88e560ab867ae3abac4c67013a7a577f7b69878f20c7939fa99e179. I have not tried to add FAILURE handling in again as this might be something you do differently with your current work, and so this just reverts the change back to a working JavaScript. If you have an opportunity to test and review we'll get this merged and it should be one less annoyance 🙂
Recorded tonight: