Add <product> element validation for SPS 1.10 compliance#1134
Add <product> element validation for SPS 1.10 compliance#1134
Conversation
- Create product model (packtools/sps/models/product.py) - Create product validation (packtools/sps/validation/product.py) - Create product rules JSON (packtools/sps/validation_rules/product_rules.json) - Create comprehensive tests (tests/sps/validation/test_product.py) - Integrate into orchestrator (xml_validations.py, xml_validator.py) Implements 7 validation rules: - @product-type presence (CRITICAL) - @product-type value (ERROR) - <source> presence (CRITICAL) - article-type consistency (ERROR) - Author presence (WARNING) - Publisher name presence (WARNING) - Year presence (WARNING) All 53 tests pass. Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com>
Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com>
Rossi-Luciano
left a comment
Rossi-Luciano
left a comment
There was a problem hiding this comment.
Casos de teste realizados:
| Caso | Elemento | Defeito | Esperado no CSV | Status |
|---|---|---|---|---|
| PR-OK1 | product-type="book" canônico |
nenhum | ausente | OK |
| PR-PT1 | sem @product-type |
@product-type ausente |
CRITICAL @product-type attribute | OK |
| PR-PT1 SKIP | sem @product-type |
validate_product_type_value |
ausente (SKIP) | OK |
| PR-PT2 | @product-type="journal" |
valor fora da lista | ERROR @product-type value | OK |
| PR-SRC1 | sem <source> |
<source> ausente |
CRITICAL source element | OK |
| PR-SRC2 | <source> vazio |
<source> com texto vazio → "" (falsy) |
CRITICAL source element | OK |
| PR-AU1 | sem person-group type="author" |
apenas editor presente | WARNING author in product | OK |
| PR-PUB1 | sem <publisher-name> |
<publisher-name> ausente |
WARNING publisher-name in product | OK |
| PR-YR1 | sem <year> |
<year> ausente |
WARNING year in product | OK |
| PR-AT1 | article-type ≠ "book-review" |
não testável neste XML — requer documento separado com article-type diferente de "book-review" |
— | — |
There was a problem hiding this comment.
Pull request overview
Adds SPS 1.10 validations for the <product> element (focused on article-type="book-review"), integrating the new checks into the existing XML validation orchestrator.
Changes:
- Introduces a
<product>extraction model (ArticleProducts) and corresponding validators (ProductValidation,ArticleProductValidation). - Adds configurable validation rule levels via
validation_rules/product_rules.json. - Integrates the new
"product"group into the XML validation pipeline and adds extensive unit tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
packtools/sps/models/product.py |
Extracts <product> data from XML for downstream validation. |
packtools/sps/validation/product.py |
Implements SPS 1.10 validation rules for <product> (presence/value checks + recommended fields). |
packtools/sps/validation_rules/product_rules.json |
Provides default rule severity levels and allowed product-type values. |
packtools/sps/validation/xml_validations.py |
Wires <product> validation into the functional validation entrypoints. |
packtools/sps/validation/xml_validator.py |
Adds "product" as a validation group in the orchestrator. |
tests/sps/validation/test_product.py |
Adds unit tests for <product> extraction and validation behaviors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| publisher_name_elem = product.find("publisher-name") | ||
| has_publisher_name = ( | ||
| publisher_name_elem is not None | ||
| and bool((publisher_name_elem.text or "").strip()) | ||
| ) | ||
|
|
||
| year_elem = product.find("year") | ||
| has_year = year_elem is not None and bool((year_elem.text or "").strip()) | ||
|
|
||
| isbn_elem = product.find("isbn") | ||
| isbn = (isbn_elem.text or "").strip() if isbn_elem is not None else None | ||
|
|
There was a problem hiding this comment.
publisher-name, year, isbn, publisher-loc, and size are also read via .text, which has the same mixed-content loss issue as <source>. For consistency and to avoid false negatives, extract these values using node_plain_text(...)/.itertext() before trimming.
| error_level = self.rules.get("product_type_presence_error_level", "CRITICAL") | ||
| product_type = self.data.get("product_type") | ||
| is_valid = bool(product_type and product_type.strip()) | ||
|
|
||
| advice_text = _( | ||
| 'Add @product-type attribute to <product>.' | ||
| ' Expected value: "book"' | ||
| ) | ||
| advice_params = {} | ||
|
|
||
| return build_response( | ||
| title="@product-type attribute", | ||
| parent=self.parent, | ||
| item="product", | ||
| sub_item="@product-type", | ||
| validation_type="exist", | ||
| is_valid=is_valid, | ||
| expected='@product-type attribute present with value "book"', |
There was a problem hiding this comment.
validate_product_type_presence() hardcodes the expected value/message to "book" instead of deriving it from product_type_list. This makes the rule harder to configure and can produce incorrect guidance if the allowed list changes. Consider building expected/advice_text from self.rules.get("product_type_list", ...) and also adjusting the advice to handle the “attribute present but empty” case (current "Add @product-type" is inaccurate when the attribute exists but is blank).
| error_level = self.rules.get("product_type_value_error_level", "ERROR") | ||
| product_type = self.data.get("product_type") | ||
| expected_values = self.rules.get("product_type_list", ["book"]) | ||
|
|
||
| # Skip if product_type is absent (handled by validate_product_type_presence) | ||
| if not product_type or not product_type.strip(): | ||
| return None | ||
|
|
||
| is_valid = product_type in expected_values | ||
|
|
||
| advice_text = _( | ||
| 'Replace @product-type="{product_type}" with "book".' | ||
| " Valid values: {allowed_values}" | ||
| ) | ||
| advice_params = { | ||
| "product_type": product_type, | ||
| "allowed_values": ", ".join(expected_values), | ||
| } |
There was a problem hiding this comment.
validate_product_type_value() supports configurable product_type_list, but the advice text always says to replace the value with "book". If product_type_list is expanded, the guidance becomes wrong. Suggest wording the advice in terms of the allowed values (e.g., “replace with one of: {allowed_values}”) and avoid hardcoding "book" here.
| source_elem = product.find("source") | ||
| source = None | ||
| if source_elem is not None: | ||
| source = (source_elem.text or "").strip() |
There was a problem hiding this comment.
Text extraction uses .text for <source>, which drops mixed content (e.g., <source>Title <italic>Part</italic></source>). This can incorrectly flag valid <source> as missing/empty. Prefer extracting with node_plain_text(source_elem) / "".join(source_elem.itertext()) so inline markup is preserved before .strip().
| source = (source_elem.text or "").strip() | |
| source = "".join(source_elem.itertext()).strip() |
O que esse PR faz?
Implementa 7 das 10 regras de validação para o elemento
<product>(70% de conformidade SPS 1.10), cobrindo resenhas de livros (book-review).Regras implementadas:
@product-typepresença@product-typevalor"book"<source>presençaarticle-type"book-review"quando<product>presente<person-group person-group-type="author">recomendado<publisher-name>recomendado<year>recomendadoArquivos criados:
packtools/sps/models/product.py— modelo de extração de dados do XMLpacktools/sps/validation/product.py—ProductValidationeArticleProductValidationpacktools/sps/validation_rules/product_rules.json— níveis de erro configuráveistests/sps/validation/test_product.py— 53 testes unitáriosArquivos modificados:
packtools/sps/validation/xml_validations.py—validate_products()+ importpacktools/sps/validation/xml_validator.py— grupo"product"no orquestradorOnde a revisão poderia começar?
packtools/sps/validation/product.py— contém toda a lógica de validação. O modelo empacktools/sps/models/product.pyextrai os dados do XML.Como este poderia ser testado manualmente?
Testes unitários:
Algum cenário de contexto que queira dar?
Segue o padrão estabelecido por
ext_link.pyetablewrap.py: usabuild_response()com suporte completo a i18n (advice_text/advice_params), modelo separado da validação, regras configuráveis via JSON, e integração no orquestrador viaxml_validations.py+xml_validator.py.Todas as mensagens usam
gettext.gettext(_()) para internacionalização. Validações condicionais retornamNonequando não aplicáveis (ex:validate_product_type_valueretornaNonese o atributo está ausente).Screenshots
N/A — sem impacto em UI.
Quais são tickets relevantes?
Criar validações para o elemento
<product>.Referências
<product><product>Original prompt
This section details on the original issue you should resolve
<issue_title>Criar validações para o elemento </issue_title>
<issue_description>## Objetivo
Implementar validações para o elemento
<product>conforme a especificação SPS 1.10, aumentando a conformidade de X% para 70% (7 de 10 regras).Nota: Algumas validações para
<product>podem já estar parcialmente implementadas no repositório. Este Issue visa reavaliar, complementar e garantir cobertura completa das regras SPS 1.10.Contexto
O elemento
<product>é usado para marcação da referência de resenha quando relacionada a um livro ou capítulo de livro. Deve aparecer em artigos com@article-type="book-review". Validações corretas garantem que o atributo obrigatório esteja presente, que elementos essenciais da referência bibliográfica estejam completos, e que haja consistência com o tipo de artigo.Conformidade atual: X de 10 regras implementadas (X%)
Meta após implementação: 7 de 10 regras (70%)
Documentação SPS
Referência oficial: https://docs.google.com/document/d/1GTv4Inc2LS_AXY-ToHT3HmO66UT0VAHWJNOIqzBNSgA/edit?tab=t.0#heading=h.product
Regras principais conforme SPS 1.10:
Ocorrência:
<product>pode aparecer zero ou mais vezes em<article-meta>Atributo obrigatório:
@product-type="book"é obrigatório em<product>Contexto de uso:
<product>deve aparecer em artigos com@article-type="book-review"Elementos tipicamente presentes:
<person-group person-group-type="author">- Autor(es) do livro resenhado<source>- Título do livro<publisher-name>- Editora<publisher-loc>- Local de publicação<year>- Ano de publicação<isbn>- ISBN do livro<size units="pages">- Número de páginas<person-group person-group-type="translator">- Tradutor(es) quando aplicávelElementos essenciais para referência bibliográfica:
<source>), editora, ano são elementos fundamentaisRegras a Implementar
P0 – Críticas (implementar obrigatoriamente)
@product-type@product-typeé obrigatório em<product>@product-type@product-typedeve ser"book"para resenhas de livros<source><source>(título do livro) é obrigatório em<product><product>está presente,<article>deve ter@article-type="book-review"P1 – Importantes (implementar se possível)
<product>contenha<person-group person-group-type="author">com informações do(s) autor(es)<product>contenha<publisher-name><product>contenha<year>P2 – Futuras (fora do escopo deste Issue)
<product>quando@article-typenão é "book-review"Arquivos a Criar/Modificar
Avaliar existentes (podem ter validações parciais):
packtools/sps/models/product.pyou similar – Verificar se modelo existepacktools/sps/validation/product.py– Verificar validações existentespacktools/sps/validation/rules/product_rules.jsonou similar – Verificar configuraçãoCriar (se não existirem):
packtools/sps/models/product.py– Modelo de extração de dadospacktools/sps/validation/product.py– Validaçõespacktools/sps/validation/rules/product_rules.json– Configuração de níveis de errotests/sps/validation/test_product.py– Testes unitáriosReferenciar (implementações similares):
packtools/sps/validation/article_contribs.py– Validação de person-grouppacktools/sps/validation/utils.py– Funções auxiliares (build_response)Exemplos de XML
XML Válido (deve passar sem erros):