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

Feature/add dashboard creation #36

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Dynnammo
Copy link
Contributor

@Dynnammo Dynnammo commented Oct 24, 2022

As a data analyst, I would like to take advantage of the POST /api/dashboard method that allows me to create a dashboard with the name provided and the collection information needed to create it at the right place.

@Dynnammo Dynnammo marked this pull request as ready for review October 27, 2022 11:43
@@ -642,6 +642,35 @@ def create_segment(self, segment_name, column_name, column_values, segment_descr



def create_dashboard(self, name, description=None, parameters=None, cache_ttl=None, collection_id=None, collection_position=None, collection_name=None):
Copy link
Owner

Choose a reason for hiding this comment

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

"parameters" and "cache_ttl" are not used in the function.
The function should allow passing custom json as argument to customize the dashboard (I assume the "parameters" option was supposed to do that). Currently only the name and description of the dashboard are customizable using this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, sorry to bother, I'm correcting this asap

@Dynnammo
Copy link
Contributor Author

Some thoughts after your remarks :

  • I tried to make a compromise between the API specification declared here and the way other helpers were previously designed (in this case I added the collection_name which is more muggles-compliant, and return_dashboard, which correspond to what you've done for create_card, create_dashboard, etc...)

I'll try to work on this until you consider it ready-to-merge. I turned it back into draft, since I want to test this on a local instance tomorrow. Thanks for maintaining this anyway, it helped us built some nice stuff here, so consider that we owe you some dev time if asked 🙌

@Dynnammo Dynnammo marked this pull request as draft October 27, 2022 20:33
@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Nov 27, 2022
@vvaezian vvaezian removed the stale label Nov 27, 2022
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.

None yet

2 participants