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

Create FlowDefinitions for deleted Flows #588

Closed
YodaDaCoda opened this issue May 3, 2023 · 17 comments
Closed

Create FlowDefinitions for deleted Flows #588

YodaDaCoda opened this issue May 3, 2023 · 17 comments
Assignees
Labels
destructive enhancement New feature or request flow good first issue Good for newcomers

Comments

@YodaDaCoda
Copy link

Is your proposal related to a problem?

I'd like to be able to leverage SGD to delete flows, but due to a Salesforce limitation ("Working as Expected"), I doubt that's ever going to be the case.

Describe a solution you propose

While Flows can't easily be deleted in destructiveChanges.xml, we can deactivate flows by deploying a FlowDefinition with activeVersionNumber set to 0. We can later purge inactive flows.

e.g.

My_Flow.flow-meta.xml

<?xml version ="1.0" encoding="UTF-8"?>
<FlowDefinition xmlns="http://soap.sforce.com/2006/04/metadata">
    <activeVersionNumber>0</activeVersionNumber>
</FlowDefinition>

I propose adding functionality to SGD that would allow for creation of these FlowDefinitions. Given how this would work, it'd likely have to be behind an additional flag (e.g. --transform-destructive-flows), and would only work with --generate-delta (the README would have to make this very clear, and use of the new flag without --generate-delta should probably throw an error.

Describe alternatives you've considered

I've considered writing a script to do this, which wouldn't be complicated, but I think this fits in within the goals of SGD.

Additional context

Earlier related issue: #366
Salesforce Known Issue: https://issues.salesforce.com/issue/a028c00000gAwixAAC/deletion-of-flow-metadata-through-destructive-changes-not-supported

@YodaDaCoda YodaDaCoda added the enhancement New feature or request label May 3, 2023
@scolladon scolladon added the good first issue Good for newcomers label May 3, 2023
@YodaDaCoda
Copy link
Author

I'm unclear what you mean, the FlowDefinitions don't need to be versioned in the repo, just generated for deployment purposes (e.g. in a CI environment).

@AllanOricil
Copy link
Contributor

AllanOricil commented May 4, 2023

How would sgd determine that a flow was deleted? If a flow is deleted, then sgd would create these flow definitions, and add the deleted flows back? This would require sgd to add a new commit. Or it could be done with a pre commit, or even pre push, ocliff hook to avoid an extra commit.

  1. developer delete flow and create a commit with these deletions.
  2. a pre commit hook runs, which does the following:
    2.1 recrete the deleted flows
    2.2 create flow definitions

I would control the hook automation with an environment variable.

@YodaDaCoda
Copy link
Author

Apologies if I haven't been clear.

SGD already detects that a Flow was deleted and puts it in the destructiveChanges.xml. My proposal is to add functionality that removes the Flow from the destructiveChanges.xml and creates a corresponding FlowDefinition which would deactivate the Flow when deployed. I recognise this doesn't actually delete the flow in the target org, but it does achieve something similiar in terms of functionality (particularly for record-triggered flows), and allows the inactive flows to be cleaned up as a post-deployment step (such as with hardis:flow:purge).

IMO Adding a FlowDefinition to the repo would be problematic for the scenario when you're deploying the whole codebase to a fresh sandbox or scratch org.

Another (though maybe less relevant) argument against adding FlowDefinitions to the repo is that Salesforce recommends against using them since APIv44.

@scolladon
Copy link
Owner

Hi @YodaDaCoda !

Thanks for raising this issue and thanks for contributing in making this project better!
Thanks for all the digging in the doc and in our issues related to that, it saves me a ton of work 😅

That being said, we all know it is kind of tricky to deal with Flow and their version.
I'll quote what I said in the #366:

From my experience, there are multiple ways to deal with this situation and I have seen it differently implemented in different project with different outcome in term of result, developer experience and release experience.
The main question is where the team want to put the burden...

Some facts so far:

  • As you said @YodaDaCoda Salesforce recommends against using them since APIv44.
  • SGD support API version from v46 to v57 (for now, we are spiking on how to deal differently with API versions)
  • FlowDefinition is specific metadata located in the flowDefinitions directory (not in the flows)
  • sfdx-git-delta will put deleted flow metadata file from the repo in the destructiveChanges (no particular treatment so far)

If I understand properly the need here:

  • Have a way to handle flow delete.
  • sfdx-git-delta driven.

The solution proposed in this enhancement request:

When sfdx-git-delta detect a flow is deleted, instead of put it in the destructiveChanges.xml it should add it to the package.xml and copy a <flowname>.flowDefinition-meta.xml file with the following content (when -d parameter is added):

