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

GridFieldOrderableRows - No support for ManyManyThroughList #254

Closed
mfendeksilverstripe opened this issue Apr 8, 2018 · 7 comments · Fixed by #260
Closed

GridFieldOrderableRows - No support for ManyManyThroughList #254

mfendeksilverstripe opened this issue Apr 8, 2018 · 7 comments · Fixed by #260

Comments

@mfendeksilverstripe
Copy link
Contributor

mfendeksilverstripe commented Apr 8, 2018

There is no support in GridFieldOrderableRows for ManyManyThroughList. Note that ManyManyThroughList doesn't extend ManyManyList.

We need to find all cases where a ManyManyList is handled and handle ManyManyThroughList in that place as well.

@robbieaverill
Copy link
Contributor

Damn, it would've been nice if ManyManyThroughList extended ManyManyList.

@tractorcow
Copy link

It doesn't because the classes aren't substitutable for one another; There are similarities between the two, but they do have separate behaviour that doesn't always imply one is a superset of another. For now you should just check either case.

It's about 5 hours of work IMO to fix and test.

@tractorcow
Copy link

We could have an interface for "relation with getExtraFields" I guess instead of a core ManyManyList.

@dhensby
Copy link
Contributor

dhensby commented May 11, 2018

OK - I've looked through this and the primary issue is that ManyManyThroughList (MMTL) doesn't share inheritance or an API with ManyManyList. MMTL isn't really fit for purpose because all the knowledge is in the ManyManyThroughQueryManipulator which is not accessible outside the MMTL class and the information it holds is not exposed in any way, either.

The most important API we need on ManyManyThroughList is getJoinTable which is used to create the query that writes the sorts.

I've added a PR to core to add this (silverstripe/silverstripe-framework#8065)

I've added a PR to this repo (#258) to add MMTL support, which is dependant on the above framework PR.

@mfendeksilverstripe
Copy link
Contributor Author

Nice one, will have a look at it.

@tractorcow
Copy link

Ok silverstripe/silverstripe-framework#8065 is merged

@NightJar
Copy link
Contributor

NightJar commented Jun 4, 2018

#260 resolves this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment