New React-based UI for the plugin #58

AbhyudayaSharma · 2020-05-25T12:43:28Z

New React-based UI for the plugin

AbhyudayaSharma · 2020-05-25T12:45:03Z

At the moment, React appears to be working from the configuration page.

The JavaScript for this is generated by Webpack and converted to a single minified JS file which is imported using a <script> tag.

Unlike working hours plugin, this doesn't use an iframe.

timja · 2020-05-25T13:05:42Z

Nice! I didn't like how the working hours plugin ran separately

Avoids having two copies of node_modules when running npm outside of maven.

src/main/frontend/src/resetEnvironment.js

Co-authored-by: Tim Jacomb <t.jacomb@kainos.com>

timja · 2020-05-26T08:55:29Z

cc @fqueiruga

Tabs look interesting, I wouldn't have the header colour though

timja · 2020-05-26T08:59:12Z

Thinking aloud here, I've been considering the concept of 'Built-in roles',

or separating Roles from Scope

Consider the following roles:

Basic read - can see jobs and logs
Global Read - can see agent, job and system configuration
Admin
Builder - can trigger and cancel jobs

These are all very re-usable roles I would expect, and could be defined by the plugin by default.

A problem with the current implementation is that roles are defined at 'Global, Agent or Folder'

What if roles were defined separately and then assigned to a scope level?

Feel free to disregard as out of scope but bringing it up here because a UI re-design could be a good time to implement this

AbhyudayaSharma · 2020-05-26T09:09:43Z

Tabs look interesting, I wouldn't have the header colour though

That's just the default color that comes with the library. They have a palette customization option: https://material-ui.com/customization/palette/

If there's a color theme specification for Jenkins, it can be easily added.

Consider the following roles:

Basic read - can see jobs and logs

Global Read - can see agent, job and system configuration

Admin

Builder - can trigger and cancel jobs

These are all very re-usable roles I would expect, and could be defined by the plugin by default.

An Admin role is automatically created when the plugin is initialized for the first time. All these things look like they are suitable for Global roles. Just like the admin role, we can create more roles if it helps.

res0nance · 2020-05-26T09:16:30Z

Thinking aloud here, I've been considering the concept of 'Built-in roles',

or separating Roles from Scope

Consider the following roles:
* Basic read - can see jobs and logs

* Global Read - can see agent, job and system configuration

* Admin

* Builder - can trigger and cancel jobs
These are all very re-usable roles I would expect, and could be defined by the plugin by default.

A problem with the current implementation is that roles are defined at 'Global, Agent or Folder'

What if roles were defined separately and then assigned to a scope level?

Feel free to disregard as out of scope but bringing it up here because a UI re-design could be a good time to implement this

This sounds like its exactly https://docs.cloudbees.com/docs/admin-resources/latest/plugins/rbac

timja · 2020-05-26T09:26:33Z

This sounds like its exactly https://docs.cloudbees.com/docs/admin-resources/latest/plugins/rbac

Looks very similar, although that plugin is proprietary and has a very dated looking UI 👅

fqueiruga · 2020-05-26T09:53:33Z

This is a great initiative. I have 2 pieces of feedback for now:

I would suggest using react hooks instead of class components. It's more future-proof and easier to use. I can help you out with this if you want.
I wouldn't use material UI. There are many examples of tabbars out there, and it's really easy to come up with styles for those. You could even copy & paste the bootstrap CSS for the tabs. The material-ui styles clash a lot with the Jenkins look & feel.

res0nance · 2020-05-26T11:27:38Z

This sounds like its exactly https://docs.cloudbees.com/docs/admin-resources/latest/plugins/rbac

Looks very similar, although that plugin is proprietary and has a very dated looking UI 👅

It has one really nice feature which users who perhaps have folder level admin can grant other users permissions from that level onwards

AbhyudayaSharma · 2020-05-27T04:55:39Z

Thanks @fqueiruga !

I would suggest using react hooks instead of class components. It's more future-proof and easier to use. I can help you out with this if you want.

Does that mean using function components along with useState()?

I wouldn't use material UI. There are many examples of tabbars out there, and it's really easy to come up with styles for those. You could even copy & paste the bootstrap CSS for the tabs. The material-ui styles clash a lot with the Jenkins look & feel.

Would you suggest using React Bootstrap or picking up components from npm and writing my own CSS?

AbhyudayaSharma · 2020-05-27T12:23:39Z

I tried using react-bootstrap with Bootstrap 4 but the CSS conflicts with that exported by Jenkins. Will have to look for something else.

timja · 2020-05-27T12:30:43Z

I tried using react-bootstrap with Bootstrap 4 but the CSS conflicts with that exported by Jenkins. Will have to look for something else.

there's a flag on layout.jelly you can use to not load the ~'bootstrap 3' grid
https://github.com/jenkinsci/jenkins/blob/master/core/src/main/resources/lib/layout/layout.jelly#L61

AbhyudayaSharma · 2020-05-27T13:59:45Z

I tried using react-bootstrap with Bootstrap 4 but the CSS conflicts with that exported by Jenkins. Will have to look for something else.

there's a flag on layout.jelly you can use to not load the ~'bootstrap 3' grid
https://github.com/jenkinsci/jenkins/blob/master/core/src/main/resources/lib/layout/layout.jelly#L61

That looks interesting. I'll try it out. Thanks!

…auth-plugin into react-ui

AbhyudayaSharma · 2020-05-28T06:37:28Z

...esources/io/jenkins/plugins/folderauth/FolderAuthorizationStrategyManagementLink/index.jelly

@@ -1,309 +1,10 @@
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:l="/lib/layout">
-    <l:layout permission="${app.ADMINISTER}" norefresh="true" title="${%title}">
+    <l:layout permission="${app.ADMINISTER}" norefresh="true" title="${%title}" nogrid="true">


That works (with some hacks) but the sidebar breaks when nogrid="true".

Would it make sense to remove the sidebar?

@uhafner thoughts? I'm guessing there's some minimal styles you can add to fix the sidebar, or the sidebar shouldn't be relying on those styles and should be fixed in Jenkins core, cc @fqueiruga

Which Jenkins version are you using? It still shows the old UI. Maybe that is the problem? I am already using the nogrid flag and the sidebar does not break.

When you use something that loads Bootstrap 4 then you also need jquery3, which leads to some additional problems. I needed to add the following fix on all my pages:
https://github.com/jenkinsci/bootstrap4-api-plugin/blob/master/src/main/webapp/js/no-prototype.js

@uhafner Jenkins 2.235

When you use something that loads Bootstrap 4 then you also need jquery3, which leads to some additional problems. I needed to add the following fix on all my pages

I'm using react-bootstrap which does not need jquery. Quoting from their website:

React-Bootstrap replaces the Bootstrap JavaScript. Each component has been built from scratch as a true React component, without unneeded dependencies like jQuery.

So we only need the bootstrap css

@uhafner is there any way to load the bootstrap layout just for the CSS without loading the bootstrap JS? That would be a huge win for many plugins.

Yes, that should be possible. Currently

<st:adjunct includes="io.jenkins.plugins.bootstrap4"/>

includes both. But you can already include it using

<link type="text/css" rel="stylesheet" href="${resURL}/plugin/bootstrap4-api/css/bootstrap.min.css"/>

The. problem is: you don't know which components require JS and which not.

@uhafner Jenkins 2.235

Maybe the problem is, that if you do not load the BS3 grid then you rather need to load the BS4 grid. So in my layouts I am using nogrid and BS4 grid. If you do not load a grid it might break things.

AbhyudayaSharma · 2020-05-28T06:38:19Z

src/main/frontend/src/css/App.css

@@ -0,0 +1,5 @@
+a.tab:link,
+a.tab:visited {
+    color: #007bff;


Hack to unset global a:link and a:visited.

Yeah, that's a known PITA, I want to fix that

AbhyudayaSharma · 2020-05-28T06:39:44Z

pom.xml

@@ -27,9 +27,11 @@
    <properties>
        <revision>1.3</revision>
        <changelist>-SNAPSHOT</changelist>
-        <jenkins.version>2.164.1</jenkins.version>
+        <jenkins.version>2.235</jenkins.version>


Also needs a version bump to at least 2.235

fqueiruga · 2020-05-28T13:27:26Z

Does that mean using function components along with useState()?

Indeed.

Would you suggest using React Bootstrap or picking up components from npm and writing my own CSS?

Depends on what you're most comfortable doing. I'd vote for custom components. There is already CSS for tabs on Jenkins, and I don't see it changing. You could use those CSS classes (for tabbarFrame, tab, etc)

src/main/frontend/src/App.tsx

AbhyudayaSharma · 2020-06-13T10:22:07Z

src/main/java/io/jenkins/plugins/folderauth/FolderAuthorizationStrategyManagementLink.java

+        response.setCharacterEncoding("utf-8");
+        Writer writer = response.getWriter();
+        FolderBasedAuthorizationStrategyWrapper wrapper = new FolderBasedAuthorizationStrategyWrapper((FolderBasedAuthorizationStrategy) strategy);
+        JsonGenerator generator = jsonFactory.createGenerator(writer);


net.sf.json was able to convert the AuthorizationStrategy to JSON but during the process it was generating over 1000 lines of warnings. I think using Jackson for it is a better solution. Please let me know if there is some better alternative.

pom.xml

timja · 2020-06-13T10:45:04Z

src/main/java/io/jenkins/plugins/folderauth/FolderAuthorizationStrategyManagementLink.java

+        Writer writer = response.getWriter();
+        FolderBasedAuthorizationStrategyWrapper wrapper = new FolderBasedAuthorizationStrategyWrapper((FolderBasedAuthorizationStrategy) strategy);
+        JsonGenerator generator = jsonFactory.createGenerator(writer);
+        generator.setCodec(new ObjectMapper());


best to cache this, you only need to create one once

vonEdfa · 2021-09-11T14:59:11Z

Is it ok if I pick up work on this? I don't have much experience with Java, but I know js/ts and React pretty well and I think a new manage UI would be golden.

AbhyudayaSharma · 2021-09-12T12:44:01Z

Is it ok if I pick up work on this? I don't have much experience with Java, but I know js/ts and React pretty well and I think a new manage UI would be golden.

@vonEdfa thanks a lot. This would be great! I started working on this last year but it got lost thanks to to me having way too much work. I will try to build this branch and share resources to help you get started tomorrow.

vonEdfa · 2021-09-12T14:26:23Z

I did a trial run and did get it up and running and made a small demo front-end design. I am tempted to update some dependencies on the node side, but I didn't really dare to at this point.
Nothing is pushed at this point though, but I could merge it in once the branch is ready?

Only snag I met was that the folder-auth api only provides the IDs for the enabled permissions and I think we'd need a list of all available permissions to get react to draw them (if we don't want to hard code them, that is, but I'm assuming that is risky as new permissions can be introduced by other plugins, right?). Could perhaps be done by changing the structure of the json so each role includes all permission IDs together with a bool or having a second set of data where we ask Jenkins to give us all currently available permissions for the role type we're editing right now.

AbhyudayaSharma · 2021-09-13T06:22:56Z

I did a trial run and did get it up and running and made a small demo front-end design. I am tempted to update some dependencies on the node side, but I didn't really dare to at this point.

We use https://github.com/eirslett/frontend-maven-plugin which downloads and installs node and npm. Node is installed in /node. The node and npm versions are specified as properties in pom.xml. Most Jenkins plugins use plugin-pom which provides common Maven configuration. Because the file .mvn_exec_node exists, Maven and plugin-pom will ensure that npm run mvnbuild will be run before the plugin is built and Jenkins is started.

Webpack is configured to output the JS bundle to src/main/webapp/js/folder-auth-bundle.js. Jenkins plugins traditionally use Jelly, a templated XML format that generates HTML; UI resources are stored in src/main/webapp. For folder-auth, src/main/resources/io/jenkins/plugins/folderauth/FolderAuthorizationStrategyManagementLink/index.jelly is rendered on the server side by Jenkins when a user visits JENKINS_URL/folder-auth. Instead of putting the front-end logic in Jelly, we just create a div with id root and import the JS compiled by webpack using <script>. Then ReactDOM renders the new UI in #root.

Only snag I met was that the folder-auth api only provides the IDs for the enabled permissions and I think we'd need a list of all available permissions to get react to draw them.

Right. The plugin currently has no API that exposes loaded permissions and permission groups. There could be one from Jenkins core but I would need to check on that. Otherwise, we will need to build one.

if we don't want to hard code them, that is, but I'm assuming that is risky as new permissions can be introduced by other plugins, right?

Yes, permissions can be added by other plugins. Hard coding them is not really an option.

Could perhaps be done by changing the structure of the json so each role includes all permission IDs together with a bool or having a second set of data where we ask Jenkins to give us all currently available permissions for the role type we're editing right now.

The data visible inside the folder roles tab right now is just a slightly modified version of the JCasC YAML in JSON. I think that the current draft could actually be better version of #69.

Currently, there is no hot-reloading for the plugin UI. When you're running the plugin with mvn hpi:run and Jenkins is fully up and running, you can run npm build:dev in parallel to watch changes. You would need to hard refresh the folder-auth page to get the latest changes.

Another major change needed is to fix the sidebar CSS to not depend on bootstrap as described in #58 (comment). Again, we need to check with a new Jenkins version. I'll try to check that later today.

@vonEdfa Thanks a lot for working on this. I hope that this information is useful. If you have any other questions, I would be happy to help.

AbhyudayaSharma

So all dependencies are now updated to the latest versions. Bumping up everything also fixed the broken sidebar.

vonEdfa · 2021-09-19T08:18:58Z

Nice! Seems I was starting it in the right way then (mvn hpi:run and npm build:dev)!

I noticed issues with my default system node versions not matching the repository's (which is quite common) and I do use version managers for node, so it it ok if I add such files to the repo? I don't know if nvm is still the most common one as I switched to asdf a while ago, but if we prefer nvm terms I think asdf supports it.

On to the look of things: I don't know if you already had anything in mind for the design to use. My initial thought was something similar to a permission matrix, but then I asked myself if we could build something more manageable? I personally don't find permissions matrices easy to use as they easily get too wide and selecting multiple bool checkboxes can be quite tedious. Let me know if I'm alone in this opinion.

Based on this, when I was playing around I took a similar approach to what Discord currently has for their server role management interface: First view is an overview listing all available roles which you'd then open to manage it's settings. My thinking is that you usually set a role and it's permissions up once and then you mostly don't need or want to edit it much (save for adding new members to it).

Here's an example of the overview list (built pre your latest commit):

I didn't implement the edit view in my demo, but if anyone's not familiar with Discord's edit role view it's a tab based design that looks like this:

AbhyudayaSharma · 2021-09-20T06:18:53Z

I noticed issues with my default system node versions not matching the repository's (which is quite common) and I do use version managers for node, so it it ok if I add such files to the repo? I don't know if nvm is still the most common one as I switched to asdf a while ago, but if we prefer nvm terms I think asdf supports it.

@vonEdfa When you run mvn hpi:run, node is downloaded and installed in PROJECT_ROOT/node. To avoid conflicts with the global npm installation, we should run ./node/npm instead.

The UI prototype looks great! I was also thinking of something along these lines. I don't like the checkbox matrix either. It can get confusing when there are a lot of permissions.

Copying the discord permission modification view looks like a great idea. I would probably suggest a modal dialog instead of a taking to a new screen to avoid routing complications on the server side.

Webpack generated React for configuration page

9a15314

AbhyudayaSharma requested review from timja, res0nance and oleg-nenashev May 25, 2020 12:43

AbhyudayaSharma added the enhancement New feature or request label May 25, 2020

AbhyudayaSharma added 3 commits May 26, 2020 10:23

Unset working directory for maven-frontend-plugin

e588d7c

Avoids having two copies of node_modules when running npm outside of maven.

Avoid collisions with JSON setup by prototype.js

b974e18

Add support for loading CSS from JS

44c9790

AbhyudayaSharma mentioned this pull request May 26, 2020

JENKINS-62443 Add Full Name lookup for SIDs #59

Open

timja reviewed May 26, 2020

View reviewed changes

src/main/frontend/src/resetEnvironment.js Outdated Show resolved Hide resolved

Material UI based layout

d1f09b8

This comment has been minimized.

Sign in to view

Update src/main/frontend/src/resetEnvironment.js

bb19ea8

Co-authored-by: Tim Jacomb <t.jacomb@kainos.com>

Require admin permission for accessing the AuthorizationStrategy

ef95ac1

AbhyudayaSharma added 2 commits May 28, 2020 11:55

react-bootstrap tabs

9861540

Merge branch 'react-ui' of https://github.com/AbhyudayaSharma/folder-…

32534c5

…auth-plugin into react-ui

AbhyudayaSharma commented May 28, 2020

View reviewed changes

AbhyudayaSharma requested a review from fqueiruga May 28, 2020 06:38

AbhyudayaSharma commented May 28, 2020

View reviewed changes

AbhyudayaSharma mentioned this pull request May 28, 2020

New UI for Folder Auth plugin jenkinsci/ui-ux-hackfest-2020#142

Closed

baymac reviewed May 29, 2020

View reviewed changes

src/main/frontend/src/App.tsx Outdated Show resolved Hide resolved

AbhyudayaSharma mentioned this pull request Jun 8, 2020

JENKINS-62285 Don't use browser alert for notifications and confirmation #60

Closed

Use Jackson to generate JSON for FolderBasedAuthorizationStrategy

dcf5ed8

AbhyudayaSharma commented Jun 13, 2020

View reviewed changes

timja reviewed Jun 13, 2020

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

timja reviewed Jun 13, 2020

View reviewed changes

Update Dependencies and use jackson2-api plugin

4f30320

AbhyudayaSharma commented Sep 13, 2021

View reviewed changes

New React-based UI for the plugin #58

Are you sure you want to change the base?

New React-based UI for the plugin #58

Conversation

AbhyudayaSharma commented May 25, 2020

AbhyudayaSharma commented May 25, 2020 • edited Loading

timja commented May 25, 2020

This comment has been minimized.

timja commented May 26, 2020

timja commented May 26, 2020

AbhyudayaSharma commented May 26, 2020

res0nance commented May 26, 2020

timja commented May 26, 2020 • edited Loading

fqueiruga commented May 26, 2020

res0nance commented May 26, 2020

AbhyudayaSharma commented May 27, 2020 • edited Loading

AbhyudayaSharma commented May 27, 2020

timja commented May 27, 2020

AbhyudayaSharma commented May 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AbhyudayaSharma May 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fqueiruga commented May 28, 2020 • edited Loading

AbhyudayaSharma Jun 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vonEdfa commented Sep 11, 2021

AbhyudayaSharma commented Sep 12, 2021

vonEdfa commented Sep 12, 2021

AbhyudayaSharma commented Sep 13, 2021

AbhyudayaSharma left a comment

Choose a reason for hiding this comment

vonEdfa commented Sep 19, 2021

AbhyudayaSharma commented Sep 20, 2021

AbhyudayaSharma commented May 25, 2020 •

edited

Loading

timja commented May 26, 2020 •

edited

Loading

AbhyudayaSharma commented May 27, 2020 •

edited

Loading

AbhyudayaSharma May 28, 2020 •

edited

Loading

fqueiruga commented May 28, 2020 •

edited

Loading

AbhyudayaSharma Jun 13, 2020 •

edited

Loading