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

Add transform to remove invalid references from profiles #3688

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

aditya-balachander
Copy link
Contributor

@aditya-balachander aditya-balachander commented Oct 25, 2023

W-11744607
W-11744695

Implemented Functionality to clean profiles and permission sets of invalid references.

Issues (corner cases that are not supported currently)

  1. Since we are using the approach of parsing the xml files in the source zip and retrieving all entities using package.xml, we are unable to retrieve:
    i. Couple of sObjects (some of which are not supported, and Document which is a metadata of it's own but comes under ObjectPermissions)
    ii. Couple of tabs (These tabs are permissioned in the profile but are not present in the TabDefinition table or the tabs page in setup of the org. Most likely these are linked tabs as part of applications or other metadata types)
    iii. Couple of fields (which are permissioned in the org but are missing in the sObject itself)
  2. For User Permissions:
    i. Some UserPermissions are dependent on other UserPermissions. Say permission A depends on B (both A and B are present in the target org). There is no current check to see if B is present when A is present in the profile or permissionset file. Without B, deployment fails when you try to deploy A.

We have created a WI to resolve these issues: W-14363935

Copy link
Contributor

@davidmreed davidmreed left a comment

Choose a reason for hiding this comment

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

@aditya-balachander This looks great. Appreciate the depth of research you put into edge cases here. I have a few questions about aspects I didn't understand. We might actually be doing more than we need to!

cumulusci/tasks/salesforce/Deploy.py Outdated Show resolved Hide resolved
@@ -220,6 +226,9 @@ def _get_package_zip(self, path) -> Union[str, dict, None]:
"clean_meta_xml": process_bool_arg(
self.options.get("clean_meta_xml", True)
),
"clean_profiles": process_bool_arg(
self.options.get("clean_profiles", True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's think this through - defaulting to True is sort of a breaking change. @jstvz what are your thoughts? @sumeet-parmar? @jkasturi-sf?

Say we have an existing project. Today, it will fail to deploy into an org that is missing specific features to expose permissioned components in its metadata bundle. If we default this to True, that project may begin to deploy cleanly.

What's the state of the user's org, then? In most or all cases, I think it will simply work. That's because the success of the deploy will only change if the Profile or PermSet reference was the only reference to the missing component. Anything else would cause a failure that we would not stop here (nor would we want to).

I'm leaning towards agreeing with the decision to default this to True.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The clean_invalid_ref defaults to False. Again sorry for not removing the old code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh shall I keep it to True?

cumulusci/utils/clean_invalid_references.py Show resolved Hide resolved
cumulusci/utils/clean_invalid_references.py Show resolved Hide resolved
cumulusci/utils/clean_invalid_references.py Outdated Show resolved Hide resolved
cumulusci/utils/clean_invalid_references.py Outdated Show resolved Hide resolved
cumulusci/utils/clean_invalid_references.py Outdated Show resolved Hide resolved
cumulusci/utils/clean_invalid_references.py Outdated Show resolved Hide resolved
cumulusci/utils/clean_invalid_references.py Outdated Show resolved Hide resolved
cumulusci/utils/clean_invalid_references.py Outdated Show resolved Hide resolved
@aditya-balachander
Copy link
Contributor Author

W-14363935

Most of the corner cases resolved. There is still an issue with some of the UserPermissions not being retrieved (in certain API versions). This is because in core, these UserPermissions either have a maxApiVersion defined (and our api version is more than that) or has apiAccess=IsSfdcInternal making it unexposed to public API.

The problem occurs because the SOAP call to retrieve the profile/permissionset gives all the UserPermissions regardless of which API Version is mentioned. However when we make a rest/tooling API call to retrieve these UserPermissions, only the ones present in that particular API version are retrieved.

@davidmreed kindly review the PR

@jstvz jstvz changed the title Feature/clean profiles deploy mutation Add transform to remove invalid references from profiles Nov 9, 2023
Copy link
Contributor

@jkasturi-sf jkasturi-sf left a comment

Choose a reason for hiding this comment

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

If a profile has references to an entity which doesn't exists in the org, then this change will help us to remove the references to those entities and deploy. This will allow us to deploy profiles without throwing an error

@jstvz jstvz added the internal Pull requests from IPDE team label May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Pull requests from IPDE team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants