Tools trend chart should use names and not IDs in legend#3236
Tools trend chart should use names and not IDs in legend#3236uhafner merged 8 commits intojenkinsci:mainfrom
Conversation
|
Can you please review the changes? @uhafner |
uhafner
left a comment
There was a problem hiding this comment.
Looks good in my Jenkins instance. Can you add a before and after image in the pull request description?
plugin/src/main/java/io/jenkins/plugins/analysis/core/model/ToolNameRegistry.java
Show resolved
Hide resolved
plugin/src/main/java/io/jenkins/plugins/analysis/core/model/ToolNameRegistry.java
Outdated
Show resolved
Hide resolved
plugin/src/main/java/io/jenkins/plugins/analysis/core/model/ToolNameRegistry.java
Outdated
Show resolved
Hide resolved
plugin/src/main/java/io/jenkins/plugins/analysis/core/model/ToolNameRegistry.java
Outdated
Show resolved
Hide resolved
plugin/src/main/java/io/jenkins/plugins/analysis/core/model/ToolNameRegistry.java
Outdated
Show resolved
Hide resolved
|
Can you always check or resolve the conversations in a review? Otherwise I don't see when everything is finished... |
Yes, I have implemented all the suggested changes. Please review the changes. @uhafner |
But you did not answer the questions! |
|
And the review comments that are no questions need to be closed |
|
Hmm, and where is the answer to "Can you check if those names are rendered correctly in ECharts with the escaping?" |
Yes, I have verified that the names are rendered correctly in ECharts with the escaping. This test verify that: assertThatJson(model.getSeries().get(0))
.node("name")
.isEqualTo("<script>alert('xss')</script>"); |
I added one... |
|
Thank you! |
Trend chart should use names and not IDs in legend
Fixes #3075
Before
After
Testing done
Submitter checklist