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

Allow ignoring query parameters on list views #88

Closed

Conversation

tomaszn
Copy link
Contributor

@tomaszn tomaszn commented Jul 2, 2017

I'd like to parametrize views using links. Unfortunately list views use all query parameters to filter data, so context.list becomes empty. This change allows ignoring prefixed parameters, and the prefix is configurable.

@tomaszn tomaszn force-pushed the allow_ignoring_params_on_list_pages branch 2 times, most recently from 070ffef to f7b983e Compare July 3, 2017 21:50
@sheppard
Copy link
Member

Thanks, this is useful though I have some thoughts about the API.

  1. Could this be a list of keywords instead of a prefix? You could still use prefixed parameters as long as you list all of them in the configuration.
  2. Could this be set at the wq/model.js configuration level rather than in wq/app.js? Different models might use different non-filtering keywords. (If you use the same keywords on all models you could just repeat the configuration for each model).

I ask this because I would like to synchronize this functionality with wq.db, specifically the (currently undocumented) attribute on views called ignore_kwargs (see filters.py). We would change that from a view attribute to a configuration option that would match whatever we do here.

@sheppard
Copy link
Member

sheppard commented Jul 12, 2017

An alternative (or addition) to making this configurable might be to have wq/model.js read from the list of model fields and only filter on the ones that match. This is essentially what filters.py does here. One tricky part is that filters.py uses any and all Django Model fields, whereas wq/model.js would only have access to writable serializer fields (since that's what is used to generate the form configuration). I could see a case where you would want to compute and store a read-only field and have it available for filtering.

@sheppard sheppard modified the milestones: 1.1, 1.0 Jul 12, 2017
@tomaszn
Copy link
Contributor Author

tomaszn commented Jul 12, 2017

Thanks for your feedback! I think I would favour this automatic ignoring of filtering criteria, because it means editing one files less when introducing another parameter. It also means one configuration option less. (And if it is not implemented, we probably should display or log a warning.)
When filtering the parameters we need to remember also about the model.functions which are available for filtering the list.

@tomaszn tomaszn force-pushed the allow_ignoring_params_on_list_pages branch from f7b983e to bc1dd9b Compare July 24, 2017 12:18
@tomaszn tomaszn force-pushed the allow_ignoring_params_on_list_pages branch 2 times, most recently from f16546e to 6e22b3e Compare September 26, 2017 20:34
@tomaszn tomaszn force-pushed the allow_ignoring_params_on_list_pages branch from 6e22b3e to 0eaf15a Compare September 26, 2017 20:41
@sheppard sheppard modified the milestones: 1.1, 1.2 Apr 23, 2018
sheppard added a commit that referenced this pull request Oct 7, 2019
sheppard added a commit that referenced this pull request Oct 7, 2019
@sheppard
Copy link
Member

sheppard commented Oct 7, 2019

I implemented this to default to defined fields, but allow override via config.

  • By default, only writeable fields (listed in form) and computed fields (in functions) will be used for filtering.
  • Server-defined read-only fields can be added via the new filter_fields array.
  • If a filter includes query parameters not defined in form, functions, or filter_fields, the parameters will be ignored. Since this may be unexpected behavior, a warning will be logged unless the fields are explicitly listed in the new filter_ignore option.
  • If there are model fields that should always be ignored when filtering, they can be defined in filter_ignore.

@sheppard sheppard closed this Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants