Issue#3828 Refatorar Mesa Diretora - #3829
Conversation
There was a problem hiding this comment.
A motivação conceitual do PR é correta (Mesa Diretora pertence à Legislatura, não à
SessaoLegislativa). Isso é uma das muitas "heranças malditas" da modelagem de dados (mal feita!) do SAPL 2.5 e anteriores. A simplificação do AJAX/jQuery para CRUDs padrão é bem vinda. Os problemas abaixo precisam ser resolvidos antes do merge.
| def __init__(self, *args, **kwargs): | ||
| super(ComposicaoMesaForm, self).__init__(*args, **kwargs) | ||
| self.instance.mesa_diretora = self.initial.get('mesa_diretora') | ||
| self.fields['parlamentar'].queryset = self.fields['parlamentar'].queryset.filter( |
There was a problem hiding this comment.
ComposicaoMesaForm.__init__ quebra silenciosamente sem initial
Se o form for instanciado sem initial['mesa_diretora'], self.initial.get(...) retorna None e a próxima linha lança AttributeError: 'NoneType' object has no attribute 'legislatura'.
Hoje as views CRUD sempre passam initial, então não dispara em produção, mas qualquer caller futuro (admin, comando, outra view) quebra silenciosamente. Sugestão de guarda:
mesa = self.initial.get('mesa_diretora')
if mesa is not None:
self.instance.mesa_diretora = mesa
self.fields['parlamentar'].queryset = (
self.fields['parlamentar'].queryset
.filter(mandato__legislatura=mesa.legislatura)
)| ultima_filiacao = self.filiacao_set.order_by('-data').first() | ||
| # este método conta com a ordenação default do model Filiacao para trazer a última filiação primeiro | ||
| # se order_by for adicionado aqui, o prefetch_related que inclui filiacao_set não irá pré-carregar como esperado | ||
| ultima_filiacao = self.filiacao_set.first() |
There was a problem hiding this comment.
filiacao_atual agora depende da ordenação default — vale um teste regressivo
A troca por self.filiacao_set.first() é correta porque Filiacao.Meta.ordering = ('parlamentar', '-data', '-data_desfiliacao'), e o prefetch_related adicionado na ListView depende disso para não disparar query extra. O comentário deixa o motivo claro — ótimo.
Sugestão: adicionar um teste curto que verifique o invariante (criar duas filiações em datas diferentes e checar parlamentar.filiacao_atual). Se um dia alguém mexer no Meta.ordering, o teste pega.
|
|
||
| def clean(self): | ||
| if self.data_inicio and self.data_fim: | ||
| if self.data_inicio >= self.data_fim: |
There was a problem hiding this comment.
Validação >= rejeita mesas de um único dia
Há cenários reais de mesa que dura um único dia (dissolução e recriação no mesmo dia, por cassação ou eventos extraordinários — exatamente o tipo de exceção citado na descrição do PR).
Considerar > em vez de >=, esse requisito precisa ser refinado... o que acha @edwardoliveira
|
|
||
| def get_filterset_kwargs(self, filterset_class): | ||
| fk = super().get_filterset_kwargs(filterset_class) | ||
| if 'legislatura' not in self.request.GET and not 'mesa' in self.request.GET: |
There was a problem hiding this comment.
NIT: not 'mesa' in self.request.GET → 'mesa' not in self.request.GET (PEP 8). Mesma observação na linha de baixo.
| return _('Mesa da %(sessao)s sessao da %(legislatura)s Legislatura') % { | ||
| 'sessao': self.sessao_legislativa, 'legislatura': self.sessao_legislativa.legislatura | ||
| } | ||
| return self.titulo or _('%(legislatura)s - %(data_inicio)s a %(data_fim)s') % { |
There was a problem hiding this comment.
Datas em __str__ saem em ISO (2021-01-01) em vez do formato pt-br. Como titulo agora é o caminho principal, é raro cair no fallback, mas vale usar django.utils.formats.date_format(d, "SHORT_DATE_FORMAT") para consistência com o resto da aplicação.
| {% endblock actions_search %} | ||
|
|
||
| {% block container_table_list %} | ||
| {% if perms.parlamentares.add_mesadiretora %} |
There was a problem hiding this comment.
o template usa add_mesadiretora para alternar entre listagem CRUD (admin) e visualização em abas (público). Funcionalmente está consistente com o resto do SAPL. Só ponderando se change_mesadiretora não seria semanticamente mais correto para representar "tem painel de gestão".
| {{composicao.parlamentar.nome_parlamentar}}</a> | ||
| </div> | ||
| </td> | ||
| <td>{{composicao.parlamentar.filiacao_atual}}</td> |
There was a problem hiding this comment.
Partido pode divergir do partido da época
A view antiga (partido_parlamentar_sessao_legislativa) calculava a filiação na data da sessão. Esta agora mostra sempre a filiação mais recente. Para mesas atuais é a mesma coisa; para mesas históricas pode mostrar partido errado (ex.: parlamentar que mudou de partido depois).
É bom validar esse requisito também, @edwardoliveira . Se não for o caso, vale reintroduzir a lógica por data (ex.: método filiacao_em(data) no model).
| 'cargo': new_cargo.id, | ||
| }) | ||
|
|
||
| composicao.refresh_from_db() |
There was a problem hiding this comment.
a cobertura dos forms está boa. mas acho que tem alguns pontos faltantes:
MesaDiretoraFilterSet: testar que?mesa=<id>resolve para a legislatura correta e que sem GET cai na legislatura atual; testar 404 para mesa inexistente.- View pública (
mesadiretora_filter.html): renderizar a página sem permissão de admin e validar as abas. test_composicaomesa_form_view_create/update: não fazem assert de status HTTP — adicionar pelo menosassert response.status_code in (200, 302)para detectar erros de form silenciosos.- Variáveis
mandato/mandato1/mandato2criadas mas nunca lidas — substituir por_ = baker.make(...)para deixar a intenção clara.
NIT: faltou newline final no arquivo. (conferir isso nos demais arquivos)
1b144e4 to
5af98ee
Compare
joaohortsenado
left a comment
joaohortsenado
left a comment
There was a problem hiding this comment.
Testando com uma base real de produção, encontrei mais alguns pontos que precisam de atenção, @LeandroJatai @edwardoliveira
| if MesaDiretora.objects.filter( | ||
| legislatura=self.legislatura, | ||
| data_inicio__lte=self.data_fim, | ||
| data_fim__gte=self.data_inicio | ||
| ).exclude(pk=self.pk).exists(): |
There was a problem hiding this comment.
A checagem de interseção usa intervalo fechado e rejeita mesas que apenas encostam
No QA contra uma base real de produção, rodei full_clean() nas 64 mesas migradas. Duas delas caem aqui:
[id=36] Mesa Diretora Biênio 1971/1972 (1971-02-03 a 1972-01-31, leg=4)
-> As datas da mesa diretora se sobrepõem com outra mesa diretora existente.
[id=37] Mesa Diretora Biênio 1972/1973 (1972-01-31 a 1973-01-30, leg=4)
-> As datas da mesa diretora se sobrepõem com outra mesa diretora existente.
O único dia em comum é 1972-01-31: é o data_fim de uma e o data_inicio da seguinte. Como o filtro usa data_inicio__lte / data_fim__gte, esse encosto conta como sobreposição.
Isso não é um caso raro da base — é o padrão de casas cujas sessões legislativas iam de 31 de janeiro a 30 de janeiro do ano seguinte, e a 0046 herda essas datas fielmente (correto).
Consequência prática: essas mesas ficam gravadas num estado que o model rejeita. Quem abrir uma delas no CRUD e salvar — mesmo mudando só a descrição — leva erro de validação e não consegue gravar.
Conceitualmente, uma mesa que termina no dia X e outra que começa no dia X não se sobrepõem. Sugiro intervalo semiaberto:
if MesaDiretora.objects.filter(
legislatura=self.legislatura,
data_inicio__lt=self.data_fim,
data_fim__gt=self.data_inicio
).exclude(pk=self.pk).exists():Isso resolve os dois registros acima sozinho. Vale notar que essa mudança conversa com a pergunta que deixei em r3182291982 sobre mesa de um único dia — as duas regras tratam da mesma ambiguidade de borda, e provavelmente deveriam ser decididas juntas.
| schema_editor.execute(""" | ||
| UPDATE parlamentares_mesadiretora md | ||
| SET | ||
| legislatura_id = sl.legislatura_id, | ||
| data_inicio = sl.data_inicio, | ||
| data_fim = sl.data_fim | ||
| FROM | ||
| parlamentares_sessaolegislativa sl | ||
| WHERE | ||
| sl.id = md.sessao_legislativa_id | ||
| """) |
There was a problem hiding this comment.
A migração pode gravar mesas fora do período da própria legislatura
O SQL está correto e rápido (as 59 migrations rodaram em 12,8s numa base real — o ponto do @edwardoliveira em r3093425814 ficou bem resolvido). O problema é que a SessaoLegislativa de origem nem sempre respeita o intervalo da Legislatura, e a nova regra de contenção introduzida no clean() passa a rejeitar o resultado.
Testando contra um dump de produção real, 2 das 64 mesas ficaram assim:
[id=10] Mesa Diretora 1959/1959 mesa: 1959-06-06 a 1959-12-31 | legislatura: 1959-06-06 a 1959-12-30
[id=21] Mesa Diretora 1977/1977 mesa: 1977-01-01 a 1977-12-31 | legislatura: 1977-01-31 a 1983-01-30
Somadas às 2 de borda coincidente que comentei em models.py, são 4 de 64 mesas (6%) inválidas logo após o upgrade. A migração em si não quebra — o efeito aparece depois, quando alguém tenta editar.
Não acho que a migration deva "consertar" datas silenciosamente: essa informação foi perdida pela modelagem anterior e qualquer chute pode piorar. Duas alternativas que me parecem melhores:
- Um management command de diagnóstico (algo como
./manage.py verifica_mesas_diretoras) que lista os registros que não passam nofull_clean(), para a casa ajustar manualmente — junto de uma nota na documentação de upgrade. - Ou, no mínimo, uma
RunPythonadicional que apenas registra em log quais mesas ficaram inconsistentes, para não ser uma descoberta silenciosa em produção.
(Registrando também que a preocupação que eu tinha levantado sobre IntegrityError na 0049 não se confirmou: sessao_legislativa era FK NOT NULL e as datas da sessão também, então o UPDATE sempre popula. Já resolvi aquela thread.)
| schema_editor.execute(""" | ||
| UPDATE parlamentares_mesadiretora | ||
| SET titulo = 'Mesa Diretora' || | ||
| CASE WHEN EXTRACT(YEAR FROM data_fim)::integer - EXTRACT(YEAR FROM data_inicio)::integer = 1 |
There was a problem hiding this comment.
Rótulo "Biênio" aplicado a mesas de aproximadamente um ano
A heurística compara os anos das datas, então marca como biênio qualquer mesa que apenas atravesse o Réveillon. Numa base real de produção, 12 dos 64 títulos gerados saíram assim:
Mesa Diretora Biênio 1973/1974 -> 1973-01-31 a 1974-01-30 (364 dias)
Mesa Diretora Biênio 1971/1972 -> 1971-02-03 a 1972-01-31 (362 dias)
Mesa Diretora Biênio 1969/1970 -> 1969-01-31 a 1970-01-30 (364 dias)
Mesa Diretora Biênio 1982/1983 -> 1982-01-01 a 1983-01-30 (394 dias)
Como o titulo agora é o rótulo principal na tela pública, é texto factualmente errado visível ao cidadão — e, uma vez gravado, só sai com edição manual mesa a mesa.
Comparar a duração real resolve:
SET titulo = 'Mesa Diretora' ||
CASE WHEN data_fim - data_inicio > 400
THEN ' Biênio'
ELSE ''
END || ' ' || ...(400 dias em vez de 365 para dar folga a mesas de ~13 meses, que pelo visto são comuns em bases antigas.)
| <script> | ||
| (function () { | ||
| // Suporte nativo a :has() — o CSS cuida de tudo, JS não é necessário | ||
| if (typeof CSS !== 'undefined' && CSS.supports && CSS.supports('selector(:has(*))')) return; |
There was a problem hiding this comment.
Este early-return torna o comportamento mobile 100% dependente do build
E acho que ainda precisa compilar a parada...
|
@LeandroJatai, obrigado pelo trabalho de fechar as threads do @edwardoliveira — conferi uma a uma no código e todas foram de fato atendidas. 👏 Acabei de subir uma review separada com os achados de um QA contra base real de produção — aquilo é assunto novo. Este comentário aqui é sobre outra coisa: ficaram 9 pontos da minha review de 04/05 sem retorno. Reconfirmei todos agora no HEAD atual ( Agrupei por tipo de decisão para facilitar: 🔴 Precisam de decisão de requisito (acho que travam o resto)Essas duas são perguntas de regra de negócio, não de código — vale o @edwardoliveira opinar: 1. Mesa de um único dia é cenário válido? (
>>> MesaDiretoraForm(data={'data_inicio': '15/06/2021', 'data_fim': '15/06/2021', ...}).errors
{'__all__': ['A data de início deve ser anterior à data de fim.']}A própria descrição do PR cita dissolução por cassação como exceção real. Se mesa de um dia existe, o certo é Complemento: no QA contra base real, essa mesma família de regra gerou 4 mesas migradas que o 2. Partido exibido deve ser o da época da mesa ou o atual? (
Se o requisito for mostrar o partido da época, precisaria de algo como um 🟠 Correções de código3. Esse é o único que considero bug de fato. Reproduzido agora: >>> ComposicaoMesaForm()
AttributeError: 'NoneType' object has no attribute 'legislatura'Hoje não dispara em produção porque as views CRUD sempre passam 4. Import não usado (
5. PEP 8 em if 'legislatura' not in self.request.GET and not 'mesa' in self.request.GET:→ 6. Datas em ISO no >>> str(MesaDiretora(titulo='', data_inicio=date(2021,1,1), data_fim=date(2021,12,31), legislatura=leg))
'16ª (2021 - 2024) - 2021-01-01 a 2021-12-31'Como o 🟡 Semântica de permissão7.
🟢 Cobertura de testes8. Teste do invariante de A troca por 9. Lacunas em A cobertura dos forms está boa. Faltam:
|
3 participants
Refatora Mesa Diretora para utilizar o Crud de forma simplifica da criação e edição de mesas e composições
Descrição
npm run buildIssue Relacionada
#3828
Nesta issue estão os requisitos funcionais e não funcionais, além da citação de várias outras issues que envolvem o tema
Motivação e Contexto
Mesas Diretoras nunca tiveram ligação conceitual com sessão legislativa. Sessão Legislativa possui uma definição e existência específica: inicia-se em meados de fevereiro e termina-se em meados de dezembro e diz sobre o intervalo onde haverá sessões plenárias.
Mesa diretora não: Mesa diretora, salvo raríssimas exceções, iniciam-se em 1º de janeiro e conclui-se com um ou dois anos. Dentro das exceções, estão a finalização inesperada de uma mesa e inicio de outra, seja por cassação de algum parlamentar, ou outro motivo qualquer que interrompa uma mesa e inicia-se outra.
Regras de não interseção entre mesas; de contenção em legislatura; de cargo único (que já existia); de só parlamentares da legislatura; de não duplicidade de parlamentar foram colocadas nos forms.
Como Isso Foi Testado?
foi criado o sapl/parlamentares/tests/test_mesadiretora.py que testa os forms
Capturas de Tela (se apropriado):
Tela Pública com 3 Mesas na mesma Legislatura. Nota-se os TABs com as três mesas, onde o Biênio de 23/24 possui duas mesas com datas de inicio e encerramento.
Tela de listagem do CRUDs tradicional para usuário que possui permissão de edição de mesa:
Tela de listagem das composições de uma mesa
Tipos de Mudanças
Checklist: