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

fix: include export mode when retrieving import statements VSCODE-440 #677

Merged
merged 4 commits into from
Jan 30, 2024

Conversation

paula-stacho
Copy link
Contributor

Description

The getImports transpiler method expects two arguments

  • mode: 'Pipeline' | 'Query' | 'Delete Query' | 'Update Query'
  • driverSyntax: boolean

source: https://github.com/mongodb-js/compass/blob/92f267e369d188c5f11c2450f3d65f73725558e4/packages/bson-transpilers/codegeneration/CodeGenerationVisitor.js#L94

In VSCode we have only two modes:

  • when an array is selected -> aggregate ~ Pipeline
  • when an object is selected -> find ~ Query

The main fix is sending two arguments (which allows for the second to be read correctly).

As far as I can tell, the exact mode only matters for Java. Given that the method applied is a wild guess, user might need to adjust the import together with the code anyway, but at least the imports now match the code.

Steps to reproduce:

  • select an object/array in the playground
  • export to Java
  • include Driver Syntax and Import Statements

Current behaviour:

The import statements are only

import org.bson.Document;

After the fix:

import org.bson.Document;
import com.mongodb.MongoClient;
import com.mongodb.MongoClientURI;
import com.mongodb.client.MongoCollection;
import com.mongodb.client.MongoDatabase;
import org.bson.conversions.Bson;
import java.util.concurrent.TimeUnit;
import org.bson.Document;

If you've selected an array, you'll also see -

import com.mongodb.client.AggregateIterable;

And for an object -

import com.mongodb.client.FindIterable;

Checklist

  • New tests and/or benchmarks are included
  • Documentation is changed or added

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Open Questions

I feel weird about the test placement, but this was the best I found.
I made an attempt to at least separate the test into three smaller pieces. I thought it would work since mocha runs the tests sequentially, but it didn't work and I gave up 😅 . I haven't worked with mocha for a long time, so maybe I'm missing something

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

@alenakhineika
Copy link
Contributor

Can we add one more test? :) The include driver syntax and then add import statements test, which would do all the steps that the include driver syntax (only) does. And then call once again await vscode.commands.executeCommand(actionCommand.command); to emulate the case when a user runs the same playground to export to language and sees the state of the code lenses from the previous run, like Include Import Statements | Exclude Driver Syntax | Exclude Driver Syntax. And then we call:

await vscode.commands.executeCommand(
  'mdb.changeExportToLanguageAddons',
  {
    ...mdbTestExtension.testExtensionController._playgroundController._exportToLanguageCodeLensProvider._exportToLanguageAddons,
     importStatements: true // We set a single addon here
  }
);

And check that the playground pane has both:

expect(content).to.include(mongoClientImport);
expect(content).to.include(queryImport);

@paula-stacho
Copy link
Contributor Author

Can we add one more test? :) The include driver syntax and then add import statements test, which would do all the steps that the include driver syntax (only) does. And then call once again await vscode.commands.executeCommand(actionCommand.command); to emulate the case when a user runs the same playground to export to language and sees the state of the code lenses from the previous run,

Ah, thanks! I knew we said five tests but I couldn't remember what the last one was

@paula-stacho paula-stacho removed the WIP label Jan 29, 2024
@paula-stacho paula-stacho requested a review from Anemy January 30, 2024 09:28
@paula-stacho paula-stacho merged commit 35d0eba into main Jan 30, 2024
5 checks passed
@paula-stacho paula-stacho deleted the VSCODE-440-imports branch January 30, 2024 18:28
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.

3 participants