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

Added lead save event inplace of removed old Hook storeLeadsData. #149

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

Conversation

tsarma
Copy link

@tsarma tsarma commented Dec 4, 2023

Before version 3.0, we had a storeLeadsData Hook, which was called when a lead was added. I am using this very often to modify or add some project-specific metadata to the leads. Since this hook was removed in the newer version > 3.0, I am proposing to add this pull request. With this new Event, which is dispatched inside the class ProcessFormDataListener, we would be able to modify data after the lead is saved.

@aschempp
Copy link
Member

aschempp commented Dec 5, 2023

we've recently discussed this on Slack, and you told me you actually need a custom export formatter, not an event. What has changed?

@tsarma
Copy link
Author

tsarma commented Dec 5, 2023

Export formatter, was another case, where I wanted to change the lead data during the export. That one could be solved by defining a custom exporter.

This pull request is an Event, which is triggered on saving the lead data, whereby giving a chance to manipulate or even add some custom data to the lead. Earlier I used the hook storeLeadsData for such purpose, which is now removed.

My use case:
During the discussion in Slack, I told you that I have a small game, which is basically a Contao form, and when a person submits, a random Sofort Prize is awarded. This prize value is stored in the leads. On that, you remarked that one can edit the HTML and assign a different prize. With this proposed event, I can assign the random Prize value, despite whatever comes in the POST value (user submits) and also I am adding a value to some custom fields defined to tl_lead in my own dca.

@aschempp
Copy link
Member

aschempp commented Dec 5, 2023

The issue I have with this event (and why nothing currently exists in leads v3) is that manipulating leads data always feels wrong to me. For example, you cannot export any data (additional columns) that are manually added to the lead. So it only ever makes sense to store whats in the form.

In your case, I think you should have a custom form field for the price. It might or might not generate a hidden field in the front end, and it should return the value of a price when the form is submitted ($widget->value). This value would then be available in the actual form data, possibly available to notification center or any other form extension and be stored in leads as you would expect. Using a leads hook/event would bypass the correct form handling in Contao …

@fritzmg
Copy link
Contributor

fritzmg commented Dec 5, 2023

We also frequently need this - but having to add a noop hidden form field just so that the additional data can be exported is quite cumbersome, so I wanted to create my own PR for this feature for Contao Leads.

@tsarma
Copy link
Author

tsarma commented Dec 5, 2023

I can understand your thinking that adding data to the lead itself, which can not be exported is wrong. To export you've to have a form field that can be hidden or not. But it may be cumbersome to add hidden fields always when we can do it from a script provided there is a HOOK.

With this Event, my primary aim is to add some meta-data to the leads itself.
In my case, I am using it to store some token values to validate the email and another field for url slug, to show details of the prize won. There may be other valid use cases, so I would appreciate adding this Event if it does not break anything.

@fritzmg
Copy link
Contributor

fritzmg commented Jul 5, 2024

You could add another event for the export so that additional fields can be added to the export - for any custom data added via this event.

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

Successfully merging this pull request may close these issues.

3 participants