Feat: errata validation #693

Rossi-Luciano · 2024-08-27T15:11:30Z

O que esse PR faz?

Este PR implementa a validação de artigos relacionados em documentos XML, garantindo que para cada artigo de errata ou correção, os elementos relacionados estejam presentes e sejam consistentes. Especificamente, ele valida:

Se para um artigo de errata, existe pelo menos um <related-article> do tipo "corrected-article".
Se para um artigo corrigido, existe um número correspondente de <related-article> do tipo "correction-forward" e <history> do tipo "corrected" no histórico do artigo.

Essas validações asseguram a integridade das referências entre artigos, conforme as especificações do sistema.

Onde a revisão poderia começar?

A revisão pode começar pelo arquivo onde as classes ErrataValidation e CorrectedArticleValidation estão definidas, especificamente nos métodos de validação de artigos relacionados.

Como este poderia ser testado manualmente?

Carregue um documento XML contendo artigos do tipo "correction" e "errata".
Utilize as classes ErrataValidation e CorrectedArticleValidation para validar os artigos.
Verifique se os seguintes cenários são corretamente validados:

Um artigo de errata deve conter pelo menos um <related-article related-article-type="corrected-article">.
Um artigo corrigido deve ter um <related-article related-article-type="correction-forward"> para cada <date date-type="corrected"> no histórico.

Avalie se as respostas de validação retornadas refletem corretamente a presença ou ausência dos elementos esperados.
Existem testes automatizados que cobrem os itens descritos anteriormente.

Algum cenário de contexto que queira dar?

NA.

Screenshots

NA.

Quais são tickets relevantes?

TK #691

Referências

NA.

robertatakenaka · 2024-08-28T11:26:02Z

packtools/sps/validation/errata.py

+# Classe base para validação de artigos, implementando a lógica comum de validação de artigos relacionados
+class ValidationBase:
+    def __init__(self, xml_tree, related_article_type):
+        # Inicializa a árvore XML e configura o tipo de artigo relacionado a ser validado


