Skip to content
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

Refatoração de tipo de unidade #60

Merged
merged 17 commits into from
Jul 5, 2024
Merged

Refatoração de tipo de unidade #60

merged 17 commits into from
Jul 5, 2024

Conversation

Vinigperuzzi
Copy link
Contributor

@Vinigperuzzi Vinigperuzzi commented Jul 3, 2024

Introdução
Esta PR aborda a necessidade de melhorar a segurança, a visualização e a experiência de usuário no que envolve o tipo de unidade do condomínio. Para isso, foram implementadas validações de autenticação para gerenciamento de tipo de unidade, estilização do formulário de cadastro e adicionadas novas validações de cadastro.
Adicionalmente, aborda a necessidade de padronização de código e, para tanto, foram refatoradas as flash messages de cadastro e edição de um condomínio.

Objetivos da Refatoração:

  1. Configurar autorização para o administrador: Assegurar que apenas administradores tenham acesso ao formulário de cadastro de um tipo de unidade, bem como à sua página e formulário de edição.
  2. Atualizar atributos de um tipo de unidade: Adicionar atributo 'fração ideal' para completar os dados exigidos no projeto.
  3. Estilização de Formulários: Alinhar a aparência dos formulários de cadastro e edição de um tipo de unidade ao padrão de estilização do projeto.
  4. Alterar/Criar métodos de formatação de texto: Alterar no Model UnitType o método 'p_metrage' para 'metreage_to_square_meters' para facilitar legibilidade e criar o método 'fraction_to_percentage'. O primeiro imprime em tela o valor de metragem no formato "XXm²" e o segundo imprime o valor de fração ideal no formato "X%".
  5. Aninhar tipo de unidade com o condomínio para que fiquem vinculados ao fazer o cadastro.

Detalhamento das Intervenções:

  • ao tentar acessar o formulário de cadastro ou edição sem estar logado, o usuário é redirecionado para a página de login:
    image

  • esta é a nova página de cadastro de um tipo de unidade
    image

  • Metragem e Fração Ideal devem ser maior que zero:
    image

  • Ao clicar em 'cadastrar tipo de unidade', o usuário deve selecionar um condomínio ao qual será vinculado
    image

  • Com os métodos metreage_to_square_meters e fraction_to_percentage, a visualização de detalhes de um tipo de unidade fica assim:
    image

  • padronização de flash messages no controller do condomínio
    image
    image

resolve #46

cellaaleo and others added 9 commits July 2, 2024 21:35
…formatação dos valores de metreage e fraction, refatoração de p_metreage

Co-authored-by: Marcella Aleo <[email protected]>
…a vírgula, ajustes nos testes, ajuste no label de fração do formulário de unit type

Co-authored-by: Marcella Aleo <[email protected]>
…ado automaticamente pelo windows

Co-authored-by: Marcella Aleo <[email protected]>
@Vinigperuzzi Vinigperuzzi added bug Something isn't working enhancement New feature or request labels Jul 3, 2024
@Vinigperuzzi Vinigperuzzi requested a review from akaninja July 3, 2024 21:46
@Vinigperuzzi Vinigperuzzi marked this pull request as ready for review July 3, 2024 21:51
@akaninja
Copy link
Contributor

akaninja commented Jul 4, 2024

Galera, eu acho que vamos precisar corrigir esse PR antes de começar a revisar, tudo bem?
Em primeiro lugar, o PR não tem uma descrição do objetivo clara, só tem um monte de itens. É importante dar um resumo da tarefa e da motivação do PR. Além disso, se tiverem telas que foram modificadas ou criadas, lembrem de colocar prints.

Em segundo lugar, o PR está com mais de 80 alterações. Fica inviável analisar o código... entendi que muitos arquivos estão identicos ao que estavam antes, mas se for possível evitem adicionar no commit arquivos que vocês não alteraram.

@gabdemiranda
Copy link

Gente, percebi que vocês estão refatorando o PR, mas queria pontuar sobre a estrutura da descrição dele. Percebi que estão caminhando para uma estrutura semelhante ao que pontuei no PR de "Refatorar Torres", não é organizado apenas colocar os pontos de intervenção superficialmente alternados entre prints, ainda ficará confuso de entender e desorganizado, precisamos começar a fazer PRs com descrições mais elaboradas, principalmente quando atingem diferentes pontos.

Vocês poderiam começar com um pequeno resumo inicial (antes dos prints e explicações mais detalhadas), para deixar mais claro o porquê dessa refatoração (os problemas), os objetivos que estão sendo "atacados" nessa refatoração e depois desenvolver com as imagens cada uma das partes que vocês "atacaram". Algo como:

Introdução

Este PR aborda a necessidade de melhorar X, Y e Z. Pois a aplicação isso, isso e aquilo, além de corrigir os problemas encontrados em tal e tal cenário.

Objetivos da Refatoração

  1. Configura autorização para o administrador: Assegurar que apenas administradores tenham acesso às páginas relacionadas a X, Y e Z.
  2. Estilização de Formulários: Alinhar a aparência dos formulários de X e Y ao padrão de estilização do projeto.
  3. Refatora para ser aninhado com o condomínio: É necessário fazer o vínculo entre X e Y, etc.
  4. Flash Messages: Implementar e permitir o uso de mensagens de aviso (info e warning) com múltiplas mensagens.
  5. [Outros pontos, estou colocando apenas um exemplo]

Detalhamento das Intervenções:

[aqui vocês colocam os prints, entram em mais detalhes do que foi feito parte por parte.]

Dessa forma fica mais oragnizado e fácil pra gente revisar e entender o PR de vocês =)

Além disso, muitos arquivos subiram como alterações mas não tem alteração nenhuma na realidade, é o mesmo arquivo como "diff". Precisamos que vocês fiquem atentos ao que está sendo commitado e se realmente tem alteração nos arquivos ou não, temos um PR com mais de 80 arquivos alterados e pelo que percebi, pelo menos a metade deles não tem diferença nenhuma, fica inviável revisar assim e entender o que realmente está sendo modificado e proposto como modificação nesse PR.

@Vinigperuzzi
Copy link
Contributor Author

Vinigperuzzi commented Jul 4, 2024

Então, os arquivos marcados como iguais são bem mais do que a metade, são 65 para ser exato. O que aconteceu foi que quando fiz o fetch na minha máquina o VSCode modificou o terminador de linha desses arquivos para CRLF, e na verdade eu nem entendi bem o porquê, pois a gente nem tocou nesses arquivos, acho que foi alguma rotina do bin/setup que fez com que o windows "tocasse" nesses arquivos e modificasse. Quando a gente rodou o rubocop pipocou 65 offenses do tipo: Layout/EndOfLine: Carriage return character detected.
A gente começou a desenvolver na máquina da Marcella e ela fez um push para que eu atualizasse e tentasse resolver um problema com o modal, eu resolvi e pushei de volta para a gente seguir o desenvolvimento. No git status, não estava marcando todos esses arquivos como modificados, então commitei e pushei para ela. Ao passar o rubocop na branch, fogos de artifício...

Eu modifiquei nas minhas configurações do VSCode para que o terminador de linha padrão seja o LF e vou adicionar os arquivos modificados um a um a partir de agora, pois mesmo alterando ainda tem alguns arquivos que estão como CRLF, mas não estão na branch do remoto.

@Vinigperuzzi
Copy link
Contributor Author

E já que mencionei o modal, tem uma dúvida que eu quero tirar com um de vocês também, de preferência por call no zoom.

…as e remove mensagem de erro de cadastro que estavam duplicadas.

Co-authored-by: Vinícius Peruzzi <[email protected]>
Copy link
Contributor

@akaninja akaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fiz alguns apontamentos no código para vocês olharem. Dessa vez nós revisamos, mas não vamos mais olhar PRs com esse tipo de problema com tantos arquivos modificados. Tentem resolver antes de abrir para revisão.

spec/models/unit_type_spec.rb Outdated Show resolved Hide resolved
spec/requests/unit_type_management_spec.rb Outdated Show resolved Hide resolved
spec/system/unit_types/user_manage_unit_types_spec.rb Outdated Show resolved Hide resolved
spec/models/unit_type_spec.rb Outdated Show resolved Hide resolved
spec/models/unit_type_spec.rb Outdated Show resolved Hide resolved
@Vinigperuzzi
Copy link
Contributor Author

Vinigperuzzi commented Jul 4, 2024

@akaninja e @gabdemiranda, por favor, deem uma olhada em como ficou o PR no final. Consegui remover todos os arquivos desnecessários e "não modificados". Dos que ficaram, todas as alterações são propositais e os arquivos que ficaram estão no formato de fim de linha LF (confirmado e com certeza).
Uma pergunta, os arquivos que ficarão como "last changed" por mim são apenas os 24 indicados na aba files changed, certo? Mesmo que eu tenha modificado eles e voltado para o estado original?

Sobre as alterações dos arquivos, consegui resolver localmente nas configurações de git e do próprio vscode e consegui rastrear as modificações dos arquivos e retornar somente eles ao estado original na criação da branch, com exceção dos propositalmente alterados que foram adicionalmente configurados para LF.
Ainda existem outros arquivos na MAIN que estão como CRLF, como eu disse no finalzinho da call, mas não mergeados por mim. Na daily de amanhã vou avisar os colegas quanto às configurações e procedimentos que eu realizei para alcançar esse estado, e propor o debate de como proceder com os que já estão na main.

@akaninja
Copy link
Contributor

akaninja commented Jul 5, 2024

Uma pergunta, os arquivos que ficarão como "last changed" por mim são apenas os 24 indicados na aba files changed, certo? Mesmo que eu tenha modificado eles e voltado para o estado original?

Tem que entender como funciona o Git. O Git lê histórico de modificações. Então ao longo desses 17 commits feitos, em alguns deles ainda vão aparecer os 82 arquivos modificados, em comparação com os anteriores da main. Mas no conjunto de modificações acumulados dos 17 commits, ficam aparecendo somente os 24 arquivos modificados NESTE PR.

Se você clicar na aba que lista os commits e ir clicando em cada commit no histórico, ele vai te mostrar o que foi modificado em cada commit. No commit específico 5aaeb4aaaf850bbab45c103625578ebf95eec4f8 ainda aparecem os 82 arquivos modificados, já que foi o commit em que vocês adicionaram os arquivos com a configuração de fim da linha "errada".

Mas nos seguintes não mostra mais, porque o Git está mostrando em comparação com o anterior. Não com a main.

Dai no PR ele vai considerar todo o histórico de commits da sua branch inteira, por isso no Files Changed, não aparecem os 82 arquivos modificados. Mas no histórico de commits vai sim aparecer que vocês mexeram em 82 arquivos num determinado commit e depois desfizeram.

@cellaaleo
Copy link
Contributor

Uma pergunta, os arquivos que ficarão como "last changed" por mim são apenas os 24 indicados na aba files changed, certo? Mesmo que eu tenha modificado eles e voltado para o estado original?

Dai no PR ele vai considerar todo o histórico de commits da sua branch inteira, por isso no Files Changed, não aparecem os 82 arquivos modificados. Mas no histórico de commits vai sim aparecer que vocês mexeram em 82 arquivos num determinado commit e depois desfizeram.

@akaninja, então ainda seria necessário criar um novo PR apenas com as alterações necessárias, ignorando este, ou o melhor a fazer é dar merge neste PR mesmo e apenas considerar as lições aprendidas com isso?

@Vinigperuzzi Vinigperuzzi merged commit 609f238 into main Jul 5, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refatoração de tipo de unidade
4 participants