silverstripe / silverstripe-userforms

UserForms module provides a visual form builder for the Silverstripe CMS. No coding required to build forms such as contact pages.
BSD 3-Clause "New" or "Revised" License
135 stars 225 forks source link

Add Extra Condition Options in EmailRecipientCondition #1274

Closed sukhwinder-somar closed 6 months ago

sukhwinder-somar commented 6 months ago

Description

Hi,

I am currently using the EmailRecipientCondition DataObject in my project and I've found that I need an additional option in the ConditionOption Enum field. The current options are:

IsBlank IsNotBlank Equals NotEquals ValueLessThan ValueLessThanEqual ValueGreaterThan ValueGreaterThanEqual

I would like to propose adding an "Includes" option to this list. This new option would allow users to check if the selected field includes a given text string, providing more flexibility when setting conditions for email recipients.

We will also need to update the matches function to filter result based on new Option.

I believe this enhancement could benefit other users as well who might need more diverse conditions for their email recipients.

Please let me know if this is something which we can add to code then I will create a new PR for it....

Thanks

Additional context or points of discussion

No response

Validations

PRs

michalkleiner commented 6 months ago

That sounds like it could be a useful addition, so if you have time to create a PR we can look at it then.

sukhwinder-somar commented 6 months ago

@michalkleiner - sweet, I have made the changes but while doing this, I did thought about changing it to be a Varchar and then add ability to add more options through extension by adding extends in matches function, so in future we won't have to create new PR. Please let me know if that's something you be interested in

https://github.com/silverstripe/silverstripe-userforms/pull/1275

michalkleiner commented 6 months ago

Yeah, in general I'm not a big fan of enums for that exact reason (not extensible) but I'd keep the scope of this change to adding the Include option and leave other changes for a separate issue/PR.

GuySartorelli commented 6 months ago

PR merged. This will be included in the October minor release.

sukhwinder-somar commented 6 months ago

@GuySartorelli - October ??, can't we do it any earlier

GuySartorelli commented 6 months ago

It has unfortunately missed the cut-off for new features in the April minor release. The next minor release window after that is in October as per the minor release policy.

sukhwinder-somar commented 6 months ago

sweet, all good.

I am trying to pull the latest of branch 6 with this - composer update silverstripe/userforms:dev-6#32d0158 -W but it doesn't seems to work, any idea what am I doing wrong here ??

error -


  Problem 1
    - Root composer.json requires silverstripe/userforms dev-6#32d0158, found silverstripe/userforms[dev-dependabot/npm_and_yarn/ip-2.0.1, dev-dependabot/npm_and_yarn/es5-ext-0.10.64, dev-master, dev-5.14-release, dev-pulls/5.8/second-page-intial-visibility-hide, 0.5.1, 1.0.1, 1.1.0-beta, 2.0.1-rc1, ..., 2.0.10, 3.0.0-beta1, ..., 3.1.2, 4.0.0, ..., 4.6.x-dev, 5.0.0-beta1, ..., 5.x-dev, 6.0.0-beta1, ..., 6.x-dev, 7.x-dev] but it does not match the constraint.
GuySartorelli commented 6 months ago

Try replacing dev-6 with 6.x-dev (see composer docs about branch constraints)

sukhwinder-somar commented 6 months ago

thanks, that helped but couldn't pull it as we are still on PHP 8.0.... :sob:

GuySartorelli commented 6 months ago

That must mean you're also on Silverstripe CMS 4.x - you'll need to update to 5.x to use this regardless of when we release it.

If that's not in the cards for your project right now I'd recommend making this change in a fork off the 5.15 branch until you're ready to upgrade.

sukhwinder-somar commented 6 months ago

hmm, I might will have to do that, thanks for the direction