← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] ~alvarocs/launchpad:content-templates-validator into launchpad:master

 

Alvaro Crespo Serrano has proposed merging ~alvarocs/launchpad:content-templates-validator into launchpad:master.

Commit message:
Implement content templates validation

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~alvarocs/launchpad/+git/launchpad/+merge/478379

-Make the content_templates field accessible through the API by exporting it.
-Implement new validation function for content templates 'validatie_content_templates' in app/validators/validation.py script.
-Add the validation function as constraint of the Dict field 'content_templates' in registry/interfaces/projectgroup.py class IProjectGroupPublic and bugs/interfaces/bugtarget.py class IBugTarget.
-Update doctests as 'content_templates' field is now exported.
-Unit tests for validation function.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~alvarocs/launchpad:content-templates-validator into launchpad:master.
diff --git a/lib/lp/app/validators/validation.py b/lib/lp/app/validators/validation.py
index 4a2dc62..83c1d41 100644
--- a/lib/lp/app/validators/validation.py
+++ b/lib/lp/app/validators/validation.py
@@ -6,6 +6,7 @@ __all__ = [
     "valid_cve_sequence",
     "validate_new_team_email",
     "validate_oci_branch_name",
+    "validate_content_templates",
 ]
 
 import re
@@ -114,3 +115,30 @@ def validate_oci_branch_name(branch_name):
         if "/" in segment:
             return False
     return True
+
+
+# XXX alvarocs 2024-12-13:
+# To add merge proposal templates or other templates
+# as allowed keys when implemented.
+def validate_content_templates(value):
+    # Omit validation if None
+    if value is None:
+        return True
+    allowed_keys = {
+        "bug_templates",
+        "mergeproposal_templates",
+        "answer_templates",
+    }
+    for key, inner_dict in value.items():
+        # Validate allowed keys
+        if key not in allowed_keys:
+            raise ValueError(
+                f"Invalid key '{key}' in content_templates. "
+                "Allowed keys: {allowed_keys}"
+            )
+        # Validate 'default' key exists
+        if "default" not in inner_dict:
+            raise ValueError(
+                f"The '{key}' dictionary must contain a 'default' key."
+            )
+    return True
diff --git a/lib/lp/bugs/browser/tests/test_bugtarget_configure.py b/lib/lp/bugs/browser/tests/test_bugtarget_configure.py
index 88846c7..76b53c0 100644
--- a/lib/lp/bugs/browser/tests/test_bugtarget_configure.py
+++ b/lib/lp/bugs/browser/tests/test_bugtarget_configure.py
@@ -4,6 +4,7 @@
 """Unit tests for bug configuration views."""
 
 from lp.app.enums import ServiceUsage
+from lp.app.validators.validation import validate_content_templates
 from lp.registry.interfaces.person import TeamMembershipPolicy
 from lp.testing import TestCaseWithFactory, login_person
 from lp.testing.layers import DatabaseFunctionalLayer
@@ -192,3 +193,34 @@ class TestProductBugConfigurationView(TestCaseWithFactory):
             {"bug_templates": {"default": "new lp template"}},
             self.product.content_templates,
         )
+
+
+class TestValidateContentTemplates(TestCaseWithFactory):
+    layer = DatabaseFunctionalLayer
+
+    # Test the validator for content templates
+    def test_none_value(self):
+        self.assertTrue(validate_content_templates(None))
+
+    def test_valid_content_templates(self):
+        valid_value = {
+            "bug_templates": {
+                "default": "A default bug template",
+                "security": "A bug template for security related bugs",
+            },
+        }
+        self.assertTrue(validate_content_templates(valid_value))
+
+    def test_invalid_key(self):
+        invalid_value = {"invalid_key": {"default": "A default bug template"}}
+        self.assertRaises(
+            ValueError, validate_content_templates, invalid_value
+        )
+
+    def test_missing_default(self):
+        invalid_value = {
+            "bug_templates": {"not_default": "A not default bug template"}
+        }
+        self.assertRaises(
+            ValueError, validate_content_templates, invalid_value
+        )
diff --git a/lib/lp/bugs/interfaces/bugtarget.py b/lib/lp/bugs/interfaces/bugtarget.py
index 2415010..2f4fec0 100644
--- a/lib/lp/bugs/interfaces/bugtarget.py
+++ b/lib/lp/bugs/interfaces/bugtarget.py
@@ -52,6 +52,7 @@ from lp.app.enums import (
     PROPRIETARY_INFORMATION_TYPES,
     InformationType,
 )
