Views exposed form action incorrect when view also has a page display

I can see how this might confuse the routing system. This is a form within a view within an entity, right?

The simply workaround that comes to mind would be to add an eva display option to force the view override_path to the current URL (meaning add a conditional here: https://github.com/ahebrank/eva/blob/8.x-1.x/src/Plugin/views/display/Ev...). I'm assuming this block isn't executing in your case because the page display has set override_path already?

OK, thanks for clarifying. Sounds like this is best left as an upstream issue.

This fix makes sense to me, setting to needs review to trigger the testbot, but this needs test, so needs to go to needs work.

We got a passing fix, nice!
Setting back to needs work for tests.

@jurgenhaas I agree a full integration test for the scenario you describe would be a little elaborate. So keep it simple and focus on the change being introduced in core. I would look into a kernel test that just loads the View, sets $view->override_path and then renders the View. Then check if the rendered HTML contains the correct action.

That should cover the change being introduced here. If you want to do a full integration test for the scenario you describe, that might be better to put into EVA, since there such a scenario might be much more relevant.

I did a quick look but couldn't find a test for \Drupal\views\Form\ViewsForm you can use as a starting point for this unfortunately.

Bit of a nudge, config errors can be tricky to figure out.

This test covers the change to \Drupal\views\Form\ViewsExposedForm, so we still need one for the change to \Drupal\views\Form\ViewsForm right?

Also, it's a good idea to add an interdiff when you post a patch, makes reviewing much easier. Look here for a how-to.

\Drupal\views\Form\ViewsForm is used whenever a View field contains a form element (a handler that implements viewsForm).

Maybe take a look at what \Drupal\views\Tests\ViewsFormMultipleTest is doing. And look at \Drupal\views_test_data\Plugin\views\field\FieldFormButtonTest for an example of a handler implementing this.

Shameless plug: check the PR in #2834812: Add a views handler for a per-variation add to cart button for a real life example of its use.

Patch in #25 assumes views is in modules/views rather than core/modules/views. Modifying the paths in the patch fixed the problem for me.

Another scenario that seems to exhibit this issue is with attached displays to block displays.

For example, I have a block view display that renders a list of content related to the currently displayed page. This block includes bulk operations for the related content. It also includes a display attachment via the views_data_export module for exporting a CSV of the related content.

I'm still tracing through the code to understand what is happening, but it does appear that in this case there was an intentional decision in DisplayPluginBase.php to set the form action to the URL of an attached view when the display is a block.

Related ticket in the views_data_export module issue queue: https://www.drupal.org/node/2881076

In my case, I had a view whose main display is a block (with an exposed form), then I added a second display that is a REST Export. The block doesn't have its own route, so when it was just the block in that view, it correctly used the current route for the action of the form. Once I added the REST Export display, which does have its own route, the block started using that route instead of the current route where the block was placed.

This is a result of the technique used to find a valid route: the getLinkDisplay() method is used to find a display within the view that actually does have its own route. The Block display plugin does not override the default logic of this method, which simply returns the first display that has a route. When the block was the only display in the view, this would return NULL and the form action would default to the current route. Once I added another display which had a route, that display was being returned and used for the form action.

I believe this behavior is incorrect, because the Block display plugin exposes an option to choose a linked display, and by default, the value is "None":

Therefore, defaulting to any other random display that the view contains to use as the linked display when determining the action value for the block's exposed form is a bug IMO. The attached patch makes sure the user-selected linked display is used by a block, which may be "None." The "Custom URL" option is not handled, as I don't personally have a need and it seemed more involved, but I would expect that option to be honored by the block display plugin as well.

This problem occurs for exposed forms in entity attachments too. Don't think the patch applies to dev any more, let's see...

Same patch as #33 ... go testbot ...

The embed display type has the same problem. I cant add a patch right now.

Here is a new patch for both block and embed displays.

re-roll wrong path in the last patch.

one last time

Re-roll of #42 for 9.1.x

Choose wrong core version to test with.

@carsoncho Patch #42 applied successfully on 9.1.x, So there is no re-roll required, also the re-roll you provided in #48 is incorrect according to the #42, For the reference I have added test on #42, Please have a look.

Also there is custom command failed issue because comment added after statement.

Try to fixed Cc Issue #48, #49 && Attached patch against Drupal 10.1.x

Cc Issue Fixed #54

added updated patch try to fix CCF #54

#42 Works well for D9. Thanks!

FYI patch #42 still works, tested on Drupal 10.1.6
Patches posted later doesn't work - tested with View with exposed filters using views_embed_view(), where is attached Data export display (module views_data_export, reported in issue 3166881 of this module).

With patch #56 applied to Drupal 10.1.6 I get this error in watchdog when submitting (pressing submit/apply button) with an embedded view with AJAX enabled and with exposed filters:

access denied	Warning	Path: /views/ajax?_wrapper_format=drupal_ajax&title=&field_event_date_value=2023-12-04&field_event_date_end_value=&field_event_tags_target_id=&moderation_state=All&view_name=group_dashboar

And it seems that the exposed filters have no affect on the results.

EDIT: This was not caused by the patch. It was caused by restricting access to the view by permission. An unrelated issue apparently.

Additionally when resetting the view using the reset button, despite having AJAX enabled for the view, the entire page is reloaded and I am taken to the top of the page. This is less than ideal.

EDIT: This is the behavior of the reset button regardless of whether the patch is applied or not. Still not ideal.

I also had this issue and solved it with the patch from #42. I didn't try any of the other patches based on #60
I am on views_data_export 1.2 on drupal 9.5.11.

Patch #42 is working for us on Drupal 10.1.

Still needs tests. Also issue summary should follow the standard issue template please.

Hiding problematic patches, since #42 is still what people are using.

Everything should be converted to a merge request as well.

Opened a merge request with the patch in #42.

The patch on #56( 2840283-56.patch) fails to apply on Drupal version 10.4.1

As part of fixing the test failures I realise this change is going to be a bit problematic for BC. I.e users may have blocks or embeds that they want to automatically link to the page display which would no longer be the case after this change, and at worst could throw a 500 with You cannot create a URL to a display without routes. which was one of failures in ArugmentDefaultTest

There is another scenario that needs to be considered: when setting the action path, if the view was refreshed via AJAX, then the path is set to <current>, which translates to "view/ajax...". Submitting the form then fails.
I've added a new MR (MR !13479) that extends the existing work, adding a new method to determine the action's URL taking AJAX into account.

#2844823: Views exposed form action incorrect for embedded views' displays with other displays with paths has been marked as possible duplicate and may also contain additional relevant information

I’ve hidden MR !8053 to avoid a bit of confusion and/but opened a new MR !14412, to preserve some of the work and testing I’d already done in #2844823: Views exposed form action incorrect for embedded views' displays with other displays with paths.

That testing was done manually on a real project (VBO + exposed filters on a block display with a data_export display), which helped validate a few edge cases.

I didn’t want to step on the work in MR !13479. The approach is very similar, so I’m comparing against it and folding in only what helps reduce the diff and close any remaining gaps.

@joelpittet thanks for the tidy up, does your MR address the issues raised in #75?

MR !14412 is ready for review.

This builds on a lot of hard work that’s already gone into this issue across multiple MRs. My intent here is not to step on toes, but to try to move things a bit closer to a finish line on a long-running and understandably tricky issue. Aside from a small class property name change, I did not merge the other MRs directly, though they very much informed this approach.

I added a post_update hook to preserve BC while removing the guessing behaviour, to avoid unexpected side effects. This will affect my own project(s), but I believe the end result is more predictable behaviour overall.

If this isn’t the right direction, or if other MRs are closer to the mark, I’m completely fine with this one being closed. I’m not attached to it (EDIT: well a little bit attached 😉 but I can deal!). My hope is that, even if this MR isn’t taken, some of the ideas here are useful and help move the issue forward! ➡️

This likely needs a CR. I’m happy to draft one if this approach looks acceptable (though I don't want to create one if I am off base).

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

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

The review bot went after the other MR. I just fixed the PHPCS issue it reported to keep it from moving back to NW. Please review #86

I think we just close the old MR at this point?

Left a question on the MR but it also needs a rebase.

Also can probably start the CR.

Thanks!

CR added https://www.drupal.org/node/3579301
The reroll was a bit tricky but I think I got it. Responded to MR comments.

MR!14412 is failing to apply for me now on Drupal core 11.3.5. Not sure if this is as a result of the rebase?

Thanks @joelpittet responded to the comment with a small change request to use constructor promotion. Less lines.

I have added constructor property promotion for the new property.

@d.fisher re #95 that is very unlucky but also very possible – we are applying it against the main branch which might have changes that don’t gel well with Drupal 11.3.5.

It seems like we are on the homestretch for this issue (knock on wood) but you can create a patch out of the older commits with git diff.​​​​​​​​​​​​​​​​
(Hot tip, exclude tests from the diff/patch will yield less conflicts)

Yes I suspect it will be some difference that has crept in between the tagged release and the main branch which is unfortunate but always something that can happen. I was just testing using the MR and adding .patch to the end. As it happens we've just switched to AJAX so this is no longer an immediate issue on the site I was testing it on but good to know this one is moving along! Thank you for all the hard work going on in here!