Skip to content

fix: human output no longer displaying undefined #307

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.

Sign up for GitHub

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

Merged
WillieRuemmele merged 12 commits into from
Jun 3, 2022
Merged

fix: human output no longer displaying undefined #307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
4e8c94f
fix: human output no longer displaying undefined
peternhale May 31, 2022
f0de9f7
chore: never return undefined
peternhale May 31, 2022
af35ade
Merge branch 'main' into phale/W-11216045
WillieRuemmele Jun 2, 2022
a1fa148
chore: modify existing test to verify nested contacts
peternhale Jun 2, 2022
b7fbcb9
chore: use os.EOL in lieu of \n
peternhale Jun 2, 2022
1549b6f
chore: try different split pattern
peternhale Jun 2, 2022
721824a
chore: debug
peternhale Jun 2, 2022
810e2d7
chore: more debug
peternhale Jun 2, 2022
18af6b9
chore: fix test
peternhale Jun 2, 2022
f5bd404
chore: really fix the test
peternhale Jun 2, 2022
c2dbc57
chore: wip
peternhale Jun 3, 2022
812ca74
Merge branch 'main' into phale/W-11216045
WillieRuemmele Jun 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 26 additions & 8 deletions src/reporters.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,15 @@ export class HumanReporter extends QueryReporter {
(field: string) =>
(formattedColumns[field] = {
header: field.toUpperCase(),
get: (row): string => get(row, field) as string,
get: (row): string => {
Copy link
Copy Markdown
Contributor Author

@peternhale peternhale May 31, 2022

Choose a reason for hiding this comment

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

Modified the get to first check if the field, as given, can be referenced directly, This solves case where property has a name like foo.bar. If cannot be directly referenced then delegate to get

// first test if key exists, if so, return value
if (Reflect.has(row, field)) {
return (Reflect.get(row, field) as string) || '';
} else {
// if not, try to find it query
return (get(row, field) as string) || '';
}
},
})
);
return formattedColumns;
Expand All @@ -136,7 +144,7 @@ export class HumanReporter extends QueryReporter {
// some fields will return a JSON object that isn't accessible via the query (SELECT Metadata FROM RemoteProxy)
// some will return a JSON that IS accessible via the query (SELECT owner.Profile.Name FROM Lead)
// querying (SELECT Metadata.isActive FROM RemoteProxy) throws a SOQL validation error, so we have to display the entire Metadata object
queryResults.map((qr) => {
queryResults.forEach((qr) => {
const result = qr as Record<string, unknown>;
this.data.columns.forEach((col) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
Expand Down Expand Up @@ -167,6 +175,7 @@ export class HumanReporter extends QueryReporter {
}
}

const subResults: Array<Record<string, unknown>> = [];
if (children.length > 0) {
const childrenRows: Record<string, unknown> = {};
children.forEach((child) => {
Expand All @@ -180,19 +189,28 @@ export class HumanReporter extends QueryReporter {
if (childO) {
const childRecords = getArray(childO, 'records', []);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
childRecords.forEach((record: unknown) => {
childRecords.forEach((record: unknown, index) => {
Copy link
Copy Markdown
Contributor Author

@peternhale peternhale May 31, 2022

Choose a reason for hiding this comment

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

There can be more than one child record from a subquery, Account record can have more than one contact associated to it. For subquery records 2-n a synthetic record needs to be added to the results to produce output like

force:data:soql:query -u myalias -q Select name, (Select FirstName, LastName from Contacts) from Account
 NAME                            CONTACTS.FIRSTNAME CONTACTS.LASTNAME 
 ─────────────────────────────── ────────────────── ───────────────── 
 Test                                                                 
 Sample Account for Entitlements Peter              Hale              
                                 null               Test              
Total number of records retrieved: 2.

Copy link
Copy Markdown
Contributor

@WillieRuemmele WillieRuemmele Jun 1, 2022

Choose a reason for hiding this comment

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

can we add a NUT to cover this case?

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const newResult: Record<string, unknown> = {};
Object.entries(record as never).forEach(([key, value]) => {
// merge subqueries with the "parent" so they are on the same row
Reflect.defineProperty(result as Record<string, unknown>, `${child.toString()}.${key}`, {
value: value ? value : chalk.bold('null'),
});
if (!index) {
Reflect.defineProperty(result as Record<string, unknown>, `${child.toString()}.${key}`, {
Copy link
Copy Markdown
Contributor Author

@peternhale peternhale May 31, 2022

Choose a reason for hiding this comment

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

Add subquery data to first query record

value: value ? value : chalk.bold('null'),
});
} else {
Reflect.defineProperty(newResult, `${child.toString()}.${key}`, {
Copy link
Copy Markdown
Contributor Author

@peternhale peternhale May 31, 2022

Choose a reason for hiding this comment

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

add subquery data to subquery records 2-n

value: value ? value : chalk.bold('null'),
});
}
});
if (index) {
subResults.push(newResult);
}
});
}
});
}
newResults.push(result);
newResults.push(result, ...subResults);
Copy link
Copy Markdown
Contributor Author

@peternhale peternhale May 31, 2022

Choose a reason for hiding this comment

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

Push parent and additional subquery records into result.

return newResults;
}, [] as unknown[]);
}
Expand Down
5 changes: 3 additions & 2 deletions test/commands/force/data/soql/query/dataSoqlQuery.nut.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,15 @@ describe('data:soql:query command', () => {
});
it('should return account records with nested contacts', () => {
const query =
"SELECT Id, Name, Phone, Website, NumberOfEmployees, Industry, (SELECT Lastname, Title, Email FROM Contacts) FROM Account WHERE Name LIKE 'SampleAccount%' limit 1";
"SELECT Id, Name, Phone, Website, NumberOfEmployees, Industry, (SELECT Lastname, Title, Email FROM Contacts) FROM Account WHERE Name LIKE 'SampleAccount%'";

const queryResult = runQuery(query, { ensureExitCode: 0, json: false }) as string;

expect(queryResult).to.match(
/ID\s+?NAME\s+?PHONE\s+?WEBSITE\s+?NUMBEROFEMPLOYEES\s+?INDUSTRY\s+?CONTACTS.LASTNAME\s+?CONTACTS.TITLE\s+?CONTACTS.EMAIL/g
);
expect(queryResult).to.match(/Total number of records retrieved: 1\./g);
expect(queryResult).to.match(/\sSmith/g);
expect(queryResult).to.match(/Total number of records retrieved: 2\./g);
});
it('should handle count()', () => {
const queryResult = execCmd('force:data:soql:query -q "SELECT Count() from User"', {
Expand Down