Skip to content

Commit

Permalink
fix: fix 3 data bugs
Browse files Browse the repository at this point in the history
* fix: @W-10290520@ '[object Object]' now stringified

* fix: @W-10290524@ fix json parsing on record:update

* chore: subqueries now display on the a single row

* chore: fix conflicting query results and parsing

* chore: recursively show query results

* test: added tests for complex subquery

* chore: try/catch around JSON parsing for data that contains {}

* chore: try changing cron to ' to match other repos for dependabot automerge

Co-authored-by: Shane McLaughlin <[email protected]>
  • Loading branch information
WillieRuemmele and mshanemc authored Feb 7, 2022
1 parent 7e45aa7 commit 801a7aa
Show file tree
Hide file tree
Showing 9 changed files with 389 additions and 43 deletions.
8 changes: 4 additions & 4 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ parameters:
description: |
By default, the latest version of the standalone CLI will be installed.
To install via npm, supply a version tag such as "latest" or "6".
default: ""
default: ''
type: string
repo_tag:
description: The tag of the module repo to checkout, '' defaults to branch/PR
default: ""
default: ''
type: string
npm_module_name:
description: The fully qualified npm module name, i.e. @salesforce/plugins-data
default: ""
default: ''
type: string
workflows:
version: 2
Expand Down Expand Up @@ -109,7 +109,7 @@ workflows:
dependabot-automerge:
triggers:
- schedule:
cron: "0 2,5,8,11 * * *"
cron: '0 2,5,8,11 * * *'
filters:
branches:
only:
Expand Down
26 changes: 18 additions & 8 deletions src/commands/force/data/soql/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
ensureString,
getArray,
isJsonArray,
JsonArray,
toJsonMap,
} from '@salesforce/ts-types';
import { Tooling } from '@salesforce/core/lib/connection';
Expand Down Expand Up @@ -66,8 +67,13 @@ export class SoqlQuery {
// eslint-disable-next-line no-underscore-dangle
const columnUrl = `${connection._baseUrl()}/query?q=${encodeURIComponent(query)}&columns=true`;
const results = toJsonMap(await connection.request(columnUrl));

return this.recursivelyFindColumns(ensureJsonArray(results.columnMetadata));
}