+from lp.app.validators.validation import validate_content_templates
 from lp.bugs.interfaces.bugtask import IBugTask
 from lp.bugs.interfaces.bugtasksearch import (
     BugBlueprintSearch,
@@ -276,26 +277,27 @@ class IBugTarget(IHasBugs):
         )
     )
 
-    # XXX alvarocs 2024-11-27:
-    # To be exported to the API once a validator is added.
-    content_templates = Dict(
-        title=("Templates to use for reporting a bug"),
-        description=(
-            "This pre-defined template will be given to the "
-            "users to guide them when reporting a bug. "
-        ),
-        key_type=TextLine(),
-        value_type=Dict(
+    content_templates = exported(
+        Dict(
+            title=("Templates to use for reporting a bug"),
+            description=(
+                "This pre-defined template will be given to the "
+                "users to guide them when reporting a bug. "
+            ),
             key_type=TextLine(),
-            value_type=Text(
+            value_type=Dict(
+                key_type=TextLine(),
+                value_type=Text(
+                    required=False,
+                    max_length=50000,
+                ),
                 required=False,
                 max_length=50000,
             ),
             required=False,
             max_length=50000,
-        ),
-        required=False,
-        max_length=50000,
+            constraint=validate_content_templates,
+        )
     )
 
     bug_reported_acknowledgement = exported(
diff --git a/lib/lp/registry/interfaces/projectgroup.py b/lib/lp/registry/interfaces/projectgroup.py
index 0b39dc4..e8324c1 100644
--- a/lib/lp/registry/interfaces/projectgroup.py
+++ b/lib/lp/registry/interfaces/projectgroup.py
@@ -33,6 +33,7 @@ from lp.app.interfaces.launchpad import (
     IServiceUsage,
 )
 from lp.app.validators.name import name_validator
+from lp.app.validators.validation import validate_content_templates
 from lp.blueprints.interfaces.specificationtarget import IHasSpecifications
 from lp.blueprints.interfaces.sprint import IHasSprints
 from lp.bugs.interfaces.bugtarget import IHasBugs, IHasOfficialBugTags
@@ -384,26 +385,27 @@ class IProjectGroupPublic(
         )
     )
 
-    # XXX alvarocs 2024-11-27:
-    # To be exported to the API once a validator is added.
-    content_templates = Dict(
-        title=("Templates to use for reporting a bug"),
-        description=(
-            "This pre-defined template will be given to the "
-            "users to guide them when reporting a bug. "
-        ),
-        key_type=TextLine(),
-        value_type=Dict(
+    content_templates = exported(
+        Dict(
+            title=("Templates to use for reporting a bug"),
+            description=(
+                "This pre-defined template will be given to the "
+                "users to guide them when reporting a bug. "
+            ),
             key_type=TextLine(),
-            value_type=Text(
+            value_type=Dict(
+                key_type=TextLine(),
+                value_type=Text(
+                    required=False,
+                    max_length=50000,
+                ),
                 required=False,
                 max_length=50000,
             ),
             required=False,
             max_length=50000,
-        ),
-        required=False,
-        max_length=50000,
+            constraint=validate_content_templates,
+        )
     )
 
     bug_reported_acknowledgement = exported(
diff --git a/lib/lp/registry/stories/webservice/xx-distribution-source-package.rst b/lib/lp/registry/stories/webservice/xx-distribution-source-package.rst
index a848237..0a69eff 100644
--- a/lib/lp/registry/stories/webservice/xx-distribution-source-package.rst
+++ b/lib/lp/registry/stories/webservice/xx-distribution-source-package.rst
@@ -14,6 +14,7 @@ Source packages can be obtained from the context of a distribution.
     >>> pprint_entry(mozilla_firefox)
     bug_reported_acknowledgement: None
     bug_reporting_guidelines: None
+    content_templates: None
     display_name: 'mozilla-firefox in Debian'
     distribution_link: 'http://.../debian'
     name: 'mozilla-firefox'
diff --git a/lib/lp/registry/stories/webservice/xx-distribution.rst b/lib/lp/registry/stories/webservice/xx-distribution.rst
index 1d9a8e9..f873924 100644
--- a/lib/lp/registry/stories/webservice/xx-distribution.rst
+++ b/lib/lp/registry/stories/webservice/xx-distribution.rst
@@ -32,6 +32,7 @@ And for every distribution we publish most of its attributes.
     code_admin_link: None
     commercial_subscription_is_due: False
     commercial_subscription_link: None
+    content_templates: None
     current_series_link: 'http://.../ubuntu/hoary'
     date_created: '2006-10-16T18:31:43.415195+00:00'
     default_traversal_policy: 'Series'
diff --git a/lib/lp/registry/stories/webservice/xx-distroseries.rst b/lib/lp/registry/stories/webservice/xx-distroseries.rst
index 51a089a..1dd4aeb 100644
--- a/lib/lp/registry/stories/webservice/xx-distroseries.rst
+++ b/lib/lp/registry/stories/webservice/xx-distroseries.rst
@@ -75,6 +75,7 @@ For distroseries we publish a subset of its attributes.
     bug_reporting_guidelines: None
     changeslist: 'hoary-changes@xxxxxxxxxx'
     component_names: ['main', 'restricted']
+    content_templates: None
     date_created: '2006-10-16T18:31:43.483559+00:00'
     datereleased: None
     description: 'Hoary is the ...
diff --git a/lib/lp/registry/stories/webservice/xx-person.rst b/lib/lp/registry/stories/webservice/xx-person.rst
index 7e93f8b..4ae1a58 100644
--- a/lib/lp/registry/stories/webservice/xx-person.rst
+++ b/lib/lp/registry/stories/webservice/xx-person.rst
@@ -813,6 +813,7 @@ Subscribed packages can be listed with getBugSubscriberPackages:
     ---
     bug_reported_acknowledgement: None
     bug_reporting_guidelines: None
+    content_templates: None
     display_name: '...'
     distribution_link: '...'
     name: 'fooix'
diff --git a/lib/lp/registry/stories/webservice/xx-project-registry.rst b/lib/lp/registry/stories/webservice/xx-project-registry.rst
index 37b1061..b6bdb0a 100644
--- a/lib/lp/registry/stories/webservice/xx-project-registry.rst
+++ b/lib/lp/registry/stories/webservice/xx-project-registry.rst
@@ -79,6 +79,7 @@ host.
     bug_reported_acknowledgement: None
     bug_reporting_guidelines: None
     bug_tracker_link: None
+    content_templates: None
     date_created: '...'
     description: 'The Mozilla Project...'
     display_name: 'The Mozilla Project'
@@ -177,6 +178,7 @@ Projects are available at their canonical URL on the API virtual host.
     bug_tracker_link: None
     commercial_subscription_is_due: False
     commercial_subscription_link: None
+    content_templates: None
     date_created: '2004-09-24T20:58:02.185708+00:00'
     date_next_suggest_packaging: None
     description: 'The Mozilla Firefox web browser'
@@ -841,6 +843,7 @@ virtual host.
     branch_link: 'http://.../~babadoo-owner/babadoo/fooey'
     bug_reported_acknowledgement: None
     bug_reporting_guidelines: None
+    content_templates: None
     date_created: '...'
     display_name: 'foobadoo'
     driver_link: None
diff --git a/lib/lp/registry/stories/webservice/xx-source-package.rst b/lib/lp/registry/stories/webservice/xx-source-package.rst
index d09785d..813e1b8 100644
--- a/lib/lp/registry/stories/webservice/xx-source-package.rst
+++ b/lib/lp/registry/stories/webservice/xx-source-package.rst
@@ -32,6 +32,7 @@ distribution series.
     >>> pprint_entry(evolution)
     bug_reported_acknowledgement: None
     bug_reporting_guidelines: None
+    content_templates: None
     displayname: 'evolution in My-distro My-series'
     distribution_link: 'http://.../my-distro'
     distroseries_link: 'http://.../my-distro/my-series'

Follow ups