-
Notifications
You must be signed in to change notification settings - Fork 148
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
Configurable anonymous access to the list of failure causes #31
Conversation
Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests |
The build failed because of Checkstyle errors - but no detail is given in the build :( Any idea? |
Checkstyle issue fixed. |
@@ -32,7 +32,7 @@ def f = namespace(lib.FormTagLib) | |||
def l = namespace(lib.LayoutTagLib) | |||
def j = namespace(lib.JenkinsTagLib) | |||
|
|||
l.layout(permission: PluginImpl.UPDATE_PERMISSION) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since VIEW_PERMISSION is inferred by UPDATE_PERMISSION you should be able to put permission: PluginImpl.VIEW_PERMISSION
here for safety.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
A test case or two testing the permissions are working as expected would be nice. Example:
There are some helper methods in JenkinsRule to help you setup security and test these scenarios. |
I'll work on the tests a bit later today. |
Although it doesn't show up here, the checkstyle still seems to be failing?
|
On last build (https://jenkins.ci.cloudbees.com/job/plugins/job/build-failure-analyzer-plugin/92/console), it looks OK. I have made a correction since then: 5f3e14e |
No worries, I won't be able to look at it until earliest tomorrow anyways. |
Another thing, if this pull request is merged back and released, the Wiki page at https://wiki.jenkins-ci.org/display/JENKINS/Build+Failure+Analyzer will have also to be adapted. |
Most likely, I hope someone remembers to do that ;) I barely remember to update the changelog when I release :D |
I have tried to create some unit tests (see 4686648, on a separate branch) but I face the following error when running them:
Does it ring a bell? |
Funny enough, when I run the test on the command line, it's working file. It's only in my IDE (Intellij) where I have this problem. I will go on using the command line right now. |
I've now added 4 unit tests to test different combinations of security accesses. They work when ran locally using the command line ( Damien. |
Some types of source code changes needs to have an updated manifest file for the hpi, running the tests from IDEA doesn't generate that metadata, but running |
Understood. Thanks for the tip. |
Configurable anonymous access to the list of failure causes
Do you know when we could expect a new release containing this enhancement? It's for us to know if we have to wait or if we have to patch our Jenkins in the meantime? We'll use this feature to provide documentation to our different projects. |
I think I can cut a release now |
In some contexts, it is interesting to provide authorised users a read-only access to list of failure causes.
This pull request creates a new
ViewCauses
permission, granted automatically for users having alreadyUpdateCauses
. When a user has thisViewCauses
permission granted, he can access the list of existing failures.Best regards,
Damien.