<?xml version ="1.0" encoding="UTF-8"?>
<FlowDefinition xmlns="http://soap.sforce.com/2006/04/metadata">
    <activeVersionNumber>0</activeVersionNumber>
</FlowDefinition>

I see multiple risks here:

  • It uses a metadata feature from v44 and below (even if it seems to still be accepted, this is not the recommended way to go)
  • It can lead to different behaviors depending on the way the deployment is done. If deployment is done via package.xml then nothing will change, the flow will still be activated. If deployment is done via source then the flow will be deactivated.

And, I'm not (yet) convinced it is something sfdx-git-delta should handle. I don't want the plugin to force a workflow and prefer the user to not be constraint in one use case so everyone can innovate and deal with their situation the best they think they should. I like this agnostic aspect of the plugin, I think the versatility of its usage is something powerful for the end user.

Alternative approach

I'm not sure if it would be suitable for your situation @YodaDaCoda but let me try something:

  1. You could change the status to be Draft in the flow metadata file (instead of Active) and commit it in the repo.
  2. sfdx-git-delta will detect this change and then deploy the flow as inactive (I have not tested this though)
  3. Then use the sfdx hardis:purge:flow (potentially on all your sandboxes...)
  4. Then you'll have to version this purge => delete all the purged flow from the repository and commit it
    => This will add those flows in the destructiveChanges.xml which risk to fail the next deploy because they should already be deleted. To avoid issue here, the deployment should be set to ignore warning (-g or --ignorewarnings)

I'm not very happy with this alternative workflow, it is me thinking out loud, but I thought it could be a start to think about it and maybe it could lead to new ideas.
Could you tell us more about your situation, need and approach? Would this alternative approach lead to something you could use?

@AllanOricil
Copy link
Contributor

AllanOricil commented May 4, 2023

@scolladon

I believe the alternative approach can be reduced to a single commit if:

  • sgd stops adding deleted flows in the destructiveChanges manifests (pre/post as well)
  • sgdoutputs a list of metadata that were deleted in its json output.

Then the rest is orchestrated in the CICD pipeline.

Steps:

  1. flow is deleted in a given commit
23echsd      deletes Flow X because ...
  1. pipeline run sgd to detect all changes between --from and --to
  2. sgd creates deployment job manifests at -o <DIR>, and it also outputs a json (stdout only) that has a list of deleted metadata. In this case we are only interested in knowing which flows were deleted. Flows are not included in the destructiveChanges.xml
{
   ...
   "deleted_metadata": {
        "flow": [ //metadata type keys follow the official metadata api specs
            {
               "path" : "./metadata/path", //we will need this information on step 4.1
               ...
            }
         ],
        ...
   },
   ...
}

The list of delete metadata is used on steps 4.1 and 6.1 (pre|post deployment steps) to avoid wasting time reading git branch history again

  1. pre deployment steps are executed
    4.1 all deleted flows have their status changed to Draft
  2. changes are deployed. In this step deleted flows are changed to Draft in the environment because of step 5.
  3. post deployment steps are executed
    6.1 all deleted flows are purged with sfdx hardis:purge:flow
  4. end

wouldn't this work?

@scolladon
Copy link
Owner

@AllanOricil I think it would work, yes (though I have not tested it) and with this we can just use the flow deletion in the repo with one commit 🎩
And I think it could be scriptable as well. it could probably be a good feature in a plugin like hardis btw (FYI @nvuillam, if you have bandwidth or maybe a POV on this matter, feel free to share with us)

Step 3 could be done without sgd, using git and grep would do the trick:

$ git diff --diff-filter=D --name-only <from> <to> | grep "\.flow-meta.xml$"

So to recap:

  1. commit 56dru135 (you got the ref? 😉) deletes a flow
  2. run sfdx-git-delta on a commit range including this commit
  3. store the deleted flow on this commit range (git diff --diff-filter=D --name-only <from> <to> | grep "\.flow-meta.xml$")
  4. add metadata related to deleted flow with Draft status in the output folder.
  • git log -1 --pretty=format:"%h" -- <flowPath> for each flowPath from 3, to get the commit where the flow has been deleted, and store it in a variable (flowDeleteSha)
  • git --no-pager show "$flowDeleteSha~1":<flowPath> for each flow path
  • sed (or awk) to remove the status xml element (sed -n '/<status>/!p') and then copy the file in the output folder (> output/<flowPath>).
  1. Deploy the output folder with ignore warning
  2. sfdx hardis:purge:flow or with a manual script or

Alternatively for step 5 and 6
5. Retrieve FlowVersion using the Tooling API for each (SELECT Id, Definition.DeveloperName, VersionNumber, Status FROM Flow WHERE Status != 'Active' AND Definition.DeveloperName in ('flowPath')), add them in a destructiveChangesPost.xml and deploy them.
6. Deploy the output folder with ignore warning

As this process forces the usage of deployment using delta sources instead of just using delta package.xml against sources from the repo I would prefer to keep it outside sfdx-git-delta.
This way we avoid forcing the users to follow a specific process for this situation and allow them to have their process.
What I don't really like as well is that it forces to have a repo "delta" state not deployable without all those steps... As it is not possible to delete via destructiveChanges.xml a flow, it should not be deleted from the repo?... 🤔

Interesting to see that Copado and Gearset deal with that using manual action.

I feel we are close to find a "nearly generic" way to deal with Flow deletion here.
Using the flowDefinitions could be a strong lead as well but I would rather avoid it as it is more a legacy way to deal with flow version and could be deprecated soon.

@AllanOricil
Copy link
Contributor

@scolladon
I know the ref is a joke but I did not understand it. Could you explain it?

Why not include the metadata that was deleted in sgd`s output?

@nvuillam
Copy link
Contributor

nvuillam commented May 4, 2023

EVERYTHING is scriptable :)

Thanks for the ping @scolladon, I quickly read in 3mn , let's discuss it further this evening , but I think there is something to be done

Meanwhile.... when you need to delete a flow (that should not happen so often) , just use https://sfdx-hardis.cloudity.com/hardis/org/purge/flow/ and then delete it manually before deployment ^^

@AllanOricil sfdx-git-delta generates a destructiveChanges.xml , it's easy to parse it to play with it, that's what we do to automatically update package.xml in sfdx-hardis :)

To verify that, you can play with sfdx-git-delta by manually selecting commits in VsCode SFDX Hardis UI, the command is available in "Nerdy Stuff" menu :)

image

@scolladon
Copy link
Owner

scolladon commented May 4, 2023

@AllanOricil I would prefer not include the metadata that was deleted in sgd's output because:

  • It is possible to get this information in the destructiveChanges.xml
  • It can be huuuuge for some customer. It may be truncated or behave badly in some terminal / arch
  • it makes us implement this output just for the purpose of a workflow/process, I would prefer to not tight features to some specific usage/workflow/process. I think it is best for versatility / innovation.

I know the ref is a joke but I did not understand it. Could you explain it?

The sha ref is written in simple leet 😄

@nvuillam 👍

@YodaDaCoda
Copy link
Author

SGD Rules! 😉

After a bit of testing, I've determined a couple things that lead me to conclude that Flow metadata can't be relied on as a source-of-truth (at least not with some workarounds). Unfortunately, since FlowVersions are org-specific, using them during deployment is not a viable option without retrieving them from the target org & processing (outside the purview of SGD). I think that using the FlowDefinition metadata as discussed earlier works around this nicely. I want to note that despite Salesforces instructions to move away from using FlowDefinitions, they're not deprecated (yet?).

  1. When deploying a Flow, Salesforce only works with the FlowVersion with the highest version number (i.e. the most recent version).
  2. Deploying Flow metadata with a status of Draft/Obsolete, when the most recent version is Active, does not deactivate the corresponding flow version (it remains Active).
  3. Deploying Flow metadata with a status of Active, where the most recent version of the flow in Salesforce matches the metadata (except for the Active status), and an older version is Active, will not deactivate the old version and activate the new version - the older version remains active (see testing details below).

My thoughts regarding development practice is to:

  • Only ever commit Flows to git with a status of Active, and only ever Delete flows to deactivate - never use a Status other than Active in git
  • Through the use of hardis:purge:flow pre-deployment, ensure that deploying an Active Flow will either create a new Active version (in the case when the Flow was changed) or leave the current most recent version Active (in the case the flow was unchanged) or re-create the Flow/Version and set it Active (in the case where an older version was made active, or the flow was deleted in the org)
  • Through the use of deploying a FlowDefinition as described earlier, ensure any git-deleted flows are made Inactive
  • Use hardis:purge:flow post-deployment to ensure newly-Inactive flows are Deleted
Details of testing

once a version of a flow has been active, that version cannot be changed, only made inactive.
deploy/retrieve with SFDX always operates on the latest version of a flow (the highest version number).

test 1:
edit a flow in salesforce, save as new version
retrieve the flow, observe the changes & the draft status
change the status to active in salesforce
retrieve the flow, observe the changed status to active
PASS

test 2:
edit a flow in salesforce, save as new version
retrieve the flow, observe the changes & the draft status
change the status to Active in the metadata
deploy the flow, observe the latest version is now active in salesforce
PASS

test 3:
edit a flow's metadata from Active to Draft
deploy the metadata
observe the flow is now inactive in salesforce
FAIL - flow is not deactivated

test 4:
edit a flow's metadata from Active to Obsolete
deploy the meteadata
observe the flow is now inactive in Salesforce
FAIL - flow is not deactivated

test 5:
edit a flow in salesforce, save as new version, leave as draft (do not activate)
deploy the old version of the flow, still active
observe that the new version of the flow is changed such that it is the same as the previous version, and is now active.
PASS

test 6:
edit a flow in salesforce, save as new version.
activate the new version.
retrieve the metadata for the new version.
activate an older version of the flow in salesforce.
re-deploy the metadata for the new version.
observe that the new version is now active in salesforce, and the old version is now inactive.
PASS WITH NOTES - the existing inactive version is not activated, instead a new version is created and made active

test 7:
edit a flow in salesforce, save as new version.
activate the new version.
retrieve the metadata for the new version.
activate an older version of the flow in salesforce.
edit the metadata to make the new version inactive.
deploy the metadata.
observe that the new version is active in salesforce, and the old version is made inactive.
FAIL - the old version is still Active and the newer version is unchanged

conclusion: when there are multiple versions of a flow in salesforce, the git metadata cannot be trusted as a source of truth without additional effort

@scolladon
Copy link
Owner

Hi @YodaDaCoda, thank you very much for those details, very helpful.

I guess your conclusion also explain why Gearset and Copado choose to deal with those Flow version the way they do.
FlowDefinition seems reliable, I do not know why it is discouraged by the doc (even if still working).

Thanks for you thought, very helpful, that's a big takeaway for anyone.

Before closing this enhancement request, what would be your workflow in the end ?
How are you going to deal with Flow version from now on ?
What would be your process in your projects with CI/CD ?

@AllanOricil
Copy link
Contributor

AllanOricil commented May 10, 2023

@YodaDaCoda @scolladon The build job that runs inside salesforce changes depending on the API version used to deploy things, right? Did you run the same set of tests using different API versions? Maybe running the same tests from 44.0 to latest should be done in order to assert that the API version does not affect ur conclusion.

@scolladon
Copy link
Owner

Thanks for thinking about it @AllanOricil
That is a very good point and would help us to be sure.

@nvuillam
Copy link
Contributor

nvuillam commented May 10, 2023

Deploying Flow metadata with a status of Active, where the most recent version of the flow in Salesforce matches the metadata (except for the Active status), and an older version is Active, will not deactivate the old version and activate the new version - the older version remains active (see testing details below).

Are you 100% sure ?
I have many CI/CD clients using flows and (we don't use FlowDefinition), and I'm pretty sure than when we deploy an active flow, after deployment is becomes the active flow in the target org
But at the first deployment, the flow indeed needs to be manually activated

@YodaDaCoda
Copy link
Author

YodaDaCoda commented May 12, 2023

@AllanOricil I agree testing with API versions 44.0 through to latest is a Good Idea. I'll try to make time for this over the next week. I haven't much patience for manual testing so I'll try to script it, which will also mean those here can replicate my result easily.

@nvuillam 100%. I double-checked. If the metadata of the Flow you're deploying exactly matches the flow that's already in Salesforce (with the exception of the <status>) the flow is unchanged. Said another way, it appears that the status only takes effect if something else in the rest of the metadata is different, causing Salesforce to generate a new version with the given status.

This means if someone activates an older version of a flow directly in the org, then the latest version is re-deployed (e.g. as part of a CI/CD job), then the old version will remain active. ☠️

@scolladon my end workflow will be to:

  • ensure flows are commit as Active, or deleted. No other statusus will be permitted. Ideally this will be done via a CI job on the PR, though will initially be manual review.
  • pre-deployment purge inactive flows (to avoid the situation where a not-latest version of a flow is active in the org, and the flow being deployed is unchanged)
  • generate FlowDefinition(version=0) for deleted flows (i'd still love it if sgd could do this bit)
  • force:source:deploy
  • post-deployment purge inactive flows (to actually delete the deleted flows)

@scolladon
Copy link
Owner

Ok thank @YodaDaCoda

So you'll go for the FlowDefinition method, interesting 👍
If there is more traction from the community to go for this way to deal with the Flow deletion and it becomes a best practice we will definitely propose something to support this workflow.

Thanks for the discussion here guys, I hope FlowDefinition will not be removed from the metadata API someday, because it looks like a proper way to deal with that.

@amtrack
Copy link
Contributor

amtrack commented Jul 17, 2024

@scolladon I just tested deactivating a Flow by using the FlowDefinition > activeVersionNumer=0 workaround and this still works with API version 61.0 and I think this is the proper way to do handle deleted Flows at the moment.

I've just created a prototype: #893

  • Are you interested in having this?
  • Do you want to need a special feature flag / config.
  • Any thoughts on whether this can/should be applied without --generate-delta?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
destructive enhancement New feature or request flow good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants