Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixes #16854: Users with readonly rights should be able to view the node properties and their value #2856

Merged
merged 1 commit into from
Apr 7, 2020
Merged

Fixes #16854: Users with readonly rights should be able to view the node properties and their value #2856

merged 1 commit into from
Apr 7, 2020

Conversation

ElaadF
Copy link
Member

@ElaadF ElaadF commented Apr 2, 2020

@ElaadF ElaadF requested a review from fanf April 2, 2020 07:28
@ElaadF ElaadF added the WIP Use that label for a Work In Progress PR that must not be merged yet label Apr 2, 2020
@ElaadF ElaadF force-pushed the bug_16854/users_with_readonly_rights_should_be_able_to_view_the_node_properties_and_their_value branch from ea46362 to 0b9146c Compare April 2, 2020 13:25
@ElaadF
Copy link
Member Author

ElaadF commented Apr 2, 2020

Commit modified

@ElaadF ElaadF force-pushed the bug_16854/users_with_readonly_rights_should_be_able_to_view_the_node_properties_and_their_value branch from 0b9146c to b697cf0 Compare April 2, 2020 14:13
@ElaadF
Copy link
Member Author

ElaadF commented Apr 2, 2020

Commit modified

@ElaadF ElaadF removed the WIP Use that label for a Work In Progress PR that must not be merged yet label Apr 2, 2020
VinceMacBuche
VinceMacBuche requested changes Apr 2, 2020
View reviewed changes
...s/rudder/rudder-web/src/main/webapp/templates-hidden/components/ComponentNodeProperties.html
@@ -102,12 +102,12 @@
<small class="text-danger" ng-if="!editedProperties[property.name].new.isValid">JSON check is enabled, but the value format is invalid</small>
</div>
</td>
<td class="text-center" ng-class="{'default-actions':!isEdited(property.name), 'edit-actions is-edited':isEdited(property.name)}">
<div ng-if="!isEdited(property.name) && property.rights !== 'read-only'">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should not remove this, it's to prevent edition of inventory/datasources properties that cannot be modified

@ElaadF
Copy link
Member Author

ElaadF commented Apr 2, 2020

PR updated with a new commit

VinceMacBuche
VinceMacBuche reviewed Apr 2, 2020
View reviewed changes
webapp/sources/rudder/rudder-web/src/main/webapp/javascript/rudder/angular/nodeProperties.js
@@ -94,10 +95,8 @@ app.controller('nodePropertiesCtrl', function ($scope, $http, DTOptionsBuilder,
DTColumnDefBuilder.newColumnDef(2).withOption("sWidth",'5%')
];
var currentNodeId
$scope.init = function(properties, nodeId, right){
if(!right){
$scope.columns[2].notVisible();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could have just changed the 2 to a 3 here :)

Copy link
Member Author

@ElaadF ElaadF Apr 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tab disapear when I used column 3

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/2856
-- Your faithful QA
Kant merge: "All our knowledge begins with the senses, proceeds then to the understanding, and ends with reason. There is nothing higher than reason."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/22591/console)

@ElaadF
Copy link
Member Author

ElaadF commented Apr 7, 2020

OK, squash merging this PR

@ElaadF ElaadF force-pushed the bug_16854/users_with_readonly_rights_should_be_able_to_view_the_node_properties_and_their_value branch from 7c786ee to 0367283 Compare April 7, 2020 12:04
@ElaadF ElaadF merged commit 0367283 into Normation:branches/rudder/5.0 Apr 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VinceMacBuche VinceMacBuche VinceMacBuche approved these changes

@fanf fanf Awaiting requested review from fanf

@RaphaelGauthier RaphaelGauthier Awaiting requested review from RaphaelGauthier

Assignees
No one assigned
Labels
qa: Can't merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ElaadF @Normation-Quality-Assistant @VinceMacBuche