Feat: filter sensors and assets by ID#2231
Conversation
…ndpoints Signed-off-by: Nicolas Höning <nicolas@seita.nl>
…orks for the other fields Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Protect GET /assets/<id>/sensors with the same read permission guard as the asset detail endpoint, with a regression test for cross-organisation access. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Use the SearchFilterField output directly so multi-term and quoted searches behave like the other filtered endpoints. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Replace text casts on integer IDs with equality and range predicates, and tighten fresh-db coverage with prefix decoys and exact counts. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Expose the filter metadata in the asset and sensor schemas and clarify that ID prefixes return only matching records. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
BelhsanHmida
left a comment
There was a problem hiding this comment.
I tested the PR manually works as expected!
I checked that filtering works on /assets, /assets/<id>/sensors, and /sensors. I tested ID-prefix searches with matching records and decoy records where the digits appear later in the ID, and the endpoints returned only the prefix matches with the expected counts. I also tested multi-term filtering on /assets/<id>/sensors.
For a code review, I found a few issues and fixed them:
The main one was that GET /assets/<id>/sensors returned sensors without first checking read access on the asset. I added the same read permission guard used by the sibling asset endpoint, and added a regression test for the cross-organisation 403.
I also found that asset_sensors only used the first parsed filter term. Since SearchFilterField already parses the full filter into terms, I changed the endpoint to use that parsed list directly, so a query like filter=no-match 123 now considers both terms.
For ID-prefix filtering, I changed the implementation away from cast(id, String).like(...) and into integer equality/range predicates. This should make the search faster on larger tables because the database can still use the normal integer primary-key index, instead of casting every ID to text before comparing it. It keeps the same intended prefix behavior: for example, 12 matches 12, 120, and 1234, but not 912.
I also tightened the tests so the explicit-ID cases use fresh DB coverage, include decoy IDs, and assert the exact returned records and counts.
nhoening
left a comment
There was a problem hiding this comment.
Some nice improvements, thanks.
But also one thing that definitely needs some fixing, I believe.
Rely on PaginationSchema for inherited filter parameters and refresh the generated OpenAPI spec. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Document that false() makes the ID-prefix branch match no rows for non-canonical digit strings. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Rename the ID-prefix helper argument so it is clear the original digit string is needed for range length calculations. Signed-off-by: Mohamed Belhsan Hmida <mohamedbelhsanhmida@gmail.com>
Description
We have API endoints to list sensors and assets. We use them in several UI views, and there we have a search function. The search functions uses a filter capability in the endpoints - the UI sends a
filter=XYZquery parameter, the endoint figures out if that matches against a list of supported db fields).Now, the user can filter by name (of sensor or asset), and also account name I believe.
Users told us that sometimes they want to filter by ID (of sensor or asset), as well.
This PR enables this extra filtering in the API endpoints (AssetAPI:index, AssetAPI:asset_sensors, SensorAPI:index).
Also add a test case for
I expect no changes to be necessary on the UI side.
Bonus question: Where is the AssetAPI:asset_sensors used in our UI, if at all?