-
Notifications
You must be signed in to change notification settings - Fork 0
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
Adicionado o Controller 'sync-shets', também uma função para estruturar dados no WriteSheetsService. #11
base: main
Are you sure you want to change the base?
Conversation
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.
@DevGranemann PRs grandes, geram reviews grandes e várias idas e vindas até conseguirmos mergear. Esse é um ótimo exemplo disso acontecendo.
TLDR; Depois de configurar tudo, o PR funciona bem, evoluímos bastante aqui! Precisamos agora tratar erros e outros ajustes para podermos mergear.
Ambiente de desenvolvimento
Rodamos o servidor local com o comando Symfony serve;
Temos um docker configurado, então podemos usar este ambiente para padronizar nosso desenvolvimento. Por favor tire um tempo para configurar seu ambiente com docker, se tiver dúvidas me avise.
Seguem os passos para ter ele funcionando localmente:
- Instale docker e docker-compose na sua máquina
- Faça novo build do container com
docker-compose build
- Desative container antigo (se estiver ativo) com
docker-compose down
- Ative o novo container atualizado:
docker-compose up -d
- Entre no container:
docker exec -it cbmsc_booker_web /bin/bash
- Instale dependências novas do symfony:
composer install
Aqui um problema causado por não usarmos o mesmo ambiente. No linux (ambiente do nosso container docker), ele entende sheetController.php
como sendo um arquivo diferente de SheetController.php
, então como dentro do arquivo, a classe dele está em pascal case
, o arquivo deve serguir a mesma convenção.
Eu corrigi este problema e ajustei o docker neste commit.
Melhores instruções para rodar o PR
Como este PR espera que tenhamos a variável de ambiente GOOGLE_AUTH_CONFIG
configurada, senti falta dessa descrição no PR ou no README do repositório.
Lembre-se que o dev que vai avaliar seu PR, geralmente terá menos contexto do que você sobre a modificação que estará sendo feita, o que é exatamente nosso caso. Idealmente, eu devo apenas seguir passo a passo suas instruções para testar o PR e devo conseguir fazer com sucesso.
Aí depois de testar o "caminho feliz", vou investir o resto do tempo que aloquei para revisar seu PR, tentando encontrar pontos falhos e de melhorias.
Como as planilhas são públicas, poderíamos também já colocar os IDs de teste para rodar o código:
- ID da planilha de escolha de horários: 1eImum6WiihpCqGaDISo-RWj-q0QPfy64PLIJj50dT5M
- ID da planilha preliminar:
Tratamento de erros
Eu não esperava ver o frontend no mesmo PR que faz a cópia de dados de uma planilha para outra. O problema que isso gera é que o PR fica grande, difícil de avaliar e difícil de mergear.
Como temos os dois aqui, vou trazer um feedback para a interface também:
- Precisamos melhorar o tratamento de erros, o que acontece se não colocarmos nenhuma informação e dermos enter?
- O que acontece se algum dos IDs de planilhas estiver incorretos e não carregar a planilha esperada?
- Se eu digito a primeira planilha correta e a segunda incorreta, ele zera todas as informações e preciso redigitar tudo
Cuidado com dados públicos
Vi que você tomou cuidado para não postar o print com os nomes dos bombeiros aqui no GH, obrigado por cuidar!
Vamos dar um passo a mais: vamos usar somente as planilhas de testes desta pasta para referência aqui, tudo bem?
Assim evitamos qualquer chance de vazar dados não previstos.
$dadosEstruturados = $writeSheetsService->estruturarDados($result); | ||
|
||
$writeSheetsService->configureClient($credentialsPath, $sheetIdB); | ||
$writeSheetsService->appendData("A13:AH13", $dadosEstruturados); |
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.
No futuro podemos tornar estes parâmetros das planilhas, configuráveis.
|
||
catch (\Exception $e) | ||
{ | ||
$this->addFlash('Erro', 'Erro ao sincronizar planilhas: ' . $e->getMessage()); |
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.
Eu gostei da ideia de usar addFlash
, mas não está funcionando pra mim.
Essa abordagem é interessante, mas pode ser perigosa, porque se os serviços abaixo não tratarem as mensagens de erro em algo para nível do usuário, poderemos compartilhar informações sensíveis num erro path de arquivos do servidor, ou ainda dados mais críticos dependendo de como fizermos os throws
abaixo.
Tarefa #9
Sincronização dos dados entre os serviços
Criei mais um controller em nosso projeto, é o 'sync-sheets'. O que ele faz, basicamente, é sincronizar os dados da planilha A com a planilha B. Fiz isso utilizando os métodos presentes nos Services criados anteriormente. Também foi necessário criar uma função que organiza a estrutura dos dados lidos pelo Service 'GoogleSheetsService'.
Como fazer a transferência de dados entre as planilhas?
Symfony serve
;/d/
e/edit
;/
, e depois dela insira o ID da planilha destino.Após esses passos, se estiver tudo certo, aparecerá uma mensagem de confirmação para você.
Depois disso, você pode conferir na planilha onde enviou os dados se eles estão lá mesmo. Na minha máquina funciona 🤓 :
Interface Gráfica
Criei uma interface gráfica simples pensando na experiência do usuário: