feat: support split CustomLabels on deploy and retrieve #278

mdonnalley · 2021-03-29T16:56:19Z

What does this PR do?

Support deploying and retrieving CustomLabels that are split across multiple package directories

The following configuration options are added to the metadata registry:

ignoreParentName: indicates whether or not the parent name should be included when building out the component's fullName
uniqueIdAttribute: indicates which xml attribute has the type's unique name. Required in order to matchup incoming children with existing children
strategies.recomposition: Used to change the behavior of RecompositionFinalizer.recompose. Only option is startEmpty which indicates whether or not the parent object should be read via parseXml or instantiated as an empty object

What issues does this PR fix or reference?

@W-9016030@

Functionality Before

All labels were retrieved into default package directory

Functionality After

Labels are retrieved into the directory to which they belong. New labels are added to the default directory

shetzel

Nice!

shetzel · 2021-03-30T15:58:50Z

src/convert/metadataConverter.ts

@@ -52,6 +52,7 @@ export class MetadataConverter {
    try {
      // it's possible the components came from a component set, so this may be redundant in some cases...
      const manifestContents = new ComponentSet(components, this.registry).getPackageXml();
+      console.log(manifestContents);


Worth converting to Logger.debug()?

brpowell · 2021-03-30T18:52:23Z

src/metadata-registry/sourceComponent.ts

+    if (this.content && !this.parent && this.type.children) {
+      return this.getDecomposedChildren(this.content);
+    } else if (!this.parent && this.type.children) {
+      return this.getNonDecomposedChildren();
+    } else {
+      return [];
+    }


Suggested change

if (this.content && !this.parent && this.type.children) {

return this.getDecomposedChildren(this.content);

} else if (!this.parent && this.type.children) {

return this.getNonDecomposedChildren();

} else {

return [];

}

if (!this.parent && this.type.children) {

if (this.content) {

return this.getDecomposedChildren(this.content);

}

return this.getNonDecomposedChildren();

}

return [];

brpowell · 2021-03-30T19:05:45Z

src/metadata-registry/sourceComponent.ts

+      const contents = fs.readFileSync(this.xml);
+      return this.parse<T>(contents.toString());


Normally I try to use the tree to do IO, in this case this.tree.readFile. There isn't a "sync" method on the tree interface yet but you could add one - should be pretty simple. Especially if modifying getChildren to be async is too destructive with the resolver and everything

@brpowell I originally tried adding a readSync to the treeContainer classes but I got stumped by ZipTreeContainer which cannot be implemented synchronously because of the zip library we're using.

I wasn't sure if the ZipTreeContainer was ever used in this code path, so I decided the safest thing was to just use fs instead

brpowell · 2021-03-30T19:11:44Z

src/metadata-registry/sourceComponent.ts

+    const xmlPathToChildren = `${this.type.name}.${this.type.directoryName}`;
+    // WARNING: for NonDecomposed children we expect the first child type to be the only child type,
+    // which might not be a valid assumption long term
+    const [childTypeId] = Object.keys(this.type.children.types);


What if instead we iterate through Object.values(this.type.children.types) and concat the components for each type?

brpowell · 2021-03-30T19:15:37Z

src/convert/metadataConverter.ts

@@ -52,6 +52,7 @@ export class MetadataConverter {
    try {
      // it's possible the components came from a component set, so this may be redundant in some cases...
      const manifestContents = new ComponentSet(components, this.registry).getPackageXml();
+      console.log(manifestContents);


Suggested change

console.log(manifestContents);

brpowell · 2021-04-05T21:21:19Z

@mdonnalley Looking good on the tests 👍 . Few more places to increase coverage on:

The new child parsing code in the SourceComponent class
The new switch case in the SourceAdapterFactory
Possibly increase the Branch category for ConvertContext stuff. It's actually a higher percentage than what it was which is good. If it's easy enough to bump to green go for it, otherwise no worries.

brpowell · 2021-04-07T14:21:05Z

src/registry/types.ts

+   * The xml attribute used as the unique identifier when parsing the xml
+   */
+  uniqueIdAttribute?: string;


This is a nit, but is it the XML attribute or element?

* feat: support split CustomLabels on deploy and retrieve * chore: rename uniqueIdAttribute to uniqueIdElement * fix: allow unprocessed component to be the default component * chore: bump shelljs

mdonnalley requested a review from a team as a code owner March 29, 2021 16:56

mdonnalley mentioned this pull request Mar 29, 2021

Support split CustomLabels #273

Closed

2 tasks

mdonnalley self-assigned this Mar 29, 2021

mdonnalley force-pushed the mdonnalley/custom-labels-deploy-retrieve branch from c29849e to c64aaee Compare March 29, 2021 17:07

shetzel approved these changes Mar 30, 2021

View reviewed changes

brpowell reviewed Mar 30, 2021

View reviewed changes

mdonnalley force-pushed the mdonnalley/custom-labels-deploy-retrieve branch from 94cfd40 to 9e58fe0 Compare April 1, 2021 14:34

mdonnalley requested a review from brpowell April 1, 2021 14:52

mdonnalley force-pushed the mdonnalley/custom-labels-deploy-retrieve branch 3 times, most recently from c78ca35 to 8cbe41f Compare April 5, 2021 20:14

mdonnalley force-pushed the mdonnalley/custom-labels-deploy-retrieve branch from e723338 to 14c112a Compare April 6, 2021 20:44

brpowell reviewed Apr 7, 2021

View reviewed changes

mdonnalley force-pushed the mdonnalley/custom-labels-deploy-retrieve branch from 1752ce1 to a771804 Compare April 7, 2021 14:51

mdonnalley requested a review from brpowell April 7, 2021 14:53

mdonnalley force-pushed the mdonnalley/custom-labels-deploy-retrieve branch from ba4cc8c to f4dc49d Compare April 7, 2021 16:57

brpowell approved these changes Apr 7, 2021

View reviewed changes

mdonnalley added 3 commits April 8, 2021 12:49

feat: support split CustomLabels on deploy and retrieve

f74d33b

chore: rename uniqueIdAttribute to uniqueIdElement

66ddda0

fix: allow unprocessed component to be the default component

992830b

mdonnalley force-pushed the mdonnalley/custom-labels-deploy-retrieve branch from f4dc49d to 992830b Compare April 8, 2021 18:49

chore: bump shelljs

e34707b

mdonnalley merged commit 82826ff into develop Apr 8, 2021

mdonnalley deleted the mdonnalley/custom-labels-deploy-retrieve branch April 8, 2021 21:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: support split CustomLabels on deploy and retrieve #278

feat: support split CustomLabels on deploy and retrieve #278

Uh oh!

mdonnalley commented Mar 29, 2021

Uh oh!

shetzel left a comment

Uh oh!

shetzel Mar 30, 2021

Uh oh!

brpowell Mar 30, 2021

Uh oh!

brpowell Mar 30, 2021 •

edited

Loading

Uh oh!

mdonnalley Mar 31, 2021 •

edited

Loading

Uh oh!

brpowell Mar 30, 2021

Uh oh!

brpowell Mar 30, 2021

Uh oh!

brpowell commented Apr 5, 2021

Uh oh!

brpowell Apr 7, 2021

Uh oh!

Uh oh!

		const contents = fs.readFileSync(this.xml);
		return this.parse<T>(contents.toString());

feat: support split CustomLabels on deploy and retrieve #278

feat: support split CustomLabels on deploy and retrieve #278

Uh oh!

Conversation

mdonnalley commented Mar 29, 2021

What does this PR do?

What issues does this PR fix or reference?

Functionality Before

Functionality After

Uh oh!

shetzel left a comment

Choose a reason for hiding this comment

Uh oh!

shetzel Mar 30, 2021

Choose a reason for hiding this comment

Uh oh!

brpowell Mar 30, 2021

Choose a reason for hiding this comment

Uh oh!

brpowell Mar 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdonnalley Mar 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brpowell Mar 30, 2021

Choose a reason for hiding this comment

Uh oh!

brpowell Mar 30, 2021

Choose a reason for hiding this comment

Uh oh!

brpowell commented Apr 5, 2021

Uh oh!

brpowell Apr 7, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brpowell Mar 30, 2021 •

edited

Loading

mdonnalley Mar 31, 2021 •

edited

Loading