Problem/Motivation

Currently, a blank parameters array is passed into $form_state->setRedirect(), which doesn't allow dynamic routes.

Getting raw parameters from RouteMatch will allow dynamic routes and at the same time does not affect routes without required parameters.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

None

API changes

\Drupal\views\Plugin\views\field\BulkForm constructor changes. See https://www.drupal.org/node/3115868

Data model changes

None

Release notes snippet

Issue fork drupal-2901412

Command icon Show commands

Start 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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

grathbone created an issue. See original summary.

grathbone’s picture

Status: Active » Needs review
FileSize
846 bytes

Populated route parameters from raw parameters of current RouteMatch and passed the array into $form_state->setRedirect()

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

grathbone’s picture

Version: 8.5.x-dev » 8.6.x-dev
grathbone’s picture

Rerolled for changes in 8.5

grathbone’s picture

Component: system.module » views.module
Lendude’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

This sounds more like a task or a feature request. Not sure what is broken here?

We'd need some kind of test to make sure this does what we want it to.

+++ b/core/modules/views/src/Plugin/views/field/BulkForm.php
@@ -376,7 +376,8 @@ public function viewsFormSubmit(&$form, FormStateInterface $form_state) {
+        $parameters = \Drupal::routeMatch()->getRawParameters()->all();

We'd want to use dependency injection for this.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

claudiu.cristea’s picture

Title: Pass current RouteMatch parameters to $form_state->setRedirect » Pass current route parameters to the confirmation form route
Assigned: grathbone » Unassigned
Category: Bug report » Feature request
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
12.5 KB
12.58 KB

Fixed #7 and added tests.

Status: Needs review » Needs work

The last submitted patch, 11: 2901412-11.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

claudiu.cristea’s picture

Status: Needs work » Needs review
FileSize
5.21 KB
17.21 KB

Fixing the unit tests & coding standards.

Lendude’s picture

Status: Needs review » Reviewed & tested by the community

So the actual change comes down to this:

+++ b/core/modules/views/src/Plugin/views/field/BulkForm.php
@@ -435,7 +452,8 @@ public function viewsFormSubmit(&$form, FormStateInterface $form_state) {
-        $form_state->setRedirect($operation_definition['confirm_form_route_name'], [], $options);
+        $route_parameters = $this->routeMatch->getRawParameters()->all();
+        $form_state->setRedirect($operation_definition['confirm_form_route_name'], $route_parameters, $options);

Nice test coverage, don't see any nits to pick.

This would be hard to accomplish in contrib, so it makes sense to me to add this to core. Just not sure if it is 8.9.x eligable.

pfrenssen’s picture

Patch looking good, RTBC+1.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

xjm’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Thanks for your work on this!

The patch doesn't apply to 9.1.x, so we need a new version. Thanks!

ravi.shankar’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
17.17 KB

Here I have done patch #13 on Drupal 9.1.x.

Status: Needs review » Needs work

The last submitted patch, 18: 2901412-18.patch, failed testing. View results

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

claudiu.cristea’s picture

Status: Needs work » Needs review

claudiu.cristea’s picture

Version: 9.5.x-dev » 10.0.x-dev

saidatom made their first commit to this issue’s fork.

idimopoulos’s picture

Just a note here for someone that is blocked by it, the patch does not apply to 9.3.14 after https://www.drupal.org/project/drupal/issues/2917239 was merged which conflicts with the current patch.

Anyone that is on 9.3.14 should include the patch in https://www.drupal.org/project/drupal/issues/2917239 too.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
FileSize
150 bytes

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

claudiu.cristea’s picture

Status: Needs work » Needs review
pfrenssen’s picture

Status: Needs review » Reviewed & tested by the community

Looking good, patch has been updated to work with D10, tests are passing.

quietone’s picture

Version: 10.0.x-dev » 11.x-dev
Issue summary: View changes
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record updates

I'm triaging RTBC issues. I re-read the IS and the comments. I didn't find any unanswered questions.

I read the patch (not a review) and found that the version numbers for the deprecation message are incorrect. I am setting to NW for that. The same is true for the Change record. I was thinking the title should have more details. More like this, "\Drupal\views\Plugin\views\field\BulkForm::__construct now requires \Drupal\Core\Routing\ResettableStackedRouteMatchInterface" Although, PHPStorm is complaining that
ResettableStackedRouteMatchInterface does not match the declaration.

The diff applies to 11.x but there is no MR against 11.x. So, we should get it testing on 11.x.

I added the standard template to the IS in case it was needed. Setting to NW for the above.

quietone’s picture

Title: Pass current route parameters to the confirmation form route » Add current route parameters to the confirmation form route

I think add is better, since this is adding a new argument.

claudiu.cristea’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs change record updates

Thank you:

  • I've opened a new MR against 11.x: https://git.drupalcode.org/project/drupal/-/merge_requests/4671. Still keeping the old MR for sites currently on Drupal 10.1
  • I've updated the CR's branch & version but I don't think the title should contain such full-namespaced method names because it would make it hard to read. But if the core committer disagrees, they can fix this when they publish the CR.
  • I think add is better, since this is adding a new argument.

    The main scope of this issue is not to add a new argument to that constructor but to forward/pass the bulk form route parameters to the confirmation form. However, I think using "add" would not make any difference.

Moving back to RTBC.

alexpott made their first commit to this issue’s fork.

alexpott changed the visibility of the branch 2901412-pass-current-route-9.1.x to hidden.

alexpott changed the visibility of the branch 2901412-pass-current-route-d10 to hidden.

alexpott changed the visibility of the branch 2901412-pass-current-route to hidden.

alexpott changed the visibility of the branch 2901412-pass-current-route-d10.1 to hidden.

alexpott’s picture

I've changed the deprecation to be 10.3.0 as this has missed 10.2

alexpott’s picture

Fixing issue credit

alexpott’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

Committed 79a07a4 and pushed to 11.x. Thanks!

  • alexpott committed 79a07a4e on 11.x
    Issue #2901412 by claudiu.cristea, pfrenssen, grathbone, ravi.shankar,...

Status: Fixed » Closed (fixed)

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