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

Open up the SendPasswordResetInstructionsAction #5589

Merged
merged 2 commits into from
Jan 12, 2023

Conversation

leleuj
Copy link
Contributor

@leleuj leleuj commented Jan 10, 2023

This PR allows to customize the event returned in case of an invalid contact.

@leleuj
Copy link
Contributor Author

leleuj commented Jan 10, 2023

Backported to: #5590

@apereocas-bot apereocas-bot added this to the 7.0.0-RC4 milestone Jan 10, 2023
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@mmoayyed
Copy link
Member

I understand what you intend to do, but this is unnecessary. We don't need to open this up. All you need to do is write a configurer that executes last and auto-configures the webflow by replacing the right transition with what you need. Extending the class here in fact is a poor choice.

@leleuj
Copy link
Contributor Author

leleuj commented Jan 10, 2023

I was reluctant to touch the webflow as it's always hard to maintain and understand, but if you prefer that way, no problem.

@leleuj leleuj closed this Jan 10, 2023
@leleuj leleuj deleted the resetpwd branch January 10, 2023 12:59
@mmoayyed
Copy link
Member

Ah sorry let me clarify; we can certainly move fwd with this, but you will be forced to extend the class and bring down a number of dependencies into compilation just to modify one line. You could do that for sure, but it's easier to modify the flow without having a dependency on the flow itself. (You could do this in fact with Groovy).

If you find this approach here easier, we could sure merge.

@mmoayyed
Copy link
Member

mmoayyed commented Jan 10, 2023

(Doing this with Groovy is easier also in the sense that the change can be easily removed or skipped later, if requirements change. otherwise you will need to rebuild and recompile again)

@leleuj
Copy link
Contributor Author

leleuj commented Jan 10, 2023

I think you're right: the Groovy script (for such a limited changed) will be the easiest and lightest solution.

@leleuj leleuj restored the resetpwd branch January 11, 2023 07:00
@leleuj leleuj reopened this Jan 11, 2023
@leleuj
Copy link
Contributor Author

leleuj commented Jan 11, 2023

@mmoayyed In fact, changing the webflow (Groovy or not) does not work. I would need to replace the error event by the success event but the error event is returned in several places and I want to change it only when the contact is not found.

I reopened both PRs for merge. Thanks

@mmoayyed mmoayyed merged commit 51309ae into apereo:master Jan 12, 2023
@leleuj leleuj deleted the resetpwd branch January 12, 2023 06:54
@leleuj
Copy link
Contributor Author

leleuj commented Jan 12, 2023

Thank you

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

Successfully merging this pull request may close these issues.

4 participants