New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implementação do Endpoint de Feriados V2 por Estado e Município no BrasilAPI #558
base: main
Are you sure you want to change the base?
Conversation
… com feriados por municipios e estados
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A proposta do PR é bastante interessante, deixei algumas observações, se puder ver se fazem sentido para você ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notei que houve uma mistura de variáveis com nomes em português e outras com nome em inglês.
Apesar de não ser um block, talvez fosse interessante manter uma padronização.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gosto da sugestão de manter o código em ingles tbm! ^^
if (uf != "") { | ||
let urlUF = 'https://servicodados.ibge.gov.br/api/v1/localidades/estados'; | ||
const responseUF = await fetch(urlUF); | ||
const htmlUF = await responseUF.text(); | ||
const objetoUF = JSON.parse(htmlUF); | ||
|
||
var achou = false; | ||
|
||
objetoUF.forEach((item) => { | ||
if (item["sigla"] == uf.toUpperCase()) { | ||
achou = true; | ||
} | ||
}); | ||
|
||
if (!achou) { | ||
throw new NotFoundError({ | ||
name: 'NotFoundError', | ||
message: `Sigla não corresponde a nenhum estado existente.`, | ||
type: 'uf_range_error', | ||
}); | ||
} | ||
|
||
if (city != "") { | ||
const urlCity = `https://servicodados.ibge.gov.br/api/v1/localidades/estados/${uf.replaceAll(" ", "_")}/municipios`; | ||
const responseCity = await fetch(urlCity); | ||
const htmlCity = await responseCity.text(); | ||
|
||
const objetoCity = JSON.parse(htmlCity); | ||
|
||
achou = false; | ||
|
||
objetoCity.forEach((item) => { | ||
if (item["nome"].toUpperCase() == city.toUpperCase()) { | ||
achou = true; | ||
} | ||
}); | ||
|
||
if (!achou) { | ||
throw new NotFoundError({ | ||
name: 'NotFoundError', | ||
message: `Cidade não existente ou não correspondete ao estado informado.`, | ||
type: 'feriados_error', | ||
}); | ||
} | ||
|
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Será que não faria sentido ter essa camada de consulta a dados em um serviço externo?
Hoje já temos um service para dados do ibge (services/ibge/gov.js), talvez fosse interessante reaproveitar o que puder dele e somente criar novas funções dentro do mesmo arquivo, visando o princípio da responsabilidade única.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pelo que vi do serviço usado parece que a url é sempre montada com o nome da cidade sem acentuação, seria interessante testar esse cenário tentando enviar uma cidade com o acento no nome, o que acha?
|
||
if (uf != "") { | ||
let urlUF = 'https://servicodados.ibge.gov.br/api/v1/localidades/estados'; | ||
const responseUF = await fetch(urlUF); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a lista de siglas de estados é algo que muda muito pouco, talvez não justifique pegar do ibge dinamicamente sempre, poderia estar estática no código, ou num lugar também estático que faça parte da build, para economizar um http get, (caro, sujeito a erros de rede, etc)
}); | ||
} | ||
|
||
const url = `https://www.feriados.com.br/feriados-${city.replaceAll(" ", "_")}-${uf.replaceAll(" ", "_")}.php?ano=${year}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seria interessante marcar com um comentário tipo // TODO: ou FIXME: um pedido de trocar a fonte dos feriados por algo que fosse mais canonico/reconhecido, talvez Wikipedia
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apesar da intenção incrível do PR talvez isso seja necessário pra uma primeira versão especialmente pelo ponto que a consulta dos dados é via interpretação de HTML que pode subtamente mudar
if(!('estado' in request.query)) { | ||
request.query.estado = ''; | ||
} | ||
if(!('cidade' in request.query)) { | ||
request.query.cidade = ''; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Será que isso é realmente necessário dado que caso não haja na query esses parametros eles serão falsy? 😬
Se a gnt só ignora isso e passa pro getHolidays lá a gnt só precisa validar se o valor contido no caminho existe e é truthy ^^
Descrição da Feature
Nesta atualização do BrasilAPI, expandimos a funcionalidade do endpoint de feriados para incluir não apenas os feriados nacionais, mas também os específicos de cada estado e município brasileiro. Agora, o endpoint é capaz de fornecer informações precisas sobre feriados locais em qualquer ano solicitado.
Motivação
A motivação para esta atualização surgiu da necessidade de fornecer dados mais precisos e abrangentes sobre feriados no Brasil. Observamos que a versão anterior do endpoint de feriados era limitada, pois retornava apenas feriados nacionais, muitos dos quais não eram observados em todos os municípios. Além disso, a seleção de feriados de todo o Brasil resultava na exibição de feriados de uma capital aleatória, como São Paulo ou Rio de Janeiro, em vez de todos os feriados nacionais.
Detalhes Técnicos
Benefícios para Usuários e Desenvolvedores
Esta atualização beneficiará diretamente desenvolvedores, empresas e usuários que necessitam de informações precisas sobre feriados em diferentes regiões do Brasil. Desenvolvedores poderão integrar facilmente esses dados em aplicativos e sistemas, enquanto empresas poderão planejar melhor suas operações considerando feriados locais. Para os usuários, essa funcionalidade traz a conveniência de acessar informações sobre feriados em suas localidades de maneira rápida e confiável.
Solicitação de Integração
Solicitamos a revisão e aceitação desta implementação pela comunidade do BrasilAPI, acreditando que ela adiciona valor significativo ao projeto, ampliando sua utilidade e precisão para todos os usuários.