Skip to content
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

Add API routes to request multiple of an item #70

Merged
merged 13 commits into from
Oct 5, 2020
Merged

Conversation

Geometrically
Copy link
Member

No description provided.

Copy link
Contributor

@Aeledfyr Aeledfyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One, fix the failing checks & run cargo fmt.

I'm not sure about moving the version routes to the api root; It makes sense, but then it makes less sense to have mod/{mod_id}/versions return a populated list of versions rather than a list of ids.

The routes for getting a list of entries seem useful, but is the naming the best? It may become confusing to have both /api/v1/mod and /api/v1/mods as routes, one that searches and one that requests by id.

src/database/models/user_item.rs Outdated Show resolved Hide resolved
src/routes/mod.rs Outdated Show resolved Hide resolved
@Geometrically
Copy link
Member Author

Geometrically commented Oct 4, 2020

One, fix the failing checks & run cargo fmt.

I'm not sure about moving the version routes to the api root; It makes sense, but then it makes less sense to have mod/{mod_id}/versions return a populated list of versions rather than a list of ids.

The routes for getting a list of entries seem useful, but is the naming the best? It may become confusing to have both /api/v1/mod and /api/v1/mods as routes, one that searches and one that requests by id.

Fixed failing checks

The reason why it won't return all the versions is because we have the get multiple versions route, so the user can decide what versions they actually need/want.

It might be confusing, but that is what documentation is for. I don't know a better way of doing it

Copy link
Contributor

@Aeledfyr Aeledfyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Batched database queries should be an order of magnitude faster, so we should at least try them. If it doesn't work out then what we have may be good enough, but it seems like it should be easy enough to implement.

src/routes/mod.rs Outdated Show resolved Hide resolved
src/routes/mods.rs Outdated Show resolved Hide resolved
src/routes/users.rs Outdated Show resolved Hide resolved
src/routes/versions.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@Aeledfyr Aeledfyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still a few things left, but it's almost done. Now it's mostly just the array types for the batch get routes and database methods.

src/database/models/mod_item.rs Outdated Show resolved Hide resolved
src/routes/mods.rs Outdated Show resolved Hide resolved
src/routes/mods.rs Show resolved Hide resolved
src/routes/users.rs Show resolved Hide resolved
@Geometrically Geometrically merged commit 2719ae5 into master Oct 5, 2020
@Geometrically Geometrically deleted the new-utils branch October 20, 2020 02:25
thesuzerain pushed a commit that referenced this pull request Dec 5, 2023
* Change header name

* Add default bio value

* Remove default

* Make name null

* Run prepare

* Add new API Routes for requesting multiple of an item

* Run formatter

* Simplify get mods query

* Run prepare

* Refactor to use one query for most routes, change version create route to have mod_id in data

* More fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants