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

Invalid SQL query when reordering items via GridFieldOrderableRows for the first time #241

Closed
michalkleiner opened this issue Jan 28, 2018 · 7 comments

Comments

@michalkleiner
Copy link
Collaborator

The problem is in https://github.com/symbiote/silverstripe-gridfieldextensions/blob/master/src/GridFieldOrderableRows.php#L564

The $baseTable is not correctly derived from the list (using the name of the db table as the input) and therefore it assumes the table holding the Sort column also holds the LastEdited column (which is then offending in the SQL).

This can be replicated by installing standard SilverStripe CMS recipe, adding "dnadesign/silverstripe-elemental": "2.x-dev", "dynamic/silverstripe-elemental-blocks": "dev-master" and creating and reordering Accordion Panels inside the Accordion block.

It only happens for the first time when the records don't have any Sort value assigned.

@michalkleiner michalkleiner changed the title Invalid SQL query while reordering items via GridFieldOrderableRows Invalid SQL query while reordering items via GridFieldOrderableRows for the first time Jan 28, 2018
@michalkleiner michalkleiner changed the title Invalid SQL query while reordering items via GridFieldOrderableRows for the first time Invalid SQL query when reordering items via GridFieldOrderableRows for the first time Jan 28, 2018
@mak001
Copy link

mak001 commented Apr 6, 2018

bump

@dhensby
Copy link
Contributor

dhensby commented Apr 13, 2018

@michalkleiner @mak001 either of you want to create a PR?

@michalkleiner
Copy link
Collaborator Author

I may have some time as we will potentially need it for the coming project.

@robbieaverill
Copy link
Contributor

@NightJar has done some work recently for supporting many many through lists in reordering, I wonder whether his changes in #260 might fix this

@NightJar
Copy link
Contributor

It depends on versioning status. If models being ordered are versioned the component uses the ORM - I imagine that in this case things should be safe (completely unverified; thinking out loud).

However if items are not versioned, and are not many_many through, then a manual SQLUpdate call is generated for each row. This would be the code executed to cause the description of the issue in the OP, and I don't think anything I've done will have affected this.

Still, could we worth trying @michalkleiner :) Never hurts to get an extra set of eyes on a PR anyway :P

@robbieaverill
Copy link
Contributor

We've done a bunch of work on reordering with this component lately, including with around many many through lists and getting the correct sort column in these and has many lists. If the issue persists, feel free to reopen this issue.

@NightJar NightJar self-assigned this Oct 19, 2018
@NightJar
Copy link
Contributor

I think this is still valid. I'll leave it closed, but have assigned myself as a reminder to check.

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

5 participants