Skip to content

Conversation

@GilbertCherrie
Copy link
Member

@GilbertCherrie GilbertCherrie commented May 24, 2024

Follow up to pr: #9191

When the current state is done its value is not removed from the context.State but also exists in the context.StateHistory. This pr prevents this state from rendering in the table as both a past state and current state and instead will only render it as a past state.

Before:
Screenshot 2024-05-24 at 10 21 57 AM

After:
Screenshot 2024-05-24 at 10 22 57 AM

@Fryguy
Copy link
Member

Fryguy commented May 24, 2024

As discussed in #9191 (comment), I don't think we can use id because they aren't unique.

Re-asking here: @agrare One more question. When the entire flow is done, does the "current" state have a "finished time"? If so, that might be the simplest check.

@agrare
Copy link
Member

agrare commented May 24, 2024

One more question. When the entire flow is done, does the "current" state have a "finished time"? If so, that might be the simplest check.

Yes,

   "State"=>
    {"Name"=>"NextState",
     "Input"=>{"foo"=>1, "result"=>{"foo"=>"bar", "bar"=>"baz"}},
     "Guid"=>"81de8e05-8118-49d0-b97e-779904fb07c3",
     "EnteredTime"=>"2024-05-24T14:34:29Z",
     "Output"=>{"foo"=>1, "result"=>{"foo"=>"bar", "bar"=>"baz"}},
     "NextState"=>nil,
     "FinishedTime"=>"2024-05-24T14:34:29Z",
     "Duration"=>0.348082391}

@Fryguy
Copy link
Member

Fryguy commented May 24, 2024

@GilbertCherrie So, I think the path forward, is when you check for the presence of State, simply also check that FinishedTime is not present.

if (rows.length > 0) {
lastId = rows[rows.length - 1].id;
}
if (response.context && response.context.State) {
Copy link
Member

Choose a reason for hiding this comment

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

So basically, something like the following (not sure this works, but you get the idea)

Suggested change
if (response.context && response.context.State) {
if (response.context && response.context.State && !response.context.State.FinishedTime) {) {

Copy link
Member

Choose a reason for hiding this comment

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

and then of course you don't need any of the other changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, made the fix and repushed

@GilbertCherrie GilbertCherrie force-pushed the remove_current_workflow_state branch 2 times, most recently from 5763990 to d70ec18 Compare May 24, 2024 14:42
@GilbertCherrie GilbertCherrie force-pushed the remove_current_workflow_state branch from d70ec18 to 635dbcc Compare May 24, 2024 14:42
@GilbertCherrie
Copy link
Member Author

@Fryguy I made the fix to check for the FinishedTime

@miq-bot
Copy link
Member

miq-bot commented May 24, 2024

Checked commit GilbertCherrie@635dbcc with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
0 files checked, 0 offenses detected
Everything looks fine. 🍪

@Fryguy Fryguy merged commit ecce835 into ManageIQ:master May 24, 2024
@Fryguy
Copy link
Member

Fryguy commented May 24, 2024

Backported to radjabov in commit cf3f4c0.

commit cf3f4c08fa463b2b3869e6d33afe4b0dc0df931d
Author: Jason Frey <[email protected]>
Date:   Fri May 24 17:25:52 2024 -0400

    Merge pull request #9195 from GilbertCherrie/remove_current_workflow_state
    
    Remove current workflow state if its done
    
    (cherry picked from commit ecce83550d0f9f6cd81df5a0ffe5a80a5229dd3b)

Fryguy added a commit that referenced this pull request May 24, 2024
…state

Remove current workflow state if its done

(cherry picked from commit ecce835)
@GilbertCherrie GilbertCherrie deleted the remove_current_workflow_state branch May 27, 2024 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants