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

Sql explorer save query params #487

Conversation

dwmorrison33
Copy link

@dwmorrison33 dwmorrison33 commented Sep 1, 2022

Description

This provides the functionality to save query params so that the queries execute with them each time one comes to the Query Detail page.

Deployment Procedures

  • Migration (Please specify app(s) below)
    • ./manage.py migrate explorer

Tests

Execute Manual UI Tests like the following:

  1. Create a new query with a query param and save, note the same functionality exists as previously where the query does not execute due to no value for the new param:

Screen Shot 2022-09-01 at 10 57 38 AM

  1. Input a value for the parameter and save:

Screen Shot 2022-09-01 at 10 57 52 AM

  1. Now add a new parameter and save:

Screen Shot 2022-09-01 at 10 58 39 AM

  1. Now save a new value to the second parameter:

Screen Shot 2022-09-01 at 10 58 53 AM

  1. Now navigate back to the Query List page and click on the test query:

Screen Shot 2022-09-01 at 10 59 08 AM

  1. Validate that the new QueryParams persist and the query executed:

Screen Shot 2022-09-01 at 10 59 41 AM

Also, you will want to verify that the csv and JSON exports still work correctly like:

Screen Shot 2022-09-01 at 11 08 54 AM

Screen Shot 2022-09-01 at 11 09 13 AM

@dwmorrison33
Copy link
Author

@marksweb Thank you for the instructions here: #482 (comment) They were very helpful and I learned a couple new things.

@marksweb
Copy link
Collaborator

marksweb commented Sep 1, 2022

@dwmorrison33 You're welcome. I'll take a look over this.

First thing that I notice is the inclusion of unicode_literals. Are you using python 2 still? If you are, you should move to python 3.

@dwmorrison33
Copy link
Author

@dwmorrison33 You're welcome. I'll take a look over this.

First thing that I notice is the inclusion of unicode_literals. Are you using python 2 still? If you are, you should move to python 3.

@marksweb Im not sure why that was auto generated since I am on Python3 and these code changes were tested against python3.7 and django1.11

@marksweb
Copy link
Collaborator

marksweb commented Sep 1, 2022

@dwmorrison33 So just trying to understand this, because as I said, I don't use query params in my queries.

If a query parameter ends up as a database object linked to the query - why not just put the value into the query and have different queries for however many values you need? Because I'm just thinking about efficiency. If there is value in having the query parameters in the database, I'd maybe have done a JSON field so all the params with a query go into 1 object.

@dwmorrison33
Copy link
Author

dwmorrison33 commented Sep 1, 2022

@dwmorrison33 So just trying to understand this, because as I said, I don't use query params in my queries.

If a query parameter ends up as a database object linked to the query - why not just put the value into the query and have different queries for however many values you need? Because I'm just thinking about efficiency. If there is value in having the query parameters in the database, I'd maybe have done a JSON field so all the params with a query go into 1 object.

@marksweb I thought about doing it that way, however chose not to since JSONField is database dependent. IIRC, the use of JSONField is specific to Postgres databases and not usable by MySQL, Oracle, etc.. I would think since there are only 2 queries performed in the create_query_params, it should be performant enough for this use case.

@dwmorrison33
Copy link
Author

dwmorrison33 commented Sep 1, 2022

If a query parameter ends up as a database object linked to the query - why not just put the value into the query and have different queries for however many values you need?

@marksweb I just realized that I did not respond to the first question you asked and the reason why we wouldnt want that is because we have roughly 1000 queries and that could become cumbersome to save a bunch of the same queries with different values. The desired behavior is that we can store the query parameters for users who are not experienced with writing SQL and would not have to update values directly, as well as so that after one navigates away from the Query Detail and then comes back that the params would persist for the next user who is looking at that Query Detail.

@dwmorrison33
Copy link
Author

Closing in favor of #489

@dwmorrison33 dwmorrison33 deleted the sql-explorer-save-query-params branch September 6, 2022 19:12
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.

2 participants