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

FIX: Compatibility with versioned DataObjects #38

Closed
nedmas opened this issue Nov 18, 2013 · 13 comments
Closed

FIX: Compatibility with versioned DataObjects #38

nedmas opened this issue Nov 18, 2013 · 13 comments

Comments

@nedmas
Copy link
Contributor

nedmas commented Nov 18, 2013

As in #21 it would be great if the GridFieldOrderableRows supported versioned DataObjects. So I thought it would be good to start a discussion regarding the best solution as I agree with @ajshort that coupling the components should be avoided.

My suggestion is that the current implementation that uses DB::query() to update the sort order is replaced with DataObject::write(). This would ensure that all the relevant pre/post write code gets run (including all the versioned related code).

However I recognise that this will add a certain amount of overhead to re-ordering, is this acceptable or is there a more elegant solution?

@ajshort
Copy link
Contributor

ajshort commented Nov 19, 2013

A benchmark is probably the only way to find out.

@MattyBalaam
Copy link

Did this ever get looked at?

@nedmas
Copy link
Contributor Author

nedmas commented Feb 13, 2015

Hey @MattyBalaam I don't think that this has been looked at (I certainly haven't I'm afraid). However there is this module (https://github.com/heyday/silverstripe-gridfieldversionedorderablerows), that I've used on a couple of projects, which might help.

@MattyBalaam
Copy link

Thank you, I will investigate.

@nyeholt
Copy link
Contributor

nyeholt commented Feb 18, 2015

(Andrew's assigned the module over as he's taking a back-seat in SS land for a while, hence the lack of attention to the issue).

My understanding of the way the sort functionality is being triggered is to avoid (potentially) loading lots of data objects and triggering lots of ->write() calls when re-ordering items - given moving an item at the head of the list towards the back of the list triggers updates on all the items in-between.

Now, if the current list being displayed is relatively small, that's not really a problem, but if you're displaying 100s at once, it might be; has someone got a sample implementation? It's probably something that could be done as a configurable option?

@MattyBalaam
Copy link

The module above shows how it can be done, it is an extension to this. Moving objects is a slight fudge as any changes seem to go live immediately, rather than just to a draft. But, data objects have always seemed to work this way anyway.

@nyeholt
Copy link
Contributor

nyeholt commented Feb 18, 2015

Yep, implementation wise there's no real issue, but there is the problem of which implementation approach should be taken, given the possible tradeoffs.

  • Direct DB::query() calls (fast, but don't trigger write() based extensions). Can be adapted to update _Live tables immediately to directly publish changes to sort order
  • Calling write() for each data object affected - could be 'expensive' for gridfields set to display a large number of items - and again, should it immediately call 'publish' after 'write'?

So there's two somewhat related implementation problems; is the sort update done using DB::query or ->write() (regardless of versioned status of the dataobjects being manipulated), and separately, should all items be immediately ->published() afterwards?

I'm thinking that both could be solved with configurable settings when creating the GridField config - will take a look over the next few days (and see if there's some code I have from a client project that might help for a 'publish all changes' type of button).

@colymba
Copy link
Contributor

colymba commented Aug 26, 2017

Any update on this? Especially now with SS4 taking an "everything Versioned" approach, this is a big issue...

@mlewis-everley
Copy link

Just to pipe up here, we have tried https://github.com/heyday/silverstripe-gridfieldversionedorderablerows as mentioned by @nedmas and it works pretty well.

The auto publishing is an issue though, I have to admit the only way I could see fix it might be to use a gridfieldcomponent that auto publishes it's list when the parent is published (rather than automatically publishing on change)?

@ivoba
Copy link
Contributor

ivoba commented Jan 19, 2018

Just stumpled over this as well with SS4.
Current state is when reordering versioned DataObjects only the DataObject table is updated.
No Draft is mentioned on owning Object and when publishing the Owner it doesnt have any effect on the ordered DataObject.
So basically have to save every DataObject that was affected by the ordering to take effect on Publish.

Im not sure how the Draft state is triggered but i would welcome if the silverstripe-gridfieldversionedorderablerows module mentioned by @mlewis-everley would trigger this instead of a automatic publish.
The current automatic publish bypasses also onAfterPublish hooks.

In the meantime i will try silverstripe-gridfieldversionedorderablerows.

@ivoba
Copy link
Contributor

ivoba commented Jan 31, 2018

i think this was mostly solved with d1021ac

No?

@dhensby
Copy link
Contributor

dhensby commented Jan 31, 2018

fixed by #243

@dhensby dhensby closed this as completed Jan 31, 2018
@kinglozzer
Copy link
Collaborator

I’m not sure this is entirely resolved - this module will still only save draft versions when changing the sort order, there’s no way to automatically propagate that change to live. Use case example: a lumberjack GridField containing a lot of child pages - auto-publishing the children when the parent is written is really slow, as a bunch of changeset records are created

I’d like to see this as an opt-in API, I wrote this extension of the existing GridFieldOrderableRows class which achieves that: https://gist.github.com/kinglozzer/c585e0ffe67757c29f15c44349a4e4e2

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

10 participants