Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DPL-1059: Group graph pipelines #1559

Merged
merged 21 commits into from
Jan 26, 2024
Merged

Conversation

StephenHulme
Copy link
Contributor

@StephenHulme StephenHulme commented Jan 22, 2024

Closes #1558

Changes proposed in this pull request

  • extend pipelines.json to include pipeline groups
  • add a button to switch between showing pipeline groups and the original sub-pipelines
  • filter by group or sub-pipeline as appropiate
  • tweaks to make all of the above work
  • add tests for the additional filtering code

Before:
Screenshot 2024-01-23 at 15 00 11

After:
Screenshot 2024-01-23 at 14 59 36

Instructions for Reviewers

[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to main]
    - Check story numbers included
    - Check for debug code
    - Check version

app/javascript/pipeline-graph/index.js Outdated Show resolved Hide resolved
app/javascript/pipeline-graph/index.js Outdated Show resolved Hide resolved
app/controllers/pipelines_controller.rb Outdated Show resolved Hide resolved
@StephenHulme StephenHulme added Size: S Small - low effort & risk Value: 4 Value to the insitute is high Pipeline visualisation OKR labels Jan 22, 2024
end

def calculate_pipeline_edges
calculate_edges_with_pipeline_data.map do |edge|
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 2 locations. Consider refactoring.

end
end

def calculate_group_edges
calculate_edges_with_pipeline_data
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 2 locations. Consider refactoring.

Copy link
Contributor

@KatyTaylor KatyTaylor left a comment

Choose a reason for hiding this comment

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

Haven't looked over all of it yet, but might not have time today so I thought I'd publish my comments so far. Looks great in general. Thanks.

app/controllers/pipelines_controller.rb Show resolved Hide resolved
app/controllers/pipelines_controller.rb Show resolved Hide resolved
app/javascript/pipeline-graph/filterFunctions.js Outdated Show resolved Hide resolved
app/javascript/pipeline-graph/index.js Outdated Show resolved Hide resolved
@StephenHulme
Copy link
Contributor Author

Thanks for the feedback @KatyTaylor, much appreciated!

app/javascript/pipeline-graph/graphFunctions.js Outdated Show resolved Hide resolved
* @returns {cytoscape.Collection} - A collection of nodes and edges that match the query.
*/
const findResults = (cy, query) => {
const findResults = (cy, filter) => {
Copy link

Choose a reason for hiding this comment

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

Function findResults has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.

Copy link

codeclimate bot commented Jan 25, 2024

Code Climate has analyzed commit 7432b32 and detected 4 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1
Duplication 3

The test coverage on the diff in this pull request is 36.3% (50% is the threshold).

This pull request will bring the total coverage in the repository to 90.1% (-0.1% change).

View more on Code Climate.

Copy link
Contributor

@KatyTaylor KatyTaylor left a comment

Choose a reason for hiding this comment

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

Only thing remaining - maybe an explanatory comment on calculate_edges ? Thanks.

@StephenHulme StephenHulme merged commit 072d67e into develop Jan 26, 2024
8 checks passed
@StephenHulme StephenHulme deleted the dpl-1028-group-graph-pipelines branch January 26, 2024 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pipeline visualisation OKR Size: S Small - low effort & risk Value: 4 Value to the insitute is high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DPL-1059 Filter pipelines by group
2 participants