adobe / adobe-dx

A toolkit for AEM to help build exceptional digital experiences.
Apache License 2.0
35 stars 33 forks source link

Adds another search criteria to the find function of the replace pipe #192

Closed luckyluke-adobe closed 2 years ago

luckyluke-adobe commented 2 years ago

Description

The current replace function allows to find resources with a specific property and replace the property value. This PR extends the functionality, that another criteria is added to the find xpath function, which allows to search for two properties (e.g. a resourceType and a template in combination)

Motivation and Context

The current replace function does not allow to have more constraints. In most of the projects such a scenario is seldom. It's more often, that two properties/constraints are needed to find specific resources. The motivation is, that this change leads to a more reusable pipe.

How Has This Been Tested?

The code was built and deployed to a local AEM 6.5.7 instance (only scripts package). The pipe has been run via curl command:

curl -u admin:admin http://localhost:4502/apps/dx/scripts/libs/replace.json -X POST -v -F bindings="{'root':'/content/experience-fragments','type':'cq:PageContent','property':'sling:resourceType','oldValue':'project/components/structure/xfpage','newValue':'project2/components/structure/xfpage', 'criteria':'cq:template', 'criteriavalue':'/conf/project/settings/wcm/templates/experience-fragment'}" -F dryRun=true

Screenshots (if appropriate):

Types of changes

Checklist:

Ben-Zahler commented 2 years ago

@luckyluke-adobe looks perfectly fine to me, but IMHO this is a breaking change as the pipe does not work if the "criteria" parameter is not set.

luckyluke-adobe commented 2 years ago

@luckyluke-adobe looks perfectly fine to me, but IMHO this is a breaking change as the pipe does not work if the "criteria" parameter is not set.

yes, you're right - I've updated the description.

luckyluke-adobe commented 2 years ago

I need to close and reopen the PR, so it get's Adobe CLA signed.

npeltier commented 2 years ago

@luckyluke-adobe why not just remove the property=value predicate? so to sum up the pipe it's traverse | xpath ${criteria} | write ${property}=${value} would look better to me

luckyluke-adobe commented 2 years ago

ok, nice idea 🙂 - would you open the criteria strong such as expr='/jcr:root${root}//element(*,${type})[${criteria}] and give a lot more freedom, or not too strong expr='/jcr:root${root}//element(*,${type})[@${criteria}="${criteriavalue}"]

npeltier commented 2 years ago

we could even have a ref pipe as search that depending on binding switch to a search mode or another, but let's keep things simple for now expr='/jcr:root${root}//element(*,${type})[${criteria}] is good to me (you might have some fun with special characters though)

npeltier commented 2 years ago

thanks for the contribution @luckyluke-adobe !