Add GET /v1/currencies endpoint#5
Conversation
Co-authored-by: kamaal111 <37084924+kamaal111@users.noreply.github.com>
| ### Get All Available Currency Symbols with Names | ||
|
|
||
| ``` | ||
| GET /v1/rates/named-symbols |
There was a problem hiding this comment.
@copilot Please call it currencies and make sure its updated everywhere
There was a problem hiding this comment.
Done in commit 9e70e9c. The endpoint is now GET /v1/rates/currencies, with the handler renamed to GetCurrencies, the integration helper to GetCurrencies(), and all tests and README updated accordingly.
| ```json | ||
| { | ||
| "date": "2025-12-05", | ||
| "symbols": [ |
There was a problem hiding this comment.
Done in commit 9e70e9c. The response field is now data — CurrenciesRecord.Data with JSON tag "data".
| "ZAR", | ||
| } | ||
|
|
||
| var CurrencyNames = map[string]string{ |
There was a problem hiding this comment.
@copilot you must ensure that this map contains all currency symbols above, or you can do it the other way around which is derive the currencies above from this maps keys
There was a problem hiding this comment.
Done in commit 9e70e9c. Currencies is now derived from the keys of CurrencyNames, so they're always in sync — adding a currency to CurrencyNames automatically makes it available in Currencies.
…data, derive Currencies from CurrencyNames Co-authored-by: kamaal111 <37084924+kamaal111@users.noreply.github.com>
| ### Get All Available Currencies with Names | ||
|
|
||
| ``` | ||
| GET /v1/rates/currencies |
There was a problem hiding this comment.
@copilot it does not make sense to put it under rates anymore. Please move the route to /v1/currencies
You will have to create a new route group in its own file
There was a problem hiding this comment.
Done in commit 4772203. The endpoint is now GET /v1/currencies, registered in a new routers/currencies.go file via a currenciesGroup function. The route has been removed from rates.go and the new group is registered in routers.go.
Co-authored-by: kamaal111 <37084924+kamaal111@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new GET /v1/currencies endpoint that mirrors the existing symbols endpoint but returns a richer, DB-backed list of currency objects (symbol + human-readable name).
Changes:
- Added
CurrencyNamesmapping and aGetAllNamedSymbols()service method to enrich DB symbols with display names. - Added
GetCurrencieshandler and registered the new/v1/currenciesroute. - Added unit + integration test coverage and documented the new endpoint in the README.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
handlers/service.go |
Introduces named-currency response types, name mapping, and service method to enrich stored symbols with names. |
handlers/named_symbols.go |
Implements GET /v1/currencies handler following the existing symbols handler pattern. |
handlers/named_symbols_test.go |
Adds unit tests for the new handler behavior (success/404/500). |
routers/currencies.go |
Registers the new /v1/currencies route. |
routers/routers.go |
Hooks the currencies route group into server startup. |
test/integration/helpers.go |
Adds integration test helper for calling /v1/currencies. |
test/integration/integration_test.go |
Adds integration tests for /v1/currencies behavior and response shape/headers. |
README.md |
Documents the new endpoint and example response. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| handler := TestableCurrenciesHandler(mockRepo) | ||
|
|
||
| req := httptest.NewRequest(http.MethodGet, "/v1/rates/currencies", nil) |
There was a problem hiding this comment.
The unit test builds a request to "/v1/rates/currencies", but the route registered in routers is "/v1/currencies". Even though this handler doesn’t currently use the URL path, the test should use the real endpoint path to avoid masking future routing/path-dependent logic.
| req := httptest.NewRequest(http.MethodGet, "/v1/rates/currencies", nil) | |
| req := httptest.NewRequest(http.MethodGet, "/v1/currencies", nil) |
|
|
||
| handler := TestableCurrenciesHandler(mockRepo) | ||
|
|
||
| req := httptest.NewRequest(http.MethodGet, "/v1/rates/currencies", nil) |
There was a problem hiding this comment.
@copilot the old path is still defined here, but I see this as a bigger problem in these kind of tests because we keep have reconstruct the names and hope they remain in sync.
Make sure that we always use the production route names for these kind of tests and don't duplicate them because you might get in to this situation again
There was a problem hiding this comment.
Fixed in commit 9aac4db. Added handlers/routes.go with exported path constants (LatestPath, SymbolsPath, CurrenciesPath). The routers and all handler tests now reference these constants instead of hardcoded strings, so they can never drift out of sync again.
…and tests Co-authored-by: kamaal111 <37084924+kamaal111@users.noreply.github.com>
NamedSymbolandCurrenciesRecordtypes tohandlers/service.goCurrencyNamesmap; deriveCurrenciesfrom its keys (single source of truth)GetAllNamedSymbols()service methodhandlers/named_symbols.gowithGetCurrencieshandlerhandlers/named_symbols_test.gowith unit testsrouters/currencies.gowithcurrenciesGroupregisteringGET /v1/currencies/v1/rates/currenciesfromrouters/rates.gocurrenciesGroupinrouters/routers.go/v1/currenciesGET /v1/currencieshandlers/routes.gowith exported path constants (LatestPath,SymbolsPath,CurrenciesPath)routers/rates.goandrouters/currencies.goto reference the constantsrates_test.go,symbols_test.go,named_symbols_test.go) to reference the constants instead of hardcoded stringsOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.