"More link" does not take display arguments into account

This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 200, following Review a patch or merge require as a guide.

This appears like it could still be relevant (code still seems to be the same but didn't test issue)

This will need a valid test to verify the bug and that it's fixed.

- Added the test cases and test-only patch to reproduce the issue.
- Test case is passing on local with the patch.

Thanks for the test!

Testing the code using the view from the patch

On Drupal 10.1.x with a standard install
Place the view block in the content reason
Create a few pieces of content of articles and basic pages
While on one of the pages I filter for articles.
Click "More" links and it takes me to the view page
Confirmed the parameter is based also.

Confirmed without the fix following the same steps the parameter is not based.

Looks like "test-only" patch is triggered by the bot for re-tests.
Not sure what is the right approach to let test bot know about picking up correct patch.
Updating weight of the patch to see how it goes.

Re-uploading the same patch again as bot is testing test-only patch.

1. +++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
   @@ -2137,33 +2137,34 @@ protected function getMoreUrl() {
        // Return the display URL if there is no custom url.
        if ($this->getOption('link_display') !== 'custom_url' || empty($path)) {
   

   I think the inline comment here needs to be updated, because we are no longer returning early.

2. +++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
   @@ -2137,33 +2137,34 @@ protected function getMoreUrl() {
   +    else {
   +      $parts = UrlHelper::parse($path);
   

   The "else" also probably needs an inline comment now too, starting with "Otherwise..." and describing what it is doing.
    

3. I checked and the only code that happens after the new "else" closes is:

       // Merge the exposed query parameters.                                      
       if (!empty($this->view->exposed_raw_input)) {
         $url->mergeOptions(['query' => $this->view->exposed_raw_input]);
       }
       return $url;
   

   I was trying to figure out if that should apply to both the cases in this if, and if we needed test coverage for both of them:

   +++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
   @@ -2137,33 +2137,34 @@ protected function getMoreUrl() {
        if ($this->getOption('link_display') !== 'custom_url' || empty($path)) {
   

   I could not figure out what custom_url meant from the code, so I installed core and poked around the Views UI, under the "More" link settings, under the pager settings, under the display path settings... still for the life of me could not find it. This was my best clue:

         case 'link_display':
           $form['#title'] .= $this->t('Which display to use for path');           
           $options = [FALSE => $this->t('None'), 'custom_url' => $this->t('Custom URL')];
   

   This is what the UI says about it:
   

   ...But I still could not find a "link display" section under "pager", or a "link display setting".

   I grepped and found a view where that setting was used:

   core/modules/user/config/optional/views.view.user_admin_people.yml:      link_display: page_1
   

   Poked around that view in the UI. Still can't find it. From the structure of the yaml, I expect it to be a top-level setting of the view, toward the end. All the references I could find to it in tests seemed to set the option manually, rather than via the form. I finally found this line in the functional (non-JS) display tests:
   $link_display_path = 'admin/structure/views/nojs/display/test_display/block_1/link_display';

   So in desperation I disabled JavaScript and navigated to http://127.0.0.1:8888/admin/structure/views/nojs/display/user_admin_people/page_1/link_display

   Which is this:
   

   Okay, so that looks like the right thing. Except after I submit that form, there seems to be no way to get back to it... indeed, link_display does not appear anywhere in the page source for the view with JS off.

   Re-reading the help text, I saw that it said something about being used if there was more than one page display. Except admin/people only has one page display, so why would it be set then? I tried adding a second page display... still could not figure out how to get to that form from the main views UI.

   So how on earth does this get configured by the user? Is it dead code? An orphaned setting? It's been awhile, and I worked on the Views in Drupal Core initiative full-time, and I'm nominally still listed as a maintainer, but I cannot tell after the better part of an hour where this option is configured. Can anyone help?

   So, needs tests... maybe? And needs followup... maybe?

Aha, I finally found it! It's a setting that only applies to block displays. As per the STR in the IS and the whole point of the issue. I get this option if I add a block display to admin/people:

So we definitely need a followup to conditionally display the "More link" section only on block displays, because it's confusing as heck on page displays. Also why is the form accessible with JS off for page displays?

Edit: The other question is, why is this set on the user view and other views where there is no block display, only a page display?

Based on all that, I think we need test coverage for both cases. Edit: When a custom URL is set but the path is empty (?).

#20 shows the relevance of #2087097: "More link" 's "Link display" option needs better UX

Hi @xjm
I've fixed the following issues from comment #19

• Point 1
• Point 2

Comment is updated for both the cases.

Regarding point 3:
- I've added the new test that validates both the points.
Fixed few more things in the MR

About the custom_url option:
- Primarily it is visible on the block plugin display and other non-page display.
So I believe we should hide it from the page display?

Probably we can use the inputs from the sub-system maintainer.
This might cause confusion to people as `More` link on the page display might not make sense.

I notice that there is no option to configure the link, it just allows to enable/disable link.
Which is correct if we want to keep option.

I'm happy to walk you through the views setup, feel free to ping me in #bugsmash.

Regarding #20
I agree with you for this one.
We can disable it from the page display and keep it active for block display.

Hiding the patches since we have a MR now.
Keeping issue in needs works since there are a few unrelated test failures that I'm looking into it.