JohnCoker / thrustcurve3

ThrustCurve.org third revision
ISC License
12 stars 7 forks source link

API: Include `availability` in motor search results #34

Closed broofa closed 2 years ago

broofa commented 3 years ago

Currently there's no good way of knowing which motors are available and which aren't in the search.json API. You can query for all motors (1093 hits), and you can query for "available" motors (783 hits), but the search results don't provide any indication of whether a motor is available or not.

I'm a little unsure what to suggest for a fix because the Swagger documentation suggests there's additional availability state (e.g. "OOP", "occasional") that isn't actually supported by the API. There is also the "regular" .vs. "available" state, which is slightly different. Specifically, the Estes D11 (motorId: 5f4294d2000231000000001f), and the Loki N3800 (motorId: 5f4294d200023100000002f2) motors show up in "availability=available" queries, but not in "availability=regular" queries.

This all suggests that the definition of "available" vs., say, "OOP" is a little complicated. But if I had my druthers I'd suggest adding an"availability": "OOP" (Out Of Production) for non-available motors. And omit this property for "available" motors (i.e. motor availability defaults to "available" unless explicitly defined otherwise.)

This is useful state to have so for thrustcurve-db (for now) I'm going to run the "all" and "available" queries and add "availability": "OOP" for any motors that are missing from the latter results. Does that make sense?

JohnCoker commented 3 years ago

In the DB, this is an enum of 'regular', 'occasional' and 'OOP'. However, 'occasional' never really got used. (No manufacturers are willing to admit that some of their motors are occasionally available). 'available' is a pseudo value which includes both 'regular' and 'occasional'.

See schema.js for details: https://github.com/JohnCoker/thrustcurve3/blob/master/database/schema/schema.js

At some point, I may switch things so that this field becomes a boolean for simplicity, but right now it basically is.

> db.motors.count({ availability: 'OOP' })
313
> db.motors.count({ availability: 'regular' })
778
> db.motors.count({ availability: 'occasional' })
2
> db.motors.count()
1093

The D11 and N3800 must be marked as 'occasional', which is why they fall through the cracks. It appears the D11 isn't available any longer, so I'll mark that as OOP. The N300 is still available (although special-order), so I'll mark that as regular.

JohnCoker commented 3 years ago

Do we want to keep adding to the current API? I thought the plan was to build a newer generation API? (I don't want to mess with v1 too much because it's used by some programs which I can't force to change (RockSim, OpenRocket).

broofa commented 3 years ago

heh Looks like we ninja'ed eachother on the "should this be a boolean" idea.

Do we want to keep adding to the current API?

I'll leave that up to you. Mostly I'm just trying to be diligent about documenting the pain points I'm finding in working with the current API. How those do / don't manifest in v1 or v2 is TBD.

I agree that v1 shouldn't change in a way that breaks existing clients. That's a no-brainer. But creating a v2 will require a significant amount of work just to get the basic API going, and I'm not sure either one of us has the time for that. But some changes (such as adding a discontinued property to the motor data) can be made in v1 w/out breaking things, so might be worth doing.

Speaking of implementing a v2, I've avoided diving in the code because I don't have a way of running it on my local machine. The main obstacle being no Mongo database to run against. That's going to be an impediment to anyone wanting to contribute to this project. I'm not sure what the best solution there is. Obviously people shouldn't have access to your production DB, but is there a way to provide a copy of that? If/when you have time, it might be worth putting a wiki page together that describes how someone like me could get a local development build up and running.

JohnCoker commented 3 years ago

There are instructions for setting up a DB and importing the test data set, see https://github.com/JohnCoker/thrustcurve3/tree/master/database.

Note that the API tests and the Selenium tests depend on the test DB (so queries are stable).

broofa commented 2 years ago

Looks like availability is now included in "search" results, and you've removed the "occasional" motors. This addresses the concerns I had so I'm going to close this out. Thank you!

Regarding v2 of the API... I think the thrustcurve-db module is going to give me everything I need. And I've started a new job, so don't have a lot of time to devote to this. (Sorry! I would love to help out more. Maybe in a few months when I'm a bit more settled in my job?)