Revolution: Form customization for BOTH update and create

[ncrossland]

ncrossland created Redmine issue ID 3772

It would be useful if there was the ability to create a rule for BOTH the update and creation of resources - it's very rare I'd want to do something different, and would remove a lot of duplication of effort in setting up every rule twice.

[gerritvanaaken]

gerritvanaaken submitted:

I can second that!

[modxbot]

rethrash submitted:

+1

[splittingred]

splittingred submitted:

While nice in theory, this does not work pragmatically - constraints are _impossible_ to apply without separated create/update rules.

If I set a constraint of 'template' => '4', one would assume that means don't do FC rules except on Resources with Template of 4.

However, on a 'Create Resource' page, no new resource has a template of 4 (unless done via a default_template setting or another FC rule). So, currently, FC gets around this by having Constraints apply to the parent of the newly made Resource. This could not be done without separation of update/create.

[ncrossland]

ncrossland submitted:

I don't know it's impossible - when I built ManagerManager for Evo I overcame this so only one rule was created which applied to both create and update.

Sorry I'm not that familiar with the Revo codebase yet, so apologies if I'm misunderstanding, but logically, if I create a set of FC rules for Create, and then duplicate it exactly but apply it to Update instead, then the same rules are being called for both create and update?

In your example there is a point where the code has concluded that the current template should be considered to be 4 (whether that's the actual template of an existing resource, or derived by deduction from a default_template or parent). This is where it decides whether to apply the rules or not?

[gadamiak]

gadamiak submitted:

Nick Crossland wrote:

In your example there is a point where the code has concluded that the current template should be considered to be 4 (whether that's the actual template of an existing resource, or derived by deduction from a default_template or parent). This is where it decides whether to apply the rules or not?

I would expect it work that way too. +1

[adamwintle]

I definitely think this is something that needs to be revisited and thought about again. Having an option for "Update and Create" would be a very useful time saver.

[argnist]

I think grid with rule sets should be editable. So workflow would be: add rule for resource/create, copy it, click on copy's action column and change to resource/update.

[gerritvanaaken]

Changes after copying must be performed on both rule sets, which is a real pain.

[argnist]

@gerritvanaaken you can copy again after change :)

[gerritvanaaken]

You know what I mean, come on ;-)

[neoneddy]

+1 on this.

[gpsietzema]

+1 from me as-well.

There's an important thing though: can there be situations where you want different rules for create and update? I can't think of any right now. But if there are, this isn't a good idea.

Any ideas?

[johanmolen]

+1

[OptimusCrime]

Please take a look at answer given earlier:

splittingred submitted:
While nice in theory, this does not work pragmatically - constraints are impossible to apply without separated create/update rules.
If I set a constraint of 'template' => '4', one would assume that means don't do FC rules except on Resources with Template of 4.
However, on a 'Create Resource' page, no new resource has a template of 4 (unless done via a default_template setting or another FC rule). So, currently, FC gets around this by having Constraints apply to the parent of the newly made Resource. This could not be done without separation of update/create.

I think is good enough reason to close the request, unless someone with good knowledge in the implemenation can tell if doing a workaround could be possible without redoing too much of the code. I have never looked at how FC is implemented "under the hood", so I can't tell.

[gpsietzema]

That's not a reason not to do it. If a rule has a constraint for template==4 and you're creating a resource where template==4 doesn't apply, it just doesn't work. That's that.

There is a possibility to build in something like template inheritance. We use a plugin for this: when creating new resources it checks a system setting for template inheritance, based on the parent template.

This could be something for a new MODX release I guess. We'll how we can implement this.

[isaacniebeling]

I'm with @gpsietzema on this one... If it doesn't apply, it doesn't apply, but keeping update and create rules in sync has had me tearing out my hair on more than one occasion. As long as FC is (one of) the last bits of JS to fire on resource load, it should be able to cover just about any constraint the same on Create as on Update.

[SnowCreative]

Wouldn't it be easy enough to just add a third option? "resource/create", "resource/update", and "resource/create or update"? One would assume that programmers won't be trying to add constraints that don't apply if "resource/create or update" is the selected option. Then, when the manager is grabbing customizations to alter the layout, it would, for example, look for (resource/create OR resource/create or update) instead of just (resource/create), or however this works behind the scenes. This problem is near the top of my list of headaches in MODX. I've never, EVER, needed to use different setups for creating and updating resources in the hundreds of websites I've created, and it's a pain to have to edit two different customization settings every time I need to update an existing manager customization.

There MUST be a way to do this, using some creative thinking. As for constraints, if this is really such a problem, how about having TWO constraints fields, one for create and one for modify? Then MODX would use the appropriate one. Or, as gpsietzema said above, if an entered constraint doesn't work, it just won't do anything, so what's the problem?

[Mark-H]

The Form Customization is not really a simple feature, so while it's easy to say how it should work, it's a bit tricky to implement.

I do agree that if possible it should be a new option ("both") as that sidesteps backwards compatibility issues and still allows different options for create/update for those that want it.

Maybe I'll take a look later when I'm waiting for my flight to see what I can do...

[Mark-H]

Some notes from my looking into this... for each rule (stored as modActionDom object, cause naming logic) the for_parent column is being set to 1 along when controller is resource/create. The modManagerController->checkFormCustomizationRules method filters rules on both the controller and the for_parent, but I've not really found any particular reason for this for_parent column in the source. It's only used in the loading of rules as far as I can tell, and also not exposed in a UI anywhere.

I'm thinking that we should get rid of the for_parent check, and instead should change the filter so it can do a IN (resource/create, both) type filter instead.

[Mark-H]

Managed to get it to work! #13775

Testers would be much appreciated ;)

[SnowCreative]

I've got a non-production site right now I can test this on.

[SnowCreative]

This seems to be working just fine! I wonder, though, why you chose to use a wildcard ('/*') rather than something like "/both". I tested your way, then changed to "/both" throughout and that seems to work just as well, plus I think it's nicer to see 'resource/both' in the Action column when viewing the list of customizations rather than 'resource/*'.

I suppose we could change the view for the customizations list to show the lexicon translation of the action rather than the raw value, so then people could call this whatever they want to see there.

[Mark-H]

The list of sets is already updated to show the lexicons, unless we're talking about a different page?

I went for the wildcard instead of /both as it is technically a wildcard. resource/* will also match a resource/foo action, if that would exist.

[SnowCreative]

I'm talking about this screen (note the first row):

we could say "resource/all" instead of "resource/both"

[Mark-H]

That's already switched to lexicons. Perhaps you're seeing cached behavior. Here's what it should look like:

[SnowCreative]

Hmm. Cleared all my caches multiple times. Maybe I missed one of the file updates.

FYI, line 196 of core/model/modx/modformcustomizationset.class.php still has " 'action' => $this->get('action'), " instead of " 'action' => $baseAction, ". Is that intentional?

[SnowCreative]

Yup, missed the manager grid file. Updated that, and all is well now. This is great! There will be much celebrating in the streets . . .

[alroniks]

Closing as related PR was merged.