@Rossi-Luciano para documentação da classe use """, pois os textos marcados desta forma geram documentação. Já os comentários com # não, mas que devem conter algo a mais de detalhes para a compreensão de outros desenvolvedores.

@Rossi-Luciano Existe um guia de estilo do python chamado PEP8. https://peps.python.org/pep-0008/#block-comments
Em resumo, os comentários devem anteceder os blocos de códigos. E os comentários na mesma linha ou desnecessários porque o código já está explícito devem ser evitados.

OK, @robertatakenaka

robertatakenaka · 2024-08-28T11:36:59Z

packtools/sps/validation/errata.py

+            if (
+                not self.related_articles
+            ):  # Se não houver artigos relacionados, retorna um erro
+                yield self._format_error_response(
+                    expected=f'at least one <related-article related-article-type="{expected_related_article_type}">',
+                    advice=f'provide <related-article related-article-type="{expected_related_article_type}">',
+                    error_level=error_level,
+                )
+            else:
+                # Se houver artigos relacionados, valida cada um e retorna um sucesso
+                for article in self.related_articles:
+                    yield self._format_success_response(
+                        article, expected_message, error_level
+                    )


@Rossi-Luciano tente usar a estrutura for+ else

for n in range(2, 10): for x in range(2, n): if n % x == 0: print(n, 'equals', x, '*', n//x) break else: # loop fell through without finding a factor print(n, 'is a prime number')

OK, @robertatakenaka

robertatakenaka · 2024-08-28T11:37:14Z

packtools/sps/validation/errata.py

+    ):
+        if (
+            self.article_type == "correction"
+        ):  # Verifica se o tipo de artigo é uma correção


@Rossi-Luciano comentário desnecessário

@Rossi-Luciano No lugar de "correction", passe o valor como parâmetro (expected_article_type)

OK, @robertatakenaka

robertatakenaka · 2024-08-28T11:38:16Z

packtools/sps/validation/errata.py

+        expected_message,
+        advice,
+        error_level="ERROR",
+    ):


@Rossi-Luciano adicionar docstring no lugar de #. O conteúdo pode ser Valida somente se o artigo é do tipo errata

OK, @robertatakenaka

robertatakenaka · 2024-08-28T11:45:54Z

packtools/sps/validation/errata.py

+    # Método auxiliar para formatar uma resposta de erro
+    def _format_error_response(self, expected, advice, error_level):
+        return format_response(
+            title="errata",


@Rossi-Luciano o valor de title não pode ser errato por dois motivos: 1. isso é uma classe Base, deveria ter um conteúdo genérico; 2. O que está validando deveria ser o title

OK, @robertatakenaka

robertatakenaka · 2024-08-28T11:46:14Z

packtools/sps/validation/errata.py

+    # Método auxiliar para formatar uma resposta de sucesso
+    def _format_success_response(self, article, expected_message, error_level):
+        return format_response(
+            title="errata",


@Rossi-Luciano mesmo comentário anterior

@Rossi-Luciano estes dois métodos format não me parecem úteis

OK, @robertatakenaka

robertatakenaka · 2024-08-28T11:49:47Z

packtools/sps/validation/errata.py

+
+# Classe base para validação de artigos, implementando a lógica comum de validação de artigos relacionados
+class ValidationBase:
+    def __init__(self, xml_tree, related_article_type):


@Rossi-Luciano tem que adicionar o expected_article_type como parâmetro

Trocar related_article_type por expected_related_article_type

Ou seja, você terá os dois parâmetros: expected_article_type e expected_related_article_type

OK, @robertatakenaka

robertatakenaka · 2024-08-28T11:52:17Z

packtools/sps/validation/errata.py

+    # Método principal de validação de artigos relacionados, que pode ser sobrescrito por subclasses
+    def validate_related_article(
+        self,
+        expected_related_article_type,


@Rossi-Luciano você não está usando o self.expected_related_article_type

OK, @robertatakenaka

robertatakenaka · 2024-08-28T11:56:55Z

packtools/sps/validation/errata.py

+        # Verifica se o número de datas corrigidas é menor que o número de artigos relacionados
+        history_date_count = len(self.history_dates)
+        related_article_count = len(self.related_articles)
+
+        if history_date_count < related_article_count:
+            # Retorna um erro se o número de datas corrigidas for insuficiente
+            yield format_response(
+                title="errata",
+                parent="article",


@Rossi-Luciano separa em outra validação

OK, @robertatakenaka

robertatakenaka

@Rossi-Luciano no geral ficou muito bom a questão da herança. Mas tem algumas questões de estilo do python PEP8 sobre os comentários. Além disso, outras melhorias e um bug.

robertatakenaka · 2024-08-29T11:15:24Z

packtools/sps/validation/errata.py

-        ]
+        self.validation_title = validation_title
+        self.article_lang = xml_tree.get("{http://www.w3.org/XML/1998/namespace}lang")
+        self.article_type = xml_tree.xpath(".")[0].get("article-type")


@Rossi-Luciano para estes casos use o find que pega o nó diretamente

OK, @robertatakenaka

robertatakenaka · 2024-08-29T11:20:49Z

packtools/sps/validation/errata.py

-        super().__init__(xml_tree, related_article_type="corrected-article")
+        super().__init__(xml_tree, related_article_type="corrected-article", validation_title="errata")
+
+    def validate_related_article(


@Rossi-Luciano você pode eliminar este método inteiro, pois a classe Base já o implementou de forma genérica.

OK, @robertatakenaka

robertatakenaka · 2024-08-29T11:24:12Z

packtools/sps/validation/errata.py

 class ValidationBase:
-    def __init__(self, xml_tree, related_article_type):
-        # Inicializa a árvore XML e configura o tipo de artigo relacionado a ser validado
+    def __init__(self, xml_tree, related_article_type, validation_title):


@Rossi-Luciano tinha pedido para você adicionar expected_article_type e expected_related_article_type (mudar o nome de related_article_type)

OK, @robertatakenaka

robertatakenaka · 2024-08-29T11:26:56Z

packtools/sps/validation/errata.py

+                advice=f'provide <related-article related-article-type="{expected_related_article_type}">',
+            )
+
+    def _get_related_articles(self, xml_tree, related_article_type):


@Rossi-Luciano aqui vc não precisa de xml_tree, related_article_type como parâmetros já que eles estariam acessíveis no momento que se instancia a classe. Crie atributos com eles self.expected_related_article_type e use self.xml_tree.

OK, @robertatakenaka

robertatakenaka · 2024-08-29T11:34:11Z

packtools/sps/validation/errata.py

-                yield self._format_error_response(
-                    expected=f'at least one <related-article related-article-type="{expected_related_article_type}">',
-                    advice=f'provide <related-article related-article-type="{expected_related_article_type}">',
+    def validate_related_article(self, expected_article_type, expected_related_article_type, expected_response,


@Rossi-Luciano o único parâmetro realmente necessário é o error_level. Os expected_article_type e expected_related_article_type tem que estar no init e o expected_response, você cria usando o expected_related_article_type

OK, @robertatakenaka

robertatakenaka · 2024-08-30T12:56:02Z

packtools/sps/validation/errata.py

+            yield self._format_response(
+                title=title,
+                validation_type="exist",
+                expected=f'at least one <related-article related-article-type="{self.expected_related_article_type}">',


@Rossi-Luciano expected=expected_response,

robertatakenaka · 2024-08-30T12:57:59Z

packtools/sps/validation/errata.py

+            title=title,
+            parent=related_article.get("parent") if related_article else "article",
+            parent_id=related_article.get("parent_id") if related_article else None,
+            parent_article_type="correction",


@Rossi-Luciano na classe base não deveria ter dados definidos. Trocar 'correction' por variável

robertatakenaka · 2024-08-30T13:00:02Z

packtools/sps/validation/errata.py

+        """
+        yield from super().validate_related_article(error_level=error_level, title=title)
+
+    def validate_related_articles_and_history_dates(self, error_level="ERROR"):


