#3046752: AJAX error when editing term content removed the core feature of Taxonomy Manager: to be able to quickly navigate a taxonomy with an AJAX user interface. This avoids unnecessary full page loads.
Some users reported AJAX errors, but for other users the AJAX functionality is working normally and a very important feature of taxonomy manager.
Therefore I think this issue is critical.
Install taxonomy manager and go to https://jd.ddev.site/admin/structure/taxonomy_manager/voc/tags . Click on a term. Note that the browser reloads the page on every click making navigating and inspecting terms extremely slow.
Restore the AJAX functionality.
Review merge request and agree on the approach.
Quick AJAX navigation between terms is restored.
None.
None.
| Comment | File | Size | Author |
|---|
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !64
Comments
Comment #3
klausimerge request created.
Comment #4
klausiUploading patch file for composer patches.
Comment #5
vladimirausComment #6
vladimirausTested 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 #3046752: AJAX error when editing term content is that it stopped working with media libraries and other Ajax dependent module.
Comment #7
vladimirausI would recommend a solution similar to views where we can toggle ajax / not ajax in config.
Comment #9
torfjI 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.
Comment #10
torfjComment #11
torfjThank 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.
Comment #12
klausiThanks a lot, nice progress!
Left some more small comments on the PR that we should address.
Comment #13
torfjThank you, I addressed the comments. I think we need to discuss further how to handle having the tid in the URL.
Comment #14
klausiThanks!
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.
Comment #15
torfjGood 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.
Comment #16
klausiNice, 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.
Comment #17
torfjThanks 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.
Comment #18
jannakha commentedI'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?
Comment #19
jannakha commentedComment #20
torfj@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.
Comment #21
klausiThank you!
Looking good to me, I think we are ready to commit.
Comment #22
torfjThanks 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
Comment #23
torfjComment #25
klausiMerged manually, thanks!
Will fix the broken pipelines in a follow-up.