wagtail-grapple icon indicating copy to clipboard operation
wagtail-grapple copied to clipboard

Apply query_params to plural field. Fixes #219

Open flyte opened this issue 2 years ago • 7 comments

Applies the query_params to the plural field as well.

A more ideal solution might be to separate out the singular and plural query helpers, as some query params might not be appropriate for both queries.

The main 'bug fix' for this is to use the kwargs in resolve_queryset() to enable custom filtering. In order to make this work, I needed to remove in_site from kwargs by setting them as a keyword argument on resolve_pages(), where that flag is actually used. Even if you don't want to alter the API behaviour of register_query_field(), this functionality makes it possible to create the separate helpers, as I mentioned above.

flyte avatar Apr 09 '22 09:04 flyte

I just noticed that there's already a singular helper, and that I hadn't made the same change to the paginated helper. Please don't merge yet, as I'll make this missing change and add a 'plural only' helper too.

flyte avatar Apr 09 '22 10:04 flyte

OK, I've reverted register_query_field() to its original behaviour and added register_plural_query_field().

There's quite a lot of duplicated code in these helpers, which isn't necessarily bad, but they do have some subtle differences in behaviour that I noticed:

  • Default query parameters are different between each helper
  • Differences between returning a result or None on a singular query if multiple results are returned
  • Different handling of urlPath queries
  • No default 'order' on non-Page model based paginated queries on register_paginated_query_field()'s resolve_plural() which raises a warning if using a QuerySet that's not ordered by default, like on snippets: UnorderedObjectListWarning: Pagination may yield inconsistent results with an unordered object_list

Hopefully this hasn't changed the Python API too drastically, but it's made Grapple a lot more flexible for my use-case.

flyte avatar Apr 09 '22 12:04 flyte

Hey @flyte, thank you for this. Will aim to review in the coming week or so

zerolab avatar Apr 10 '22 08:04 zerolab

@flyte do you still have any inclination to work on this issue?

dopry avatar May 12 '23 02:05 dopry

@dopry hi, I'm afraid I don't now. The project I was working on is complete and I'm no longer using this.

flyte avatar May 12 '23 08:05 flyte

@flyte that is cool. Life happens. Is it okay if another contributor takes over this work?

dopry avatar May 12 '23 15:05 dopry

Yes, of course! 😊

flyte avatar May 12 '23 16:05 flyte