Rewrite Pipeline Graph in Vue.js to allow realtime
What does this MR do?
Frontend part of making pipelines graph realtime.
Transform existing haml into vue:
-
Create vue bundle -
Create graph component -
Create stage component -
Create badge component reusable between the big graph and the dropdown with jobs -
Create dropdown component -
Create tests for all new components -
Fixes HTML structure of job dropdown -
- [ ] Remove USJ for actions callsLet's go with baby steps and remove this in a second iteration
Screenshots
Note: the arrow in the dropdown is not visible because this branch is not synced to master - this bug is already fixed in master
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary - Tests
-
Added for this feature/bug -
All builds are passing
-
-
Conform by the merge request performance guides -
Conform by the style guides -
Branch has no merge conflicts with master(if it does - rebase it please) -
Squashed related commits together
What are the relevant issue numbers?
Closes #25226 (closed) Closes #31557 (closed)
Merge request reports
Activity
added 6 commits
-
f9ce909b...b69b4b73 - 2 commits from branch
zj-real-time-pipelines - dc853cc1 - Render badges
- bf05a7c6 - Merge branch 'zj-real-time-pipelines' into 25226-realtime-pipelines-fe
- 7fdd8b57 - Add badge and dropdown
- 8ea361d1 - Merge branch 'zj-real-time-pipelines' into 25226-realtime-pipelines-fe
Toggle commit list-
f9ce909b...b69b4b73 - 2 commits from branch
added 1 commit
- 99ea7300 - Creates CI Icon component as a shared component
@iamphill This is already big
🙊 It's missing tests mostly, won't plan on changing the architecture:
This MR creates:
- a CI Icon component that renders the correct icon and css class given the an object with the status information;
- A component that renders the CI icon and the name of each job. This is reusable in 3 places: The node that represents a badge, the dropdown button that also looks like a badge, and in each job in the dropdown
- An action component reused in the same places as last point
- A Stage component to render each column
- The graph component that holds it all together.
A Store and a Service were created in order to hold data for show view although in this first iteration we will only save data related to the graph.
While I write the missing tests, can you please take a look?
added 2 commits
added 2 commits
marked the checklist item Squashed related commits together as completed
marked the checklist item Squashed related commits together as incomplete
marked the checklist item Conform by the style guides as completed
marked the checklist item Conform by the merge request performance guides as completed
marked the checklist item Changelog entry added, if necessary as completed
-
@iamphill this has 28 filed changes and no tests yet.
🙈 And still have to delete the old haml files😂 Some of the changes are whitespace.
I am going to create a new branch from this one with the tests and without the old haml files, or this will be impossible to review.
-
Builds in this MR will fail, tests are in this MR: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10972
@iamphill some tiny details were changes on that MR
😊 can you please review?Edited by Filipa Lacerda assigned to @iamphill
assigned to @filipa
mentioned in issue #31557 (closed)
added 2 commits
-
@iamphill I believe I fixed them all :)
Going to update the part 2 with these changes, can you please review this one again? Thank you!
(we should merge the part 2 into this one first, I think)
assigned to @iamphill
@filipa What are the steps after this is completed? Get the tests merged into this & then this into master?
assigned to @filipa
mentioned in issue #31570 (closed)
-
@iamphill steps are:
-
Merge the tests one first https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10972 into this MR. - we can do that now if you think they are good
-
Merge this MR into @zj MR/branch - https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10777
-
@ayufan will probably review and merge into master :)
This can't be merged into master directly.
Also, the API changes in this one https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10777 break the pipelines table - mini graph is not rendered with the data we receive now, so we wither revert the changes done to
pipelines.jsonor we open a new MR to fix the frontend. @ayufan @zj let me know what we should do here😊 -
mentioned in merge request !10972 (merged)
-
@iamphill we need to wait for backend to be merged first. I will ping you when we can merge this one into master!
💪 mentioned in merge request !11033 (merged)
added 770 commits
-
36e86035...b50a73b5 - 769 commits from branch
zj-real-time-pipelines - b3bdb894 - Merge branch 'zj-real-time-pipelines' into 25226-realtime-pipelines-fe
-
36e86035...b50a73b5 - 769 commits from branch
mentioned in issue #31415 (closed)
added 3 commits
-
f74a883e...bae2b0f2 - 2 commits from branch
zj-real-time-pipelines - da27f41b - Merge branch 'zj-real-time-pipelines' into 25226-realtime-pipelines-fe
-
f74a883e...bae2b0f2 - 2 commits from branch
-
@iamphill added 2 changes:
-
Fix HTML dropdown structure - https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10878/diffs?diff_id=4234237&start_sha=7eaf0b0f005b621ed5736679523d6d0139f4d797
-
Adjust to new data schema - https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10878/diffs?diff_id=4235471&start_sha=da27f41bb7eceffa750df92cc0b48af30b2e5cce
Can you please review them?
Then we just need to wait for this one https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10777 to be merged and then we can merge it
💪 cc @ayufan -
assigned to @iamphill
@filipa Changes look good. Assigning back to you until we are ready to merge
👍 assigned to @filipa
added 116 commits
-
3c1a878d...e9e71a9c - 115 commits from branch
zj-real-time-pipelines - dc1b5d96 - Merge branch 'zj-real-time-pipelines' into '25226-realtime-pipelines-fe'
-
3c1a878d...e9e71a9c - 115 commits from branch
mentioned in commit aa440eb1
@filipa I did rebase and squash your changes on top of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10777 which is merged to master.
Is it safe for me to merge that as it seems that frontend is reviewed already? is that correct @iamphill @filipa?
added 183 commits
-
dc1b5d96...c17e6a6c - 182 commits from branch
master - aa440eb1 - Single commit squash of all changes for https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10878
-
dc1b5d96...c17e6a6c - 182 commits from branch
added 54 commits
-
aa440eb1...6ad3814e - 50 commits from branch
master - 13b31d25 - Fix broken css class
- cc27180e - Merge branch 'master' into 25226-realtime-pipelines-fe
- 8f64091a - Adds missing CSS class
- b46d38ae - Move file loading to the top of the file
Toggle commit list-
aa440eb1...6ad3814e - 50 commits from branch
-
@ayufan one of the tests is failing but I do not how to fix it: https://gitlab.com/gitlab-org/gitlab-ce/builds/15747350
The reason why is breaking is because is not waiting for Vue to mount the component. It shows the haml container
#js-pipeline-graph-vue. Vue component should mount on this and replace it, but this div is empty.It is probably not waiting for this file to run
app/assets/javascripts/pipelines/graph_bundle.jsbut I am not sure how to make it wait.I am not sure why it is failing or how to fix it.
🤔 😭 Do you have any idea? assigned to @ayufan
-
@ayufan I am not sure if this makes any sense but I don't think we use views test to test code with JS
🤔 and the failing test is covered on this onespec/features/projects/pipelinesalready, does it make sense to delete it?🤔 -
assigned to @filipa
@filipa I moved the test to a controller as you need Javascript helpers to work. It displays Pipeline Graph now, so you have to tune selectors only and we should be just fine.
added 1 commit
- c195b313 - Make test that actually displays pipeline graph
mentioned in commit 8cd578a7
mentioned in commit f671f819
mentioned in merge request !11167 (merged)
mentioned in issue #36072 (closed)
