Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
rest.module
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
18 Sep 2014 at 09:42 UTC
Updated:
11 May 2017 at 14:46 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
R.Muilwijk commentedUsing the exposed filters is fairly easy and could be a valuable addition to a GET REST endpoint. Patch attached.
Comment #2
sebas5384 commentedHey @R.Muilwijk thanks!! you saved my day ;)
This worked exactly like I was expecting to work, I don't know if I'm the right person to review this because I don't know what kind of problems could bring this feature.
But it worked!, and for curious I would love to know why this wasn't released at first.
Please!! somebody from the core, this is a really important feature, not just for filters, for example exposed pagination.
Cheers!
Comment #3
klausiMakes sense, can we get a test case for this?
Comment #4
kalabroI'm working on this at DrupalCamp Baltics 2015 Code Sprint!
Comment #5
kalabro@klausi Could you please explain what kind of test would you like to see here? As I can see, REST module isn't responsible for Views logic tests. Views exposed filters are tested in Views module itself.
Comment #6
klausiWe should write a web test that does something similar as described in the issue summary. Create a REST view, add some nodes, sent a request with an exposed filter, make sure you only get filtered results back.
Comment #7
johnennew commentedPlease find a new patch with a test attached.
1. Adds a new test view with an exposed title filter set to filter by titles which start with the provided value
2. Creates 3 nodes specifying the title
3. Queries without the title query parameter to check all 3 nodes are returned - this shows the patch is not changing the no query parameter behaviour
4. Queries again with a title query parameter which should reduce the number of nodes returned to be 2 - this shows providing a parameter applies the query and filters the results.
Comment #8
dawehnerThank you for adding a test!
I think we should better test the cacheabilty metadata introduced by those exposed filters as well, just to be sure. So use \Drupal\system\Tests\Cache\AssertPageCacheContextsAndTagsTrait and check assertCacheContexts() that it includes the proper query parameters
Comment #9
johnennew commentedHi @dawehner - I've added a test which makes use of assertCacheContexts(). I couldn't see if it was possible to check if the specific query parameters were being included in the cache - is this possible? I'm at DrupalCon Barcelona today (Friday 25th) if you want to ping me any instructions Drupal Contribute as ceng_
Comment #10
Anonymous (not verified) commentedAny chance to get this into final Drupal 8.0?
Comment #11
dawehnerWow nice, I'd not have expected that its sad easy. Nice work
Comment #12
xjmRetitling to clarify the bug. @effulgentsia and I agreed that this can be fixed during RC because this is a common usecase, the fix is non-disruptive, and we don't actually need to change anything except enabling the functionality.
I do find myself wondering why this got set to false in the first place, though. I might ask @damiankloip or check the git history. Leaving at RTBC but I'd want to double-check that before committing this patch myself--someone else can still commit it though if they want.
Comment #13
dawehnerWell, I just think we didn't believed that it actually works that easy.
Comment #14
xjm@phenaproxima checked and it's been set to FALSE since comment #9 of #1819760-9: Add a REST export display plugin and serializer integration., which was basically the first patch in that issue based on the display plugin refactor. It's not discussed on the issue at all there, so indeed it was not disabled for any specific reason other than what @dawehner suggests in #13 or scope management or whatnot.
Committed and pushed to 8.0.x. Thanks!
Comment #16
damiankloip commentedYes. I think we just had it disabled....because. I wouldn't be surprised if there are a few issues, and expectations based on a regular exposed form. It works so easily because views exposed filters/form uses GET anyway.
Over all +1. Just don't blame me if some things don't work :)
Comment #19
khaled.zaidan commentedThis problem seems to be broken again (despite the display type still being set as "usesExposed").
After doing a bit of debugging, I've found that in the case of the REST export, the exposed filter is completely missing from the views filters (in the $view object), whereas it's present in other display types (e.g. page).
Looking further into it right now.
Comment #20
khaled.zaidan commentedI'm actually just completely wrong... I was editing one view while looking at another!
Please ignore my previous comment :)