Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pagination with 1 item per page breaks breaks when using prev/next #11231

Open
2 tasks done
lekoala opened this issue May 10, 2024 · 8 comments
Open
2 tasks done

Pagination with 1 item per page breaks breaks when using prev/next #11231

lekoala opened this issue May 10, 2024 · 8 comments

Comments

@lekoala
Copy link
Contributor

lekoala commented May 10, 2024

Module version(s) affected

5.x

Description

Stumbled upon this recently when doing some tests

When having a nested gridfield in a model admin, if you have setItemsPerPage = 1 for the nested grid field, when using the next arrow, it stops working after three clicks, as if there was no record anymore

Eg: i have 9 pages

image

It stops on record number 3

image

It works just fine with 2 items per page
Refreshing the page doesn't help
I guess something is wrong with the +1/-1 offset logic

How to reproduce

  • With a dataobject with a many many relation to another object
  • Using modeladmin navigate to the dataobject, seeing the gridfield with the many many relation that contains 3+ object
  • Given that the pagination is 1 item per page (see snippet below)
  • Click on the first record in the grid, using the arrow navigation in the bottom right corner. At the third record, the next becomes disabled even if there are more records
 public function getCMSFields()
    {
        $fields = parent::getCMSFields();

        $ManyRelation = $fields->dataFieldByName('ManyRelation');
        if ($ManyRelation) {
            $config = $ManyRelation->getConfig();
            $paginator = $config->getComponentByType(GridFieldPaginator::class);
            $paginator->setItemsPerPage(2);
        }

        return $fields;
    }

Possible Solution

No response

Additional Context

No response

Validations

  • Check that there isn't already an issue that reports the same bug
  • Double check that your reproduction steps work in a fresh installation of silverstripe/installer (with any code examples you've provided)
@GuySartorelli
Copy link
Member

Just to clarify, when you say "prev/next", is that the "previous page" and "next page" buttons in the paginator when you're viewing the GridField as a whole? Or is that the "previous record" and "next record" buttons when accessing the edit form for a record managed by the GridField?

@GuySartorelli
Copy link
Member

Also your "how to reproduce" is just some code without context. Please give some steps along with the code, so it's easy to reproduce by just following some steps. Especially double check the code since you mention 1 item per page, but the code shows 2.

@lekoala
Copy link
Contributor Author

lekoala commented May 14, 2024

Updated the how to reproduce. It's when using the detail form using the prev/next arrow next to the delete button

@emteknetnz emteknetnz self-assigned this May 15, 2024
@emteknetnz
Copy link
Member

emteknetnz commented May 15, 2024

I cannot replicate on 5.2.x-dev or 5.x-dev

Please let me know exact version and exact code used

Here's the code I failed to replicate with:

// app/src/MyModelAdmin.php

<?php

use SilverStripe\Admin\ModelAdmin;

class MyModelAdmin extends ModelAdmin
{
    private static $url_segment = 'MyModelAdmin';

    private static $menu_title = 'My model admin';

    private static $managed_models = [
        MyDataObject::class,
    ];
}

// app/src/MyDataObject.php

<?php

use SilverStripe\Forms\GridField\GridField;
use SilverStripe\Forms\GridField\GridFieldConfig_RecordEditor;
use SilverStripe\ORM\DataObject;
use SilverStripe\Forms\GridField\GridFieldPaginator;

class MyDataObject extends DataObject
{
    private static $table_name = 'MyDataObject';

    private static $db = [
        'Title' => 'Varchar'
    ];

    private static $many_many = [
        'MySubDataObjects' => MySubDataObject::class
    ];

    public function getCMSFields()
    {
        $fields = parent::getCMSFields();
        $gridField = new GridField('MySubDataObjects', 'MySubDataObjecs', $this->MySubDataObjects());
        $config = GridFieldConfig_RecordEditor::create();
        $gridField->setConfig($config);
        $paginator = $config->getComponentByType(GridFieldPaginator::class);
        $paginator->setItemsPerPage(1);
        $fields->insertAfter('Title', $gridField);
        return $fields;
    }
}

// app/src/MySubDataObject.php

<?php

use SilverStripe\ORM\DataObject;

class MySubDataObject extends DataObject
{
    private static $table_name = 'MySubDataObject';

    private static $db = [
        'Title' => 'Varchar'
    ];

    private static $belongs_many_many = [
        'MyDataObjects' => MyDataObject::class
    ];
}

@lekoala
Copy link
Contributor Author

lekoala commented May 15, 2024

here is a video maybe it's better:

Silverstripe---MyModel.mp4

it's on silverstripe/framework 5.2.6 using chrome/windows

@emteknetnz
Copy link
Member

emteknetnz commented May 16, 2024

OK thanks that video helps a lot

I've done some debugging, looks like in the GridFieldDetailForm_ItemRequest the $this->gridField is the top level DataObject. Rather than the one being edited. This means that "gridstate" which is ?gridState-MySubDataObjects-1=%7B%22GridFieldPaginator%22%3A%7B%22currentPage%22%3A5%7D%7D thing in the URL will not match because it's using the wrong key, in the case of the code sample I had on this issue it $gridField->name should be 'MySubDataObjects' but it's mistakenly 'MyDataObject' instead

The full url of my request is /admin/MyModelAdmin/MyDataObject/EditForm/field/MyDataObject/item/1/ItemEditForm/field/MySubDataObjects/item/5?gridState-MySubDataObjects-1=%7B%22GridFieldPaginator%22%3A%7B%22currentPage%22%3A5%7D%7D

@lekoala
Copy link
Contributor Author

lekoala commented May 16, 2024

yes, so while my issue (who would be using 1 item per page :) ) is pretty minor, i'm sure it leads to a larger issue that could cause tricky bugs

maybe related:
#9556

@emteknetnz
Copy link
Member

Apparently we might look at a new way of managing the gridstate, so in the mean time I might just park this one for a wee while since it seems like it's very hard to fix

@emteknetnz emteknetnz removed their assignment May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants