feat: buscar coordenadas do abrigo #87
Conversation
- Finish interaction with Google Maps API through src/googleMaps/mapsApi.ts. - Now retrieving Shelter coordinates from Google Maps API everytime a new shelter is created or a already existent shelter is updated by admin. - If API fetch is not working, it does not break the creation or update features. - Loading Maps API key from .env file. - Update env.example: Create MAPS_API_URL and MAPS_API_KEY variables.
wOL-Lucas
changed the title
src/shelter/shelter.controller.ts
Outdated
| @@ -49,13 +49,14 @@ export class ShelterController { | |||
| @UseGuards(StaffGuard) | |||
| async store(@Body() body) { | |||
| try { | |||
| await fetchShelterCoordinates(body); | |||
There was a problem hiding this comment.
❓ Já que esta mutando o body adicionando lat/lng, não seria melhor fazer isso direto no service?
There was a problem hiding this comment.
Realmente :D
Vou alterar, obrigado.
There was a problem hiding this comment.
A propósito, acabei de ver que este PR esta utilizado o Google Maps para mostrar o local do abrigo com base no endereço
There was a problem hiding this comment.
Ah sim, correto.
Mas se considerarmos o uso das informações como longitude e latitude do abrigo teríamos que usar a api ou semelhante. - Claro, se a implementação de futuras funcionalidades como "Achar abrigo mais próximo" ou calcular "distância" da localização atual do usuário até o abrigo x, y ainda forem relevantes.
Acha interessante prosseguir?
There was a problem hiding this comment.
Na minha opnião "achar abrigo mais próximo" por exemplo é bem interessante sim, apesar de talvez não ser um item de alta prioridade no momento? Estou tendo problemas pra acessar meu Discord 😓, talvez seja interessante anunciar por lá pra ver o que outras pessoas acham.
There was a problem hiding this comment.
Ótimo. Eu gostaria de fazer parte da comunidade, mas não encontrei link para o discord.
Eu poderia ter acesso?
There was a problem hiding this comment.
@wOL-Lucas eu peguei aqui nessa issue
Move the concern of retrieving the shelter coords from api and updating payload, from controller to shelter.service
src/shelter/shelter.service.ts
Outdated
| @@ -49,6 +58,16 @@ export class ShelterService { | |||
|
|
|||
| async fullUpdate(id: string, body: z.infer<typeof FullUpdateShelterSchema>) { | |||
| const payload = FullUpdateShelterSchema.parse(body); | |||
| if (payload.address) { | |||
There was a problem hiding this comment.
Que tal passar para um método privado essa lógica, que se repete aqui e no método store? Assim evita duplicidade e isola o preenchimento de coordenadas... esse método pode ser chamado apenas na hora de concatenar o payload na resposta, algo do tipo:
...generatePayloadWithCoordinates(payload),
There was a problem hiding this comment.
Show, obrigado.
É realmente uma boa ideia. Vou alterar.
|
@wOL-Lucas ótima idéia, deixei um comentário... Tenho mais 2 pontos, os testes unitários e padrão de nomenclaturas (CamelCase vs PascalCase)) mas devido a urgência do propósito da aplicação, não sei como estamos lidando aqui (vou deixar essa dúvida no Discord). |
|
@wOL-Lucas mais uma dúvida... ainda não consegui subir o projeto então não sei como está hoje, mas e caso a latitude e longitude já tenham sido preenchidos? Não vale verificar se eles estão nulos antes de seguir no fluxo de busca automática? |
|
@thaua, pelo que verifiquei hoje em prod mesmo essas informações não estão sendo preenchidas, mas obviamente é uma boa ideia manter o input original. Não me atentei nessa questão. Vou aproveitar as outras alterações que você propôs e ajustar. |
This commit refactors the shelter service by moving the logic for updating coordinates into a private method. This eliminates repetitive code in both the store and fullUpdate methods.
| latitude: number | null; | ||
| longitude: number | null; |
There was a problem hiding this comment.
Desculpem a intromissão, não entendo nada de TS, mas notei que estão usando aqui dois atributos para representar um ponto e o mesmo está refletido também na tabela do banco de dados.
No Postgres temos um tipo de dado nativo dele chamado Point que se combinado, por exemplo, com a extensão nativa earthdistance é possível realizar queries para buscar registros mais próximos ou distantes de determinado ponto.
| address: string, | ||
| ): Promise<{ lat: number | null; lng: number | null }> { | ||
| try { | ||
| const response = await fetch( |
There was a problem hiding this comment.
o NestJS possui um modulo http para fazer esse tipo de request
| | z.infer<typeof CreateShelterSchema> | ||
| | z.infer<typeof FullUpdateShelterSchema>, | ||
| ) { | ||
| if ((payload.latitude && payload.longitude) || !payload.address) { |
There was a problem hiding this comment.
No cenário do fullUpdate em que o usuário altera o local do abrigo, os valores da latitude e longitude não serão atualizados
🔨 Tenho uma nova PR para vocês revisarem
🤔 O que foi feito?
Incluído integração com API do Google Maps para buscar coordenadas dos abrigos automaticamente
quando eles são criados ou atualizados por admins. Por que? Acho interessante podermos pensar na possibilidade de mostrar a distância do usuário até o abrigo em uma futura funcionalidade no frontend. Talvez buscar os abrigos mais próximos dele. Buscar automaticamente as coordenadas através do Google Maps facilita o desenvolvimento dessa possível funcionalidade.
Importante:
Caso a API não esteja funcionando, ela não quebra as funcionalidades de Inclusão e atualização, apenas mantem os valores de longitude e latitude como null. Pelo que percebi esses valores não estão sendo informados na criação dos abrigos.
Sobre a utilização da API, ficaria feliz em contribuir com a disponibilização da API Key, se for possível e caso acatem a ideia.
📗 Checklist do desenvolvedor
Foi testado localmente? Sim.
Foi adicionado documentação necessária (swagger, testes e etc)? - Não
👀 Checklist do revisor
Revisor 1️⃣
Você entendeu o propósito desse PR?
Você entendeu o fluxo de negócio?
Você entendeu o que e como foi desenvolvido tecnicamente a solução?
Você analisou se os testes estão cobrindo a maioria dos casos?