Propose new Mautic features
#MauticRoadmap Building Mautic together!
Has your proposal been discussed on the Mautic Forums already?
Yes, multiple times. Here is a thread with one of the most recent replies: https://forum.mautic.org/t/merge-custom-fields-group-bundle-to-mautic-core/28960/4
Is your feature request related to a problem? Please describe.
Some Mautic users, would like to have additional field groups as opoose to just the default ones.
Describe the solution you'd like
At the moment you can add additional field groups by doing some coding. I would like to eliminate the coding part, I want user to be able to add additional field groups into Mautic without a need for a developer.
Describe alternatives or workarounds you've considered
An alternative to this bundle is that you extend the form type, but you need to hire a developer to handle that for you.
Additional context
As an author of the plugin, I can say that it felt like plugin really helped out some people and provided a much needed feature.
I would also like to point out that the plugin works and most of the work has already be done, so I can start with the merging to core pretty quickly.
I would kindly ask the team, in the event that this actually passes the proposal stage, that someone answers to my questions expressed in the forum post here: < https://forum.mautic.org/t/merge-custom-fields-group-bundle-to-mautic-core/28960/4?u=mzagmajster >.
Thank you.
Does this issue could impact on users private data?
No, I do not think so.
Funded by
Project funded by Adra Cloud, coded and maintained by Matic Zagmajster
Estimated cost
It'd be great to try to get this into the 5.2 release, if it's not a BC Break.
Project funded by Adra Cloud, coded and maintained by Matic Zagmajster.
This proposal has been accepted because:
Thanks for all the discussion and debate, this feature will be great to bring into Mautic.
Proposal appearing in these projects:
Report inappropriate content
Is this content inappropriate?
Comment details
You are seeing a single comment
View all comments
Conversation with John Linhart
I have demoed this plugin to the Product team at Acquia. In short, the idea is great and our customers would welcome the custom field groups feature.
I don't mind if it would be a core plugin, a core bundle or part of the LeadBundle.
We've found many issues during the demo. Some of them:
1. If you delete or even just rename a group, the contact view or edit pages throws 500 error.
2. The un/Publish feature doesn't work. Is it necessary?
3. If a group is deleted, those fields are not shown anywhere. Perhaps they should appear back in the Core group?
4. If there is no field group or is deleted (can't remember), an empty menu item shows up. It shouldn't show up at all.
5. There are no tests
6. There seem to be an API implemented but it's not documented
In summary, the current implementation is very fragile, but if it would get to a production ready state, we'll be glad if it would be added to the core.
Hi @escopecz , thanks for the feedback. I will look into detail for each point, here is my take at the moment.
& 4. More investigation needs to be done here but what you described might be happening because translation string is generated based on the name you input. We could add ability to override the translation string. The second thing is in prod. env. translator loader is using cache, so that might be way you are experiancing some of the issues. When you rename or delete the group I would also clear the cache.
The idea of unpublish / publish feature initially was to enable user to hide the entire group from the GUI temporarly, but encountered some challenges with that so at the moment features only come to the effect when using an API. Meaning if the group is unpublished its not gonna show in the results. Why did I have issues with the feature from the GUI side I cannot remember anymore, but I will investigate and let you know.
Good point, although I have to say I am against doing this implicitly. I suggest we add ability to transfer fields from one group to another and only allow to delete group if there are no fields in that group. Does that sound reasonable?
Tests are here: < https://gitlab.com/adra-network/mautic-field-groups-plugin/-/tree/dev_mautic5/Tests?ref_type=heads >, perhaps you want me to add some?
API docs are here:< https://gitlab.com/adra-network/mautic-field-groups-plugin/-/tree/dev_mautic5/docs?ref_type=heads >, is there a better place / format to use?
As for the where to place the functionality. I think its best if this is a a separate core bundle, please do know that we would also need to extend stuff in CoreBundle, specifically here: app/bundles/CoreBundle/Translation/Translator.php and here: app/bundles/CoreBundle/Loader/TranslationLoader.php.
All good! The gist is that the feature must be resilient. It cannot have features that do not work (un/publish) and cannot break UI when a group is deleted or renamed. Clearing the cache is not an option for a user-generated content. It must just work.
Hi @escopecz , I have looked over your notes and tested things see my feedback below.
I was not able get the 500 when I renamed the group even though I tested in production env. What does happen is that when you rename the group, it does not actually rename the group, what it does is actually just updates the entry in the database where all custom groups are listed.
What happened when I tested it is that the name of the group on the contact was not translated anymore, that is because the group itself still exists and containts the fields - just translation for it does not exist anymore.
Why you might have encountered the 500 is because of cache. This is fixable (see my plan below).
The reason un/publish feature is not fully utilized is because at the moment bundle is created in a way that it does not touch the lead or company templates when they render fields, this design allows us to have field group bumdle as standalone component that you can remove anytime.
If I would want to utilize that fully and not show inactive groups I would need to touch those templates. I suggest we remove it from the UI, but leave it in database for now.
I would disallow deletion of group if the fields are in the group and an ability to mass transfer fields from one group to another.
This is related to the fact that when you delete a group in this plugin you are not removing the group itself, but rather you remove translation for that group.
In regards to cache my idea is to clear the translations cache and then when you actually delete a group or rename it will show in the GUI when you refresh it, because translations cache will be regenerated.
The other option would be to completely disable the cache like its done for dev and test env. but I do not want to do that.
In addition to a simple text file, I also added openapi spec. so you can copy it in swagger editor and get a nice looking documentation.
Here are the tasks that I see need to be done in order to address issues you mentionted, let me know what you think:
Clear cache on newStandard, editStandard, deleteStandard or disable cache completely (not a good idea)
Remove the publish/unpbulish feature from GUI
Allow to transfer fields from one group to another
Only allow to delete a group if no fields are in the group (also display number of fields in the group)
Update Tests
Best, M.
All of it sounds good!
The only thing I don't understand is the translation and cache. Why do we use translation for user-generated strings? We don't do that in other parts of Mautic, right? How would users translate the groups names if the group names should be translatable?
And, do we need aliases? IDs aren't good enough?
So the way core field groups are done now is with the translations. Esentially you have something like this:
[
'mautic.lead.field.group.core' => 'core',
'mautic.lead.field.group.social' => 'social',
'mautic.lead.field.group.personal' => 'personal',
'mautic.lead.field.group.professional' => 'professional'
]
You see the key in the above array is actually translation string, so when I was writing this plugin I tried to build it in a way so that it does not against existing code base, and instead tries to reuse as much as possible and does not touch things you do not absolutely have to touch. If I would not use translation approach I would have to modify templates as well where these groups are used.
So when you enter the field group in the plugin form, plugin actually takes what you enter as a display name and behind the scenese generates translation string similar to the ones you see above.
How would users translate the groups names if the group names should be translatable?It is true that at the moment you can only add one translation for the group, but if there is interest we could add ability to the plugin to add more the one translation. That being said, I am not sure this adds much value since you do not really translate the field groups (by that logic you would also want to translate the custom fields labels, you just do not).
And when we use translations we also use translation cache, that is why the tasks related to cache were listed above.
And, do we need aliases? IDs aren't good enough?Alias is something that I took from custom fields :), but I guess, I could make it work with the IDs (would have to check though, to not have any surprises). So it would look something like:
[
'mautic.lead.field.group.1' => '1',
]
Thanks for the suggestion. One thing to keep in mind is that when you reference field groups in custom plugins for example, like when you update the field, the code is easier to read if you have some descriptive name as oppose to just an ID (but that is just from the perspective of development of custom functionality).
For example:
$lead->getFieldValue('countryiso, 'mygroup');
looks way better, then:
$lead->getFieldValue('countryiso, '1');
Updated tasks based on your notes:Clear cache on newStandard, editStandard, deleteStandard or disable cache completely (not a good idea)
Remove the publish/unpbulish feature from GUI
Allow to transfer fields from one group to another
Only allow to delete a group if no fields are in the group (also display number of fields in the group)
Update Tests
Do we use translation approach (and do not touch lead/company templates, but we need to take care of the cache) or do we leave the translations out and update the templates?
My personal take is that I do not mind touching templates, but we should use some sort of cache to load this records due to performance, since field groups is something you typically setup once and then just use it, I do not think field groups change that often.
But I will do, what you decide is best :).
We are going deeper into the implementation details, but I'd suggest to dispatch events in the core code where you need them for your plugin to work without any hacks. For example I'm looking at https://gitlab.com/adra-network/mautic-field-groups-plugin/-/blob/main/EventListener/CustomTemplateSubscriber.php?ref_type=heads and this would be a no go that a plugin is replacing core templates with some other. This will eventually break things and it will be hard to debug.
In https://gitlab.com/adra-network/mautic-field-groups-plugin/-/blob/main/Model/FieldGroupModel.php?ref_type=heads I can see that you translate all groups. Why not translate only the default ones?
On the main branch, there is Mautic 4 version of this plugin, and there was a change to the approach in Mautic 5 version, so lets just look into that and leave out the old version:
https://gitlab.com/adra-network/mautic-field-groups-plugin/-/tree/dev_mautic5
I overrided the translation loader, the coresponding configuration form type (and some templates in Mautic 4) to actually get this plugin to work. Ideally if we are talking about merging to core I would not override as much as I would replace the classes that need to replaced, same goes for the templates. If any modification would be needed I would update the core templates not override via plugin.
Why am I translating all the groups?
Because Mautic core dictates the pattern that field groups need to follow in order to show propely in the UI (somewhere in templates and forms translation string base is hardcoded).
Honestly if you are willing to put an additional event into the core and dispatch it at the right places that would be one of the better ways to handle this and I do not think we need to override any translators.
My suggestion;
Put everything into LeadBundle (db entity, api endpoints, standard controllers for gui).
Dispatch event LeadEvents::FIELD_GROUP_LIST_ON_GENERATE before displaying (in controller most likely) for the following templates:
https://gitlab.com/adra-network/mautic-field-groups-plugin/-/blob/main/Views/Field/list.html.php?ref_type=heads
https://gitlab.com/adra-network/mautic-field-groups-plugin/-/tree/main/Views/Lead?ref_type=heads
Create FieldGroupEvent, with the following implementation:
getGroups()
setGroups()
Change the templates so groups are used without hardcoding any translation strings.
Remove the publish/unpbulish feature from GUI
Allow to transfer fields from one group to another
Only allow to delete a group if no fields are in the group (also display number of fields in the group)
Update Tests
Elemnets we do not need anymore:
We do not need this: https://gitlab.com/adra-network/mautic-field-groups-plugin/-/blob/main/Form/Type/FieldGroupExtensionType.php?ref_type=heads (since we will adjust the type in the core).
We do not need this anympre: https://gitlab.com/adra-network/mautic-field-groups-plugin/-/tree/dev_mautic5/Translation?ref_type=heads (since field groups will have nothing to do with the translations when we are done)
Let me know if you are OK with this approach or is there something you would like to change / add. Thank you.
Loading comments ...