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

Feat/relatorios #133

Merged
merged 10 commits into from
Nov 10, 2023
Merged

Feat/relatorios #133

merged 10 commits into from
Nov 10, 2023

Conversation

douglastofoli
Copy link
Contributor

Descrição

Contém tela com formulário para preenchimento de relatórios de forma dinâmica, mensal, trimestral ou anual.

Stories relacionadas (Shortcut)

  • [sc-xxxx]

Pontos para atenção

  • Listar pontos para atenção no review
  • Listar pontos para atenção nos testes

Possui novas configurações?

  • Descrever as configurações alteradas ou novas

Possui migrations?

  • Sim

@douglastofoli douglastofoli self-assigned this Oct 19, 2023
@douglastofoli douglastofoli added the feature A new feature label Oct 19, 2023
Copy link
Member

@zoedsoupe zoedsoupe left a comment

Choose a reason for hiding this comment

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

Ótima pr @douglastofoli, mandou bem nas decisões de código e em como organizar a feature de criação.

<img src=“https://media.giphy.com/media/3oz9ZE2Oo9zRC/giphy.gif?cid=ecf05e47non0tswcysrymnvor4nuem9shp56kw4n61ubcyjq&ep=v1_gifs_search&rid=giphy.gif&ct=g” width=“100” height=“100” />

Apenas a fim de histórico: essa PR está com um minor bug onde os valores dos formulários de criação não estão sendo computados pela live view. Um bugfix já será feito

Isso inclui os comentários dessa PR. ficam como sugestões de melhorias futuras

Comment on lines +5 to 6
alias ModuloPesquisa.Models.RelatorioPesquisa, as: RelatorioPesquisaModel
alias ModuloPesquisa.Schemas.RelatorioPesquisa
Copy link
Member

Choose a reason for hiding this comment

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

por questão de padronização, acho melhor deixar o alias do model como ele é e só renomear o do schema para Schema, tipo:

alias Modulo.Models.X
alias Modulo.Schemas.X, as: Schema

alias ModuloPesquisa.Models.RelatorioAnualPesquisa, as: Anual
alias ModuloPesquisa.Models.RelatorioMensalPesquisa, as: Mensal
alias ModuloPesquisa.Models.RelatorioTrimestralPesquisa, as: Trimestral
alias ModuloPesquisa.Models.RelatorioPesquisa, as: RelatorioPesquisaModel
alias ModuloPesquisa.Schemas.RelatorioPesquisa

@locale Application.compile_env(:pescarte, :locale, "pt_BR")
Copy link
Member

Choose a reason for hiding this comment

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

boa! tava querendo fazer isso msm! mandou bem

Comment on lines +26 to +28
defp get_relatorio_tipo(%RelatorioPesquisa{tipo: "anual"}), do: :anual
defp get_relatorio_tipo(%RelatorioPesquisa{tipo: "mensal"}), do: :mensal
defp get_relatorio_tipo(%RelatorioPesquisa{tipo: "trimestral"}), do: :trimestral
Copy link
Member

Choose a reason for hiding this comment

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

achei que o tipo do relatório era um Ecto.Enum, deveria vir como átomo já

Comment on lines +56 to +58
|> cast_embed(:conteudo_anual)
|> cast_embed(:conteudo_mensal)
|> cast_embed(:conteudo_trimestral)
Copy link
Member

Choose a reason for hiding this comment

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

talvez seja melhor depois criar 3 changeset/2 diferentes, pra cada tipo, assim a gente consegue forças que o conteúdo seja required, tipo:

def mensal_changeset(relatorio, attrs) do
  relatorio
  |> changeset()
  |> cast_embed(:conteudo_mensal, required: true)
end

def changeset(conteudo, attrs) do
conteudo
|> cast(attrs, @fields)
|> validate_required([])
Copy link
Member

Choose a reason for hiding this comment

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

todos os campos são obrigatórios!

def changeset(conteudo, attrs) do
conteudo
|> cast(attrs, @fields)
|> validate_required([])
Copy link
Member

Choose a reason for hiding this comment

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

mesma coisa aqui

def changeset(conteudo, attrs) do
conteudo
|> cast(attrs, @fields)
|> validate_required([])
Copy link
Member

Choose a reason for hiding this comment

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

mesma coisa aqui

mes: sequence(:mes, & &1),
link: "https//datalake.com/relatorio_anual",
link: "https//datalake.com/relatorio",
conteudo_mensal: %{},
Copy link
Member

Choose a reason for hiding this comment

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

teria que criar factory pros conteúdos tbm, com strings aleatórias

alias ModuloPesquisa.Models.RelatorioPesquisa
alias ModuloPesquisa.Repository

@locale "pt_BR"
Copy link
Member

Choose a reason for hiding this comment

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

troca pela config com compile_env/3

Copy link
Member

Choose a reason for hiding this comment

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

acho que seu editor não conseguiu ler o .formatter.exs e colocou parentêses em todas as macros. não é nada urgente, mas é bom se atentar à isso

@zoedsoupe zoedsoupe merged commit b423733 into main Nov 10, 2023
0 of 3 checks passed
@zoedsoupe zoedsoupe deleted the feat/relatorios branch November 10, 2023 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature
Projects
No open projects
Status: Completo
Development

Successfully merging this pull request may close these issues.

2 participants