gems-uff / sapos

SAPOS main goal is to ease the management of information related to graduate programs such as enrollments, courses, advisement, scholarships, requirements, among others.
http://gems-uff.github.io/sapos/
MIT License
29 stars 14 forks source link

WIP: Issue #448 #497

Open IgorMonardez opened 1 week ago

JoaoFelipe commented 1 week ago

Mais alguns comentários.

Começando com perguntas que acho que @leomurta pode ajudar a dar opinião:

1) O que acontece se nenhum program_level estiver cadastrado no momento da defesa? Acho que antes da mudança aparecia o nível do programa em branco. Agora deve dar uma exceção sem indicar exatamente o que foi. Qual será que é o comportamento desejado?

2) Será que o fato do professor ter sido cadastro com afiliação deveria restringir a remoção do mesmo? Acho que faz sentido restringir nos outros campos (por exemplo, se o professor já deu aula, não deveria ser possível simplesmente remover o registro do sapos), mas não vejo muito sentido no restrict_with_exception para esse caso.

Além disso, tenho algumas sugestões de mudanças:

1) Quando institution_id de professor é nulo, a migração está criando uma afiliação com instituição nula. Faria mais sentido não criar essa afiliação. Verificar se o mesmo não acontece com program_level

2) O arquivo spec/features/custom_variables_spec.rb ainda tem a variável referente ao nível do programa (e usa isso em testes). A variável foi removida com a adição da tabela e o teste deveria refletir essa mudança.

3) O arquivo seeds.rb também está criando program_level como CustomVariable. Altere para criar ProgramLevel

4) Falta rota/active scaffold para alterar o nível do programa (e testes). Como não é mais custom variable, o local de customizar isso não existe mais e não parece ter nenhuma alternativa.

5) Em "Professores" foi adicionado um link para "Afiliações": image

Acho que seria melhor não ter esse link e as afiliações aparecerem no olhinho.

IgorMonardez commented 1 week ago

Mais alguns comentários.

Começando com perguntas que acho que @leomurta pode ajudar a dar opinião:

  1. O que acontece se nenhum program_level estiver cadastrado no momento da defesa? Acho que antes da mudança aparecia o nível do programa em branco. Agora deve dar uma exceção sem indicar exatamente o que foi. Qual será que é o comportamento desejado?
  2. Será que o fato do professor ter sido cadastro com afiliação deveria restringir a remoção do mesmo? Acho que faz sentido restringir nos outros campos (por exemplo, se o professor já deu aula, não deveria ser possível simplesmente remover o registro do sapos), mas não vejo muito sentido no restrict_with_exception para esse caso.

Além disso, tenho algumas sugestões de mudanças:

  1. Quando institution_id de professor é nulo, a migração está criando uma afiliação com instituição nula. Faria mais sentido não criar essa afiliação. Verificar se o mesmo não acontece com program_level
  2. O arquivo spec/features/custom_variables_spec.rb ainda tem a variável referente ao nível do programa (e usa isso em testes). A variável foi removida com a adição da tabela e o teste deveria refletir essa mudança.
  3. O arquivo seeds.rb também está criando program_level como CustomVariable. Altere para criar ProgramLevel
  4. Falta rota/active scaffold para alterar o nível do programa (e testes). Como não é mais custom variable, o local de customizar isso não existe mais e não parece ter nenhuma alternativa.
  5. Em "Professores" foi adicionado um link para "Afiliações": image

Acho que seria melhor não ter esse link e as afiliações aparecerem no olhinho.

A rota para o program level estaria na configuration?

leomurta commented 1 week ago
  1. O que acontece se nenhum program_level estiver cadastrado no momento da defesa? Acho que antes da mudança aparecia o nível do programa em branco. Agora deve dar uma exceção sem indicar exatamente o que foi. Qual será que é o comportamento desejado?

Eu acho que o ideal seria manter o comportamento atual.

  1. Será que o fato do professor ter sido cadastro com afiliação deveria restringir a remoção do mesmo? Acho que faz sentido restringir nos outros campos (por exemplo, se o professor já deu aula, não deveria ser possível simplesmente remover o registro do sapos), mas não vejo muito sentido no restrict_with_exception para esse caso.

Eu acho que se ele participou de alguma banca com aquela afiliação, não pode ser removido, pois quando tirarmos o histórico do aluno, não conseguiremos saber quem foi a banca dele. Mas o mero fato de ter afiliações sem estar vinculado a bancas, orientações, etc, não me parece relevante para barrar a remoção. Aí seria um cascade, que remove tanto o prof. quanto as afiliações (mantendo as instituições, obviamente).

Além disso, tenho algumas sugestões de mudanças:

  1. Quando institution_id de professor é nulo, a migração está criando uma afiliação com instituição nula. Faria mais sentido não criar essa afiliação. Verificar se o mesmo não acontece com program_level

Concordo.

  1. O arquivo spec/features/custom_variables_spec.rb ainda tem a variável referente ao nível do programa (e usa isso em testes). A variável foi removida com a adição da tabela e o teste deveria refletir essa mudança.

Concordo. Tem que testar se o nível antes da migração (com a variável) é o mesmo depois da migração (com a tabela).

  1. O arquivo seeds.rb também está criando program_level como CustomVariable. Altere para criar ProgramLevel

Concordo.

  1. Falta rota/active scaffold para alterar o nível do programa (e testes). Como não é mais custom variable, o local de customizar isso não existe mais e não parece ter nenhuma alternativa.

Aqui não sei se entendi bem. Não teria uma tela de ProgramLevel em Configurações onde podemos criar/editar/remover ProgramLevels? Na verdade, acho que são dois conceitos distintos: um é com as opções de nível (3 a 7). Talvez o nome seria ProgramLevelType para ficar parecido com o que fazemos no resto do Sapos. Outro com a atribuição de um nível ao programa numa data.

  1. Em "Professores" foi adicionado um link para "Afiliações": image

Essa imagem não apareceu.

Acho que seria melhor não ter esse link e as afiliações aparecerem no olhinho.

Talvez na edição do professor seja o melhor local para editar as afiliações.

A rota para o program level estaria na configuration?

Rota = menu, né? Acho que sim.

Leo

IgorMonardez commented 1 week ago

Aqui não sei se entendi bem. Não teria uma tela de ProgramLevel em Configurações onde podemos criar/editar/remover ProgramLevels? Na verdade, acho que são dois conceitos distintos: um é com as opções de nível (3 a 7). Talvez o nome seria ProgramLevelType para ficar parecido com o que fazemos no resto do Sapos. Outro com a atribuição de um nível ao programa numa data.

Não entendi, pode ter mais de um ProgramLevel que iriam de 3 a 7? Sobre o nome eu segui o mesmo que estava nos tipos das variáveis, acredito que não tenha por que mudar.

IgorMonardez commented 1 week ago

Eu acho que o ideal seria manter o comportamento atual.

Qual seria o comportamento atual?

leomurta commented 1 week ago

Aqui não sei se entendi bem. Não teria uma tela de ProgramLevel em Configurações onde podemos criar/editar/remover ProgramLevels? Na verdade, acho que são dois conceitos distintos: um é com as opções de nível (3 a 7). Talvez o nome seria ProgramLevelType para ficar parecido com o que fazemos no resto do Sapos. Outro com a atribuição de um nível ao programa numa data.

Não entendi, pode ter mais de um ProgramLevel que iriam de 3 a 7? Sobre o nome eu segui o mesmo que estava nos tipos das variáveis, acredito que não tenha por que mudar.

Hoje existe esses 5 níveis. Mas a CAPES pode decidir amanhã em trocar isso. O ideal seria isso ser flexível, como fazemos para vários outros dados, como tipo de matrícula, níveis, tipo de bolsa, etc.

leomurta commented 1 week ago

Eu acho que o ideal seria manter o comportamento atual.

Qual seria o comportamento atual?

Tente fazer um teste com a versão 6.4.8 para identificar e documente aqui.

JoaoFelipe commented 1 week ago

A rota para o program level estaria na configuration?

A rota em si seria /program_levels Mas o link para ela em navigation.rb estaria em Configuration sim

Aqui não sei se entendi bem. Não teria uma tela de ProgramLevel em Configurações onde podemos criar/editar/remover ProgramLevels?

Sim, é necessário ter isso. Não tinha quando revisei o PR.

Na verdade, acho que são dois conceitos distintos: um é com as opções de nível (3 a 7). Talvez o nome seria ProgramLevelType ?para ficar parecido com o que fazemos no resto do Sapos. Outro com a atribuição de um nível ao programa numa data.

Acho que criar ProgramLevelType seria complicação demais a toa. Não vejo problema em ter só ProgramLevel com datas do nível e um campo numérico (ou mesmo texto, se quiser deixar bem flexível caso a CAPES resolva mudar os níveis para letras no futuro).

Se percebermos que ProgramLevel tem características a mais de acordo com nível, passa a fazer sentido ter o ProgramLevelType

leomurta commented 1 week ago

Acho que criar ProgramLevelType seria complicação demais a toa. Não vejo problema em ter só ProgramLevel com datas do nível e um campo numérico (ou mesmo texto, se quiser deixar bem flexível caso a CAPES resolva mudar os níveis para letras no futuro).

Se percebermos que ProgramLevel tem características a mais de acordo com nível, passa a fazer sentido ter o ProgramLevelType

Beleza então. Talvez uma validação importante seja garantir que um nível não esteja com datas sobrepostas de validade, né? Deve ter uma validação parecida em bolsa.