launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #32016
[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