Changing api path #13
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
❇️ CodePress Review Summary
👋 Hey team,
Overall the changes look great! I left 2 helpful notes inline.
Here's the quick rundown:
❌ Decision: REQUEST CHANGES
While the code change is straightforward, this is a breaking change for API consumers. It requires careful coordination with client updates, which is not evident from the PR itself. Further context or a deployment plan is needed.
👍 What's working well
The updated unit tests for the new /v1/ API path prefix are commendable. This ensures continued test coverage and validates the breaking change, which is crucial for maintaining stability.
ℹ️ Other thoughts
The API path prefix update is correctly implemented, though it's important to note this is a breaking change requiring coordinated client deployment. For future extensibility and easier version management, consider making the API version a configurable constant.
| const url = `${protocol}://${apiHost}${ | ||
| apiPort ? `:${apiPort}` : "" | ||
| }/api/${apiPath}`; | ||
| }/v1/${apiPath}`; |
There was a problem hiding this comment.
ℹ️ FYI: This change correctly updates the API path prefix. As noted in the diff context, this is a breaking change and requires coordinated deployment with all API clients. For future extensibility, consider making the API version (v1) a configurable constant if there's a foreseeable need to support multiple API versions concurrently or to easily update the version string.
| const protocol = | ||
| apiHost === "localhost" || apiHost === "127.0.0.1" ? "http" : "https"; | ||
| const expectedUrl = `${protocol}://${apiHost}:${apiPort}/api/${endpoint}`; | ||
| const expectedUrl = `${protocol}://${apiHost}:${apiPort}/v1/${endpoint}`; |
There was a problem hiding this comment.
👏 PRAISE: Good to see the unit tests are updated to reflect the new /v1/ API path prefix, ensuring test coverage for this breaking change.
Update API Path Prefix from
/apito/v1This PR updates the base API path used by the server from
/api/to/v1/and adjusts related test cases to match.Key Changes:
src/server.jsto construct API URLs with a/v1/prefix instead of/api/.test/server.test.jsto reflect the new/v1/path in expected URLs for API calls.Review Notes:
/v1/path simultaneously to avoid service disruption.