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

fix: deletes singular CL when from CLs when specified #381

Merged
merged 16 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
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
7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@
"/oclif.manifest.json"
],
"dependencies": {
"@salesforce/core": "^3.34.6",
"@salesforce/core": "^3.36.0",
"@salesforce/kit": "^1.9.2",
"@salesforce/source-deploy-retrieve": "^8.0.1",
"@salesforce/source-deploy-retrieve": "^8.4.0",
"fast-xml-parser": "^4.2.2",
"graceful-fs": "^4.2.11",
"isomorphic-git": "1.23.0",
"ts-retry-promise": "^0.7.0"
Expand Down Expand Up @@ -158,4 +159,4 @@
"output": []
}
}
}
}
90 changes: 77 additions & 13 deletions src/sourceTracking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
// this is not exported by SDR (see the comments in SDR regarding its limitations)
import { filePathsFromMetadataComponent } from '@salesforce/source-deploy-retrieve/lib/src/utils/filePathGenerator';

import { XMLBuilder, XMLParser } from 'fast-xml-parser';
import { RemoteSourceTrackingService, remoteChangeElementToChangeResult } from './shared/remoteSourceTrackingService';
import { ShadowRepo } from './shared/localShadowRepo';
import { throwIfConflicts, findConflictsInComponentSet, getDedupedConflictsFromChanges } from './shared/conflicts';
Expand Down Expand Up @@ -106,6 +107,54 @@ export class SourceTracking extends AsyncCreatable {
this.subscribeSDREvents = options.subscribeSDREvents ?? false;
}

/**
* A static method to help delete custom labels from a file, or the entire file if there are no more labels
*
* @param filename - a path to a custom labels file
* @param customLabels - an array of SourceComponents representing the custom labels to delete
*/
public static async deleteCustomLabels(filename: string, customLabels: SourceComponent[]): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

if it's not using anything from the class, it could be a standalone function, top-level exported if needed by the plugins.

// for custom labels, we need to remove the individual label from the xml file
// so we'll parse the xml
const parser = new XMLParser({
ignoreDeclaration: false,
ignoreAttributes: false,
attributeNamePrefix: '@_',
});
const cls = parser.parse(fs.readFileSync(filename, 'utf8')) as {
Copy link
Contributor

Choose a reason for hiding this comment

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

alternatively, there's kit/ensureArray so you can have fewer conditionals. If you filtered the array and it ends up empty, you've got a check below for that to delete the file.

CustomLabels: { labels: Array<{ fullName: string }> | { fullName: string } };
};
if ('fullName' in cls.CustomLabels.labels) {
// a single custom label remains, delete the entire file
return fs.promises.unlink(filename);
} else {
// in theory, we should only have custom labels passed in, but filter just to make sure
const customLabelsToDelete = customLabels
.filter((label) => label.type.id === 'customlabel')
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd put this up at the top of the function. You could filter/warn/throw even before parsing the file if you got something unexpected

.map((change) => change.fullName);
// delete the labels from the json based on their fullName's
cls.CustomLabels.labels = cls.CustomLabels.labels.filter(
(label) => !customLabelsToDelete.includes(label.fullName)
);

if (cls.CustomLabels.labels.length === 0) {
// we've deleted everything, so let's delete the file
return fs.promises.unlink(filename);
} else {
// we need to write the file json back to xml back to the fs
const builder = new XMLBuilder({
attributeNamePrefix: '@_',
ignoreAttributes: false,
format: true,
indentBy: ' ',
});
// and then write that json back to xml and back to the fs
const xml = builder.build(cls) as string;
return fs.promises.writeFile(filename, xml);
}
}
}

// eslint-disable-next-line class-methods-use-this
public async init(): Promise<void> {
await this.maybeSubscribeLifecycleEvents();
Expand Down Expand Up @@ -349,9 +398,35 @@ export class SourceTracking extends AsyncCreatable {
.filter((filename) => filename)
.map((filename) => sourceComponentByFileName.set(filename, component))
);

// calculate what to return before we delete any files and .walkContent is no longer valid
const changedToBeDeleted = changesToDelete.reduce<FileResponse[]>((result, component) => {
[...component.walkContent(), component.xml].flatMap((file) => {
result.push({
state: ComponentStatus.Deleted,
filePath: file,
type: component.type.name,
fullName: component.fullName,
});
});

return result;
}, []);

const filenames = Array.from(sourceComponentByFileName.keys());
// delete the files
await Promise.all(filenames.map((filename) => fs.promises.unlink(filename)));
await Promise.all(
filenames.map((filename) => {
if (sourceComponentByFileName.get(filename)?.type.id === 'customlabel') {
return SourceTracking.deleteCustomLabels(
filename,
changesToDelete.filter((change) => change.type.id === 'customlabel')
);
} else {
return fs.promises.unlink(filename);
}
})
);

// update the tracking files. We're simulating SDR-style fileResponse
await Promise.all([
Expand All @@ -365,19 +440,8 @@ export class SourceTracking extends AsyncCreatable {
true // skip polling because it's a pull
),
]);
return filenames.reduce<FileResponse[]>((result, filename) => {
const component = sourceComponentByFileName.get(filename);
if (component) {
result.push({
state: ComponentStatus.Deleted,
filePath: filename,
type: component.type.name,
fullName: component.fullName,
});
}

return result;
}, []);
return changedToBeDeleted;
}

/**
Expand Down
150 changes: 150 additions & 0 deletions test/unit/sourceTracking.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
/*
* Copyright (c) 2020, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import * as fs from 'fs';
import * as sinon from 'sinon';
import { SourceComponent } from '@salesforce/source-deploy-retrieve';
import { expect } from 'chai';
import { SourceTracking } from '../../src';
Copy link
Contributor

Choose a reason for hiding this comment

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

can you import it from the file instead of the parent folder?


describe('SourceTracking', () => {
const sandbox = sinon.createSandbox();
let fsReadStub: sinon.SinonStub;
let fsWriteStub: sinon.SinonStub;
let fsUnlinkStub: sinon.SinonStub;

beforeEach(() => {
fsWriteStub = sandbox.stub(fs.promises, 'writeFile');
fsUnlinkStub = sandbox.stub(fs.promises, 'unlink');
fsReadStub = sandbox
.stub(fs, 'readFileSync')
.returns(
'<?xml version="1.0" encoding="UTF-8"?>\n' +
'<CustomLabels xmlns="http://soap.sforce.com/2006/04/metadata">\n' +
' <labels>\n' +
' <fullName>DeleteMe</fullName>\n' +
' <language>en_US</language>\n' +
' <protected>true</protected>\n' +
' <shortDescription>DeleteMe</shortDescription>\n' +
' <value>Test</value>\n' +
' </labels>\n' +
' <labels>\n' +
' <fullName>KeepMe1</fullName>\n' +
' <language>en_US</language>\n' +
' <protected>true</protected>\n' +
' <shortDescription>KeepMe1</shortDescription>\n' +
' <value>Test</value>\n' +
' </labels>\n' +
' <labels>\n' +
' <fullName>KeepMe2</fullName>\n' +
' <language>en_US</language>\n' +
' <protected>true</protected>\n' +
' <shortDescription>KeepMe2</shortDescription>\n' +
' <value>Test</value>\n' +
' </labels>\n' +
'</CustomLabels>\n'
);
});

afterEach(() => {
sandbox.restore();
});

describe('deleteCustomLabels', () => {
it('will delete a singular custom label from a file', async () => {
const labels = [
{
type: { id: 'customlabel', name: 'CustomLabel' },
fullName: 'DeleteMe',
} as SourceComponent,
];

await SourceTracking.deleteCustomLabels('labels/CustomLabels.labels-meta.xml', labels);
expect(fsWriteStub.firstCall.args[1]).to.not.include('DeleteMe');
expect(fsWriteStub.firstCall.args[1]).to.include('KeepMe1');
expect(fsWriteStub.firstCall.args[1]).to.include('KeepMe2');
expect(fsReadStub.callCount).to.equal(1);
});
it('will delete a multiple custom labels from a file', async () => {
const labels = [
{
type: { id: 'customlabel', name: 'CustomLabel' },
fullName: 'KeepMe1',
},
{
type: { id: 'customlabel', name: 'CustomLabel' },
fullName: 'KeepMe2',
},
] as SourceComponent[];

await SourceTracking.deleteCustomLabels('labels/CustomLabels.labels-meta.xml', labels);
expect(fsWriteStub.firstCall.args[1]).to.include('DeleteMe');
expect(fsWriteStub.firstCall.args[1]).to.not.include('KeepMe1');
expect(fsWriteStub.firstCall.args[1]).to.not.include('KeepMe2');
expect(fsReadStub.callCount).to.equal(1);
});

it('will delete the file when everything is deleted', async () => {
const labels = [
{
type: { id: 'customlabel', name: 'CustomLabel' },
fullName: 'KeepMe1',
},
{
type: { id: 'customlabel', name: 'CustomLabel' },
fullName: 'KeepMe2',
},
{
type: { id: 'customlabel', name: 'CustomLabel' },
fullName: 'DeleteMe',
},
] as SourceComponent[];

await SourceTracking.deleteCustomLabels('labels/CustomLabels.labels-meta.xml', labels);
expect(fsUnlinkStub.callCount).to.equal(1);
expect(fsReadStub.callCount).to.equal(1);
});

it('will delete the file when only a single label is present and deleted', async () => {
fsReadStub.returns(
'<?xml version="1.0" encoding="UTF-8"?>\n' +
'<CustomLabels xmlns="http://soap.sforce.com/2006/04/metadata">\n' +
' <labels>\n' +
' <fullName>DeleteMe</fullName>\n' +
' <language>en_US</language>\n' +
' <protected>true</protected>\n' +
' <shortDescription>DeleteMe</shortDescription>\n' +
' <value>Test</value>\n' +
' </labels>\n' +
'</CustomLabels>\n'
);
const labels = [
{
type: { id: 'customlabel', name: 'CustomLabel' },
fullName: 'DeleteMe',
},
] as SourceComponent[];

await SourceTracking.deleteCustomLabels('labels/CustomLabels.labels-meta.xml', labels);
expect(fsUnlinkStub.callCount).to.equal(1);
expect(fsReadStub.callCount).to.equal(1);
});

it('will not delete custom labels', async () => {
const labels = [
{
type: { id: 'apexclass', name: 'ApexClass' },
fullName: 'DeleteMe',
},
] as SourceComponent[];

await SourceTracking.deleteCustomLabels('labels/CustomLabels.labels-meta.xml', labels);
expect(fsUnlinkStub.callCount).to.equal(0);
expect(fsWriteStub.firstCall.args[1]).to.include('DeleteMe');
expect(fsReadStub.callCount).to.equal(1);
});
});
});
Loading