Wr/delete #199

WillieRuemmele · 2021-09-01T16:31:48Z

What does this PR do?

adds the source:delete command
adds unit tests
adds delete NUTs

What issues does this PR fix or reference?

@W-9137776@
@W-7552573@

shetzel · 2021-09-13T18:37:19Z

src/commands/force/source/delete.ts

+
+    // The DeleteResultFormatter will use SDR and scan the directory, if the files have been deleted, it will throw an error
+    // so we'll delete the files locally now
+    this.deleteFilesLocally();


This feels out of place in a results formatting function, and I don't think the comment applies anymore. What if we move this to either the resolveSuccess method or the run method just after calling resolveSuccess()?

src/commands/force/source/delete.ts

shetzel · 2021-09-13T18:46:06Z

src/commands/force/source/delete.ts

+    if (!(await this.handlePrompt())) {
+      // the user said 'no' to the prompt
+      this.exit(0);
+    }


What if we just used a private for this and resolveSuccess would check that too? Maybe: this.canceled = true; return;

src/commands/force/source/delete.ts

mshanemc · 2021-09-16T14:21:23Z

src/commands/force/source/delete.ts

+    if (!this.getFlag('noprompt')) {
+      const paths = this.sourceComponents.map((component) => component.content + '\n' + component.xml).join('\n');
+      const promptMessage = messages.getMessage('prompt', [paths]);
+      const answer = (await prompt(promptMessage)) as string;


I've always wondered why CLI commands use prompt instead of confirm (and then have to do the extra casing, etc).

source: https://github.com/oclif/cli-ux/blob/fd0ca55f2ae7096ea273592be35e071ee4ddcb0d/src/prompt.ts#L132

ex in use
https://github.com/forcedotcom/source-tracking/blob/e73b17609ae410be4043b423b8488968a7513afe/src/commands/force/source/tracking/reset.ts#L44

Oh, it's probably just a carry-over from TB, I'll swap it for confirm

mshanemc · 2021-09-17T14:55:33Z

QA Notes:
I'm using this repo: https://github.com/SFDC-Assets/flow-builder

after running its setup (orgInit.sh)

I'm getting some undefined in the warnings

➜  flow-builder git:(master) ✗ sfdx force:source:delete -c -m CustomTab
This operation will delete the following files on your computer and in your org: 
undefined
/Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/tabs/Assigned_Training__c.tab-meta.xml
undefined
/Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/tabs/Employee__c.tab-meta.xml
undefined
/Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/tabs/Equipment__c.tab-meta.xml

mshanemc · 2021-09-17T14:56:38Z

QA Notes: same undefined with a specifed file or using -p

➜  flow-builder git:(master) ✗ sfdx force:source:delete -c -m CustomTab:Employee__c
This operation will delete the following files on your computer and in your org: 
undefined
/Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/tabs/Employee__c.tab-meta.xml

PS: these only seem to happen when there aren't content files to go with the xml. Example of it working for an object

/Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c
/Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/Employee__c.object-meta.xml

mshanemc · 2021-09-17T15:03:35Z

QA notes:
-c yield the following stdout message...it's technically a deployment but that's a bit "weird" for a delete operation. Successfully validated the deployment. 1 components deployed and 0 tests run..

I kinda feel the same way about "Deployed Source" on the --verbose version

➜  flow-builder git:(master) ✗ sfdx force:source:delete -m CustomTab:Employee__c --verbose
This operation will delete the following files on your computer and in your org: 
undefined
/Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/tabs/Employee__c.tab-meta.xml

Are you sure you want to proceed (y/n)?: y
*** Deleting with SOAP API ***
Deploy ID: 0Af1D00001YUU1FSAX

=== Deployed Source
FULL NAME    TYPE       PROJECT PATH
───────────  ─────────  ────────────────────────────────────────────────────
Employee__c  CustomTab  force-app/main/default/tabs/Employee__c.tab-meta.xml

mshanemc · 2021-09-17T15:16:30Z

QA Notes:
good deletes with -p, -m (tried by types, my specific files, even -m CustomField:Object.Field).
it's also deleting local source (as expected)

mshanemc · 2021-09-17T15:18:25Z

QA Notes:
I deleted a file, then undeleted it on local FS, then ran delete on it again. I get this non-useful error (because other failures can be things that can't be deleted because of dependencies, and I'd really want to see that information to know what the dependency is!)

ERROR running force:source:delete:  Deploy failed.

when what I really want is in the --json

"componentFailures": {
        "changed": "false",
        "componentType": "CustomField",
        "created": "false",
        "createdDate": "2021-09-17T15:17:18.000Z",
        "deleted": "false",
        "fileName": "destructiveChanges.xml",
        "fullName": "destructiveChanges.xml",
        "problem": "No CustomField named: Employee__c.Onboarding_Buddy__c found",
        "problemType": "Warning",
        "success": "false"
      },

here's a more complex example where I try to delete an entire object

{
              "changed": "false",
              "componentType": "CustomField",
              "created": "false",
              "createdDate": "2021-09-17T15:21:24.000Z",
              "deleted": "false",
              "fileName": "objects/Employee__c.object",
              "fullName": "Employee__c.User_Record__c",
              "problem": "This custom field is referenced elsewhere in salesforce.com. : Flow Version - 3011D0000008EFE.",
              "problemType": "Error",
              "success": "false"
            },
            {
              "changed": "false",
              "componentType": "CustomField",
              "created": "false",
              "createdDate": "2021-09-17T15:21:24.000Z",
              "deleted": "false",
              "fileName": "objects/Employee__c.object",
              "fullName": "Employee__c.First_Name__c",
              "problem": "This custom field is referenced elsewhere in salesforce.com. : Flow Version - 3011D0000008EFE. This custom field is referenced elsewhere in salesforce.com. : Apex Class - ResetOnboarding.",
              "problemType": "Error",
              "success": "false"
            },
            {
              "changed": "false",
              "componentType": "CustomField",
              "created": "false",
              "createdDate": "2021-09-17T15:21:24.000Z",
              "deleted": "false",
              "fileName": "objects/Employee__c.object",
              "fullName": "Employee__c.Status__c",
              "problem": "This custom field is referenced elsewhere in salesforce.com. : Flow Version - 3011D0000008EFE. This custom field is referenced elsewhere in salesforce.com. : Apex Class - ResetOnboarding. Referenced by the Lightning page Employee Record Page : Lightning Page.",
              "problemType": "Error",
              "success": "false"
            },
            {
              "changed": "false",
              "componentType": "CustomField",
              "created": "false",
              "createdDate": "2021-09-17T15:21:24.000Z",
              "deleted": "false",
              "fileName": "objects/Employee__c.object",
              "fullName": "Employee__c.Last_Name__c",
              "problem": "This custom field is referenced elsewhere in salesforce.com. : Flow Version - 3011D0000008EFE. This custom field is referenced elsewhere in salesforce.com. : Apex Class - ResetOnboarding.",
              "problemType": "Error",
              "success": "false"
            },
            {
              "changed": "false",
              "componentType": "CustomField",
              "created": "false",
              "createdDate": "2021-09-17T15:21:24.000Z",
              "deleted": "false",
              "fileName": "objects/Employee__c.object",
              "fullName": "Employee__c.Phone__c",
              "problem": "This custom field is referenced elsewhere in salesforce.com. : Flow Version - 3011D0000008EFE.",
              "problemType": "Error",
              "success": "false"
            },

It looks like the componentFailures from SDR can be an array or a single object--and it's the non-Array that we're not handling properly. you do get an errors table if there are multiple failures.

mshanemc · 2021-09-17T15:27:31Z

QA Notes:
for decomposed source, it's presenting a somewhat misleading list of what's going to be deleted (relying on the user to know that the fields all roll up onto the object). I think I would have expected a list of fields?

➜  flow-builder git:(master) ✗ sfdx force:source:delete -m CustomField:Employee__c.Onboarding_Buddy__c,CustomObject:Employee__c       
This operation will delete the following files on your computer and in your org: 
/Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c
/Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/Employee__c.object-meta.xml

toolbelt gives the full list of deletions for the same command

➜  flow-builder git:(master) ✗ sfdx force:source:delete -m CustomField:Employee__c.Onboarding_Buddy__c,CustomObject:Employee__c
This operation will delete the following files on your computer and in your org: 
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/Employee__c.object-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/fields/City__c.field-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/fields/Department__c.field-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/fields/Email__c.field-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/fields/First_Name__c.field-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/fields/Last_Name__c.field-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/fields/Onboarding_Buddy__c.field-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/fields/Phone__c.field-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/fields/PostalCode__c.field-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/fields/Role__c.field-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/fields/Send_Notification__c.field-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/fields/Start_Date__c.field-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/fields/State__c.field-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/fields/Status__c.field-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/fields/Street_Address__c.field-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/fields/User_Record__c.field-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/compactLayouts/Default.compactLayout-meta.xml
Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/listViews/All.listView-meta.xml

this happens on LWC, too (it'll only show the bundle and its xml) which is different from toolbelt

/Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/lwc/lightningWebComponentExample
/Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/lwc/lightningWebComponentExample/lightningWebComponentExample.js-meta.xml

mshanemc · 2021-09-17T15:56:47Z

QA Notes:
When adding a non-existent type/item and a pair that does exist, we get a warning that only refers to the existing one (but doesn't inform the user that the non-existent one is a mistake)

➜  flow-builder git:(master) ✗ sfdx force:source:delete -m CustomTab:Employee__x,CustomObject:Employee__c  
This operation will delete the following files on your computer and in your org: 
/Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c
/Users/shane.mclaughlin/eng/repros/flow-builder/force-app/main/default/objects/Employee__c/Employee__c.object-meta.xml

after confirming, it doesn't look like any attempt was made to delete the non-existent one (which seems fair). I think this should have thrown an error before trying to delete.

[only applies to -m because -p checks for dir/file existing.]

Also, toolbelt has the same problem with -m [one good, one not there]

WillieRuemmele · 2021-09-21T22:16:33Z

forcedotcom/cli#971

WillieRuemmele added 5 commits September 1, 2021 08:39

feat: add source:delete command

2eb3f76

chore: handle xml or content only metadata

d4004db

chore: fix bundle types

ced2483

fix: add REST to delete, add NUTs

fe37406

docs: add long descriptions where appropriate

fe5eee2

WillieRuemmele requested a review from mshanemc September 8, 2021 15:09

shetzel requested changes Sep 13, 2021

View reviewed changes

chore: 3/4 of Steve's comments addressed

82e3745

mshanemc reviewed Sep 16, 2021

View reviewed changes

chore: swap prompt to confirm

312e6b9

shetzel approved these changes Sep 16, 2021

View reviewed changes

WillieRuemmele added 3 commits September 16, 2021 14:54

chore: attempt to create TestKit differently

fca05e4

chore: attempt to create TestKit differently

717db9a

chore: fix delete with tests on windows NUT

de9bd53

chore: backout server error messages

a20f8b6

WillieRuemmele force-pushed the wr/delete branch from 0a15295 to a20f8b6 Compare September 17, 2021 22:29

refactor: use sdr walkContent

d0bf60a

mshanemc approved these changes Sep 21, 2021

View reviewed changes

mshanemc merged commit 4c443c8 into main Sep 21, 2021

mshanemc deleted the wr/delete branch September 21, 2021 21:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wr/delete #199

Wr/delete #199

WillieRuemmele commented Sep 1, 2021 •

edited by mshanemc

Loading

shetzel Sep 13, 2021

shetzel Sep 13, 2021

mshanemc Sep 16, 2021

WillieRuemmele Sep 16, 2021

mshanemc commented Sep 17, 2021

mshanemc commented Sep 17, 2021 •

edited

Loading

mshanemc commented Sep 17, 2021 •

edited

Loading

mshanemc commented Sep 17, 2021

mshanemc commented Sep 17, 2021 •

edited

Loading

mshanemc commented Sep 17, 2021 •

edited

Loading

mshanemc commented Sep 17, 2021 •

edited

Loading

WillieRuemmele commented Sep 21, 2021

Wr/delete #199

Wr/delete #199

Conversation

WillieRuemmele commented Sep 1, 2021 • edited by mshanemc Loading

What does this PR do?

What issues does this PR fix or reference?

shetzel Sep 13, 2021

Choose a reason for hiding this comment

shetzel Sep 13, 2021

Choose a reason for hiding this comment

mshanemc Sep 16, 2021

Choose a reason for hiding this comment

WillieRuemmele Sep 16, 2021

Choose a reason for hiding this comment

mshanemc commented Sep 17, 2021

mshanemc commented Sep 17, 2021 • edited Loading

mshanemc commented Sep 17, 2021 • edited Loading

mshanemc commented Sep 17, 2021

mshanemc commented Sep 17, 2021 • edited Loading

mshanemc commented Sep 17, 2021 • edited Loading

mshanemc commented Sep 17, 2021 • edited Loading

WillieRuemmele commented Sep 21, 2021

WillieRuemmele commented Sep 1, 2021 •

edited by mshanemc

Loading

mshanemc commented Sep 17, 2021 •

edited

Loading

mshanemc commented Sep 17, 2021 •

edited

Loading

mshanemc commented Sep 17, 2021 •

edited

Loading

mshanemc commented Sep 17, 2021 •

edited

Loading

mshanemc commented Sep 17, 2021 •

edited

Loading