Create a new taglib to capture the save/apply bottom bar #9813

Vlatombe · 2024-10-02T09:19:45Z

This introduces <f:saveApplyBar/>, capturing a recurring pattern in our jelly views (originally introduced in fbb2bca)

Added save/apply pattern to

Computer
MasterComputer
LogRecorder
Needs Avoid depending on labels, rather use the button name acceptance-test-harness#1754

See JENKINS-XXXXX.

Testing done

Checked with various roles (Admin/Manage/ExtendedRead) that configuration screens were displayed the same as before.
Checked that Save/Apply works as expected for Log Recorder configuration submission.
Checked that Save/Apply works as expected for Computer configuration submission.
Checked that Save/Apply works as expected for MasterComputer configuration submission.

Proposed changelog entries

Developer: a new taglib <f:saveApplyBar/> is now available for configuration forms.

Proposed upgrade guidelines

N/A

Submitter checklist

Give feedback

The Jira issue, if it exists, is well-described.
The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples). Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
There is automated testing or an explanation as to why this change has no tests.
New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, 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 (CSP) directives (see documentation).
For dependency updates, there are links to external changelogs and, if possible, full differentials.
For new APIs and extension points, there is a link to at least one consumer.
Options

Desired reviewers

@mention

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

Maintainer checklist

Give feedback

There are at least two (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 is not blocking the change.
Changelog entries in the pull request 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, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see 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).
Options

* Add Save/Apply to LogRecorder configuration screen

Vlatombe · 2024-10-02T09:20:42Z

core/src/main/resources/jenkins/model/Jenkins/configure.jelly

-          <f:apply />
-        </f:bottomButtonBar>
-      </l:hasAdministerOrManage>
+      <j:set var="readOnlyMode" value="${!h.hasPermission(app.MANAGE)}"/>


Needed because of https://github.com/jenkinsci/jenkins/pull/9813/files#diff-04f3f5f17eaeb24ac6c8f50acae68a658010e18d2de16105942d34e151542fbfR57 overrriding the first defined value.

Vlatombe · 2024-10-02T11:05:55Z

core/src/main/resources/hudson/model/View/configure.jelly

@@ -55,12 +55,8 @@ THE SOFTWARE.
        <!-- view property configurations -->
        <f:descriptorList descriptors="${it.getVisiblePropertyDescriptors()}" instances="${it.properties}" />

-        <f:bottomButtonBar>
-          <f:submit value="${%OK}" />


This effectively changes the label OK to Save. But I don't think there was any good reason for this label to differ from other screens.

daniel-beck · 2024-10-11T12:50:29Z

core/src/main/resources/hudson/model/Computer/configure.jelly

-            <f:submit value="${%Save}"/>
-          </f:bottomButtonBar>
-        </j:if>
+        <f:saveApplyBar/>


This one is funny, after clicking Apply:

Good catch, fixed by https://github.com/jenkinsci/jenkins/pull/9813/files#diff-581669f62053a1ed8fed271068c074eec26588530194da1aef0d14f722f48b30R1516

daniel-beck · 2024-10-11T13:10:25Z

core/src/main/resources/jenkins/agents/CloudSet/index.jelly

-                <f:submit id="saveButton" value="${%Save}" clazz="jenkins-hidden" />
-              </f:bottomButtonBar>
-            </l:isAdmin>
+            <f:saveApplyBar/>


Also funny:

daniel-beck · 2024-10-11T13:12:30Z

core/src/main/resources/jenkins/model/Jenkins/MasterComputer/configure.jelly

-            <f:submit value="${%Save}"/>
-          </f:bottomButtonBar>
-        </l:hasPermission>
+        <f:saveApplyBar/>


Also funny:

Fixed by https://github.com/jenkinsci/jenkins/pull/9813/files#diff-40583d1d185a48f8484bb0b00ce88d369357e85d1667546a09eb955db42ac846R4162

obsolete

timja

/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!

Create a new taglib to capture the save/apply bottom bar

8cf0d03

* Add Save/Apply to LogRecorder configuration screen

Vlatombe added the developer Changes which impact plugin developers label Oct 2, 2024

Vlatombe commented Oct 2, 2024

View reviewed changes

Vlatombe marked this pull request as ready for review October 2, 2024 09:21

Vlatombe requested a review from a team October 2, 2024 09:22

Missing import

d30acbe

Vlatombe commented Oct 2, 2024

View reviewed changes

Vlatombe mentioned this pull request Oct 2, 2024

Avoid depending on labels, rather use the button name jenkinsci/acceptance-test-harness#1754

Merged

6 tasks

daniel-beck previously requested changes Oct 11, 2024

View reviewed changes

daniel-beck added the security-approved @jenkinsci/core-security-review reviewed this PR for security issues label Oct 11, 2024

Vlatombe added 2 commits October 11, 2024 15:14

Merge branch 'master' into save-apply-bottom-bar

53d0008

Fix up issues found by Daniel

710fc1c

Vlatombe requested a review from daniel-beck October 11, 2024 13:31

Bump ath version

c9aa779

NotMyFault approved these changes Oct 13, 2024

View reviewed changes

NotMyFault requested a review from a team October 13, 2024 09:58

timja approved these changes Oct 13, 2024

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 Oct 13, 2024

timja and others added 2 commits October 14, 2024 09:11

Merge branch 'master' into save-apply-bottom-bar

45f40a0

Merge branch 'master' into save-apply-bottom-bar

6e54132

timja merged commit fe80df5 into jenkinsci:master Oct 14, 2024
16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create a new taglib to capture the save/apply bottom bar #9813

Create a new taglib to capture the save/apply bottom bar #9813

Vlatombe commented Oct 2, 2024 •

edited

Loading

Submitter checklist

Maintainer checklist

Vlatombe Oct 2, 2024 •

edited

Loading

Vlatombe Oct 2, 2024

daniel-beck Oct 11, 2024

Vlatombe Oct 11, 2024

daniel-beck Oct 11, 2024

daniel-beck Oct 11, 2024

Vlatombe Oct 11, 2024

timja left a comment

Create a new taglib to capture the save/apply bottom bar #9813

Create a new taglib to capture the save/apply bottom bar #9813

Conversation

Vlatombe commented Oct 2, 2024 • edited Loading

Testing done

Proposed changelog entries

Proposed upgrade guidelines

Submitter checklist

Desired reviewers

Maintainer checklist

Vlatombe Oct 2, 2024 • edited Loading

Choose a reason for hiding this comment

Vlatombe Oct 2, 2024

Choose a reason for hiding this comment

daniel-beck Oct 11, 2024

Choose a reason for hiding this comment

Vlatombe Oct 11, 2024

Choose a reason for hiding this comment

daniel-beck Oct 11, 2024

Choose a reason for hiding this comment

daniel-beck Oct 11, 2024

Choose a reason for hiding this comment

Vlatombe Oct 11, 2024

Choose a reason for hiding this comment

timja left a comment

Choose a reason for hiding this comment

Vlatombe commented Oct 2, 2024 •

edited

Loading

Vlatombe Oct 2, 2024 •

edited

Loading