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

feat!: ga new tree commands #975

Merged
merged 10 commits into from
Jul 1, 2024
Merged

feat!: ga new tree commands #975

merged 10 commits into from
Jul 1, 2024

Conversation

mshanemc
Copy link
Contributor

@mshanemc mshanemc commented Jun 27, 2024

What does this PR do?

GA the new tree commands
See the plan at https://github.com/salesforcecli/plugin-data/blob/main/NewTreeCommandsPlan.md

  • update the plan
  • do the steps on the plan

unrelated

  • the data resume command was incorrectly marked deprecated, pointing people to use the old force: command. 🤷🏻‍♂️

why aren't there UT for the new commands?

  • they're simple (the functions they call have the UT)
  • NUTs are better for org import/export with an org, fs, and os issues

What issues does this PR fix or reference?

forcedotcom/cli#2738
@W-16113657@

@mshanemc mshanemc requested a review from a team as a code owner June 27, 2024 16:03
@mshanemc mshanemc changed the title Sm/ga-data-commands feat!: ga new tree commands Jun 27, 2024
Copy link

git2gus bot commented Jun 27, 2024

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

@@ -30,7 +30,7 @@ Directory in which to generate the JSON files; default is current directory.

- Export records retrieved with the specified SOQL query into a single JSON file in the current directory; the command uses your default org:

<%= config.bin %> <%= command.id %> --query "SELECT Id, Name, (SELECT Name, Address__c FROM Properties__r) FROM Broker__c"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bump dev-scripts to avoid linting the .md files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aww, it's on current dev-scripts. I don't know why that's still happening. Maybe something in vscode ext?

};

public async run(): Promise<ImportResult[] | JsonMap> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we changing so much of these commands for the GA? shouldn't this PR just be changing command files/states/warnings?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the command that was src/commands/data/import/tree.ts moved to src/commands/data/import/legacy/tree.ts and the command that was src/commands/data/import/beta/tree.ts moved here.

git doesn't know we swapped the files so it tries to diff this file

@jshackell-sfdc
Copy link
Contributor

FYI, I created a separate PR with my messages updates. I thought I was gonna have more comments, which is why I separated it out, but it turned out to be rather small: #976

@@ -46,15 +47,19 @@ export default class Export extends SfCommand<DataPlanPart[] | SObjectTreeFileCo
};

public async run(): Promise<DataPlanPart[] | SObjectTreeFileContents> {
this.info(
'Try the the new "sf data export beta tree" command. It support SOQL queries with up to 5 levels of objects!'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if they run the legacy command, you'll get a suggestion for a beta command, it should just b e"data export tree" right

 ➜  ../../oss/plugin-data/bin/run.js data export legacy tree --query "SELECT ID FROM Account"
Try the the new "sf data export beta tree" command.  It support SOQL queries with up to 5 levels of objects!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even then,

 ➜  ../../oss/plugin-data/bin/run.js data export beta tree --query "SELECT ID FROM Account"   
 ›   Error: command data:export:beta:tree not found

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just going to remove that. at this point, you're intentionally choosing this old command to avoid the new one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, you found that I had the "beta" alias wrong on the new commands.

@mshanemc mshanemc requested a review from WillieRuemmele June 28, 2024 14:47
Copy link
Contributor

@WillieRuemmele WillieRuemmele left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the beta deprecation is accurate, but there's no information on the legacy commands

 ➜  ../../oss/plugin-data/bin/run.js data export beta tree --query "SELECT ID FROM Account"
Warning: The "data export beta tree" command has been deprecated. Use "data export tree" instead.
wrote 1 records to Account.json
➜  dreamhouse-lwc git:(main) ✗  hub:(GLOBAL - DevHub) scratch:(test-enhclzordbdi@example.com)
 ➜  ../../oss/plugin-data/bin/run.js data export tree --query "SELECT ID FROM Account" 
wrote 1 records to Account.json
➜  dreamhouse-lwc git:(main) ✗  hub:(GLOBAL - DevHub) scratch:(test-enhclzordbdi@example.com)
 ➜  ../../oss/plugin-data/bin/run.js data export legacy tree --query "SELECT ID FROM Account"       
Wrote 1 records to Account.json

@mshanemc
Copy link
Contributor Author

mshanemc commented Jul 1, 2024

the beta deprecation is accurate, but there's no information on the legacy commands

 ➜  ../../oss/plugin-data/bin/run.js data export beta tree --query "SELECT ID FROM Account"
Warning: The "data export beta tree" command has been deprecated. Use "data export tree" instead.
wrote 1 records to Account.json
➜  dreamhouse-lwc git:(main) ✗  hub:(GLOBAL - DevHub) scratch:(test-enhclzordbdi@example.com)
 ➜  ../../oss/plugin-data/bin/run.js data export tree --query "SELECT ID FROM Account" 
wrote 1 records to Account.json
➜  dreamhouse-lwc git:(main) ✗  hub:(GLOBAL - DevHub) scratch:(test-enhclzordbdi@example.com)
 ➜  ../../oss/plugin-data/bin/run.js data export legacy tree --query "SELECT ID FROM Account"       
Wrote 1 records to Account.json

good catch...looks like we need both state = deprecated AND deprecationOptions for the warning to show.

@mshanemc mshanemc requested a review from WillieRuemmele July 1, 2024 20:43
@WillieRuemmele
Copy link
Contributor

QA Notes


✅ : warning on legacy
✅ : no message on 'data export tree'
✅ : deprecation on 'beta' command
✅ : --json


💡 : could improve, ok since it matches current behavior - I didn't have required information in the Account.json file

 ➜  ../../oss/plugin-data/bin/run.js data import tree --files Account.json -o test-ddwy6vblev4p@example.com
Error (ERROR_HTTP_400): {"hasErrors":true,"results":[{"referenceId":"AccountRef1","errors":[{"statusCode":"REQUIRED_FIELD_MISSING","message":"Required fields are missing: [Name]","fields":["Name"]}]}]}

✅ : no note to try beta on standard command
✅ : beta is deprecated
✅ : legacy mentions deprecation day
✅ : --json consistent (for my use case)

@WillieRuemmele WillieRuemmele merged commit cb49efc into main Jul 1, 2024
10 checks passed
@WillieRuemmele WillieRuemmele deleted the sm/ga-data-commands branch July 1, 2024 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants