Feat: related article validation #698
Conversation
|
@Rossi-Luciano revisado e aprovado até o commit 2 |
| @@ -1,5 +1,5 @@ | |||
| from packtools.sps.models.v2.related_articles import RelatedArticles | |||
| from packtools.sps.models import article_and_subarticles | |||
| from packtools.sps.models import article_and_subarticles, article_dates | |||
There was a problem hiding this comment.
@Rossi-Luciano se não me engano, foi feita a versão v2 deste modelo... se sim... qual é o mais apropriado para usar. Por favor, coloque em forma de comentário a justificativa por usar um ou o outro. Teoricamente se foi necessário criar o v2, teria que usar o v2.
There was a problem hiding this comment.
@robertatakenaka para esse caso o módulo foi renomeado de dates para article_dates e não está em v2 mas é o módulo novo.
| obtained_events = self.history_events | ||
| is_valid = expected_event in obtained_events | ||
| yield format_response( | ||
| title='Related article type validation', |
There was a problem hiding this comment.
@Rossi-Luciano o valor de title não condiz com o nome do método
There was a problem hiding this comment.
OK, @robertatakenaka
| if not correspondence_list: | ||
| raise ValidationRelatedArticleException("Function requires a dictionary with article type and history date event") | ||
|
|
||
| for related_article in self.related_articles: |
There was a problem hiding this comment.
@Rossi-Luciano todos os related article tem obrigatoriamente que ter uma data relacionadoa no histórico?
There was a problem hiding this comment.
OK, @robertatakenaka
| @@ -234,6 +234,267 @@ def test_related_articles_does_not_have_doi(self): | |||
| with self.subTest(i): | |||
| self.assertDictEqual(obtained[i], item) | |||
|
|
|||
| def test_related_articles_attribute_validation(self): | |||
There was a problem hiding this comment.
@Rossi-Luciano o nome deste teste está muito genérico. O que de fato ele está testando? Deveria ser algo como: test_related_article_type_xxx_exige_que_exista_a_data_yyy_no_historico. Tampouco há um comentário que expresse o que de fato está sendo testado.
There was a problem hiding this comment.
OK, @robertatakenaka
|
|
||
| expected = [ | ||
| { | ||
| 'title': 'Related article type validation', |
There was a problem hiding this comment.
@Rossi-Luciano title não condiz com o que está sendo validado.
There was a problem hiding this comment.
OK, @robertatakenaka
| } | ||
| }, | ||
| { | ||
| 'title': 'Related article type validation', |
There was a problem hiding this comment.
@Rossi-Luciano title não condiz com o que está sendo validado.
There was a problem hiding this comment.
OK, @robertatakenaka
| } | ||
| }, | ||
| { | ||
| 'title': 'Related article type validation', |
There was a problem hiding this comment.
@Rossi-Luciano title não condiz com o que está sendo validado.
There was a problem hiding this comment.
OK, @robertatakenaka
| <date date-type="corrected"> | ||
| <day>01</day> | ||
| <month>06</month> | ||
| <year>2012</year> |
There was a problem hiding this comment.
@Rossi-Luciano eu acho que está errado. Esta data é esperada no XML do artigo corrigido e não no XML da errata
There was a problem hiding this comment.
@Rossi-Luciano vc já fez esta validação na classe que valida errata. Considere o que já foi feito
There was a problem hiding this comment.
OK, @robertatakenaka
Será considerado em outra validação.
| @@ -156,28 +156,3 @@ def related_article_attributes_validation(self, error_level="ERROR"): | |||
| data=related_article, | |||
| error_level=error_level | |||
| ) | |||
|
|
|||
| def related_articles_matches_history_date_validation(self, correspondence_list=None, error_level="ERROR"): | |||
There was a problem hiding this comment.
@Rossi-Luciano isso está sendo atendido em outra classe? Era um questionamento
There was a problem hiding this comment.
OK, @robertatakenaka
| @@ -141,7 +141,7 @@ def related_article_attributes_validation(self, error_level="ERROR"): | |||
| for attrib in ("related-article-type", "id", "href", "ext-link-type"): | |||
| if not related_article[attrib]: | |||
| yield format_response( | |||
| title='Related article type validation', | |||
| title='Related article attributes validation', | |||
There was a problem hiding this comment.
@Rossi-Luciano gostaria que o título não tivesse validation para deixar mais curto, pois tudo é validation. Em title deixe mais específico que aspecto está sendo validado. Neste caso, use f'Related article {attrib}'
There was a problem hiding this comment.
OK, @robertatakenaka
Rossi-Luciano
changed the title
packtools/sps/validation/errata.py
Outdated
| return [ | ||
| article for article in RelatedItems(xml_tree).related_articles | ||
| if article.get("related-article-type") == expected_related_article_type | ||
| ] |
There was a problem hiding this comment.
@Rossi-Luciano será que o melhor não seria ter o filtro e iterar em todos os related-article? Desta forma não está escondendo possíveis defeitos no XML ao ignorar os related-article de tipo inesperados?
packtools/sps/validation/errata.py
Outdated
| history_date_count = len(history_dates) | ||
| related_article_count = len(self.related_articles) | ||
|
|
||
| if history_date_count < related_article_count: |
There was a problem hiding this comment.
@Rossi-Luciano não acho boa a abordagem de contagem. A abordagem a ser adotada deveria ser pareada. Porque o objetivo não é saber se a quantidade bate, mas qual é o item que tem a data faltando. E vice versa, se há a data no histórico e não há o related-article.
|
@robertatakenaka mudei a abordagem da validação. |
packtools/sps/validation/errata.py
Outdated
| self.xml_tree = xml_tree | ||
| self.correspondence_list = correspondence_list | ||
| self.article_type = xml_tree.find(".").get("article-type") | ||
| self.related_articles = list(RelatedArticles(xml_tree).related_articles()) |
There was a problem hiding this comment.
@Rossi-Luciano aqui está sendo recuperado todos os related_articles. Em RelatedArticles, crie método para filtrar somente os related_articles com os quais deseja trabalhar, no lugar de criar em RelatedArticlesValidation o método get_related_article_types_by_article_type
O que esse PR faz?
Adiciona validações para
related_article.Onde a revisão poderia começar?
Por commit.
Como este poderia ser testado manualmente?
python3 -m unittest -v tests/sps/validation/test_related_articles.pyAlgum cenário de contexto que queira dar?
NA.
Screenshots
NA.
Quais são tickets relevantes?
NA.
Referências
NA.