From 706a1f291db395f88cf127dd263134a2e9e112d1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 12:12:44 +0000 Subject: [PATCH 1/8] Initial plan From 20d8c0d0c9a03cb964ad7dee6e21db287ac14874 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 12:33:31 +0000 Subject: [PATCH 2/8] Add new issue element validations for SPS 1.10 compliance Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com> --- .../sps/validation/front_articlemeta_issue.py | 249 ++++++- src/scielo-scholarly-data | 1 + .../test_front_articlemeta_issue.py | 628 ++++++++++++++++++ 3 files changed, 877 insertions(+), 1 deletion(-) create mode 160000 src/scielo-scholarly-data diff --git a/packtools/sps/validation/front_articlemeta_issue.py b/packtools/sps/validation/front_articlemeta_issue.py index 19962e40f..89d19bf32 100644 --- a/packtools/sps/validation/front_articlemeta_issue.py +++ b/packtools/sps/validation/front_articlemeta_issue.py @@ -242,6 +242,245 @@ def validate_expected_issues(self): error_level=self.params["expected_issues_error_level"], ) + def validate_issue_element_uniqueness(self): + """ + Validates that element appears at most once in . + According to SPS 1.10, only one element is allowed. + + Returns: + dict: Validation response with results + """ + issue_elements = self.xml_tree.findall(".//front/article-meta/issue") + count = len(issue_elements) + is_valid = count <= 1 + + return build_response( + title="issue element uniqueness", + parent={"parent": "article"}, + item="issue", + sub_item=None, + validation_type="unique", + is_valid=is_valid, + expected="at most one element in ", + obtained=f"{count} element(s) found", + advice=f"Remove duplicate elements from . Found {count} elements, expected at most 1.", + data={"issue_count": count, "issue_values": [elem.text for elem in issue_elements]}, + error_level=self.params.get("issue_element_uniqueness_error_level", "ERROR"), + ) + + def validate_issue_no_punctuation(self): + """ + Validates that value does not contain punctuation marks. + According to SPS 1.10, punctuation like . , - / : ; are not allowed. + + Returns: + dict: Validation response with results + """ + if not self.article_issue.issue: + return None + + issue_value = self.article_issue.issue + # Check for common punctuation marks + punctuation_marks = ['.', ',', '-', '/', ':', ';', '!', '?', '(', ')', '[', ']', '{', '}', '"', "'"] + found_punctuation = [p for p in punctuation_marks if p in issue_value] + is_valid = len(found_punctuation) == 0 + + return build_response( + title="issue value without punctuation", + parent={"parent": "article"}, + item="issue", + sub_item=None, + validation_type="format", + is_valid=is_valid, + expected="issue value without punctuation marks", + obtained=issue_value, + advice=f"Remove punctuation marks {found_punctuation} from value '{issue_value}'", + data={"issue": issue_value, "punctuation_found": found_punctuation}, + error_level=self.params.get("issue_no_punctuation_error_level", "ERROR"), + ) + + def validate_issue_no_uppercase(self): + """ + Validates that value does not contain uppercase letters. + According to SPS 1.10, all letters must be lowercase. + + Returns: + dict: Validation response with results + """ + if not self.article_issue.issue: + return None + + issue_value = self.article_issue.issue + has_uppercase = any(c.isupper() for c in issue_value) + is_valid = not has_uppercase + + return build_response( + title="issue value without uppercase", + parent={"parent": "article"}, + item="issue", + sub_item=None, + validation_type="format", + is_valid=is_valid, + expected="issue value in lowercase only", + obtained=issue_value, + advice=f"Convert uppercase letters to lowercase in value '{issue_value}'. Expected: '{issue_value.lower()}'", + data={"issue": issue_value, "expected": issue_value.lower()}, + error_level=self.params.get("issue_no_uppercase_error_level", "ERROR"), + ) + + def validate_issue_supplement_nomenclature(self): + """ + Validates that supplement uses correct nomenclature 'suppl'. + According to SPS 1.10, must use 'suppl' not 'supl', 's', 'supplement', 'sup'. + + Returns: + dict: Validation response with results + """ + if not self.article_issue.issue: + return None + + issue_value = self.article_issue.issue + # Check if issue contains supplement-related terms + if "sup" not in issue_value.lower(): + return None + + # Check for invalid supplement nomenclatures + invalid_terms = ['supl', 'supplement', ' sup ', ' s '] + # Also check for 'sup' at the end or beginning (but not 'suppl') + invalid_patterns = [] + + for term in invalid_terms: + if term in issue_value.lower(): + invalid_patterns.append(term.strip()) + + # Check for 'sup' not followed by 'pl' + import re + if re.search(r'\bsup\b', issue_value.lower()) and 'suppl' not in issue_value.lower(): + invalid_patterns.append('sup') + + is_valid = len(invalid_patterns) == 0 and 'suppl' in issue_value.lower() + + return build_response( + title="issue supplement nomenclature", + parent={"parent": "article"}, + item="issue", + sub_item="supplement nomenclature", + validation_type="format", + is_valid=is_valid, + expected="supplement nomenclature as 'suppl'", + obtained=issue_value, + advice=f"Use 'suppl' for supplement nomenclature in value '{issue_value}'. Invalid terms found: {invalid_patterns}", + data={"issue": issue_value, "invalid_terms": invalid_patterns}, + error_level=self.params.get("issue_supplement_nomenclature_error_level", "ERROR"), + ) + + def validate_issue_special_nomenclature(self): + """ + Validates that special issues use correct nomenclature 'spe'. + According to SPS 1.10, must use 'spe' not 'esp', 'nesp', 'nspe', 'especial', 'noesp'. + + Returns: + dict: Validation response with results + """ + if not self.article_issue.issue: + return None + + issue_value = self.article_issue.issue + issue_lower = issue_value.lower() + + # Check if issue contains special issue indicators + special_indicators = ['esp', 'especial', 'nesp', 'nspe', 'noesp'] + found_invalid = [] + + for indicator in special_indicators: + if indicator in issue_lower: + found_invalid.append(indicator) + + # If no special issue indicators found, check if 'spe' is present + if not found_invalid and 'spe' not in issue_lower: + return None + + is_valid = len(found_invalid) == 0 and 'spe' in issue_lower + + return build_response( + title="issue special nomenclature", + parent={"parent": "article"}, + item="issue", + sub_item="special issue nomenclature", + validation_type="format", + is_valid=is_valid, + expected="special issue nomenclature as 'spe'", + obtained=issue_value, + advice=f"Use 'spe' for special issue nomenclature in value '{issue_value}'. Invalid terms found: {found_invalid}", + data={"issue": issue_value, "invalid_terms": found_invalid}, + error_level=self.params.get("issue_special_nomenclature_error_level", "ERROR"), + ) + + def validate_no_supplement_element(self): + """ + Validates that element does not exist in . + According to SPS 1.10, is not allowed in . + Supplements should be identified in element instead. + + Returns: + dict: Validation response with results + """ + supplement_elements = self.xml_tree.findall(".//front/article-meta/supplement") + count = len(supplement_elements) + is_valid = count == 0 + + return build_response( + title="supplement element not allowed", + parent={"parent": "article"}, + item="supplement", + sub_item=None, + validation_type="unexpected", + is_valid=is_valid, + expected="no element in ", + obtained=f"{count} element(s) found", + advice="Remove element(s) from . Use element to indicate supplements (e.g., '4 suppl 1').", + data={"supplement_count": count, "supplement_values": [elem.text for elem in supplement_elements]}, + error_level=self.params.get("no_supplement_element_error_level", "CRITICAL"), + ) + + def validate_issue_no_leading_zeros(self): + """ + Validates that numeric parts of do not have leading zeros. + According to SPS 1.10, should use '4' not '04'. + + Returns: + dict: Validation response with results + """ + if not self.article_issue.issue: + return None + + issue_value = self.article_issue.issue + parts = issue_value.split() + + # Check each numeric part for leading zeros + issues_found = [] + for part in parts: + # Check if part is numeric and has leading zero + if part.isdigit() and len(part) > 1 and part[0] == '0': + issues_found.append(part) + + is_valid = len(issues_found) == 0 + expected_value = ' '.join([part.lstrip('0') if part.isdigit() else part for part in parts]) + + return build_response( + title="issue value without leading zeros", + parent={"parent": "article"}, + item="issue", + sub_item=None, + validation_type="format", + is_valid=is_valid, + expected="numeric values without leading zeros", + obtained=issue_value, + advice=f"Remove leading zeros from numeric parts in value '{issue_value}'. Expected: '{expected_value}'", + data={"issue": issue_value, "parts_with_leading_zeros": issues_found, "expected": expected_value}, + error_level=self.params.get("issue_no_leading_zeros_error_level", "WARNING"), + ) + def validate(self): """ Performs all validation checks for the issue. @@ -255,7 +494,15 @@ def validate(self): yield self.validate_number_format() yield self.validate_supplement_format() yield self.validate_issue_format() - yield self.validate_expected_issues() + yield self.validate_expected_issues() + # New SPS 1.10 validations for element + yield self.validate_issue_element_uniqueness() + yield self.validate_issue_no_punctuation() + yield self.validate_issue_no_uppercase() + yield self.validate_issue_supplement_nomenclature() + yield self.validate_issue_special_nomenclature() + yield self.validate_no_supplement_element() + yield self.validate_issue_no_leading_zeros() class PaginationValidation: diff --git a/src/scielo-scholarly-data b/src/scielo-scholarly-data new file mode 160000 index 000000000..a2899ce8a --- /dev/null +++ b/src/scielo-scholarly-data @@ -0,0 +1 @@ +Subproject commit a2899ce8a1fa77396c516640d36686351210d606 diff --git a/tests/sps/validation/test_front_articlemeta_issue.py b/tests/sps/validation/test_front_articlemeta_issue.py index bdfc07c13..77f8eaa4b 100644 --- a/tests/sps/validation/test_front_articlemeta_issue.py +++ b/tests/sps/validation/test_front_articlemeta_issue.py @@ -943,3 +943,631 @@ def test_validation_pages_and_e_location_exists_fail(self): self.assertEqual(obtained["response"], "OK") self.assertEqual(obtained["message"], "Got elocation-id: e51467, fpage: 220, lpage: 240, expected elocation-id or fpage + lpage") self.assertIsNone(obtained["advice"]) + + +class IssueElementUniquenessTest(TestCase): + """Tests for Rule 1: Validate uniqueness of element""" + + def setUp(self): + self.expected_keys = ["title", "parent", "parent_article_type", "parent_id", "parent_lang", "response", "item", "sub_item", + "validation_type", "expected_value", "got_value", "advice", "message", "data"] + self.params = { + "issue_element_uniqueness_error_level": "ERROR", + } + + def test_single_issue_element_valid(self): + """Test with exactly one element - should pass""" + xml = """ +
+ + + 10 + 4 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_element_uniqueness() + + self.assertEqual(obtained["response"], "OK") + self.assertEqual(obtained["title"], "issue element uniqueness") + self.assertIsNone(obtained["advice"]) + + def test_no_issue_element_valid(self): + """Test with no element - should pass (0 is <= 1)""" + xml = """ +
+ + + 10 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_element_uniqueness() + + self.assertEqual(obtained["response"], "OK") + + def test_multiple_issue_elements_invalid(self): + """Test with multiple elements - should fail""" + xml = """ +
+ + + 10 + 4 + 5 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_element_uniqueness() + + self.assertEqual(obtained["response"], "ERROR") + self.assertEqual(obtained["title"], "issue element uniqueness") + self.assertIn("Remove duplicate", obtained["advice"]) + self.assertEqual(obtained["data"]["issue_count"], 2) + + +class IssueNoPunctuationTest(TestCase): + """Tests for Rule 2: Validate no punctuation in value""" + + def setUp(self): + self.expected_keys = ["title", "parent", "parent_article_type", "parent_id", "parent_lang", "response", "item", "sub_item", + "validation_type", "expected_value", "got_value", "advice", "message", "data"] + self.params = { + "issue_no_punctuation_error_level": "ERROR", + } + + def test_issue_without_punctuation_valid(self): + """Test with valid issue without punctuation""" + xml = """ +
+ + + 4 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_no_punctuation() + + self.assertEqual(obtained["response"], "OK") + self.assertEqual(obtained["title"], "issue value without punctuation") + + def test_issue_with_suppl_no_punctuation_valid(self): + """Test with supplement format without punctuation""" + xml = """ +
+ + + 4 suppl 1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_no_punctuation() + + self.assertEqual(obtained["response"], "OK") + + def test_issue_with_period_invalid(self): + """Test with period in issue value - should fail""" + xml = """ +
+ + + 4.5 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_no_punctuation() + + self.assertEqual(obtained["response"], "ERROR") + self.assertIn(".", obtained["data"]["punctuation_found"]) + + def test_issue_with_hyphen_invalid(self): + """Test with hyphen in issue value - should fail""" + xml = """ +
+ + + 4-5 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_no_punctuation() + + self.assertEqual(obtained["response"], "ERROR") + self.assertIn("-", obtained["data"]["punctuation_found"]) + + def test_issue_with_slash_invalid(self): + """Test with slash in issue value - should fail""" + xml = """ +
+ + + 4/5 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_no_punctuation() + + self.assertEqual(obtained["response"], "ERROR") + self.assertIn("/", obtained["data"]["punctuation_found"]) + + +class IssueNoUppercaseTest(TestCase): + """Tests for Rule 3: Validate no uppercase in value""" + + def setUp(self): + self.params = { + "issue_no_uppercase_error_level": "ERROR", + } + + def test_issue_lowercase_valid(self): + """Test with all lowercase - should pass""" + xml = """ +
+ + + 4 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_no_uppercase() + + self.assertEqual(obtained["response"], "OK") + + def test_issue_spe_lowercase_valid(self): + """Test with 'spe' in lowercase - should pass""" + xml = """ +
+ + + spe1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_no_uppercase() + + self.assertEqual(obtained["response"], "OK") + + def test_issue_suppl_lowercase_valid(self): + """Test with 'suppl' in lowercase - should pass""" + xml = """ +
+ + + 4 suppl 1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_no_uppercase() + + self.assertEqual(obtained["response"], "OK") + + def test_issue_with_uppercase_invalid(self): + """Test with uppercase 'Suppl' - should fail""" + xml = """ +
+ + + 4 Suppl 1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_no_uppercase() + + self.assertEqual(obtained["response"], "ERROR") + self.assertEqual(obtained["data"]["expected"], "4 suppl 1") + + def test_issue_spe_with_uppercase_invalid(self): + """Test with uppercase 'SPE' - should fail""" + xml = """ +
+ + + SPE1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_no_uppercase() + + self.assertEqual(obtained["response"], "ERROR") + self.assertEqual(obtained["data"]["expected"], "spe1") + + +class IssueSupplementNomenclatureTest(TestCase): + """Tests for Rule 4: Validate supplement nomenclature""" + + def setUp(self): + self.params = { + "issue_supplement_nomenclature_error_level": "ERROR", + } + + def test_issue_suppl_valid(self): + """Test with correct 'suppl' nomenclature - should pass""" + xml = """ +
+ + + 4 suppl 1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_supplement_nomenclature() + + self.assertEqual(obtained["response"], "OK") + + def test_issue_suppl_only_valid(self): + """Test with 'suppl' only - should pass""" + xml = """ +
+ + + suppl 1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_supplement_nomenclature() + + self.assertEqual(obtained["response"], "OK") + + def test_issue_supl_invalid(self): + """Test with 'supl' instead of 'suppl' - should fail""" + xml = """ +
+ + + 4 supl 1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_supplement_nomenclature() + + self.assertEqual(obtained["response"], "ERROR") + self.assertIn("supl", obtained["data"]["invalid_terms"]) + + def test_issue_supplement_invalid(self): + """Test with 'supplement' instead of 'suppl' - should fail""" + xml = """ +
+ + + 4 supplement 1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_supplement_nomenclature() + + self.assertEqual(obtained["response"], "ERROR") + self.assertIn("supplement", obtained["data"]["invalid_terms"]) + + def test_issue_sup_invalid(self): + """Test with 'sup' instead of 'suppl' - should fail""" + xml = """ +
+ + + 4 sup 1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_supplement_nomenclature() + + self.assertEqual(obtained["response"], "ERROR") + self.assertIn("sup", obtained["data"]["invalid_terms"]) + + +class IssueSpecialNomenclatureTest(TestCase): + """Tests for Rule 5: Validate special issue nomenclature""" + + def setUp(self): + self.params = { + "issue_special_nomenclature_error_level": "ERROR", + } + + def test_issue_spe_valid(self): + """Test with correct 'spe' nomenclature - should pass""" + xml = """ +
+ + + spe1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_special_nomenclature() + + self.assertEqual(obtained["response"], "OK") + + def test_issue_spe_with_number_valid(self): + """Test with 'spe' and number - should pass""" + xml = """ +
+ + + 4 spe 1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_special_nomenclature() + + self.assertEqual(obtained["response"], "OK") + + def test_issue_esp_invalid(self): + """Test with 'esp' instead of 'spe' - should fail""" + xml = """ +
+ + + esp1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_special_nomenclature() + + self.assertEqual(obtained["response"], "ERROR") + self.assertIn("esp", obtained["data"]["invalid_terms"]) + + def test_issue_nesp_invalid(self): + """Test with 'nesp' instead of 'spe' - should fail""" + xml = """ +
+ + + nesp1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_special_nomenclature() + + self.assertEqual(obtained["response"], "ERROR") + self.assertIn("nesp", obtained["data"]["invalid_terms"]) + + def test_issue_especial_invalid(self): + """Test with 'especial' instead of 'spe' - should fail""" + xml = """ +
+ + + especial + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_special_nomenclature() + + self.assertEqual(obtained["response"], "ERROR") + self.assertIn("especial", obtained["data"]["invalid_terms"]) + + +class NoSupplementElementTest(TestCase): + """Tests for Rule 6: Validate absence of element""" + + def setUp(self): + self.params = { + "no_supplement_element_error_level": "CRITICAL", + } + + def test_no_supplement_element_valid(self): + """Test without element - should pass""" + xml = """ +
+ + + 10 + 4 suppl 1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_no_supplement_element() + + self.assertEqual(obtained["response"], "OK") + self.assertEqual(obtained["title"], "supplement element not allowed") + + def test_with_supplement_element_invalid(self): + """Test with element - should fail""" + xml = """ +
+ + + 10 + 4 + 1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_no_supplement_element() + + self.assertEqual(obtained["response"], "CRITICAL") + self.assertIn("Remove ", obtained["advice"]) + self.assertEqual(obtained["data"]["supplement_count"], 1) + + def test_with_multiple_supplement_elements_invalid(self): + """Test with multiple elements - should fail""" + xml = """ +
+ + + 10 + 4 + 1 + 2 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_no_supplement_element() + + self.assertEqual(obtained["response"], "CRITICAL") + self.assertEqual(obtained["data"]["supplement_count"], 2) + + +class IssueNoLeadingZerosTest(TestCase): + """Tests for Rule 7: Validate no leading zeros""" + + def setUp(self): + self.params = { + "issue_no_leading_zeros_error_level": "WARNING", + } + + def test_issue_without_leading_zeros_valid(self): + """Test with issue without leading zeros - should pass""" + xml = """ +
+ + + 4 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_no_leading_zeros() + + self.assertEqual(obtained["response"], "OK") + + def test_issue_suppl_without_leading_zeros_valid(self): + """Test with supplement without leading zeros - should pass""" + xml = """ +
+ + + 4 suppl 1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_no_leading_zeros() + + self.assertEqual(obtained["response"], "OK") + + def test_issue_with_leading_zero_invalid(self): + """Test with leading zero in issue - should fail""" + xml = """ +
+ + + 04 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_no_leading_zeros() + + self.assertEqual(obtained["response"], "WARNING") + self.assertIn("04", obtained["data"]["parts_with_leading_zeros"]) + self.assertEqual(obtained["data"]["expected"], "4") + + def test_issue_suppl_with_leading_zero_invalid(self): + """Test with leading zero in supplement number - should fail""" + xml = """ +
+ + + 4 suppl 01 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_no_leading_zeros() + + self.assertEqual(obtained["response"], "WARNING") + self.assertIn("01", obtained["data"]["parts_with_leading_zeros"]) + self.assertEqual(obtained["data"]["expected"], "4 suppl 1") + + def test_issue_zero_alone_valid(self): + """Test with single zero (0) - should pass as it's not a leading zero""" + xml = """ +
+ + + 0 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_no_leading_zeros() + + self.assertEqual(obtained["response"], "OK") From 61c4e5b01b8ceb7d6c991ef20e5e1fbe7da1c118 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 12:33:41 +0000 Subject: [PATCH 3/8] Remove accidentally added src/ submodule --- .gitignore | 1 + src/scielo-scholarly-data | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 160000 src/scielo-scholarly-data diff --git a/.gitignore b/.gitignore index e330edc72..cb5191afc 100644 --- a/.gitignore +++ b/.gitignore @@ -42,3 +42,4 @@ nosetests.xml .venv .idea +src/ diff --git a/src/scielo-scholarly-data b/src/scielo-scholarly-data deleted file mode 160000 index a2899ce8a..000000000 --- a/src/scielo-scholarly-data +++ /dev/null @@ -1 +0,0 @@ -Subproject commit a2899ce8a1fa77396c516640d36686351210d606 From 4ac03ac81613ab6555665845c6919d1dfd9f0567 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 12:36:22 +0000 Subject: [PATCH 4/8] Fix validation logic based on code review feedback Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com> --- packtools/sps/validation/front_articlemeta_issue.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packtools/sps/validation/front_articlemeta_issue.py b/packtools/sps/validation/front_articlemeta_issue.py index 89d19bf32..456e99f89 100644 --- a/packtools/sps/validation/front_articlemeta_issue.py +++ b/packtools/sps/validation/front_articlemeta_issue.py @@ -1,5 +1,6 @@ from packtools.sps.models.front_articlemeta_issue import ArticleMetaIssue from packtools.sps.validation.utils import build_response +import re def is_valid_value(value, zero_is_allowed): @@ -354,11 +355,10 @@ def validate_issue_supplement_nomenclature(self): invalid_patterns.append(term.strip()) # Check for 'sup' not followed by 'pl' - import re if re.search(r'\bsup\b', issue_value.lower()) and 'suppl' not in issue_value.lower(): invalid_patterns.append('sup') - is_valid = len(invalid_patterns) == 0 and 'suppl' in issue_value.lower() + is_valid = len(invalid_patterns) == 0 return build_response( title="issue supplement nomenclature", @@ -400,7 +400,7 @@ def validate_issue_special_nomenclature(self): if not found_invalid and 'spe' not in issue_lower: return None - is_valid = len(found_invalid) == 0 and 'spe' in issue_lower + is_valid = len(found_invalid) == 0 return build_response( title="issue special nomenclature", @@ -465,7 +465,7 @@ def validate_issue_no_leading_zeros(self): issues_found.append(part) is_valid = len(issues_found) == 0 - expected_value = ' '.join([part.lstrip('0') if part.isdigit() else part for part in parts]) + expected_value = ' '.join([(part.lstrip('0') or '0') if part.isdigit() else part for part in parts]) return build_response( title="issue value without leading zeros", From 402a6255bcfeab92583d21ce7f5e3b2ed0fc4fc1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 12:38:33 +0000 Subject: [PATCH 5/8] Refactor supplement validation and remove unused test variables Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com> --- .../sps/validation/front_articlemeta_issue.py | 22 ++++++++++--------- .../test_front_articlemeta_issue.py | 4 ---- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/packtools/sps/validation/front_articlemeta_issue.py b/packtools/sps/validation/front_articlemeta_issue.py index 456e99f89..8e6e5ed7f 100644 --- a/packtools/sps/validation/front_articlemeta_issue.py +++ b/packtools/sps/validation/front_articlemeta_issue.py @@ -341,22 +341,24 @@ def validate_issue_supplement_nomenclature(self): return None issue_value = self.article_issue.issue + issue_lower = issue_value.lower() + # Check if issue contains supplement-related terms - if "sup" not in issue_value.lower(): + if "sup" not in issue_lower: return None - # Check for invalid supplement nomenclatures - invalid_terms = ['supl', 'supplement', ' sup ', ' s '] - # Also check for 'sup' at the end or beginning (but not 'suppl') + # Check for invalid supplement nomenclatures using regex invalid_patterns = [] - for term in invalid_terms: - if term in issue_value.lower(): - invalid_patterns.append(term.strip()) - - # Check for 'sup' not followed by 'pl' - if re.search(r'\bsup\b', issue_value.lower()) and 'suppl' not in issue_value.lower(): + # Check for specific invalid patterns + if re.search(r'\bsupl\b', issue_lower): + invalid_patterns.append('supl') + if re.search(r'\bsupplement\b', issue_lower): + invalid_patterns.append('supplement') + if re.search(r'\bsup\b', issue_lower): invalid_patterns.append('sup') + if re.search(r'\bs\b', issue_lower): + invalid_patterns.append('s') is_valid = len(invalid_patterns) == 0 diff --git a/tests/sps/validation/test_front_articlemeta_issue.py b/tests/sps/validation/test_front_articlemeta_issue.py index 77f8eaa4b..fc141d935 100644 --- a/tests/sps/validation/test_front_articlemeta_issue.py +++ b/tests/sps/validation/test_front_articlemeta_issue.py @@ -949,8 +949,6 @@ class IssueElementUniquenessTest(TestCase): """Tests for Rule 1: Validate uniqueness of element""" def setUp(self): - self.expected_keys = ["title", "parent", "parent_article_type", "parent_id", "parent_lang", "response", "item", "sub_item", - "validation_type", "expected_value", "got_value", "advice", "message", "data"] self.params = { "issue_element_uniqueness_error_level": "ERROR", } @@ -1019,8 +1017,6 @@ class IssueNoPunctuationTest(TestCase): """Tests for Rule 2: Validate no punctuation in value""" def setUp(self): - self.expected_keys = ["title", "parent", "parent_article_type", "parent_id", "parent_lang", "response", "item", "sub_item", - "validation_type", "expected_value", "got_value", "advice", "message", "data"] self.params = { "issue_no_punctuation_error_level": "ERROR", } From 38ae7a046c536644a9fc711251a4dd423cf5cefe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 12:41:40 +0000 Subject: [PATCH 6/8] Fix supplement nomenclature validation to avoid false positives Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com> --- packtools/sps/validation/front_articlemeta_issue.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packtools/sps/validation/front_articlemeta_issue.py b/packtools/sps/validation/front_articlemeta_issue.py index 8e6e5ed7f..165057e4f 100644 --- a/packtools/sps/validation/front_articlemeta_issue.py +++ b/packtools/sps/validation/front_articlemeta_issue.py @@ -332,7 +332,7 @@ def validate_issue_no_uppercase(self): def validate_issue_supplement_nomenclature(self): """ Validates that supplement uses correct nomenclature 'suppl'. - According to SPS 1.10, must use 'suppl' not 'supl', 's', 'supplement', 'sup'. + According to SPS 1.10, must use 'suppl' not 'supl', 'supplement', 'sup'. Returns: dict: Validation response with results @@ -357,8 +357,6 @@ def validate_issue_supplement_nomenclature(self): invalid_patterns.append('supplement') if re.search(r'\bsup\b', issue_lower): invalid_patterns.append('sup') - if re.search(r'\bs\b', issue_lower): - invalid_patterns.append('s') is_valid = len(invalid_patterns) == 0 From 370cc2f7878b2e7190faa4917d3a89005a267d91 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 9 Mar 2026 11:44:03 +0000 Subject: [PATCH 7/8] Fix test assertions and add missing test coverage - Fix test_validate_article_issue_out_of_pattern_value to match actual behavior - Fix test_validate_article_issue to expect 11 non-None results (not 10) - Fix pagination test assertions to match actual module output strings - Fix test_validation_e_location to test elocation-id present case (was duplicate) - Rename test_validation_pages_and_e_location_exists_fail to test_validation_elocation_and_pages_both_present_valid - Remove unused expected dicts from test_validate_article_issue_number_start_with_zero and test_validate_article_issue_number_value_is_not_numeric - Rename test_suppl_matches to test_suppl_invalid_non_alphanumeric (more accurate) - Rename test_suppl_no_matches to test_suppl_valid_alphanumeric (more accurate) - Add missing test coverage: - VolumeZeroTest for volume=0 validation - ExpectedIssuesTest for registered/unregistered issues and missing journal_data - PaginationAdditionalTest for elocation-only, fpage/lpage-only, and AOP cases - SpecialNomenclatureAdditionalTest for nspe and noesp rejection Addresses all issues raised in PR review comments. Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com> --- .../test_front_articlemeta_issue.py | 311 +++++++++++++----- 1 file changed, 235 insertions(+), 76 deletions(-) diff --git a/tests/sps/validation/test_front_articlemeta_issue.py b/tests/sps/validation/test_front_articlemeta_issue.py index fc141d935..244e5de4e 100644 --- a/tests/sps/validation/test_front_articlemeta_issue.py +++ b/tests/sps/validation/test_front_articlemeta_issue.py @@ -261,23 +261,6 @@ def test_validate_article_issue_number_start_with_zero(self): """ ) - expected = { - "title": "number", - "parent": "article", - "parent_article_type": None, - "parent_id": None, - "parent_lang": None, - "response": "ERROR", - "item": "number", - "sub_item": None, - "validation_type": "format", - "expected_value": "4", - "got_value": "04", - "message": "Got 04, expected 4", - "advice": "Consulte SPS documentation to complete issue element", - "data": {"number": "04", "volume": "56"}, - } - validator = IssueValidation( xml_tree, params=self.params ) @@ -307,23 +290,6 @@ def test_validate_article_issue_number_value_is_not_numeric(self): """ ) - expected = { - "title": "number", - "parent": "article", - "parent_article_type": None, - "parent_id": None, - "parent_lang": None, - "response": "OK", - "item": "number", - "sub_item": None, - "validation_type": "format", - "expected_value": "4a", - "got_value": "4a", - "message": "Got 4a, expected 4a", - "advice": None, - "data": {"number": "4a", "volume": "56"}, - } - validator = IssueValidation( xml_tree, params=self.params ) @@ -652,7 +618,7 @@ def test_validate_article_issue_number_supplement_with_dot_and_space(self): self.assertDictEqual(expected, obtained) - def test_suppl_matches(self): + def test_suppl_invalid_non_alphanumeric(self): self.maxDiff = None xml_tree = etree.fromstring( """ @@ -693,7 +659,7 @@ def test_suppl_matches(self): self.assertDictEqual(expected, obtained) - def test_suppl_no_matches(self): + def test_suppl_valid_alphanumeric(self): self.maxDiff = None xml_tree = etree.fromstring( """ @@ -823,9 +789,9 @@ def test_validate_article_issue(self): params=self.params, ) - obtained = list(validator.validate()) + obtained = [r for r in validator.validate() if r is not None] - self.assertEqual(len(obtained), 5) + self.assertEqual(len(obtained), 11) for i, item in enumerate(obtained): for key in self.expected_keys: with self.subTest(f"item: {i}, key: {key}"): @@ -863,26 +829,16 @@ def test_validation_pages(self): """ xml_tree = etree.fromstring(xml) - expected = { - "title": "Pagination", - "parent": "article", - "parent_article_type": None, - "parent_id": None, - "parent_lang": None, - "item": "elocation-id | fpage / lpage", - "sub_item": "elocation-id | fpage / lpage", - "validation_type": "match", - "response": "CRITICAL", - "expected_value": "elocation-id or fpage + lpage", - "got_value": "elocation-id: None, fpage: None, lpage: None", - "message": "Got elocation-id: None, fpage: None, lpage: None, expected elocation-id or fpage + lpage", - "advice": "Provide elocation-id or fpage + lpage", - "data": {"volume": "56"}, - } - obtained = PaginationValidation(xml_tree, self.params).validate() - self.assertDictEqual(expected, obtained) + for key in self.expected_keys: + self.assertIn(key, obtained, f"{key} not found") + + self.assertEqual(obtained["title"], "Pagination") + self.assertEqual(obtained["response"], "CRITICAL") + self.assertEqual(obtained["expected_value"], "elocation id or first and last pages") + self.assertEqual(obtained["message"], "Got elocation-id: None, fpage: None, lpage: None, expected elocation id or first and last pages") + self.assertEqual(obtained["advice"], "Mark elocation id with or first page with and last page with ") def test_validation_e_location(self): self.maxDiff = None @@ -891,34 +847,24 @@ def test_validation_e_location(self): 56 + e51467 """ xml_tree = etree.fromstring(xml) - expected = { - "title": "Pagination", - "parent": "article", - "parent_article_type": None, - "parent_id": None, - "parent_lang": None, - "item": "elocation-id | fpage / lpage", - "sub_item": "elocation-id | fpage / lpage", - "validation_type": "match", - "response": "CRITICAL", - "expected_value": "elocation-id or fpage + lpage", - "got_value": "elocation-id: None, fpage: None, lpage: None", - "message": "Got elocation-id: None, fpage: None, lpage: None, expected elocation-id or fpage + lpage", - "advice": "Provide elocation-id or fpage + lpage", - "data": {"volume": "56"}, - } - obtained = PaginationValidation(xml_tree, self.params).validate() - self.assertDictEqual(expected, obtained) + for key in self.expected_keys: + self.assertIn(key, obtained, f"{key} not found") + + self.assertEqual(obtained["title"], "Pagination") + self.assertEqual(obtained["response"], "OK") + self.assertEqual(obtained["message"], "Got elocation-id: e51467, fpage: None, lpage: None, expected elocation id or first and last pages") + self.assertIsNone(obtained["advice"]) - def test_validation_pages_and_e_location_exists_fail(self): + def test_validation_elocation_and_pages_both_present_valid(self): self.maxDiff = None xml = """
@@ -941,7 +887,7 @@ def test_validation_pages_and_e_location_exists_fail(self): self.assertEqual(obtained["title"], "Pagination") self.assertEqual(obtained["response"], "OK") - self.assertEqual(obtained["message"], "Got elocation-id: e51467, fpage: 220, lpage: 240, expected elocation-id or fpage + lpage") + self.assertEqual(obtained["message"], "Got elocation-id: e51467, fpage: 220, lpage: 240, expected elocation id or first and last pages") self.assertIsNone(obtained["advice"]) @@ -1567,3 +1513,216 @@ def test_issue_zero_alone_valid(self): obtained = validator.validate_issue_no_leading_zeros() self.assertEqual(obtained["response"], "OK") + + +# Additional test coverage for missing cases + +class VolumeZeroTest(TestCase): + """Test for volume=0 validation""" + + def setUp(self): + self.params = { + "volume_format_error_level": "CRITICAL", + } + + def test_volume_zero_invalid(self): + """Test that volume='0' is invalid (zero_is_allowed=False)""" + xml = """ +
+ + + 0 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_volume_format() + + self.assertEqual(obtained["response"], "CRITICAL") + self.assertIn("expected alphanumeric value, except 0", obtained["message"]) + + +class ExpectedIssuesTest(TestCase): + """Tests for validate_expected_issues""" + + def setUp(self): + self.params = { + "expected_issues_error_level": "CRITICAL", + } + + def test_issue_present_in_registered_list(self): + """Test that issue present in the list passes""" + xml = """ +
+ + + 56 + 4 + + +
+ """ + xml_tree = etree.fromstring(xml) + params = { + "expected_issues_error_level": "CRITICAL", + "journal_data": { + "issues": [{"volume": "56", "number": "4", "supplement": None}] + } + } + validator = IssueValidation(xml_tree, params=params) + obtained = validator.validate_expected_issues() + + self.assertEqual(obtained["response"], "OK") + + def test_issue_absent_from_registered_list(self): + """Test that issue absent from the list fails""" + xml = """ +
+ + + 56 + 99 + + +
+ """ + xml_tree = etree.fromstring(xml) + params = { + "expected_issues_error_level": "CRITICAL", + "journal_data": { + "issues": [{"volume": "56", "number": "4", "supplement": None}] + } + } + validator = IssueValidation(xml_tree, params=params) + obtained = validator.validate_expected_issues() + + self.assertEqual(obtained["response"], "CRITICAL") + + def test_journal_data_absent(self): + """Test that missing journal_data returns WARNING""" + xml = """ +
+ + + 56 + 4 + + +
+ """ + xml_tree = etree.fromstring(xml) + params = {"expected_issues_error_level": "CRITICAL"} + validator = IssueValidation(xml_tree, params=params) + obtained = validator.validate_expected_issues() + + self.assertEqual(obtained["response"], "WARNING") + self.assertIn("Unable to check", obtained["advice"]) + + +class PaginationAdditionalTest(TestCase): + """Additional pagination tests for missing coverage""" + + def setUp(self): + self.params = { + "pagination_error_level": "CRITICAL", + } + + def test_only_elocation_id_valid(self): + """Test that only elocation-id is valid""" + xml = """ +
+ + + 56 + e12345 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = PaginationValidation(xml_tree, params=self.params) + obtained = validator.validate() + + self.assertEqual(obtained["response"], "OK") + + def test_only_fpage_lpage_valid(self): + """Test that only fpage and lpage is valid""" + xml = """ +
+ + + 56 + 100 + 120 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = PaginationValidation(xml_tree, params=self.params) + obtained = validator.validate() + + self.assertEqual(obtained["response"], "OK") + + def test_aop_no_pagination_valid(self): + """Test that AOP (no volume, no issue, no pagination) is valid""" + xml = """ +
+ + + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = PaginationValidation(xml_tree, params=self.params) + obtained = validator.validate() + + self.assertEqual(obtained["response"], "OK") + + +class SpecialNomenclatureAdditionalTest(TestCase): + """Additional tests for special issue nomenclature""" + + def setUp(self): + self.params = { + "issue_special_nomenclature_error_level": "ERROR", + } + + def test_issue_nspe_invalid(self): + """Test that 'nspe' is rejected""" + xml = """ +
+ + + nspe1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_special_nomenclature() + + self.assertEqual(obtained["response"], "ERROR") + self.assertIn("nspe", obtained["data"]["invalid_terms"]) + + def test_issue_noesp_invalid(self): + """Test that 'noesp' is rejected""" + xml = """ +
+ + + noesp1 + + +
+ """ + xml_tree = etree.fromstring(xml) + validator = IssueValidation(xml_tree, params=self.params) + obtained = validator.validate_issue_special_nomenclature() + + self.assertEqual(obtained["response"], "ERROR") + self.assertIn("noesp", obtained["data"]["invalid_terms"]) From 105a0dd5040d5f141bd4e3e3c8e5739f13928729 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 13:43:11 +0000 Subject: [PATCH 8/8] Fix advice string bug in validate_issue_format() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fixed f-string typo: "by with" → "with" - Fixed object interpolation: use self.article_issue.issue (string) instead of self.article_issue (object) - Fixed template: removed closing tag from advice string - Advice now correctly displays: "Replace {issue_value} in with one of {expected}" Co-authored-by: robertatakenaka <505143+robertatakenaka@users.noreply.github.com> --- packtools/sps/validation/front_articlemeta_issue.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packtools/sps/validation/front_articlemeta_issue.py b/packtools/sps/validation/front_articlemeta_issue.py index 165057e4f..4ddd24535 100644 --- a/packtools/sps/validation/front_articlemeta_issue.py +++ b/packtools/sps/validation/front_articlemeta_issue.py @@ -185,7 +185,7 @@ def validate_issue_format(self): expected=expected, obtained=parsed_issue, advice=( - f"""Replace {self.article_issue} in {self.article_issue} by with one of {expected}""" + f"Replace {self.article_issue.issue} in with one of {expected}" if not got_valid_format else None ),