Filtering scheduled flows from the heartbeattimeline #188
Conversation
There was a problem hiding this comment.
Welcome to the UI @kingsleyb!
Completely agreed on this proposal but I don't think is the correct place to make this change - we want this component to be as agnostic to inputs as possible so this filtering should be done either on the heartbeat tiles or queries. In addition, by filtering this on the client we'll run into situations where all 10 items returned by the @/Dashboard/heartbeat.gql and @/Flow/heartbeat.gql queries are scheduled, which would result in an empty activity timeline. My suggestion would be to modify those queries to exclude scheduled states instead.
|
Made some changes based on the feedback provided on where to filter out the Scheduled states. |
src/graphql/Dashboard/heartbeat.gql
Outdated
| flow_run( | ||
| limit: 10 | ||
| where: { | ||
| flow: { project_id: { _eq: $projectId } } | ||
| state_timestamp: { _gte: $timestamp } | ||
| state: { _eq: $state } | ||
| state: { _eq: $state, _neq: $filterOutState } |
There was a problem hiding this comment.
This feels a little convoluted and will be confusing when separated from the motivation in this PR, because if someone weren't aware of how Hasura treats null variables, this looks like we're saying "give me flow runs that match a state but not a different state", which doesn't make much sense.
However, I recognize that it fits our usage. My suggestion would be to make it a _nin clause instead of a _neq clause, to make it easier to filter out potentially other states going forward.
| state: { _eq: $state, _neq: $filterOutState } | |
| state: { _eq: $state, _nin: $filterOutStates } |
There was a problem hiding this comment.
@jlowin this makes sense but should $state also be made a list whilst i'm making this change?
There was a problem hiding this comment.
I'm more hesitant to touch things that are already in use because it may require more updates elsewhere
src/graphql/Dashboard/heartbeat.gql
Outdated
| @@ -1,10 +1,10 @@ | |||
| query Heartbeat($projectId: uuid, $timestamp: timestamptz, $state: String) { | |||
| query Heartbeat($projectId: uuid, $timestamp: timestamptz, $state: String, $filterOutState: String) { | |||
There was a problem hiding this comment.
| query Heartbeat($projectId: uuid, $timestamp: timestamptz, $state: String, $filterOutState: String) { | |
| query Heartbeat($projectId: uuid, $timestamp: timestamptz, $state: String, $filterOutStates: [String!]) { |
There was a problem hiding this comment.
made suggested changes.
PR Checklist:
CHANGELOG.mdDescribe this PR
The dual timestamps on the activity timeline for scheduled flow runs is causing some confusion on the meaning of the two displayed times. Given that the scheduled flow runs are shown both in the dashboard overview and also the flow run over view in a separate window, I want to suggest removing the scheduled flow runs from the activity timeline altogether.
This PR shows how this would look.
I'll wait on feedback on the suggested change prior to adding to changelog.