feat: weather location cptec #590
Conversation
- add endpoint to get weather by location - add test to weather location - add doc of weather location api
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
I need change documentation response... |
- fix documentation with location sample of Campinas city, change to Brejo Alegre city
|
change location to Brejo Alegre city sample |
There was a problem hiding this comment.
Obrigado pela contribuição, @enieber!
Achei que o PR ficou muito bom.
Só deixei uma observação, mas não é um block, na minha opinião ;-)
revert change from pages/_app.js not require in this scope
|
Acho valido as alterações... vou implementar...
Em sex., 23 de fev. de 2024 22:04, Lorhan Sohaky ***@***.***>
escreveu:
… ***@***.**** commented on this pull request.
------------------------------
In services/cptec/weather.js
<#590 (comment)>:
> + if (parsed.cidade) {
+ const jsonData = formatPrediction(parsed);
+ if (jsonData.cidade === 'null') {
+ return null;
+ }
+ return jsonData;
+ }
+ return [];
O que acha dessa mudança? O que muda aqui é que foi adotada a estratégia
Return Early Pattern e em vez de retornar [] quando não tem cidade, é
retornado null
⬇️ Suggested change
- if (parsed.cidade) {
- const jsonData = formatPrediction(parsed);
- if (jsonData.cidade === 'null') {
- return null;
- }
- return jsonData;
- }
- return [];
+ if (!parsed.cidade) {
+ return null;
+ }
+
+ const jsonData = formatPrediction(parsed);
+ if (jsonData.cidade === 'null') {
+ return null;
+ }
+ return jsonData;
------------------------------
In tests/cpetc/previsao-v1.test.js
<#590 (comment)>:
> + test('GET /api/cptec/v1/clima/previsao/semana/:lat/:long', async () => {
+ const requestUrl = `${global.SERVER_URL}/api/cptec/v1/clima/previsao/semana/-22.90/-47.06`;
+ const response = await axios.get(requestUrl);
+
+ expect(response.status).toBe(200);
+ expect(Array.isArray(response.data)).toBe(false);
+
+ expect(Array.isArray(response.data.clima)).toBe(true);
+ expect(response.data.clima.length).toBeGreaterThan(2);
+ expect(response.data.clima.length).toBeLessThanOrEqual(7);
+
+ expect(response.data).toMatchObject({
+ cidade: 'Campinas',
+ estado: 'SP',
+ });
+ });
Se possível adicionar mais alguns teste:
- latitude inválida
- longitude inválida
- cidade inexistente
—
Reply to this email directly, view it on GitHub
<#590 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB4KN7DU64Z7JCJZDQJ7XZTYVE4ALAVCNFSM6AAAAABDWGEE6WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQOJZGEZTQOJXHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Co-authored-by: Lorhan Sohaky <16273730+LorhanSohaky@users.noreply.github.com>
- add test iinvalid latitude - add test invalid longitude - add test invalid city
|
Testes implementados, os testes que estão falhando são relacionados a:
|
There was a problem hiding this comment.
Só seria interessante entender se haveria alguma maneira de evitar esse 500. Não que o status code esteja errado, mas se pode ser algum caso que possamos tratar, por exemplo, quando não encontra a localidade, qual status code retornado pela api deles? Não daria para utilizar isso para informar que a cidade não foi encontrada?
Deveria ser 404, pois não encontrou a localidade.. |
|
|
@LorhanSohaky alterei para 404 qiuando não encontrar a cidade. |
| estado: 'SP', | ||
| }); | ||
| }); | ||
| // TODO: verify response to positive lat/long |
There was a problem hiding this comment.
Existe algo que impeça, temporariamente, este todo de se concretizar?
There was a problem hiding this comment.
A api está respondendo a cidade como São Paulo sp ao invés de Campinas.
|
I wanna create app weather without use city code only latitude and longitude, when I search in CPTEC API I found one endpoint with this resource so I add in here to facility use.
Maybe some changes not in scope because
npm run fixcommand.