review: fix waiting tasks

[oliver-sanders]

At present waiting tasks do not appear in Cylc Review due to a SQL query error.

We were joining the tasks and jobs tables. In SQL terms JOIN means intersection. As waiting tasks do not have jobs they were excluded in the intersection. I've changed the JOIN to LEFT JOIN to include tasks with out jobs.

[oliver-sanders]

@sadielbartholomew @matthewrmshin bafa9d7 changes the approach so that waiting tasks only get displayed if requested by the user. See what you think about the approach, if you approve I'll fix the tests. Otherwise the simplest solution would just be to remove the "waiting" task state from cylc review.

[sadielbartholomew]

I have not yet had a chance to review this, but Travis has picked up that one of the Cylc Review sub-tests needs one change to reflect the revised approach.

[oliver-sanders]

Base changed to 7.8.x, cylc-review temporally removed on master so doesn't need a second PR against master.

[hjoliver]

(Will we remember to make the same fix later though, when cylc review is restored to (or for - perhaps as a separate project) master?)

[kinow]

(Will we remember to make the same fix later though, when cylc review is restored to (or for - perhaps as a separate project) master?)

Good point! Maybe if we create a ticket, add the commit where it was removed, and a link to the branch 7.8.x? And only destroy the 7.8.x branch after everything has been migrated to master/Cylc 8?

[sadielbartholomew]

Issue recreated on master (7.8.x) branch, & fixed according to the solution described:

waiting tasks only get displayed if requested by the user

For ease of reference for those reading or reviewing this PR, it changes the default page-load state, & UI, of the 'Display Options' top area as follows, where the tasks then shown are those you would expect based on correct application of task status filters in the latter case :

Review service started from 7.8.x:

... & from this branch:

---

A few non-essential observations, that I think we should address before merge simply while we are considering this:

1. The icon displayed for the waiting task status is not really appropriate:
   
   Can we change it to the one I also captured here for the job status of the submitted task, with the clock (glyphicon-time from Inspection on the web element), which implies waiting or at least much more so than the current "play" symbol?

2. [See sub-comment.]

[sadielbartholomew]

Feedback addressed, where I'll bump the point about the waiting task status icon to the Issue #2833 as it not strictly the domain of this PR.

[oliver-sanders]

Can we change it to the one I also captured here for the job status of the submitted task

That wouldn't be appropriate as it suggests there is a job running. We could, however, just remove the job icon altogether.