Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
#1938980: Fix Ajax system; the last remnants of the old API must be swept away went into various troubles with using '#type' => 'ajax', so let's replace all of the places to use AjaxResponse directly.
Comment | File | Size | Author |
---|---|---|---|
#14 | interdiff.txt | 12.62 KB | mkadin |
#14 | drupal_remove_old_ajax_1957590_14.patch | 47.32 KB | mkadin |
#12 | drupal_remove_old_ajax_1957590_11.patch | 43.2 KB | mkadin |
#8 | drupal-1957590-8.patch | 20.68 KB | dawehner |
#8 | interdiff.txt | 500 bytes | dawehner |
Comments
Comment #1
dawehnerLet's see what the testbot tells us.
Comment #2
tim.plunkettThis is one of the times you should probably just have
use Drupal\Core\Ajax;
and then havenew Ajax\AjaxResponse();
inline. Drupal\views_ui\Form\Ajax\ViewsFormBase does this with Drupal\views\Ajax\*Comment #4
andypostLet's get rid of ajax_command_*() here. I think it's right way to make sure that nothing is broken
Comment #5
effulgentsia CreditAttribution: effulgentsia commentedFWIW, +1 to removing #type => 'ajax'. That was a D7 hack/abuse of render arrays, because we had no alternative. In D8, AjaxResponse is the better API.
Comment #6
dawehnerI'm wondering how much these tests are actually helpful now, because they basically test now whether new Object()->render == new Object()->render(), this doesn't seem to be that helpful.
I let some of the functions in there, as they are used, so we can remove them once all procedural code can be finally removed.
ajax_prepare_response() returned "An error occurred while handling the request: The server received invalid input." when something set an error messages. That's no longer true, as when you have an error you should handle the error all in your custom AlertCommand object.
Comment #8
dawehnerRemoved one line too much.
Comment #9
andypost#8 is awesome! Suppose to get rid of #ajax we need separate issue
Comment #10
andypostMarked as duplicate #1959574: Remove the deprecated Drupal 7 Ajax API which patch also removes ajax_render()
Comment #11
dawehner#8: drupal-1957590-8.patch queued for re-testing.
Comment #12
mkadin CreditAttribution: mkadin commentedHere's an extension of the patch from #1959574: Remove the deprecated Drupal 7 Ajax API. Crazy how similar it is to #8 without having seen it!
My work went a bit further to delete a lot of the stuff from ajax.inc that isn't in use anymore, but is otherwise the same (as best I can tell).
Comment #14
mkadin CreditAttribution: mkadin commentedComment #15
mkadin CreditAttribution: mkadin commentedThat patch incorporates the feedback from #2 and tries to fix the tests.
To trigger AjaxController, I added the custom ajax mime-type to drupalPostAJAX and drupalGetAJAX, but various tests were using drupalGetAJAX to just get / decode json, as opposed to specific ajax commands. So I created WebTestBase::drupalGetJSON() for that purpose, and applied it where needed.
I'm pretty sure this patch will fail, there are 2 fails in the ajax framework tests that I can't for the life of me figure out. They work in the real world, just not in the test environment...so I'm not sure how to debug.
Comment #17
dawehner@mkadin
Thanks for getting your work from the other issue in, but I'm asking whether we could get #8 in first.
It's nothing complicated compared to your changes, then we can continue working on it here and maybe on #1938980: Fix Ajax system; the last remnants of the old API must be swept away?
Comment #18
mkadin CreditAttribution: mkadin commentedThat's fine. In that case, I'll re-open #1959574: Remove the deprecated Drupal 7 Ajax API as postponed on this issue, and add the rest of the stuff there.
I also see #8 as RTBC.
Comment #19
Crell CreditAttribution: Crell commented+1 to committing #8 and continuing in the linked issue(s).
Comment #20
catchCommitted #8, following the follow-up :)