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.

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

grafana_folder: add permissions #231

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

CWollinger
Copy link
Contributor

SUMMARY

Able to set permissions of a folder

ISSUE TYPE
  • Bugfix Pull Request
  • Feature Pull Request
COMPONENT NAME

grafana_folder

ADDITIONAL INFORMATION

fix #213

@CWollinger CWollinger requested review from rrey and seuf as code owners April 4, 2022 18:34
@codecov
Copy link

codecov bot commented Apr 4, 2022

Codecov Report

Merging #231 (d4349c3) into main (ef170c4) will increase coverage by 0.61%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##             main     #231      +/-   ##
==========================================
+ Coverage   68.76%   69.37%   +0.61%     
==========================================
  Files          16       16              
  Lines        1700     1760      +60     
  Branches      287      304      +17     
==========================================
+ Hits         1169     1221      +52     
- Misses        398      401       +3     
- Partials      133      138       +5     
Flag Coverage Δ
integration 68.18% <83.33%> (+1.04%) ⬆️
sanity 22.41% <10.00%> (-0.56%) ⬇️
units 73.40% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
plugins/modules/grafana_folder.py 79.87% <83.33%> (+4.11%) ⬆️
plugins/modules/grafana_datasource.py 74.55% <0.00%> (ø)
plugins/callback/grafana_annotations.py 31.70% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef170c4...d4349c3. Read the comment docs.

plugins/modules/grafana_folder.py Show resolved Hide resolved
plugins/modules/grafana_folder.py Outdated Show resolved Hide resolved
plugins/modules/grafana_folder.py Outdated Show resolved Hide resolved
self._module.fail_json(failed=True, msg="User '%s' does not exists" % email)
return user.get("id")

def get_team(self, name):
Copy link
Collaborator

Choose a reason for hiding this comment

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

We already have code in grafana_team.py doing this (line 241).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shall we export these functions to module_utils and import this from both modules? Could be also interesting for other functions like

def get_version(self):

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, that is something I'd like to do at some point ; have all Grafana API call related stuff in module_utils and have our library that we can use in all modules.

I guess it will be easier if you move the team related stuff that you need to have the big change that I will never have time to review properly.

Sorry for the delay on this PR.

Comment on lines +112 to +116
- assert:
that:
- "result.changed == false"
- "result.failed == true"
- "result.msg == expected_error"
Copy link
Collaborator

Choose a reason for hiding this comment

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

@CWollinger I don't know if you still want to finish this PR but I am realizing now after coming back to this PR that we don't check the permissions in these tests.
I believe that the permissions should be in the module outputs and an assert performed to check it is the proper ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I will check this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Folder permissions
2 participants