-
-
Notifications
You must be signed in to change notification settings - Fork 0
Feat: post api boxes #616
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: post api boxes #616
Conversation
* feat(api): add api routes for /users/me * fix(tests): api me PUT * feat(api): add delete me endpoint
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.
Pull Request Overview
This PR implements POST functionality for creating new devices (boxes) via the API and updates the data format for the /users/me/boxes endpoint to match the openSenseMap API specification.
- Adds a new POST /api/boxes endpoint for creating devices with sensors
- Updates the /users/me/boxes endpoint to return data in openSenseMap-compatible format
- Ensures measurement values are returned as strings instead of numbers for app compatibility
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| app/routes/api.boxes.ts | New POST endpoint implementation with comprehensive OpenAPI documentation |
| app/routes/api.users.me.boxes.ts | Updated to use device transformation and include JWT token in response |
| app/models/device.server.ts | Enhanced createDevice function to return sensors and support additional fields |
| app/models/sensor.server.ts | Modified to cast measurement values to text format |
| app/lib/device-transform.ts | New utility for transforming device data to openSenseMap API format |
| app/lib/devices-service.server.ts | Added Zod schema for validating box creation requests |
| app/schema/sensor.ts | Added TypeScript type for lastMeasurement JSON field |
| tests/ | Added comprehensive test coverage for new functionality |
| app/routes/api.ts | Updated route configuration to enable POST /boxes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Regarding your open questions:
- What attributes should be returned after box is created and at /users/me/boxes? Is it ok not to return attributes with null values (like it was done before)?
Yes thats ok. Whatever was done before is currently our goal.
- Is it ok to return last measurement as string, not as double?
It was probably string before, so again: Yes thats ok.
- What should be returned as device.acces_token?
I am frankly somewhat confused by this. Where does this field even come from?
Why would a device respond with an access token if the client just did a request for which a token is required. That implies the client already having a token. Do we even need this?
|
@scheidtdav, thank you very much for the review and feedback! Per our conversation I removed access_token mentions and addressed your comments. Last question - because current PR was based on some other PR, which was based on another PR, now i contains commits from one/all of those prs. How would you like me to proceed? I could create a new pr and cherry-pick only my changes? Or there is a better way? |
|
@mashazyu Don't worry about those. Its a bit of a leftover from how we approached the migration previously. |
This PR adds POST functionality for creating new boxes (devices) via the API and updates the data format returned by the /users/me/boxes endpoint to match the openSenseMap API specification.
Changes
Testing
I have been testing current PR with senseBox:bike app.
Open questions