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 regenerating Query API Key #3764

Merged
merged 8 commits into from
Jun 12, 2019

Conversation

kyoshidajp
Copy link
Member

@kyoshidajp kyoshidajp commented May 6, 2019

What type of PR is this? (check all applicable)

  • Feature

Description

Support regenerating Query API Key.

Add API endpoint to regenerate.

POST /api/queries/{id}/regenerate_api_key

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

Copy link
Member

@arikfr arikfr left a 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;
Copy link
Member

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.

'object_type': 'query',
})

result = QuerySerializer(query, with_visualizations=True).serialize()
Copy link
Member

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;
Copy link
Member

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;
Copy link
Member Author

Choose a reason for hiding this comment

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

  1. Regenerate API Key
  2. Close dialog
  3. Reopen dialog (API Key isn't updated)

Need to update the query object, but I have no idea.

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 update the query.api_key for it to persist.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Fixed it.

Copy link
Collaborator

@kravets-levko kravets-levko left a 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.

@kyoshidajp
Copy link
Member Author

@kravets-levko Thanks. Please review again.

@kravets-levko kravets-levko requested a review from arikfr May 21, 2019 07:20
@arikfr arikfr merged commit 8e38dcd into getredash:master Jun 12, 2019
@arikfr
Copy link
Member

arikfr commented Jun 12, 2019

Thanks!

harveyrendell pushed a commit to pushpay/redash that referenced this pull request Nov 14, 2019
* 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
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.

3 participants