Refactor "create template commands" to reduce code duplication #1246

allileong · 2019-04-08T23:16:59Z

What does this PR do?

Refactors our "create template commands" to reduce code duplication.
Moves the LWC create command from salesforcedx-vscode-lwc to saleforcedx-vscode-core

What issues does this PR fix or reference?

W-5988755

brpowell · 2019-04-08T23:38:55Z

packages/salesforcedx-vscode-core/src/commands/templates/baseTemplateCommand.ts

+    }).execute(cancellationToken);
+
+    execution.processExitSubject.subscribe(async data => {
+      this.logMetric(execution.command.logName, startTime);


It looks like the logging for the directory type (default or custom) is missing. I think it had to do with me making this branch before I added that in 😛 . Check out one of the command files in a more recent version of develop.

Good catch!

brpowell · 2019-04-08T23:41:30Z

@allileong The forceLightningLwcCreate command should be updated to use the baseTemplateCommand now as well. That one is sneaky because it's in the lwc package. Besides that and the logging comment I left it looks good to me!

codecov · 2019-04-08T23:41:58Z

Codecov Report

Merging #1246 into develop will increase coverage by 1.83%.
The diff coverage is 83.17%.

@@             Coverage Diff             @@
##           develop    #1246      +/-   ##
===========================================
+ Coverage    70.99%   72.83%   +1.83%     
===========================================
  Files          198      201       +3     
  Lines         7471     7332     -139     
  Branches       786      761      -25     
===========================================
+ Hits          5304     5340      +36     
+ Misses        2010     1834     -176     
- Partials       157      158       +1

Impacted Files	Coverage Δ
.../salesforcedx-vscode-core/src/commands/commands.ts	`72.8% <ø> (-3.76%)`	⬇️
...re/src/commands/templates/metadataTypeConstants.ts	`100% <100%> (ø)`
...orcedx-vscode-core/src/commands/templates/index.ts	`100% <100%> (ø)`
...core/src/commands/templates/baseTemplateCommand.ts	`64.7% <64.7%> (ø)`
.../src/commands/templates/forceLightningAppCreate.ts	`86.66% <86.66%> (ø)`
...ommands/templates/forceLightningInterfaceCreate.ts	`86.66% <86.66%> (ø)`
...e/src/commands/templates/forceApexTriggerCreate.ts	`86.66% <86.66%> (ø)`
.../src/commands/templates/forceLightningLwcCreate.ts	`86.66% <86.66%> (ø)`
...c/commands/templates/forceVisualforcePageCreate.ts	`86.66% <86.66%> (ø)`
...ore/src/commands/templates/forceApexClassCreate.ts	`86.66% <86.66%> (ø)`
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3102ef9...2d886b2. Read the comment docs.

allileong · 2019-04-09T23:50:19Z

packages/salesforcedx-vscode-core/src/commands/commands.ts

@@ -30,74 +30,6 @@ import { taskViewService } from '../statuses';
 import { telemetryService } from '../telemetry';
 import { getRootWorkspacePath, hasRootWorkspace } from '../util';

-export class LightningFilePathExistsChecker


Consolidated the 2 LightningFilePathExistsCheckers (one was here and the other was in the salesforcedx-vscode-lwc extension) and the FilePathExistsChecker from this file, and moved it to the baseTemplateCommand.ts file.

allileong · 2019-04-09T23:55:41Z

packages/salesforcedx-vscode-core/src/commands/templates/baseTemplateCommand.ts

+        return inputs;
+      } else {
+        const overwrite = await notificationService.showWarningMessage(
+          nls.localize('warning_prompt_file_overwrite', this.metadataLabel),


The new warning message includes the metadata type of the entity that the user is choosing to overwrite. The new string added to the end of the i18n.ts file with names ending in `*_message_name` represent the different metadata names that could replace 'Aura Bundle' in the screenshot above.

allileong · 2019-04-09T23:58:38Z

packages/salesforcedx-vscode-core/src/commands/templates/forceLightningAppCreate.ts

+      fileNameGatherer,
+      outputDirGatherer
+    ),
+    new ForceLightningAppCreateExecutor(),


With the exception of this line, each of the forceLightning*Create() methods are exactly the same. This may be a place to further reduce code duplication in the future.

allileong · 2019-04-10T00:01:02Z

packages/salesforcedx-vscode-core/test/vscode-integration/commands/commands.test.ts

@@ -308,217 +304,6 @@ describe('Command Utilities', () => {
    });
  });

