-
Notifications
You must be signed in to change notification settings - Fork 36
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
feat: search filter on sensor API #1191
Conversation
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First batch of comments, one idea about asking for asset sensors still not clear to me either.
- relocated SearchField to dedicated file - worked on event resolution formating Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
…ring Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some quick observations
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We saw today that there are no tests yet for the sensor index endpoint. Let's add some, both with and without pagination.
One more thing: I found a query for asset sensors which the asset view is making, and which we don't have to do anymore now (I guess, please confirm). |
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
I think this is the code that was populating the Since that page is being feed by a API now I don't think its needed anymore |
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I asked for tests for the index endpoint. Right now, we are only testing the new simpler endpoint.
…ntend Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit more detail in testing, please.
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test needs work.
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think the test need one iteration.
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, much better. Almost there!
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
We discussed that I found how this endpoint needs to be enhanced even further, so we can get sensors of an asset and its sub-assets, but I will open a follow-up PR, so that this one can already be on main and serve ongoing development!
* feat: search filter on sensor API Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * feat: sensor API pagintion and search Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * refactor: reduce code repitition and code size Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * refactor: request changes - relocated SearchField to dedicated file - worked on event resolution formating Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * feat: api to fetch all sensors under an asset Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * feat: added filter feature for sensor table, both name and unit filtering Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * feat: unit filter features on get sensors API Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * refactor: asset sensor and sensors API changes Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * feat: test case for asset_sensors API Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * chore: few changes variable namings and import arrangement Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * refactor: updated testcase with more checks and removed redundant code Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * chore: little changes on test(waiting for comment resolution) Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * chore: moved formatResolution func to base.html Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * refactor: changes to asset sensors API and ISO formatter logic on forntend Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * chore: removed hard coded asset ID in asset sensors test case Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * feat: test to get all sensors Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * fix: fix wrong imports Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * refactor: expandd test cases for get sensors Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * refactor: expanded test case Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * chore: added to changelog Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * chore: updated change log Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * refactor: dynamic expected results for test case Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * refactor: dynamic sensor name for test case Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> * chore: simplified testcase Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> --------- Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com> Co-authored-by: Nicolas Höning <nicolas@seita.nl> (cherry picked from commit e8444d0)
Description
This PR adds the following to the sensor API
Look & Feel
How to test
Steps to test it or name of the tests functions.
Go to the assets page and click any of your assets that has some sensors
Further Improvements
None
Related Items
This PR closes the first task in #1076