-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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 regenerating Query API Key #3764
Conversation
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.
Thanks, @kyoshidajp . This is definitely a good feature to have.
See comments ➡️
this.jsonUrl = `${clientConfig.basePath}api/queries/${this.resolve.query.id}/results.json?api_key=${this.apiKey}`; | ||
$scope.canEdit = currentUser.id === this.resolve.query.user.id || currentUser.hasPermission('admin'); | ||
$scope.disableRegenerateApiKeyButton = false; | ||
$scope.query = this.resolve.query; |
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.
This is an Angular component, so you can access this
in the template as $ctrl
- no need for use of $scope
.
redash/handlers/queries.py
Outdated
'object_type': 'query', | ||
}) | ||
|
||
result = QuerySerializer(query, with_visualizations=True).serialize() |
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.
Let's return a simple version of the query, without visualizations, to avoid the extra queries and simplify API response.
$http | ||
.post(`api/queries/${this.resolve.query.id}/regenerate_api_key`) | ||
.success((data) => { | ||
$scope.query = data; |
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.
See my comment about simplifying the response of this API call. In this case, we only need to update the API key of the query object.
$http | ||
.post(`api/queries/${this.resolve.query.id}/regenerate_api_key`) | ||
.success((data) => { | ||
this.api_key = data.api_key; |
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.
- Regenerate API Key
- Close dialog
- Reopen dialog (API Key isn't updated)
Need to update the query object, but I have no idea.
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.
You should update the query.api_key
for it to persist.
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.
Thanks. Fixed it.
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.
Looks good (at least, JS part 🙂). @kyoshidajp please catch up with master
so I can check Percy snapshots.
dd4a52d
to
cdde2ee
Compare
@kravets-levko Thanks. Please review again. |
Thanks! |
* Add regenerate function of query's API Key * Add regenerate API Key button * Add regenerate Query API Key tests * Fix too long line * Replace with this * Return a simple version query * Update only API Key * Update API Key via query
What type of PR is this? (check all applicable)
Description
Support regenerating Query API Key.
Add API endpoint to regenerate.
Also its button (ref: the screenshot below at the bottom of this page).
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Query API Key dialog:
![after_567x335](https://user-images.githubusercontent.com/3317191/57215401-bab57380-7027-11e9-8eba-b8e7d3c6a0e5.png)