-  describe('LightningFilePathExistsChecker', () => {


These tests were moved to the baseTemplateCommand.test.ts file.

allileong · 2019-04-10T00:03:56Z

...esforcedx-vscode-core/test/vscode-integration/commands/templates/baseTemplateCommand.test.ts

+  });
+});
+
+describe('FilePathExistsChecker for Aura bundle', () => {


The tests from this line to the end of the file were moved from commands.test.ts. With more time, I would have assessed whether we should be keeping these tests and whether or not more tests need to be added.

allileong · 2019-04-10T00:13:01Z

Hey @brpowell @lcampos, these code changes are ready for review. With more time, I would have liked to:

Spend more time seeing if the tests in the baseTemplateCommand.test.ts file that are validating the FilePathExistsChecker should be updated (test cases added or removed) since the checker itself has been modified. The current tests have been around since before this refactor
Manually test the create commands more

jonnyhork · 2019-04-15T20:51:24Z

@lcampos This PR would fix the issues related to W-6017178 in Theia. I was wondering if there is anything I can do to help this get merged? I know you guys are a bit short staffed. Thanks!

lcampos · 2019-04-16T14:53:48Z

@jonnyhork I've made some updates to the PR and functionality wise it's ready, I just want to add a couple more tests and will then get it merged. I'm thinking this will be merged later today or early tomorrow.

jonnyhork · 2019-04-16T15:04:58Z

Great, thanks!

brpowell · 2019-04-16T20:07:37Z

...rcedx-vscode-core/test/vscode-integration/commands/templates/forceLightningAppCreate.test.ts

+      nls.localize('force_lightning_app_create_text')
+    );
+    expect(lightningAppCreate.getDefaultDirectory()).to.equal('aura');
+    expect(lightningAppCreate.getFileExtension()).to.equal('.app');


Since we're checking defaultDirectory and fileExtension, we might as well check the sourcePathStrategy for the commands too. For the bundles we want to make sure they're using BundlePathStrategy, o/w DefaultPathStrategy.

brpowell

Looks good. Just had a comment about checking source path strategy as part of the tests.

lcampos · 2019-04-17T01:19:01Z

fyi @jonnyhork this is now in develop branch. I'll work on PR #1259 next.

salesforce-cla · 2021-04-07T11:50:17Z

Thanks for the contribution! Unfortunately we can't verify the commit author(s): Allison Leong <a***@s***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, refresh the status of this Pull Request.

salesforce-cla · 2021-05-05T21:54:11Z

Thanks for the contribution! Unfortunately we can't verify the commit author(s): Allison Leong <a***@s***.com> Luis Campos <l***@s***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, refresh the status of this Pull Request.

allileong force-pushed the aleong/templateRefactor branch from aa32b81 to 15d3799 Compare April 8, 2019 23:18

allileong requested a review from brpowell April 8, 2019 23:19

brpowell reviewed Apr 8, 2019

View reviewed changes

allileong force-pushed the aleong/templateRefactor branch from 283ff79 to bebb903 Compare April 9, 2019 23:06

allileong commented Apr 9, 2019

View reviewed changes

allileong commented Apr 10, 2019

View reviewed changes

lcampos mentioned this pull request Apr 12, 2019

Update deprecated workspace.rootPath method to getRootWorkspacePath #1260

Merged

Allison Leong added 17 commits April 14, 2019 16:06

Add baseTemplateCommand class

4c65bfb

Pull out source path creation logic

16a0447

Migrate forceApexClassCreate to use base class

1fc0733

Convert forceApexTriggerCreate

180c69c

Convert forceLightningAppCreate

e548fb0

Convert forceLightningComponentCreate

52ff5a3

Convert forceLightningEventCreate

fd2c86a

Convert forceLightningInterfaceCreate

b044209

Convert forceVisualforceComponentCreate

759f317

Convert forceVisualforcePageCreate

782481e

Creating a path strategy

5ec518d

Refactor strategy initialization

b2db146

Add unit tests

514342a

Add default dir tracking

11a9968

Add forceLightningLwcCreate command in core

1a0d6d9

Allison Leong added 3 commits April 14, 2019 16:10

Clean up message

71f64e5

Fix vscode-integration tests

8285e9a

Fix system test

7f95300

Fix root path, additional tests

b863d5d

lcampos force-pushed the aleong/templateRefactor branch from 94c0a14 to b863d5d Compare April 16, 2019 06:04

Add tests to remaining metadata commands

7accfa5

brpowell reviewed Apr 16, 2019

View reviewed changes

brpowell approved these changes Apr 16, 2019

View reviewed changes

Include sourcePathStrategy as part of template command tests

2d886b2

lcampos merged commit 2a7d578 into develop Apr 17, 2019

lcampos deleted the aleong/templateRefactor branch April 17, 2019 01:17

forcedotcom deleted a comment from jonnyhork Apr 17, 2019

salesforce-cla bot added the cla:missing label Apr 7, 2021

This was referenced Jan 22, 2022

[Snyk] Fix for 4 vulnerabilities Sarrac3873/salesforcedx-vscode#28

Open

[Snyk] Fix for 20 vulnerabilities Sarrac3873/salesforcedx-vscode#31

Open

This was referenced Nov 5, 2022

[Snyk] Fix for 1 vulnerabilities Sarrac3873/salesforcedx-vscode#147

Open

[Snyk] Fix for 1 vulnerabilities Sarrac3873/salesforcedx-vscode#148

Open

This was referenced Dec 26, 2022

[Snyk] Fix for 1 vulnerabilities Sarrac3873/salesforcedx-vscode#159

Open

[Snyk] Fix for 1 vulnerabilities Sarrac3873/salesforcedx-vscode#160

Open

Sarrac3873 mentioned this pull request Jun 22, 2023

[Snyk] Fix for 1 vulnerabilities Sarrac3873/salesforcedx-vscode#209

Open

This was referenced Nov 28, 2023

[Snyk] Fix for 9 vulnerabilities Sarrac3873/salesforcedx-vscode#248

Open

[Snyk] Fix for 5 vulnerabilities Sarrac3873/salesforcedx-vscode#249

Open

This was referenced May 13, 2024

[Snyk] Fix for 2 vulnerabilities Sarrac3873/salesforcedx-vscode#324

Open

[Snyk] Fix for 2 vulnerabilities Sarrac3873/salesforcedx-vscode#326

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor "create template commands" to reduce code duplication #1246

Refactor "create template commands" to reduce code duplication #1246

allileong commented Apr 8, 2019 •

edited

Loading

brpowell Apr 8, 2019

allileong Apr 9, 2019

brpowell commented Apr 8, 2019 •

edited

Loading

codecov bot commented Apr 8, 2019 •

edited

Loading

allileong Apr 9, 2019 •

edited

Loading

allileong Apr 9, 2019 •

edited

Loading

allileong Apr 9, 2019

allileong Apr 10, 2019

allileong Apr 10, 2019

allileong commented Apr 10, 2019

jonnyhork commented Apr 15, 2019

lcampos commented Apr 16, 2019

jonnyhork commented Apr 16, 2019

brpowell Apr 16, 2019

brpowell left a comment

lcampos commented Apr 17, 2019

salesforce-cla bot commented Apr 7, 2021

salesforce-cla bot commented May 5, 2021

Refactor "create template commands" to reduce code duplication #1246

Refactor "create template commands" to reduce code duplication #1246

Conversation

allileong commented Apr 8, 2019 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

brpowell Apr 8, 2019

Choose a reason for hiding this comment

allileong Apr 9, 2019

Choose a reason for hiding this comment

brpowell commented Apr 8, 2019 • edited Loading

codecov bot commented Apr 8, 2019 • edited Loading

Codecov Report

allileong Apr 9, 2019 • edited Loading

Choose a reason for hiding this comment

allileong Apr 9, 2019 • edited Loading

Choose a reason for hiding this comment

allileong Apr 9, 2019

Choose a reason for hiding this comment

allileong Apr 10, 2019

Choose a reason for hiding this comment

allileong Apr 10, 2019

Choose a reason for hiding this comment

allileong commented Apr 10, 2019

jonnyhork commented Apr 15, 2019

lcampos commented Apr 16, 2019

jonnyhork commented Apr 16, 2019

brpowell Apr 16, 2019

Choose a reason for hiding this comment

brpowell left a comment

Choose a reason for hiding this comment

lcampos commented Apr 17, 2019

salesforce-cla bot commented Apr 7, 2021

salesforce-cla bot commented May 5, 2021

allileong commented Apr 8, 2019 •

edited

Loading

brpowell commented Apr 8, 2019 •

edited

Loading

codecov bot commented Apr 8, 2019 •

edited

Loading

allileong Apr 9, 2019 •

edited

Loading

allileong Apr 9, 2019 •

edited

Loading