feat: add verbose flag to bulk delete and upsert

[R0Wi]

What does this PR do?

This adds a --verbose flag to the sf data delete bulk and sf data upsert bulk commands. If set, failed records will be printed as table:

What issues does this PR fix or reference?

forcedotcom/cli#2221

Discussion

• How to write tests for this? I tried to mock the node-fetch library which is used by BulkApiV2.request but this is quite hard to accomplish. Maybe someone has some other ideas?
• Currently we skip the table printing if --json is set. This is because otherwise the printed JSON would be malformed.
• How to deal with the returned Id column? The only thing we know for sure is that SF will give us the sf__Id column. But if an error occures (for example when trying to delete records) this column won't be filled and it might make more sense to have the actual source Id column (or external id in case of upsert) printed here. This might end in a different implementation for upsert and delete.

[WillieRuemmele]

Hi @R0Wi - thanks for the contribution - this looks really good 🏆

For the testing question, I would suggest writing a few NUTs (not unit tests) for this, checkout the test/commands/data/dataBulk.nut.ts tests, you'll maybe want to add something to the test-files directory as well to create a reproducible test, it would be great to have some sample data that fails (if that's possible) so we can verify the table data is correct, and verify that the --json flag returns correctly formed output.

Running the NUTs locally might be slightly inconvent for you, it requires a dev hub that can create a scratch org... so if you want, if you provide the data, commands, and expected behavior, I could write the tests too

For the id question... would it make sense to include both of the IDs? maybe one as sf__Id and one as Id for when we error? It would be better to keep them separated to create a consistent behavior and so you don't get different Ids in the same field based on what happened during the command execution

[R0Wi]

so if you want, if you provide the data, commands, and expected behavior, I could write the tests too

That sounds great! For my personal tests I used the following setup:

[1] Testcase "should print table because --verbose flag is set and there are errors"

• Create a new file delete.csv:

Id
<Some non-existing Id>

• Run sf data bulk delete --file delete.csv --sobject Account --verbose
• The SF API services/data/v58.0/jobs/ingest/<JobId>/failedResults/ will return a CSV like this:

"sf__Id","sf__Error",Id
"","MALFORMED_ID:malformed id <ID>:--","<ID>"

[2] Testcase "should not print table because --verbose flag is set but there are no errors"

• Insert a new Account in Salesforce and keep track of the Id
• Create a new file delete.csv:

Id
<Tracked Id of Account to be deleted>

• Run sf data bulk delete --file delete.csv --sobject Account --verbose

Other Testcases can be setup by combining the above approach with --verbose and --json flags set/unset

Please let me know if you need further information.

---

would it make sense to include both of the IDs?

Yes, makes sense. Depending on the concrete operation the sf__Id might not be filled (for example if a bulk delete for a record failed). On the other hand we might need to check if the result contains an Id column or not, think that depends on the data you uploaded to SF (see above example). So we can add an Id column and fill values if there are any?

[WillieRuemmele]

Awesome @R0Wi - I've added those tests, that in theory, following your test scenarios, should pass... they might need to be tweaked based on some changes you make... I did notice that the table is being printed in --json scenarios, try

 ➜  ../../oss/rowi-data/bin/dev data:delete:bulk --file data.csv --sobject Account --verbose --wait 10 --json

Bulk Failures [1]
====================================================
| Id Error                                           
| ── ─────────────────────────────────────────────── 
|    MALFORMED_ID:malformed id 123e1232133nugGQAQ:-- 
{
  "status": 1,
  "result": {
    "jobInfo": {

so it seems like ux isn't json aware unless explicitly told by the flag

[R0Wi]

Great, thank you! Yes I already noticed that. Will commit a fix tomorrow :-)

[R0Wi]

Hi @WillieRuemmele, I adjusted the code a bit and "manually" tested the testcases. They are now working for me. Think that's it from my end at the moment, any feedback appreciated 🚀

PS: one thing came to my mind regarding tests: currently we do not have any tests for the upsert action. Of couse it would be nice to also check if the behaviour is the same there but I could imagine that provoking an upsert error could be a bit tricky. Maybe you have some ideas?

[WillieRuemmele]

all tests passed and validated, thanks for the contribution, this will be in the nightly build tonight, latest-rc next wednesday, and latest the wednesday after that.

Thanks for your contribution @R0Wi 🏆 ❤️