W-16448626 feat: sosl by WillieRuemmele · Pull Request #1025 · salesforcecli/plugin-data

WillieRuemmele · 2024-08-08T18:06:15Z

What does this PR do?

adds data search command to preform SOSL queries

What issues does this PR fix or reference?

@W-16448626@

* fix: write the help for new command * fix: add info about --result-format behavior

mshanemc · 2024-08-12T15:58:49Z

src/commands/data/search.ts

+      if (flags['result-format'] !== 'json') this.spinner.start(messages.getMessage('queryRunningMessage'));
+      const queryResult = await conn.search(queryString);
+      if (!this.jsonEnabled()) {
+        // once we overrode result-format summary, undefined became a flag option, but the default is human


I'll make a note to ask Mike about this when he gets back

mshanemc

most are comment/questions, some of which might need changes depending on the answer.

definitely fix

that empty string thing
ut using plugins-core's stubs

mshanemc · 2024-08-12T16:00:35Z

src/commands/data/search.ts

+      // --file will be present if flags.query isn't. Oclif exactlyOne isn't quite that clever
+      const queryString = flags.query ?? fs.readFileSync(flags.file as string, 'utf8');
+      const conn = flags['target-org'].getConnection(flags['api-version']);
+      if (flags['result-format'] !== 'json') this.spinner.start(messages.getMessage('queryRunningMessage'));


is it the result format or the json-enabled-ness that matters for spinners?

ex: Imagine I want --result-format csv --json because I'm a CI script. I want to get that 0/1 and file location as json output BUT I want a csv file written. I wouldn't want a spinner.

this is carried from data query, but this.spinner should be respecting the --json flag

mshanemc · 2024-08-12T16:12:25Z

src/reporters/search/reporter.ts

+import { SearchResult } from '@jsforce/jsforce-node';
+
+export abstract class SearchReporter {
+  public types: string[];


this iterates the records to collect unique types, then each reporter iterates the records again to filter one type at a time.

Why not have a Map of <type, Record[]>. Then you already have them organized (and once we can use Map.groupBy it'll be even cleaner!)

mshanemc · 2024-08-12T16:13:06Z

src/reporters/search/reporter.ts

+  public types: string[];
+  public ux = new Ux();
+  protected constructor(public result: SearchResult) {
+    this.types = [...new Set(this.result.searchRecords.map((row) => row.attributes?.type ?? ''))];


what's the important of the empty string? When does it come back empty? Leave a comment for the next person who reads this and wonders.

mshanemc · 2024-08-12T16:17:22Z

src/reporters/search/csvSearchReporter.ts

+    this.types.map((type) => {
+      const filtered = this.result.searchRecords.filter((t) => t.attributes?.type === type);
+      // remove 'attributes' property from result and csv output
+      filtered.map((r) => delete r.attributes);


both the filter and the removal of attributes seem common to both classes. Probably a shared fn?

Or, if you do move to the Map idea, then the value could be filtered to remove attributes via kit/omit .map((r) => omit(r, 'attributes'));

mshanemc · 2024-08-12T16:21:23Z

src/searchUtils.ts

+import { JsonSearchReporter } from './reporters/search/reporter.js';
+import { CsvSearchReporter } from './reporters/search/csvSearchReporter.js';
+
+export const displaySearchResults = (queryResult: SearchResult, resultFormat: 'human' | 'json' | 'csv'): void => {


if you wanted to do this without classes, this would be a function that decides which reporter function(ux, results) to call.

then you wouldn't need all the new and display() etc both here and in the tests

mshanemc · 2024-08-12T16:24:18Z

test/reporters/csvSearchReporter.test.ts

+  });
+
+  it('will write two csv files', () => {
+    const logStub = $$.SANDBOX.stub(Ux.prototype, 'log');


you should use the ux tubs from sf-plugins-core.

mshanemc · 2024-08-12T16:27:54Z

test/reporters/csvSearchReporter.test.ts

+
+  it('will write two csv files', () => {
+    const logStub = $$.SANDBOX.stub(Ux.prototype, 'log');
+    const writeFileStub = $$.SANDBOX.stub(fs, 'writeFileSync');


idea: rather than the fs stubbing, I think this would be better to write to a temp dir (and then use mocha-snap to snapshot and commit them).

Then we'd detect accidental changes (ex: to.include would pass if you accidentally included the attributes stuff)

mshanemc · 2024-08-12T16:29:12Z

test/reporters/humanSearchReporter.test.ts

+  });
+
+  it('will write two tables', () => {
+    const tableStub = $$.SANDBOX.stub(Ux.prototype, 'table');


the sf-plugins-core ux stub has table on it already

mshanemc · 2024-08-12T16:32:05Z

src/reporters/search/csvSearchReporter.ts

+      const cols = Object.keys(filtered[0]).join(',');
+      const body = filtered.map((r) => Object.values(r).join(',')).join(os.EOL);
+
+      this.ux.log(`Written to ${type}.csv`);


wouldn't the empty string be really bad here?

mshanemc · 2024-08-12T21:09:15Z

messages/data.search.md

+
+# flags.file.summary
+
+File that contains the SOQL query.


Suggested change

File that contains the SOQL query.

File that contains the SOSL query.

mshanemc · 2024-08-12T21:17:05Z

messages/data.search.md

@@ -0,0 +1,31 @@
+# summary
+
+Execute a SOSL query.


probably for Juliet...I think we should link to the SOSL syntax docs

…into wr/sosl

mshanemc · 2024-08-13T13:39:33Z

QA notes:

./bin/run.js data search -o platformers -q "find 'Shane'"
✅ shows lots of tables with IDs in them.
✅ --json looks good

🎨 I don't think the word Results on each table is adding much.

./bin/run.js data search -o platformers -q "find {Shane}" --json --result-format csv
👎🏻 I guess --json overrides the --result-format? I wonder if we could validate that better to make it clearer that the command isn't gonna do what they might think it does?

data search -o platformers -q "find {Shane}" --result-format csv
✅ got lots of CSVs
🎨 I think I might prefer these in a folder? I could go either way on that

"find {Shane} returning RecordType"
✅ shows the server error nicely

"find {Shane} returning ApexClass(name, Status), User(name, email)"
✅ tables look good with fields in them
using --result-format csv produces good csv (I also added ID tot the returning fields on each object)

✅ query with file throws good error
✅ query from a file works fine

* refactor: use oclif custom flag * refactor: exclusive json * style: remove the word "results" from table heads * test: no "Results" in test assertions

WillieRuemmele · 2024-08-13T14:48:33Z

✅ : no --json and -r json =>

 ➜  ../../oss/plugin-data/bin/run.js data search --file query.txt  -r json --json
{
  "name": "Error",
  "message": "The following error occurred:\n  \u001b[2m--json=true cannot also be provided when using --result-format\u001b[22m\nSee more help with --help",
  "exitCode": 2,
  "context": "DataSearchCommand",
  "stack": "Error: The following error occurred:\n  \u001b[2m--json=true cannot also be provided when using --result-formats```

WillieRuemmele added 6 commits August 8, 2024 10:12

feat: add data search command

faf71a3

chore: multiple object result

f78946a

chore: consolidate reporter classes

d091736

test: add NUTs

13b8194

chore: gen snapshot

c0f1314

refactor: move formatters to dirs, add UTs

f6223f4

WillieRuemmele marked this pull request as ready for review August 8, 2024 21:52

WillieRuemmele requested a review from a team as a code owner August 8, 2024 21:52

WillieRuemmele and others added 2 commits August 9, 2024 09:44

docs: provide way to override result-format summary

292192f

write help for new "data search" command (#1026)

100d4a9

* fix: write the help for new command * fix: add info about --result-format behavior

mshanemc changed the title ~~Wr/sosl~~ feat: sosl Aug 12, 2024

mshanemc reviewed Aug 12, 2024

View reviewed changes

mshanemc requested changes Aug 12, 2024

View reviewed changes

chore: review reqs

dc2e427

WillieRuemmele force-pushed the wr/sosl branch from 100d4a9 to dc2e427 Compare August 12, 2024 19:47

mshanemc approved these changes Aug 12, 2024

View reviewed changes

mshanemc reviewed Aug 12, 2024

View reviewed changes

jshackell-sfdc added 2 commits August 12, 2024 15:20

fix: write the help for new command

f5bcf4f

fix: add info about --result-format behavior

e1e714c

mshanemc approved these changes Aug 12, 2024

View reviewed changes

Merge branch 'wr/sosl' of https://github.com/salesforcecli/plugin-data …

1744039

…into wr/sosl

refactor: type-fix-for-result-format (#1033)

b64a10e

* refactor: use oclif custom flag * refactor: exclusive json * style: remove the word "results" from table heads * test: no "Results" in test assertions

WillieRuemmele added 2 commits August 13, 2024 09:34

test: fix NUT assertion

fd24a49

chore: fix NUT

0f4649f

WillieRuemmele merged commit 833fc4e into main Aug 13, 2024

WillieRuemmele deleted the wr/sosl branch August 13, 2024 17:15

iowillhoit changed the title ~~feat: sosl~~ W-16448626 feat: sosl Jan 27, 2025

	File that contains the SOQL query.
	File that contains the SOSL query.

Conversation

WillieRuemmele commented Aug 8, 2024

What does this PR do?

What issues does this PR fix or reference?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mshanemc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mshanemc commented Aug 13, 2024

Uh oh!

WillieRuemmele commented Aug 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants