migrate /boxes/:senseBoxId/locations

[scheidtdav]

Method	Implementation to be migrated
GET	boxesController.getBoxLocations

Keep in mind that the way locations are stored in the Postgres database changed compared to the existing mongo db.

[timber-they]

I implemented the endpoint, though I'm unsure if I understood the way our locations are stored correctly. Right now I receive them with this:

const device = await getDevice({ id: deviceId });
if (!device)
  return notFound("Device not found");

const locations = device.locations.filter(location =>
  new Date(location.time) >= fromDate && new Date(location.time) <= toDate);

However in our test data, always only one location is returned that way. Also I wonder if this will scale that well, since at first all locations are fetched from the database and only later is the filter applied. However, this is generally the case for the function getDevice (right?). What do you think @scheidtdav @jona159?

Other than that only the unit tests are missing for this ticket.

[timber-they]

I now changed it to only get the required locations from the database. But still it seems like a waste of computing power that the default getDevice fetches all the location entries.

[scheidtdav]

I agree, it does seem wasteful, especially considering that most devices are currently stationary and thus really only have a single location.

We might be missing something here though:
The current documentation specifies the defaults for fromDate and toDate as 48hrs ago and 'now'. To me that does sound a lot more reasonable than loading all locations. I would even argue to implement that (or something similar) even if the current implementation does not do that.
In fact I even tried the current implementation (using cURL) and it seems like the endpoint is not returning anything when called without the parameters, meaning there simply are no defaults.

Only when run with fromDate was I able to get something from this endpoint.

So what do we say @timber-they @jona159 ?
Implement the 48hr default, as the docs suggest? -> 🚀
Or make the parameters somewhat mandatory (aka when not specified, you get nothing)? -> 👀

@timber-they Whatever we decide, would you able to add that to the existing PR?

[jona159]

I agree, it does seem wasteful, especially considering that most devices are currently stationary and thus really only have a single location.

We might be missing something here though: The current documentation specifies the defaults for fromDate and toDate as 48hrs ago and 'now'. To me that does sound a lot more reasonable than loading all locations. I would even argue to implement that (or something similar) even if the current implementation does not do that. In fact I even tried the current implementation (using cURL) and it seems like the endpoint is not returning anything when called without the parameters, meaning there simply are no defaults.

Only when run with fromDate was I able to get something from this endpoint.

So what do we say @timber-they @jona159 ? Implement the 48hr default, as the docs suggest? -> 🚀 Or make the parameters somewhat mandatory (aka when not specified, you get nothing)? -> 👀

@timber-they Whatever we decide, would you able to add that to the existing PR?

I briefly looked at the current implementation of this endpoint and you are right there are simply no defaults in place despite the docs suggesting that. I think i would still lean towards implementing the from-date and to-date paramters exactly as specified in the docs (so optional and with defaults of 48h ago and now)

[timber-they]

In the existing PR I already implemented it with the documented default values (see here). However, we also still use the getDevice function from device.server.ts in other places, e.g. api.device.ts. It seems typically the locations aren't required there at all. So I guess it would be good to implement another getDevice method that is used instead of the current one that doesn't return the locations and use it except when the locations are actually needed. This seems like a different ticket though.

[scheidtdav]

Created #654 for that :-)