private recursivelyFindColumns(data: JsonArray): Field[] {
const columns: Field[] = [];
for (let column of ensureJsonArray(results.columnMetadata)) {
for (let column of data) {
column = ensureJsonMap(column);
const name = ensureString(column.columnName);

Expand All @@ -78,12 +84,17 @@ export class SoqlQuery {
name,
fields: [],
};
for (const subcolumn of column.joinColumns) {
const f: Field = {
fieldType: FieldType.field,
name: ensureString(ensureJsonMap(subcolumn).columnName),
};
if (field.fields) field.fields.push(f);
for (let subcolumn of column.joinColumns) {
subcolumn = ensureJsonMap(subcolumn);
if (isJsonArray(column.joinColumns) && column.joinColumns.length > 0) {
if (field.fields) field.fields.push(...this.recursivelyFindColumns([subcolumn]));
} else {
const f: Field = {
fieldType: FieldType.field,
name: ensureString(ensureJsonMap(subcolumn).columnName),
};
if (field.fields) field.fields.push(f);
}
}
columns.push(field);
} else {
Expand Down Expand Up @@ -131,7 +142,6 @@ export class DataSoqlQueryCommand extends DataCommand {
public static readonly description = messages.getMessage('description');
public static readonly requiresUsername = true;
public static readonly examples = messages.getMessage('examples').split(os.EOL);

public static readonly flagsConfig: FlagsConfig = {
query: flags.string({
char: 'q',
Expand Down
25 changes: 17 additions & 8 deletions src/dataCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

import { SfdxCommand } from '@salesforce/command';
import { AnyJson, Dictionary, get, Nullable } from '@salesforce/ts-types';
import { fs, Messages, SfdxError, Org } from '@salesforce/core';
import { BaseConnection, ErrorResult, Record, SObject } from 'jsforce';
import { fs, Messages, Org, SfdxError } from '@salesforce/core';
import { BaseConnection, ErrorResult, Record as jsforceRecord, SObject } from 'jsforce';
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore because jsforce doesn't export http-api
import * as HttpApi from 'jsforce/lib/http-api';
Expand Down Expand Up @@ -123,7 +123,7 @@ export abstract class DataCommand extends SfdxCommand {
return connection;
}

public async query(sobject: SObject<unknown>, where: string): Promise<Record<unknown>> {
public async query(sobject: SObject<unknown>, where: string): Promise<jsforceRecord<unknown>> {
const queryObject = this.stringToDictionary(where);
const records = await sobject.find(queryObject, 'id');
if (!records || records.length === 0) {
Expand All @@ -137,10 +137,10 @@ export abstract class DataCommand extends SfdxCommand {
);
}

return this.normalize<Record<unknown>>(records);
return this.normalize<jsforceRecord<unknown>>(records);
}

protected stringToDictionary(str: string): Dictionary<string | boolean> {
protected stringToDictionary(str: string): Dictionary<string | boolean | Record<string, unknown>> {
const keyValuePairs = this.parseKeyValueSequence(str);
return this.transformKeyValueSequence(keyValuePairs);
}
Expand Down Expand Up @@ -172,8 +172,8 @@ export abstract class DataCommand extends SfdxCommand {
*
* @param [keyValuePairs] - The list of key=value pair strings.
*/
private transformKeyValueSequence(keyValuePairs: string[]): Dictionary<string | boolean> {
const constructedObject: Dictionary<string | boolean> = {};
private transformKeyValueSequence(keyValuePairs: string[]): Dictionary<string | boolean | Record<string, unknown>> {
const constructedObject: Dictionary<string | boolean | Record<string, unknown>> = {};

keyValuePairs.forEach((pair) => {
// Look for the *first* '=' and splits there, ignores any subsequent '=' for this pair
Expand All @@ -182,7 +182,16 @@ export abstract class DataCommand extends SfdxCommand {
throw new Error(messages.getMessage('TextUtilMalformedKeyValuePair', [pair]));
} else {
const key = pair.substr(0, eqPosition);
constructedObject[key] = this.convertToBooleanIfApplicable(pair.substr(eqPosition + 1));
if (pair.includes('{') && pair.includes('}')) {
try {
constructedObject[key] = JSON.parse(pair.substr(eqPosition + 1)) as Record<string, unknown>;
} catch {
// the data contained { and }, but wasn't valid JSON, default to parsing as-is
constructedObject[key] = this.convertToBooleanIfApplicable(pair.substr(eqPosition + 1));
}
} else {
constructedObject[key] = this.convertToBooleanIfApplicable(pair.substr(eqPosition + 1));
}
}
});

Expand Down
32 changes: 26 additions & 6 deletions src/reporters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { Logger, Messages } from '@salesforce/core';
import { UX } from '@salesforce/command';
import * as chalk from 'chalk';
import { get, getArray, getNumber, isString, Optional } from '@salesforce/ts-types';
import { SoqlQueryResult, Field, FieldType } from './dataSoqlQueryTypes';
import { Field, FieldType, SoqlQueryResult } from './dataSoqlQueryTypes';

Messages.importMessagesDirectory(__dirname);
const messages = Messages.loadMessages('@salesforce/plugin-data', 'soql.query');
Expand Down Expand Up @@ -133,12 +133,28 @@ export class HumanReporter extends QueryReporter {

// eslint-disable-next-line @typescript-eslint/no-explicit-any
public massageRows(queryResults: unknown[], children: string[], aggregates: Field[]): any {
// 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) => {
const result = qr as Record<string, unknown>;
this.data.columns.forEach((col) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const entry = Reflect.get(result, col.name);
if (typeof entry === 'object' && col.fieldType === FieldType.field) {
Reflect.set(result, col.name, JSON.stringify(entry, null, 2));
} else if (typeof entry === 'object' && col.fields?.length && entry) {
col.fields.forEach((field) => {
Reflect.set(result, `${col.name}.${field.name}`, get(result, `${col.name}.records[0].${field.name}`));
});
}
});
});

// There are subqueries or aggregates. Massage the data.
let qr;
if (children.length > 0 || aggregates.length > 0) {
qr = queryResults.reduce((newResults: unknown[], result) => {
newResults.push(result);

// Aggregates are soql functions that aggregate data, like "SELECT avg(total)" and
// are returned in the data as exprX. Aggregates can have aliases, like "avg(total) totalAverage"
// and are returned in the data as the alias.
Expand Down Expand Up @@ -167,15 +183,17 @@ export class HumanReporter extends QueryReporter {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
childRecords.forEach((record: unknown) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const newRecord = Object.assign({});
Object.entries(record as never).forEach(([key, value]) => {
Reflect.defineProperty(newRecord, `${child.toString()}.${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'),
});
});
newResults.push(newRecord);
});
}
});
}
newResults.push(result);
return newResults;
}, [] as unknown[]);
}
Expand Down Expand Up @@ -223,6 +241,8 @@ export class CsvReporter extends QueryReporter {
const value = get(row, name);
if (isString(value)) {
return this.escape(value);
} else if (typeof value === 'object') {
return this.escape(JSON.stringify(value));
}
return value;
});
Expand Down
32 changes: 32 additions & 0 deletions test/commands/force/data/record/dataRecord.nut.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,4 +247,36 @@ describe('data:record commands', () => {
expect(result).to.have.property('Bool__c', false);
});
});

it('will parse JSON correctly for update', () => {
const result = execCmd('force:data:soql:query -q "SELECT Id FROM RemoteProxy LIMIT 1" -t --json', {
ensureExitCode: 0,
}).jsonOutput?.result as { records: Array<{ Id: string }> };

const update = execCmd(
'force:data:record:update ' +
'--sobjecttype RemoteProxy ' +
`--sobjectid ${result.records[0].Id} ` +
'--usetoolingapi ' +
'--values "Metadata=\'{\\"disableProtocolSecurity\\": false,\\"isActive\\": true,\\"url\\": \\"https://www.example.com\\",\\"urls\\": null,\\"description\\": null}\'"',
{ ensureExitCode: 0 }
);
expect(update).to.be.ok;
});

it('will parse invalid JSON data, but that contains {}', () => {
const result = execCmd('force:data:record:create -s Account -v "Name=Test" --json', {
ensureExitCode: 0,
}).jsonOutput?.result as { id: string };

const update = execCmd(
'force:data:record:update ' +
'--sobjecttype Account ' +
`--sobjectid ${result.id} ` +
'--values "Description=\'my new description { with invalid } JSON\'"',
{ ensureExitCode: 0 }
);

expect(update).to.be.ok;
});
});
15 changes: 15 additions & 0 deletions test/commands/force/data/soql/query/dataSoqlQuery.nut.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,5 +170,20 @@ describe('data:soql:query command', () => {
toolingApi: true,
});
});

it('should print JSON output correctly', () => {
const result = runQuery('select id, isActive, Metadata from RemoteProxy', {
ensureExitCode: 0,
json: false,
toolingApi: true,
});
expect(result).to.not.include('[object Object]');
// the Metadata object parsed correctly
expect(result).to.include('disableProtocolSecurity');
expect(result).to.include('isActive');
expect(result).to.include('url');
expect(result).to.include('urls');
expect(result).to.include('description');
});
});
});
Loading

0 comments on commit 801a7aa

Please sign in to comment.