Adapt skeleton pack generation to work with new queries panel by elenatanasoiu · Pull Request #2250 · github/vscode-codeql

elenatanasoiu · 2023-03-30T13:22:59Z

Please review this PR commit-by-commit.

In a previous PR, we implemented a feature where if you add a new database, we generate a skeleton QL pack for you to work with the database. The skeleton pack would contain an example query file and would match the language of the database.

We've found it's hard for the user to discover this workflow in the codespaces-codeql repo.

After some input from design, we want to reverse this flow by starting with generating the query file and skeleton pack first, and then downloading the database. This means the user immediately focuses on the query file instead of having to switch away from the extension and look for the query file in the explorer view. (see videos below for a visual illustration of what this all means)

This new flow should trigger via a command in the extension ("Create Query") which takes you through a wizard and generates everything.

When the new queries panel is ready, we can also hook up this command with the link suggesting how to create a query ("Create one to start"):

Finally, we're making sure that both the codespaces-codeql and vscode-codeql-starter repos work as expected with this new command:

codespaces-codeql repo

Screen.Recording.2023-03-30.at.14.25.05.mov

vscode-codeql-starter repo

Screen.Recording.2023-03-30.at.14.26.50.mov

elenatanasoiu · 2023-04-03T13:32:17Z

CI has been failing remotely, but not locally. I hope I've cleaned up the remote failures as well. 🤞

We initially defined the default database to download as one from the `github/codeql` repo as it was convenient. However, this repo doesn't have a lot of vulnerabilities to discover. Let's use repos that are in our MRVA top 10 list to allow users to write more interesting queries.

Instead of assigning this property in a method, let's make the method return a value and assign it more visibly.

Replaces `file[0]` with a more meaningful `filename`.

Let's only ask for the language when the language is not provided OR it's invalid. Let's also add tests for these cases.

And add tests for getFirstStoragePath method

We've now added more tests and pushed the total duration over 5 seconds for all the tests in this file. This limitation seems to be a recent development where files with tests that last longer than 5 seconds start failing in jest. We're bumping the timeout limit to 40 seconds for now.

When we try to determine the next file name for our example query, we only look at `example<n>.ql` files. e.g. if the files in the folder are: - `example.ql` - `example2.ql` - `MyQuery.ql` we will create an `example3.ql` file. Previously we were counting all existing `.ql` files.

elenatanasoiu · 2023-04-11T15:12:43Z

Thanks for the reviews @koesie10 & @shati-patel! I'm going to pair with @shati-patel tomorrow to look at the windows issues she's discovered. I plan to address those in a subsequent PR as this is getting quite heavy on replies + reviews.

In the meantime, this PR is ready for review again.

shati-patel

Thanks! 🎉 I'm also keen to get this merged, since I'm struggling to review/keep track of the parallel PRs and changes 🙈

I've left a bunch of comments/questions, but they are all non-blocking 💚 The main important thing for me is to (temporarily) hide the command, so that we can do more testing/debugging without disrupting users.

shati-patel · 2023-04-12T14:55:52Z

+      await this.selectExistingDatabase();
+    } else {
+      // generate a new skeleton QL pack with query file
+      await this.createQlPack();


(Discussed offline: Turns out this isn't a Windows thing)

Let's fix this is in a follow-up issue/PR, since we might want to discuss the options with product/design.

Do we want to just add the custom-queries folder to whichever place happens to be the first workspace folder, or should it be a top-level workspace folder?

If the latter, where do we actually create the folder on-disk? (In the extension's workspace storage?)

elenatanasoiu · 2023-04-12T16:07:06Z

Thanks for the reviews @koesie10 & @shati-patel 🙇‍♀️

I'm gonna merge this and follow up with you on the remaining issues tomorrow.

elenatanasoiu force-pushed the elena/yer-a-wizard-query branch from 0ee62f8 to 29fc648 Compare March 30, 2023 13:36

elenatanasoiu marked this pull request as ready for review March 30, 2023 14:28

elenatanasoiu requested a review from a team as a code owner March 30, 2023 14:28

elenatanasoiu force-pushed the elena/yer-a-wizard-query branch from 29fc648 to 09b50eb Compare March 30, 2023 15:06

github-advanced-security AI found potential problems Mar 30, 2023

View reviewed changes

Comment thread extensions/ql-vscode/src/extension.ts Fixed

elenatanasoiu force-pushed the elena/yer-a-wizard-query branch from 09b50eb to 926688e Compare March 30, 2023 15:10

elenatanasoiu requested a review from a team March 30, 2023 18:22

elenatanasoiu added the secexp label Mar 30, 2023

elenatanasoiu requested a review from adityasharad March 31, 2023 16:29

elenatanasoiu mentioned this pull request Mar 31, 2023

Improvements to skeleton query flow #2256

Merged

3 tasks

elenatanasoiu force-pushed the elena/yer-a-wizard-query branch 3 times, most recently from e1cb0b9 to c00240a Compare April 3, 2023 13:31

elenatanasoiu force-pushed the elena/yer-a-wizard-query branch 4 times, most recently from 2f70963 to 0ec1c6c Compare April 3, 2023 14:10

github-advanced-security AI found potential problems Apr 3, 2023

View reviewed changes

elenatanasoiu force-pushed the elena/yer-a-wizard-query branch 11 times, most recently from ebf9c5b to 0545eb5 Compare April 3, 2023 16:46

elenatanasoiu added 17 commits April 11, 2023 14:59

Shorten beforeEach to decrease chances of timeout

95ed076

Don't obfuscate assignment when we choose language

061f347

Instead of assigning this property in a method, let's make the method return a value and assign it more visibly.

Fail gracefully if we can't open the new query file

30011aa

Get rid of unnecessary ternary

fe3e9a7

Make it clear we're checking the filename

9c0deae

Replaces `file[0]` with a more meaningful `filename`.

Fix error message

f949eda

Use a regular const instead of storing this on the class

911c3af

Use Record

97fb4ea

Remove unnecessary casting to DatabaseItem

b794427

Convert folderName property into getter method

59909e2

Shorten logic for deciding when to ask for language

16a8289

Let's only ask for the language when the language is not provided OR it's invalid. Let's also add tests for these cases.

Set storage path after the user selects language

62bebc0

And add tests for getFirstStoragePath method

Rename workoutNextFileName -> determineNextFileName

c7d9407

Add language to list of params

460da1e

elenatanasoiu force-pushed the elena/yer-a-wizard-query branch from e198a79 to 460da1e Compare April 11, 2023 15:00

elenatanasoiu mentioned this pull request Apr 11, 2023

Refactor digForDatabaseItem and digForDatabaseWithSameLanguage methods for skeleton wizard #2297

Merged

3 tasks

Merge branch 'main' into elena/yer-a-wizard-query

e4406f4

elenatanasoiu mentioned this pull request Apr 12, 2023

Fix problem with detecting storage folder on windows #2298

Merged

koesie10 reviewed Apr 12, 2023

View reviewed changes

Comment thread extensions/ql-vscode/src/skeleton-query-wizard.ts

Comment thread extensions/ql-vscode/src/skeleton-query-wizard.ts

Comment thread extensions/ql-vscode/src/skeleton-query-wizard.ts

Comment thread extensions/ql-vscode/src/skeleton-query-wizard.ts

shati-patel approved these changes Apr 12, 2023

View reviewed changes

elenatanasoiu mentioned this pull request Apr 12, 2023

Small fixes to skeleton query wizard #2305

Merged

elenatanasoiu merged commit f377bb9 into main Apr 12, 2023

elenatanasoiu deleted the elena/yer-a-wizard-query branch April 12, 2023 16:07

This was referenced Apr 13, 2023

Stop pushing QL pack as top level folder to avoid confusing the user #2310

Merged

Filter out databases with errors #2312

Merged

Conversation

elenatanasoiu commented Mar 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

codespaces-codeql repo

vscode-codeql-starter repo

Uh oh!

Uh oh!

elenatanasoiu commented Apr 3, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elenatanasoiu commented Apr 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shati-patel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shati-patel Apr 12, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elenatanasoiu commented Apr 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

elenatanasoiu commented Mar 30, 2023 •

edited

Loading

elenatanasoiu commented Apr 11, 2023 •

edited

Loading

elenatanasoiu commented Apr 12, 2023 •

edited

Loading