modxcms / revolution

MODX Revolution - Content Management Framework
https://modx.com/
GNU General Public License v2.0
1.36k stars 529 forks source link

[bug] Change resource meniundex through drag and drop at the time of resource editing #14299

Closed Ruslan-Aleev closed 5 years ago

Ruslan-Aleev commented 5 years ago

Bug report

Summary

If you edit a resource and at the same time move it in the tree through drag and drop, and then save it, the resource position (menuindex) will remain the same at the time of opening the resource.

However, if you edit a resource and change its parent (by moving through drag and drop to another folder) and then save, everything will be fine with the new parent.

This bug is pretty old, but I did not find it in the list. Customers often complained about this issue.

More comment info - https://github.com/modxcms/revolution/issues/14299#issuecomment-456017453

Environment

MODX 2.7.0

Jako commented 5 years ago

I suggest to remove the menuindex input from the resource edit form by a system setting (that part has to be coded). Until then you could try to remove the field with Form Customisation.

Ruslan-Aleev commented 5 years ago

@Jako I tried to remove the menuindex from the form (through Form Customization), but the problem persists. Here the bug is in the simultaneous resource editing (meniundex can not edit at all) and dragging of the resource in the tree.

sottwell commented 5 years ago

If you drag-and-drop the resource in the tree, it's automatically saved (AJAX) in the database from that operation. However, the open resource editing form is not changed unless the page is reloaded at that point, so when you now save that editing form, the original menuindex value still in the form field gets saved. So how is that open editing form's value to be changed if the resource was dragged in the Tree? Perhaps dragging a resource in the Tree should be prevented if its editing form is open?

sottwell commented 5 years ago

The same thing will happen if you use Quick Edit to edit any part of a resource while that same resource is open for editing. Save the Quick Edit form, then view the page. You'll see that change. Now save the open resource editing page, and look at the page again.

sottwell commented 5 years ago

It's perfectly logical; the last value to be saved will be the one in the database. The only solution is to prevent the Tree quick editing and moving functions if a resource is already open for editing.

Jako commented 5 years ago

Another solution would be to prevent saving the menuindex during editing a resource.

Jako commented 5 years ago

@Ruslan-Aleev Maybe the Form Customization does only hide the input and does not remove it from the form (or it is posted back somehow else).

sottwell commented 5 years ago

I cannot like that. If I want to edit the menuindex, I should certainly be able to. What happens in the case where the user does not have permission to view the Tree, but can edit resources? For example, using the QuickBar front-end editing menu?

Jako commented 5 years ago

As I wrote above, this has to be enabled by an option. Drag&Drop one resource is a lot easier than manually changing the menuindex in a lot of resources.

sottwell commented 5 years ago

Very true. The question is, should these quick and easy Tree-based functions be enabled when the same resource is open for editing? I know that this menuindex issue is perhaps the most obvious, but what about all of the right-click functions in the Tree? Publish, Delete... the same situation applies, what is done in the Tree will be overridden by what is in the same resource's form when it's saved.

sottwell commented 5 years ago

The same thing happens with elements and categories; if you are editing an element that is in category A and drag it into category B in the Tree, then save it, it will be back in category A.

sottwell commented 5 years ago

Files are a bit smarter. Open a file for editing, then move it to another folder, it will throw an error if you now try to save it while the form field has the old path.

Ruslan-Aleev commented 5 years ago

@Jako In addition to removing the menuindex from the resource form, i think, need to correct the script, because by changing the parent resource by drag and drop - there is no such problem. But it's probably more difficult :)

Jako commented 5 years ago

Removing the right click functions in the tree for the current resource should be not enough, since you could drag&drop another resource before/after the current resource and the menuindex of the current resource would be changed too.

Jako commented 5 years ago

Maybe an alert should be generated, when you edit a resource and modify the resource tree at the same time. The same could work, when you edit an element and modify the element tree (i.e. by changing the category).

Ruslan-Aleev commented 5 years ago

@Jako Why changing the parent by drag when editing the same resource there is no such problem? Maybe the alert is not necessary, and we need fix the script, or situation with changing the parent is easier than with menuindex?

JoshuaLuckers commented 5 years ago

I agree with @Ruslan-Aleev.

Jako commented 5 years ago

@Ruslan-Aleev Is parent updated during upgrading a resource? If not, why? If yes, then there should be an issue too.

Ruslan-Aleev commented 5 years ago

@Jako I did not quite understand the question. If you edit the resource and move it in the tree - move it to the folder to the new parent or change the menuindex and save the edit, the parent will update to the one we moved to the tree, but the menuindex will return to the one that the resource had at the moment start editing.

Jako commented 5 years ago

If you edit a resource and save it, values will be posted. There is a menuindex and a parent field in the resource and both fields should post a value and those values are written to the database. If you change the tree (before saving the current resource) by drag and drop, some values are changed in in the database (menuindex, parent). If you save the resource after, the changed fields were overwritten by the values of the current edited resource.

Since there is no issue with the parent field, please look, wether the parent field of the current edited resource is changed during that drag and drop action. That could be the reason, why you don't face the issue in that part.

Ruslan-Aleev commented 5 years ago

Yes, it is. The parent changes in the form of editing the resource, but not the menuindex. So I'm trying to say that it is more correct to do the same behavior for menuindex. The question is not in the Form Customization, or in a Alert window, but in the script. Or are there any restrictions that do not allow it?

p.s. The same behavior is necessary for elements and categories, folders and files, etc., if there are no technical limitations.

Jako commented 5 years ago

Now the issue is clear. I did not know that the parent input of a resource edit panel will change, when you drag the resource in the tree.

There has to be a success listener after the resource is dragged in the tree. Inside of this method an Ajax request for the current edited resource could occur after drag and drop.

Jako commented 5 years ago

Changing the parent combo happens here: https://github.com/modxcms/revolution/blob/2.x/manager/assets/modext/widgets/resource/modx.tree.resource.js#L370-L386