Conversation
Implement a new POST /walkers endpoint that allows authenticated users to add new walkers to their clan. The endpoint accepts optional fields like owner, type, use, ready status, and description, with sensible defaults. This enables clans to programmatically add walkers to their inventory through the API.
The endpoint summary and operationId were changed from 'addWalker' to 'addWalkerFromUser' to better reflect its specific purpose and distinguish it from other potential walker creation methods.
Contributor
Reviewer's GuideAdds a new authenticated POST /walkers endpoint that lets a user create a walker record tied to their clan/Discord server, extends walker usage enum values, and wires the new request type into the API while also standardizing string literals and error handling in the walkers routes. Updated class diagram for walker request types and WalkerUse enumclassDiagram
class WalkerUse {
<<enumeration>>
PVP
FARMING
PERSONAL
RAM
SCOUT
RAIDER
SUPPORT
HAULER
CRAFT
STORAGE
}
class WalkerType {
<<enumeration>>
}
class AddWalkersRequest {
+Body body
}
class Body {
+string name
+string owner
+WalkerUse use
+boolean ready
+WalkerType type
+string description
}
AddWalkersRequest --> Body : has
Body --> WalkerUse : uses
Body --> WalkerType : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Add SCOUT, RAIDER, SUPPORT, HAULER, CRAFT, and STORAGE to WalkerUse enum to expand walker functionality classification.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The POST
/walkershandler mixes callback-styleserver.mysql.querywith earlyreturnfrom the route without always guaranteeing a single response; consider making the handlerasyncand using the promise API (or wrapping in a promise) so you canawaitthe insert andreturnexactly once on all paths. - The manual
if (!request?.body?.name)400 response returns an empty body, which conflicts with the documentedError400Defaultschema; either rely on Fastify's schema validation for thenamerequirement or return an error object that matches the 400 schema.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The POST `/walkers` handler mixes callback-style `server.mysql.query` with early `return` from the route without always guaranteeing a single response; consider making the handler `async` and using the promise API (or wrapping in a promise) so you can `await` the insert and `return` exactly once on all paths.
- The manual `if (!request?.body?.name)` 400 response returns an empty body, which conflicts with the documented `Error400Default` schema; either rely on Fastify's schema validation for the `name` requirement or return an error object that matches the 400 schema.
## Individual Comments
### Comment 1
<location path="src/routes/walkers/index.ts" line_range="229-238" />
<code_context>
+ 201: Type.Object({
+ message: Type.String(),
+ }),
+ 400: Error400Default,
+ 401: Error401Default,
+ 405: Error405Default,
+ 503: Error503Default,
+ },
+ },
+ },
+ (request, reply) => {
+ if (!request?.dbuser) {
+ reply.code(401);
+ return new Error("Invalid token JWT");
+ }
+ if (!request?.dbuser.clanid || !request?.dbuser.serverdiscord) {
+ reply.code(405);
+ return new Error("No clan");
+ }
+ if (!request?.body?.name) {
+ return reply.code(400).send();
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Align 400 error handling with the declared Error400Default schema
The 400 response is documented as `Error400Default`, but the missing `name` case sends a 400 with no body. This mismatch can break clients expecting the structured error. Please return an `Error400Default`-shaped payload here (and in any other 400 paths).
</issue_to_address>
### Comment 2
<location path="src/routes/walkers/index.ts" line_range="276-285" />
<code_context>
+ ready,
+ description,
+ ],
+ (insertErr, insertResult) => {
+ if (insertResult) {
+ return reply.code(201).send({
+ message: "Walker created",
+ });
+ }
+ if (insertErr) {
+ return reply.code(503).send();
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Prioritize handling `insertErr` before checking `insertResult` in the insert callback
Right now the callback returns a 201 whenever `insertResult` is truthy, even if `insertErr` is also set. To avoid returning success on error and to follow common Node.js patterns, handle `insertErr` first and return early, then handle the success case in the `else` branch.
```suggestion
],
(insertErr, insertResult) => {
if (insertErr) {
return reply.code(503).send();
}
if (insertResult) {
return reply.code(201).send({
message: "Walker created",
});
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Previously, when the request body lacked a 'name' field, the endpoint would send an empty 400 response. This change adds a descriptive "Bad Request" message to improve API clarity and client-side debugging.
Update the package version from 3.6.0 to 3.7.0 in preparation for a new release. Also reformat the keywords array for improved readability.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add an authenticated endpoint for users to create walkers and extend walker usage classifications.
New Features:
Enhancements: