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: resolve strictDirectoryName types in mdapi format #601

Merged
merged 4 commits into from
Apr 1, 2022

Conversation

shetzel
Copy link
Contributor

@shetzel shetzel commented Mar 31, 2022

What does this PR do?

Fixes MetadataResolver to match a file path in source format and mdapi format when testing types with strictDirectoryNames that also do not have an adapter strategy defined (those types are resolved properly another way).

This should fix the following types:

  • CustomSite
  • IntegrationHubSettings
  • AppointmentSchedulingPolicy
  • RestrictionRule
  • ObjectHierarchyRelationship
  • IndustriesManufacturingSettings
  • DataSource
  • FieldRestrictionRule
  • AppointmentAssignmentPolicy

What issues does this PR fix or reference?

forcedotcom/cli#1448
@W-10910004@

Functionality Before

Would not resolve files in mdapi format with certain strictDirectoryName types such as CustomSite. This would cause (e.g.) a retrieval of these types to fail to be written to the file system.

Functionality After

Files resolve properly. Metadata retrieval of these files are successful.

@shetzel shetzel requested review from a team as code owners March 31, 2022 17:51
@shetzel shetzel requested a review from jag-j March 31, 2022 17:51
@@ -142,6 +142,46 @@ describe('MetadataResolver', () => {
expect(access.getComponentsFromPath(path)).to.deep.equal([matchingContentFile.COMPONENT]);
});

it('Should determine type for metadata file with known suffix and strictDirectoryName', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made sure the test failed without the fix.

Copy link
Contributor

@mshanemc mshanemc left a comment

Choose a reason for hiding this comment

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

let's test with emailServicesFunction

// the suffix matches the type we think it is
(type.suffix && fsPath.endsWith(`${type.suffix}${META_XML_SUFFIX}`)) ||
// the file suffix (in source or mdapi format) matches the type suffix we think it is
(type.suffix && [type.suffix, `${type.suffix}${META_XML_SUFFIX}`].some((s) => fsPath.endsWith(s))) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

that exception is that little punk emailServicesFunction for whom .xml is the suffix.

now that we're testing with a real registry, can you make sure that it's handled correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested manually and added a unit test for it. The fix doesn't affect it since the matching wouldn't get that far with the EmailServicesFunction type; it's not defined as "strictDirectoryName": true

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, I was wanting it in a strictDir. ESF has a habit of matching way too promiscuously and I'm trying to keep that from happening.

https://github.com/forcedotcom/source-deploy-retrieve/pull/595/files#diff-af7a780a5aa66c4831707c3ccd4e24d4f88a6639346bc0b1897c287efcbe14c9

So...what would happen if it were in strictDir?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Testing would help us out, but I think it would still work just fine because it would only get to the suffix matching if the path contained the expected directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After testing, ESF still retrieves and deploys successfully when defined as a strictDirectoryName.

@WillieRuemmele
Copy link
Member

QA notes:
✅ Using CustomSite it worked for retrieve
image
✅ : source:convert -> mdapi:beta:deploy
✅ : local change, picked up source:status
✅ : source:push

@WillieRuemmele WillieRuemmele merged commit 823966e into main Apr 1, 2022
@WillieRuemmele WillieRuemmele deleted the sh/convert-custom-sites branch April 1, 2022 22:13
@ifeanyikyra
Copy link

Thanks for the fix. My project is impacted by it. When will it be made available to customers?

@WillieRuemmele
Copy link
Member

Hi @ifeanyikyra this will be available in the latest-rc build of the sfdx-cli on Thursday 4/7 and GA 4/14

@ifeanyikyra
Copy link

Thanks @WillieRuemmele .... Would running "sfdx update" get me the latest-rc on 4/7? Otherwise, is there a link that explains it

@WillieRuemmele
Copy link
Member

the release notes explain how to get the RC version sfdx update stable-rc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants