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

Support roles as a list property of Dashboards #31

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

ljpeters
Copy link

The roles input field is revealed after enabling DASHBOARD_RBAC flag.
image

The information was already retrieved, but now also stored in the Dashboard during json processing.

supersetapiclient/base.py Outdated Show resolved Hide resolved
@ljpeters
Copy link
Author

I am still working on saving a Dashboard with roles attached. This is not working yet because the save function in base.py is not yet converting the list of Role objects to json.

Luuk Peters added 3 commits August 18, 2022 16:34
…eters/superset-api-client into support-roles-property-of-dashboards
… into support-roles-property-of-dashboards

# Conflicts:
#	supersetapiclient/client.py
@ljpeters ljpeters force-pushed the support-roles-property-of-dashboards branch from 5cd9f32 to adae42d Compare August 21, 2022 21:08
@ljpeters
Copy link
Author

Hi @ecederstrand or other supersetapiclient developer, I'm done with the changes I needed to store the Roles of a Dashboard. I have applied these changes to my superset setup and they work. I would like to follow through on this PR and merge it.

  • Is the correct target branch set on this PR? I noticed some more changes on develop.
  • Are units test available which I can expand on?
  • I would like to expand documentation in README.md regarding the feature I'm adding, but I would like to do it in a separate PR, is that ok?

@opus-42
Copy link
Owner

opus-42 commented Aug 31, 2022

Hello @ljpeters ,

Thank you for your contribution. I was on vacations and unable to respond sooner.

  • The correct target branch should be develop
  • On master & develop, there is a skeleton for unit testing the client but it does not yet cover everything.
  • Yes please do it in a separate PR, I will then approve it. At some point we should also release some official documentation on a website.

Thanks again

@ecederstrand
Copy link
Collaborator

ecederstrand commented Sep 15, 2022

@ljpeters If you merge the latest develop into this branch, then there's now a full test suite you can add test cases to, with lots of examples on how to write tests cases.

@Ian2012
Copy link

Ian2012 commented May 25, 2023

Is this still in progress?

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

Successfully merging this pull request may close these issues.

4 participants