Restore term clicking AJAX functionality

merge request created.

Uploading patch file for composer patches.

Tested on 10.4. Ajax functionality is back, but styling for media library and clicking media library is broken.

One of the reasons Ajax was removed in 🐛 AJAX error when editing term content Postponed: needs info is that it stopped working with media libraries and other Ajax dependent module.

I would recommend a solution similar to views where we can toggle ajax / not ajax in config.

I concur with @klausi’s assessment of the critical nature of this issue, as the AJAX functionality is central to the module’s purpose.

After investigating the problems with AJAX interactions in media libraries and the non-functional “Add another item” button (which appears when a field has unlimited cardinality), I found that initiating the AJAX call to render the term edit form directly in JavaScript resolves these issues.

I plan to conduct further testing and clean up the code next week when I have more time.

Currently, both the AJAX that renders the term edit form when clicking in the tree and the AJAX and styling within the term edit form are functioning well.

Thank you @klausi for the feedback! Having someone like you help maintain this module would it greatly.

I cleaned up the code, addressed the concerned in the MR and checked for code quality warnings.
For me it works well now. The only thing missing is that before the term ID was added to the URL. I think this should be a follow up issue, but let me know.

Thanks a lot, nice progress!

Left some more small comments on the PR that we should address.

Thank you, I addressed the comments. I think we need to discuss further how to handle having the tid in the URL.

Thanks!

One issue I found is that when you save a term it redirects you to the term page. But I would expect that I stay in Taxonomy Manager. Should be fixable by setting the correct form state redirect.

I did not test the media library integration - does that work as well that there are no AJAX errors? Then we could avoid introducing a setting to switch on/off Ajax mode as @vladimiraus suggested.

Good point! I was able to fix the redirect issue by overwriting the redirect in the save function of the TaxonomyManagerTermForm. Let me know if you have a better way of doing that.
Also, I removed an unused function (taxonomyTermSubmitHandler) from the TaxonomyManagerForm class.

I did some more testing with media library integration and it works well for me now. In addition I also checked that you can still use the 'Add another item' button (if you have a field with unlimited amount of values allowed) without it causing any AJAX errors.

This should mean that we don't need a setting to enable/disable AJAX.

Nice, looks good to me! Please fix the coding standard issues, otherwise I think we are ready to merge. The CI pipeline is failing, but that is independent of this issue and we can ignore here.

Marking this as RTBC.

@vladimiraus I would wait a couple of days if there are any objections and then merge this.

Thanks again @klausi for all the help on this! I fixed the two coding standard issues you noticed.

I did do some more testing with a user that doesn't have edit term permissions and noticed that when clicking on terms in the tree it will cause an ajax error. I added some more logic to make sure that the ajax request is only send to the server if the user has the edit term permission. In addition, I noticed console errors when a user doesn't have the delete term permission, which I also fixed.

I've tested MR - it looks like it's working with media as required.

I think it still needs more testing - especially for security/permissions.

@torfj - what were your test cases for term permissions?

@jannahka I tested with a user that only has permission to edit terms in a specific vocabulary.
I also tested with a user that has the administer taxonomy permission which allows a user to edit any taxonomy term.

Thank you!

Looking good to me, I think we are ready to commit.

Thanks again @klausi for working with me on this! I added the missing title.

Please note the following issue can also be closed once the changes has been committed: #3491197 📌 Wrong Toolbar position after updating to 2.0.16 Active

Merged manually, thanks!

Will fix the broken pipelines in a follow-up.

klausi → closed merge request !64

Automatically closed - issue fixed for 2 weeks with no activity.