forcedotcom / salesforcedx-vscode

Salesforce Extensions for VS Code
https://developer.salesforce.com/tools/vscode
BSD 3-Clause "New" or "Revised" License
953 stars 406 forks source link

feat: do not prompt for output dir when called from context menu #5544

Closed aheber closed 5 months ago

aheber commented 5 months ago

What does this PR do?

Make the Apex Class and Apex Test Class context options take the output directory from context instead of prompting the user.

What issues does this PR fix or reference?

4294

Functionality Before

User is prompted for output directory VSCode-4294-CreateApex-Before

Functionality After

Output directory is taken from context VSCode-4294-CreateApex-After

aheber commented 5 months ago

This also changes the tests so they are correct in isolation as well as when run as a group. Had to leak implementation details a bit to make it all work. We can remove or change as needed.

There is risk here, I changed the order of the parameters on apexGenerateUnitTestClass to match the parameter order when invoked from a Context menu. As far as I can tell this exists for testing but if this is used elsewhere then we may need to undo that decision.

I also tested this from the Command Pallet at the behaviors are not modified, output directory is still prompted.

aheber commented 5 months ago

@AnanyaJha @randi274 I think this would replace #4901 if accepted.

aheber commented 5 months ago

If we're happy with this implementation I'm happy to look at the other context menu options like creating LWCs for a follow up PR.

git2gus[bot] commented 5 months ago

This issue has been linked to a new work item: W-15547200

aheber commented 5 months ago

@peternhale looks great! Appreciate the speedy review and changes.

peternhale commented 5 months ago

@aheber FYI, we will need our product manager to review the functional changes to make sure they fit in well with other similar functionality, so the PR may "sit" for a bit before we see movement.

aheber commented 5 months ago

No problem.

peternhale commented 5 months ago

@aheber The logic looks solid and I dug around a bit with different test scenarios, specifically projects with multiple "packageDirectories" in sfdx-project.json.

I made some additional changes and pushed them here

The branch also includes some changes to get rid of ParameterGatherer.

@aheber Product and our UX folks like this change so we can proceed. I see the suggestions I made have yet to be applied to your PR. Once those changes are made, I will move this PR forward. Thanks again for the contribution!

aheber commented 5 months ago

Great news!

My fault on the suggestions, I misunderstood. I'll take care of it now.

aheber commented 5 months ago

@peternhale did I brake it by cherry picking your [skip-ci] commit? I seem to remember the CI jobs normally starting but maybe that took intervention.

Hopefully that should do it but if I need to do something else please let me know.

peternhale commented 5 months ago

@aheber your contribution has been accepted via a PR I opened to allow tests to run. Thanks for the contribution!

I will be closing this PR.

aheber commented 5 months ago

Thank you!