trocar por validate_history_dates

robertatakenaka · 2024-08-30T13:09:30Z

tests/sps/validation/test_errata.py

+                ]
+            }
+        ]
+        for i, item in enumerate(obtained):


@Rossi-Luciano expected

@Rossi-Luciano em todos adiciton o assert para verificar o tamanho da lista obtained

robertatakenaka · 2024-08-30T13:09:43Z

tests/sps/validation/test_errata.py

+                "data": None,
+            }
+        ]
+        for i, item in enumerate(obtained):


@Rossi-Luciano expected

robertatakenaka · 2024-08-30T13:09:53Z

tests/sps/validation/test_errata.py

+                },
+            }
+        ]
+        for i, item in enumerate(obtained):


@Rossi-Luciano expected

robertatakenaka

@Rossi-Luciano fazer correções

robertatakenaka · 2024-09-02T12:35:19Z

packtools/sps/validation/errata.py

@@ -70,7 +70,7 @@ def _format_response(self, title, validation_type, expected, error_level, is_val
            title=title,
            parent=related_article.get("parent") if related_article else "article",
            parent_id=related_article.get("parent_id") if related_article else None,
-            parent_article_type="correction",
+            parent_article_type=related_article.get("parent_article_type") if related_article else None,


@Rossi-Luciano forma alternativa:

parent_article_type=related_article and related_article.get("parent_article_type")

robertatakenaka · 2024-09-02T12:37:52Z

tests/sps/validation/test_errata.py

@@ -25,7 +25,7 @@ def test_validate_related_article_not_found(self):
                "title": "validation matching 'correction' and 'corrected-article'",
                "parent": "article",
                "parent_id": None,
-                "parent_article_type": "correction",
+                "parent_article_type": None,


@Rossi-Luciano Por que é None? não deveria ser 'correction'?

@robertatakenaka corrigido

robertatakenaka

@Rossi-Luciano Verificar os testes

robertatakenaka · 2024-09-03T11:14:10Z

packtools/sps/validation/errata.py

+            parent=related_article.get("parent") if related_article else "article",
+            parent_id=related_article.get("parent_id") if related_article else None,
+            parent_article_type=related_article.get("parent_article_type") if related_article else self.article_type,
+            parent_lang=related_article.get("parent_lang") if related_article else self.article_lang,


@Rossi-Luciano olhando estes vários if que são todos referentes a existência ou não do related_article, fica evidente que def _format_response é desnecessário. Esta verificação de existência já ocorreu no trecho das linhas 31 a 52. Sugiro remover e adequar o trecho 31 a 52.

OK @robertatakenaka

Rossi-Luciano added 2 commits August 27, 2024 11:59

Adiciona validação para 'errata'

98f34b8

Adiciona testes para validação de 'errata'

1f104d9

Rossi-Luciano requested a review from robertatakenaka August 27, 2024 15:11

robertatakenaka reviewed Aug 28, 2024

View reviewed changes

robertatakenaka requested changes Aug 28, 2024

View reviewed changes

Rossi-Luciano added 2 commits August 28, 2024 14:48

Refatora classe de validação de "errata"

31ba389

Adapta teste

5b141cd

robertatakenaka reviewed Aug 29, 2024

View reviewed changes

Rossi-Luciano added 4 commits August 29, 2024 10:43

Refatora classe de validação de "errata"

27ed2eb

Adapta os testes

696ccf5

Adiciona "title" como parâmetro de validação

c661bf8

Adapta os testes

72d897c

Rossi-Luciano requested a review from robertatakenaka August 29, 2024 14:14

robertatakenaka reviewed Aug 30, 2024

View reviewed changes

tests/sps/validation/test_errata.py Outdated

"data": None,

}

]

for i, item in enumerate(obtained):

Copy link

Member

robertatakenaka Aug 30, 2024

Choose a reason for hiding this comment

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

@Rossi-Luciano expected

robertatakenaka reviewed Aug 30, 2024

View reviewed changes

tests/sps/validation/test_errata.py Outdated

},

}

]

for i, item in enumerate(obtained):

Copy link

Member

robertatakenaka Aug 30, 2024

Choose a reason for hiding this comment

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

@Rossi-Luciano expected

robertatakenaka requested changes Aug 30, 2024

View reviewed changes

Rossi-Luciano added 2 commits September 1, 2024 15:58

Corrige validação

28787e7

Corrige testes

93fa1e2

Rossi-Luciano requested a review from robertatakenaka September 1, 2024 18:58

robertatakenaka reviewed Sep 2, 2024

View reviewed changes

robertatakenaka requested changes Sep 2, 2024

View reviewed changes

Rossi-Luciano added 2 commits September 2, 2024 10:00

Corrige obtenção de "parent_article_type"

cb3829e

Corrige testes

f2d5135

Rossi-Luciano requested a review from robertatakenaka September 2, 2024 13:00

robertatakenaka reviewed Sep 3, 2024

View reviewed changes

Remove "_format_response"

659088b

Rossi-Luciano requested a review from robertatakenaka September 3, 2024 12:20

robertatakenaka approved these changes Sep 3, 2024

View reviewed changes

robertatakenaka merged commit d96beaa into scieloorg:master Sep 3, 2024
2 checks passed

Feat: errata validation #693

Feat: errata validation #693

Conversation

Rossi-Luciano commented Aug 27, 2024

O que esse PR faz?

Onde a revisão poderia começar?

Como este poderia ser testado manualmente?

Algum cenário de contexto que queira dar?

Screenshots

Quais são tickets relevantes?

Referências

Choose a reason for hiding this comment

robertatakenaka Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robertatakenaka Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robertatakenaka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robertatakenaka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robertatakenaka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robertatakenaka Aug 28, 2024 •

edited

Loading

robertatakenaka Aug 28, 2024 •

edited

Loading