[JENKINS-69339] Add support for badge icons in Management links #6995

mawinter69 · 2022-08-15T13:04:38Z

Allow to define a badge icon and a tooltip for all management links.

Adjust PluginsLink to the new method
Add implementations for OldDataMonitor and NodesLink

See JENKINS-69339.

Proposed changelog entries

Add support for badge icons in Management links

Proposed upgrade guidelines

N/A

Submitter checklist

(If applicable) Jira issue is well described
Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change) and are in the imperative mood. Examples
- Fill-in the Proposed changelog entries section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
Appropriate autotests or explanation to why this change has no tests
New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadoc, as appropriate.
New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO") if applicable.
New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content-Security-Policy directives (see documentation on jenkins.io).
For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@mention

Maintainer checklist

Before the changes are marked as ready-for-merge:

There are at least 2 approvals for the pull request and no outstanding requests for change
Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
Changelog entries in the PR title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood
Proper changelog labels are set so that the changelog can be generated automatically
If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

Allow to define a badge icon and a tooltip for all management links

core/src/main/java/hudson/model/ManagementLink.java

fix compile errors

daniel-beck · 2022-08-16T08:24:54Z

The weird thing about the existing badge is that it only exists for a feature that does not have a corresponding admin monitor (plugin updates). My guess is that it's probably deliberately so, because there are always plugin updates, especially since JEP-229, so having an admin monitor that's always on would be spammy and quickly ignored by many admins.

Other features like Old Data Monitor do have an admin monitor when they have useful content. Usually because this is something that needs attention (in that case, loss of configuration data, loss of functionality, etc.).

If this is added, we need to provide guidelines to explain when you would have the admin monitor, when you would have the badge, and when you would have both.

An interesting case is In-Process Script Approval (script-security plugin): It currently has a badge-like feature (the label changes). This feature should be migrated to badges if this is merged (would make a nice downstream PR for this change), and I wonder whether an admin monitor would make sense there as well. Could probably go either way.

daniel-beck

I really like this PR. It cleans up the existing tooltip for plugin updates and adds useful additional tooltips (although I'm not entirely sure the node one is useful as-is, given agents on a schedule etc.).

An open question remains about the guidelines for plugin developers on how to use this feature. We should have documentation when to (not) use this ready at the same time we publish this API addition. If we keep the String type, the docs should also explain it's for short labels only (probably mostly numeric; e.g., 4 of 12 might be OK, 21 available updates is not). This should be added to Javadoc and/or https://plugins.jenkins.io/design-library/

Otherwise (and assuming a quick re-test confirms the non-HTML tooltip behavior) this looks good 👍

daniel-beck · 2022-12-15T13:25:47Z

core/src/main/resources/hudson/model/ManageJenkinsAction/index.jelly

-                        <div tooltip="${m.getUpdateCount() == 1 ? '%updateAvailable(m.getUpdateCount())' : '%updatesAvailable(m.getUpdateCount())'}" class="jenkins-section__item__icon__badge">
-                          ${m.getUpdateCount()}
+                      <j:if test="${m.badge != null}">
+                        <div tooltip="${m.badgeTooltip}" class="jenkins-section__item__icon__badge">


This is now (once re-merged with master) a non-HTML tooltip. That seems fine.

daniel-beck · 2022-12-15T22:41:09Z

assuming a quick re-test confirms the non-HTML tooltip behavior

Confirmed, non-HTML tooltip and content when merged with master.

mawinter69 · 2022-12-16T14:46:51Z

I'm wondering if there are other places where such badges might be useful. If so it might make sense to create a Badge class that contains the data, tooltip and maybe a severity so the badge can have different colors. Even when not used somewhere else it might help to avoid running some calculations twice (once for the badge and once for the tooltip.
For severity, e.g. when there are plugin updates that fix security issues display it in red, otherwise in orange.

mawinter69 · 2022-12-16T15:20:36Z

(although I'm not entirely sure the node one is useful as-is, given agents on a schedule etc.).

The agents on a schedule wouldn't be a problem, but the ondemand would show up whenever they have nothing to do and thus disconnect. Maybe restrict the check to agents that use the always online strategy which is the default and most people use I guess.

daniel-beck · 2022-12-18T22:54:52Z

avoid running some calculations twice (once for the badge and once for the tooltip

For expensive calculations, could be handled by individual ManagementLink in a cache. I don't think there are that many cases where computation would be expensive?

Feel free to sketch this out, might be reasonable for future extension (e.g. severity/color), but IMO all that's missing here is related documentation.

Maybe restrict the check to agents that use the always online strategy which is the default and most people use I guess.

Right, something along those lines. Might be simpler to remove this from the PR for now and look into it as followup. Migrating Manage Plugins to this is already pretty nice cleanup, and Manage Old Data is a reasonable new use.

Badge class takes message, tooltip and severity Do not create badge for NodesLink. It will not work with the agent-maintenance plugin as this configures as well a retention strategy that can't be queried here. To get the reliably working Demand and SimpleScheduled would need some reworking at least. The plugin link badge now changes color when there is an update that fixes a security vulnerability.

mawinter69 · 2022-12-19T13:33:06Z

I removed the badge for the nodes now, there is no way of reliably determining this information, e.g. when the agent-maintenance enabled for an agent, which is proxying the retentionstrategy
Added a Badge class to wrap all the information
Plugin update badge is now red when updates fix security vulnerabilities and orange otherwise. Tooltip shows the actual numbers for total updates and updates with security fixes. One could think of extending the badge/tooltip also for plugins that have incompatible changes

core/src/main/java/jenkins/management/Badge.java

daniel-beck · 2022-12-19T22:00:03Z

core/src/main/java/hudson/model/ManagementLink.java

+     * @return badge or {@code null} if no badge should be shown.
+     * @since TODO
+     */
+    public @CheckForNull Badge getBadge() {


One drawback of the new Badge type is that it requires a new core dependency. Previously, the String based API could be implemented even by plugins with an old core dependency.

I don't think this change must be reverted, but considering simple page navigation shouldn't be expensive anyway, the argument for one call instead of two isn't that strong (expensive computation should be done elsewhere and cached values shown, kind of like available updates).

core/src/main/java/jenkins/management/PluginsLink.java

core/src/main/java/jenkins/management/Badge.java

core/src/main/resources/hudson/model/ManageJenkinsAction/index.jelly

war/src/main/less/modules/section.less

add info about incompatible changes

timja

LGTM just one wording question in the Javadoc but no blockers, tested locally.

core/src/main/java/jenkins/management/Badge.java

core/src/main/resources/hudson/model/ManageJenkinsAction/index.jelly

Co-authored-by: Tim Jacomb <21194782+timja@users.noreply.github.com>

addressed

timja · 2022-12-31T16:48:52Z

/label ready-for-merge

This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

daniel-beck · 2023-01-02T19:11:43Z

timja dismissed daniel-beck’s stale review Dec 31, 2022
addressed

In multiple comments I kept requesting developer documentation. Could you point me to it?

#6995 (comment):

If this is added, we need to provide guidelines to explain when you would have the admin monitor, when you would have the badge, and when you would have both.

#6995 (review):

An open question remains about the guidelines for plugin developers on how to use this feature. We should have documentation when to (not) use this ready at the same time we publish this API addition. If we keep the String type, the docs should also explain it's for short labels only (probably mostly numeric; e.g., 4 of 12 might be OK, 21 available updates is not). This should be added to Javadoc and/or https://plugins.jenkins.io/design-library/

#6995 (review):

FWIW I look forward to reading in docs about the interaction between admin monitors and badges here :)

timja · 2023-01-02T19:14:56Z

In multiple comments I kept requesting developer documentation. Could you point me to it?

jenkins/core/src/main/java/jenkins/management/Badge.java

Lines 33 to 50 in a6d5425

    
           /** 
        
            *  Definition of a badge that can be returned by a {@link ManagementLink} implementation. 
        
            *  The badge is shown as a small overlay over the corresponding icon on the {@code Manage Jenkins} page, 
        
            *  it can display additional information in a tooltip and change it's color depending on the severity. 
        
            * 
        
            *  <p> 
        
            *  A badge mainly serves as a fast feedback for the corresponding management page. 
        
            *  It could be used to just display some short status information or hint that some action can be taken. 
        
            *  For example the badge on {@code Manage Plugins} shows information about the number of available updates 
        
            *  and in its tooltip additionally how many updates contain incompatible changes or fix 
        
            *  security vulnerabilities. It also changes its color when there are security fixes available. 
        
            * 
        
            *  <p> 
        
            *  A badge might display the same information as an {@link AdministrativeMonitor}. While an {@link AdministrativeMonitor} 
        
            *  can be disabled, a badge will always be shown. E.g. the badge of {@link OldDataMonitor.ManagementLinkImpl} always shows the number of old data entries. 
        
            * 
        
            *  @since TODO 
        
            */

jenkins/core/src/main/java/hudson/model/ManagementLink.java

Lines 218 to 223 in a6d5425

    
               /** 
        
                * A {@link Badge} shown as overlay over the icon on "Manage Jenkins". 
        
                * 
        
                * @return badge or {@code null} if no badge should be shown. 
        
                * @since TODO 
        
                */

If we keep the String type, the docs should also explain it's for short labels only (probably mostly numeric; e.g., 4 of 12 might be OK, 21

jenkins/core/src/main/java/jenkins/management/Badge.java

Lines 57 to 66 in a6d5425

    
               /** 
        
                * Create a new Badge 
        
                * 
        
                * @param text  The text to be shown in the badge. 
        
                *      Keep it short, ideally just a number. More than 6 or 7 characters do not look good. Avoid spaces as they will lead to line breaks. 
        
                *      as this might lead to line breaks. 
        
                * @param tooltip  The tooltip to show for the badge. 
        
                *      Do not include html tags. 
        
                * @param severity  The severity of the badge (danger, warning, info) 
        
                */

daniel-beck · 2023-01-02T19:17:19Z

Thanks, missed it (or forgot in the mean time) 👍

* Init * Update core/src/main/resources/lib/layout/task.jelly Co-authored-by: Alexander Brandes <brandes.alexander@web.de> * Remove duplicated class * Update core/src/main/resources/lib/layout/task.jelly Co-authored-by: Daniel Beck <1831569+daniel-beck@users.noreply.github.com> * Add support for badges in menus + manage jenkins page * Update index.jelly * Update ModelObjectWithContextMenu.java * Merge with #6995 * Add tooltip in task links * Add tooltip support to badges * Lint * Escape badge tooltip/text in menus * Use xmlEscape for escaping attributes * Support nulls * Update breadcrumbs.js * Update breadcrumbs.js --------- Co-authored-by: Alexander Brandes <brandes.alexander@web.de> Co-authored-by: Daniel Beck <1831569+daniel-beck@users.noreply.github.com> Co-authored-by: Tim Jacomb <21194782+timja@users.noreply.github.com> Co-authored-by: Tim Jacomb <timjacomb1@gmail.com> Co-authored-by: Alexander Brandes <mc.cache@web.de>

[JENKINS-69339] badge icon for Management links

ffd4b76

Allow to define a badge icon and a tooltip for all management links

daniel-beck reviewed Aug 15, 2022

View reviewed changes

core/src/main/java/hudson/model/ManagementLink.java Outdated Show resolved Hide resolved

daniel-beck reviewed Aug 15, 2022

View reviewed changes

core/src/main/java/hudson/model/ManagementLink.java Outdated Show resolved Hide resolved

allow null for the tooltip

df10189

fix compile errors

daniel-beck self-requested a review August 15, 2022 13:55

NotMyFault added web-ui The PR includes WebUI changes which may need special expertise rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted needs-security-review Awaiting review by a security team member labels Aug 15, 2022

NotMyFault requested a review from a team August 15, 2022 19:27

basil added 2 commits August 25, 2022 15:53

Merge branch 'master' into JENKINS-69339

c055737

Merge branch 'master' into JENKINS-69339

200d1e4

timja mentioned this pull request Oct 24, 2022

Add updates count badge to Updates sidebar item #7084

Merged

13 tasks

Merge branch 'master' into JENKINS-69339

a29bf2f

daniel-beck reviewed Dec 15, 2022

View reviewed changes

daniel-beck added security-approved @jenkinsci/core-security-review reviewed this PR for security issues and removed needs-security-review Awaiting review by a security team member labels Dec 15, 2022

mawinter69 added 3 commits December 19, 2022 14:15

minor fixes to javadoc and whitespace

1540702

set badge var just before we use it

853c80b

remove text color for badge item

77e9f70

daniel-beck previously requested changes Dec 19, 2022

View reviewed changes

improve documentation

de9128e

add info about incompatible changes

timja requested a review from a team December 29, 2022 20:15

Merge branch 'master' into JENKINS-69339

0a92ec8

Copyediting

5db3133

timja approved these changes Dec 30, 2022

View reviewed changes

core/src/main/java/jenkins/management/Badge.java Outdated Show resolved Hide resolved

core/src/main/resources/hudson/model/ManageJenkinsAction/index.jelly Show resolved Hide resolved

core/src/main/resources/hudson/model/ManageJenkinsAction/index.jelly Show resolved Hide resolved

timja requested review from daniel-beck and a team December 30, 2022 20:49

Merge branch 'master' into JENKINS-69339

4246e34

timja added a commit to timja/jenkins that referenced this pull request Dec 30, 2022

Merge with jenkinsci#6995

659de13

timja added a commit to timja/jenkins that referenced this pull request Dec 31, 2022

Merge with jenkinsci#6995

dc79e8a

Update core/src/main/java/jenkins/management/Badge.java

ff818d4

Co-authored-by: Tim Jacomb <21194782+timja@users.noreply.github.com>

NotMyFault approved these changes Dec 31, 2022

View reviewed changes

comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Dec 31, 2022

Merge branch 'master' into JENKINS-69339

838971e

timja merged commit 622e4c6 into jenkinsci:master Jan 2, 2023

NotMyFault changed the title ~~[JENKINS-69339] badge icon for Management links~~ [JENKINS-69339] Add support for badge icons in Management links Jan 2, 2023

timja added a commit to janfaracik/jenkins that referenced this pull request Jan 2, 2023

Merge with jenkinsci#6995

70966cf

daniel-beck mentioned this pull request Jul 12, 2023

allow string for badge attribute of l:task #8260

Closed

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-69339] Add support for badge icons in Management links #6995

[JENKINS-69339] Add support for badge icons in Management links #6995

mawinter69 commented Aug 15, 2022 •

edited by timja

Loading

daniel-beck commented Aug 16, 2022

daniel-beck left a comment

daniel-beck Dec 15, 2022

daniel-beck commented Dec 15, 2022

mawinter69 commented Dec 16, 2022

mawinter69 commented Dec 16, 2022

daniel-beck commented Dec 18, 2022

mawinter69 commented Dec 19, 2022

daniel-beck Dec 19, 2022

timja left a comment

timja commented Dec 31, 2022

daniel-beck commented Jan 2, 2023

timja commented Jan 2, 2023 •

edited

Loading

daniel-beck commented Jan 2, 2023

[JENKINS-69339] Add support for badge icons in Management links #6995

[JENKINS-69339] Add support for badge icons in Management links #6995

Conversation

mawinter69 commented Aug 15, 2022 • edited by timja Loading

Proposed changelog entries

Proposed upgrade guidelines

Submitter checklist

Desired reviewers

Maintainer checklist

daniel-beck commented Aug 16, 2022

daniel-beck left a comment

Choose a reason for hiding this comment

daniel-beck Dec 15, 2022

Choose a reason for hiding this comment

daniel-beck commented Dec 15, 2022

mawinter69 commented Dec 16, 2022

mawinter69 commented Dec 16, 2022

daniel-beck commented Dec 18, 2022

mawinter69 commented Dec 19, 2022

daniel-beck Dec 19, 2022

Choose a reason for hiding this comment

timja left a comment

Choose a reason for hiding this comment

timja commented Dec 31, 2022

daniel-beck commented Jan 2, 2023

timja commented Jan 2, 2023 • edited Loading

daniel-beck commented Jan 2, 2023

mawinter69 commented Aug 15, 2022 •

edited by timja

Loading

timja commented Jan 2, 2023 •

edited

Loading