launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #33176
[Merge] ~enriqueesanchz/launchpad:add-channel-url into launchpad:master
Enrique Sánchez has proposed merging ~enriqueesanchz/launchpad:add-channel-url into launchpad:master.
Commit message:
Add channel to external package bugtask url
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~enriqueesanchz/launchpad/+git/launchpad/+merge/494829
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~enriqueesanchz/launchpad:add-channel-url into launchpad:master.
diff --git a/lib/lp/bugs/browser/bugtask.py b/lib/lp/bugs/browser/bugtask.py
index 270167d..9c65b9d 100644
--- a/lib/lp/bugs/browser/bugtask.py
+++ b/lib/lp/bugs/browser/bugtask.py
@@ -127,10 +127,7 @@ from lp.registry.interfaces.distributionsourcepackage import (
IDistributionSourcePackage,
)
from lp.registry.interfaces.distroseries import IDistroSeries, IDistroSeriesSet
-from lp.registry.interfaces.externalpackage import (
- IExternalPackage,
- IExternalURL,
-)
+from lp.registry.interfaces.externalpackage import IExternalPackage
from lp.registry.interfaces.externalpackageseries import IExternalPackageSeries
from lp.registry.interfaces.ociproject import IOCIProject
from lp.registry.interfaces.person import IPersonSet
@@ -307,9 +304,6 @@ class BugTaskURL:
@property
def path(self):
- if IExternalURL.providedBy(self.context.target):
- return f"+bug/{self.context.bug.id}/+bugtask/{self.context.id}"
-
return "+bug/%s" % self.context.bug.id
@@ -347,17 +341,9 @@ class BugTargetTraversalMixin:
# rather than making it look as though this task was "not found",
# because it was filtered out by privacy-aware code.
- # Check if it uses +external url
- is_external = IExternalURL.providedBy(context)
-
for bugtask in bug.bugtasks:
target = bugtask.target
-
- if is_external:
- if context.isMatching(target):
- # Security proxy the object on the way out
- return getUtility(IBugTaskSet).get(bugtask.id)
- elif target == context:
+ if target == context:
return getUtility(IBugTaskSet).get(bugtask.id)
# If we've come this far, there's no task for the requested context.
@@ -383,23 +369,6 @@ class BugTaskNavigation(Navigation):
usedfor = IBugTask
- @stepthrough("+bugtask")
- def traverse_bugtask(self, id):
- bugtask = getUtility(IBugTaskSet).get(int(id))
- # Jumping to a not matching bugtask is not allowed
- if bugtask.bug.id != self.context.bug.id:
- return
- if bugtask.sourcepackagename != self.context.sourcepackagename:
- return
- if bugtask.distribution != self.context.distribution:
- return
- if bugtask.distroseries != self.context.distroseries:
- return
- if bugtask.packagetype != self.context.packagetype:
- return
-
- return bugtask
-
@stepthrough("attachments")
def traverse_attachments(self, name):
"""traverse to an attachment by id."""
diff --git a/lib/lp/bugs/browser/tests/test_bugtask_navigation.py b/lib/lp/bugs/browser/tests/test_bugtask_navigation.py
index 8cf2537..a208eeb 100644
--- a/lib/lp/bugs/browser/tests/test_bugtask_navigation.py
+++ b/lib/lp/bugs/browser/tests/test_bugtask_navigation.py
@@ -73,7 +73,6 @@ class TestBugTaskTraversal(TestCaseWithFactory):
)
def test_traversal_to_external_bugtask(self):
- # Test that traversal using +bugtask/id works
# Test that we can differ between bugtasks with same packagename and
# distribution/distroseries, but different packagetype or channel
bug = self.factory.makeBug()
@@ -91,31 +90,31 @@ class TestBugTaskTraversal(TestCaseWithFactory):
distribution=distribution,
sourcepackagename=spn,
packagetype=ExternalPackageType.SNAP,
- channel=("11", "stable"),
+ channel=("11", "stable", None),
),
self.factory.makeExternalPackage(
distribution=distribution,
sourcepackagename=spn,
packagetype=ExternalPackageType.SNAP,
- channel=("11", "edge"),
+ channel=("11", "edge", "branch1"),
),
self.factory.makeExternalPackage(
distribution=distribution,
sourcepackagename=spn,
packagetype=ExternalPackageType.CHARM,
- channel=("11", "stable"),
+ channel=("11", "stable", None),
),
self.factory.makeExternalPackageSeries(
distroseries=distroseries_1,
sourcepackagename=spn,
packagetype=ExternalPackageType.CHARM,
- channel=("11", "stable"),
+ channel=("11", "stable", None),
),
self.factory.makeExternalPackageSeries(
distroseries=distroseries_2,
sourcepackagename=spn,
packagetype=ExternalPackageType.CHARM,
- channel=("11", "stable"),
+ channel=("11", "stable", None),
),
)
@@ -132,36 +131,40 @@ class TestBugTaskTraversal(TestCaseWithFactory):
# Check externalpackage urls
for bugtask in bugtasks[:3]:
- self.assertEqual(
- canonical_url(bugtask),
- "http://bugs.launchpad.test/%s/+%s/%s/+bug/%d/";
- "+bugtask/%s"
- % (
- bugtask.distribution.name,
- bugtask.target.packagetype.name.lower(),
- bugtask.target.name,
- bugtask.bug.id,
- bugtask.id,
- ),
+ target = bugtask.target
+ track, risk, branch = target.channel
+ # Helper to build the channel URL part
+ channel_url = f"/+track/{track}/+risk/{risk}"
+ if branch is not None:
+ channel_url += f"/+branch/{branch}"
+
+ expected_url = (
+ f"http://bugs.launchpad.test/{target.distribution.name}";
+ f"/+{target.packagetype.name.lower()}/{target.name}"
+ f"{channel_url}/+bug/{bugtask.bug.id}"
)
+ self.assertEqual(expected_url, canonical_url(bugtask))
+
obj, _, _ = test_traverse(canonical_url(bugtask))
self.assertEqual(bugtask, obj)
# Check externalpackageseries urls
for bugtask in bugtasks[3:]:
- self.assertEqual(
- canonical_url(bugtask),
- "http://bugs.launchpad.test/%s/%s/+%s/%s/+bug/%d/";
- "+bugtask/%s"
- % (
- bugtask.target.distribution.name,
- bugtask.distroseries.name,
- bugtask.target.packagetype.name.lower(),
- bugtask.target.name,
- bugtask.bug.id,
- bugtask.id,
- ),
+ target = bugtask.target
+ track, risk, branch = target.channel
+ # Helper to build the channel URL part
+ channel_url = f"/+track/{track}/+risk/{risk}"
+ if branch is not None:
+ channel_url += f"/+branch/{branch}"
+
+ expected_url = (
+ f"http://bugs.launchpad.test/{target.distribution.name}";
+ f"/{target.distroseries.name}"
+ f"/+{target.packagetype.name.lower()}/{target.name}"
+ f"{channel_url}/+bug/{bugtask.bug.id}"
)
+ self.assertEqual(expected_url, canonical_url(bugtask))
+
obj, _, _ = test_traverse(canonical_url(bugtask))
self.assertEqual(bugtask, obj)
@@ -178,16 +181,20 @@ class TestBugTaskTraversal(TestCaseWithFactory):
naked_view.target,
canonical_url(bug.default_bugtask, rootsite="bugs"),
)
+
+ bugtask = bug.default_bugtask
+ distro_name = bugtask.distribution.name
+ packagetype_name = bugtask.target.packagetype.name.lower()
+ target_name = bugtask.target.name
+ bug_id = bugtask.bug.id
+ track = bugtask.channel[0]
+ risk = bugtask.channel[1]
+ # branch is None so we don't add it
+
self.assertEqual(
removeSecurityProxy(view).target,
- "http://bugs.launchpad.test/%s/+%s/%s/+bug/%d/+bugtask/%s";
- % (
- bug.default_bugtask.distribution.name,
- bug.default_bugtask.target.packagetype.name.lower(),
- bug.default_bugtask.target.name,
- bug.default_bugtask.bug.id,
- bug.default_bugtask.id,
- ),
+ f"http://bugs.launchpad.test/{distro_name}/+{packagetype_name}/";
+ f"{target_name}/+track/{track}/+risk/{risk}/+bug/{bug_id}",
)
def test_traversal_to_default_external_package_series_bugtask(self):
@@ -214,17 +221,20 @@ class TestBugTaskTraversal(TestCaseWithFactory):
naked_view.target,
canonical_url(bug.default_bugtask, rootsite="bugs"),
)
+ distro_name = bug.default_bugtask.target.distribution.name
+ distroseries_name = bug.default_bugtask.distroseries.name
+ packagetype_name = bug.default_bugtask.target.packagetype.name.lower()
+ target_name = bug.default_bugtask.target.name
+ bug_id = bug.default_bugtask.bug.id
+ track = bug.default_bugtask.channel[0]
+ risk = bug.default_bugtask.channel[1]
+ # branch is None so we don't add it
+
self.assertEqual(
removeSecurityProxy(view).target,
- "http://bugs.launchpad.test/%s/%s/+%s/%s/+bug/%d/+bugtask/%s";
- % (
- bug.default_bugtask.target.distribution.name,
- bug.default_bugtask.distroseries.name,
- bug.default_bugtask.target.packagetype.name.lower(),
- bug.default_bugtask.target.name,
- bug.default_bugtask.bug.id,
- bug.default_bugtask.id,
- ),
+ f"http://bugs.launchpad.test/{distro_name}/{distroseries_name}/";
+ f"+{packagetype_name}/{target_name}/+track/{track}/+risk/{risk}/"
+ f"+bug/{bug_id}",
)
def test_traversal_to_default_external_package_bugtask_on_api(self):
@@ -238,16 +248,19 @@ class TestBugTaskTraversal(TestCaseWithFactory):
bug.default_bugtask.bug.id,
)
)
+ bugtask = bug.default_bugtask
+ distro_name = bugtask.distribution.name
+ packagetype_name = bugtask.target.packagetype.name.lower()
+ target_name = bugtask.target.name
+ bug_id = bugtask.bug.id
+ track = bugtask.channel[0]
+ risk = bugtask.channel[1]
+ # branch is None so we don't add it
+
self.assertEqual(
removeSecurityProxy(view).target,
- "http://api.launchpad.test/1.0/%s/+%s/%s/+bug/%d/+bugtask/%s";
- % (
- bug.default_bugtask.distribution.name,
- bug.default_bugtask.target.packagetype.name.lower(),
- bug.default_bugtask.target.name,
- bug.default_bugtask.bug.id,
- bug.default_bugtask.id,
- ),
+ f"http://api.launchpad.test/1.0/{distro_name}/+{packagetype_name}/";
+ f"{target_name}/+track/{track}/+risk/{risk}/+bug/{bug_id}",
)
def test_traversal_to_default_external_package_series_bugtask_on_api(self):
@@ -270,16 +283,18 @@ class TestBugTaskTraversal(TestCaseWithFactory):
bug.default_bugtask.bug.id,
)
)
+ distro_name = bug.default_bugtask.target.distribution.name
+ distroseries_name = bug.default_bugtask.distroseries.name
+ packagetype_name = bug.default_bugtask.target.packagetype.name.lower()
+ target_name = bug.default_bugtask.target.name
+ bug_id = bug.default_bugtask.bug.id
+ track = bug.default_bugtask.channel[0]
+ risk = bug.default_bugtask.channel[1]
+ # branch is None so we don't add it
+
self.assertEqual(
removeSecurityProxy(view).target,
- "http://api.launchpad.test/1.0/%s/%s/+%s/%s/+bug/%d/";
- "+bugtask/%s"
- % (
- bug.default_bugtask.target.distribution.name,
- bug.default_bugtask.distroseries.name,
- bug.default_bugtask.target.packagetype.name.lower(),
- bug.default_bugtask.target.name,
- bug.default_bugtask.bug.id,
- bug.default_bugtask.id,
- ),
+ f"http://api.launchpad.test/1.0/{distro_name}/{distroseries_name}/";
+ f"+{packagetype_name}/{target_name}/+track/{track}/+risk/{risk}/"
+ f"+bug/{bug_id}",
)
diff --git a/lib/lp/bugs/configure.zcml b/lib/lp/bugs/configure.zcml
index ea98516..8ebd8c2 100644
--- a/lib/lp/bugs/configure.zcml
+++ b/lib/lp/bugs/configure.zcml
@@ -209,6 +209,7 @@
product_id
productseries_id
packagetype
+ channel
sourcepackagename_id
task_age
bug_subscribers
diff --git a/lib/lp/bugs/interfaces/bugtasksearch.py b/lib/lp/bugs/interfaces/bugtasksearch.py
index 55e69ae..6a27fec 100644
--- a/lib/lp/bugs/interfaces/bugtasksearch.py
+++ b/lib/lp/bugs/interfaces/bugtasksearch.py
@@ -156,6 +156,7 @@ class BugTaskSearchParams:
assignee=None,
sourcepackagename=None,
packagetype=None,
+ channel=None,
owner=None,
attachmenttype=None,
orderby=None,
@@ -202,6 +203,7 @@ class BugTaskSearchParams:
self.assignee = assignee
self.sourcepackagename = sourcepackagename
self.packagetype = packagetype
+ self.channel = channel
self.owner = owner
self.attachmenttype = attachmenttype
self.user = user
@@ -315,14 +317,14 @@ class BugTaskSearchParams:
def setExternalPackage(self, externalpackage):
"""Set the externalpackage context on which to filter the search."""
self.distribution = externalpackage.distribution
- # Currently we are not filtering by channel
+ self.channel = externalpackage.channel
self.packagetype = externalpackage.packagetype.value
self.sourcepackagename = externalpackage.sourcepackagename
def setExternalPackageSeries(self, externalpackageseries):
"""Set the externalpackage context on which to filter the search."""
self.distroseries = externalpackageseries.distroseries
- # Currently we are not filtering by channel
+ self.channel = externalpackageseries.channel
self.packagetype = externalpackageseries.packagetype.value
self.sourcepackagename = externalpackageseries.sourcepackagename
@@ -349,6 +351,9 @@ class BugTaskSearchParams:
)
from lp.registry.interfaces.distroseries import IDistroSeries
from lp.registry.interfaces.externalpackage import IExternalPackage
+ from lp.registry.interfaces.externalpackageseries import (
+ IExternalPackageSeries,
+ )
from lp.registry.interfaces.milestone import IMilestone
from lp.registry.interfaces.ociproject import IOCIProject
from lp.registry.interfaces.product import IProduct
@@ -379,6 +384,8 @@ class BugTaskSearchParams:
self.setSourcePackage(target)
elif IExternalPackage.providedBy(instance):
self.setExternalPackage(target)
+ elif IExternalPackageSeries.providedBy(instance):
+ self.setExternalPackageSeries(target)
elif IProjectGroup.providedBy(instance):
self.setProjectGroup(target)
elif IOCIProject.providedBy(instance):
diff --git a/lib/lp/bugs/model/bugnomination.py b/lib/lp/bugs/model/bugnomination.py
index 50d1b0a..3e883e6 100644
--- a/lib/lp/bugs/model/bugnomination.py
+++ b/lib/lp/bugs/model/bugnomination.py
@@ -27,6 +27,7 @@ from lp.bugs.interfaces.bugnomination import (
)
from lp.bugs.interfaces.bugtask import IBugTaskSet
from lp.registry.interfaces.distroseries import IDistroSeries
+from lp.registry.interfaces.externalpackageseries import IExternalPackageSeries
from lp.registry.interfaces.person import validate_public_person
from lp.registry.interfaces.productseries import IProductSeries
from lp.registry.interfaces.sourcepackage import ISourcePackage
@@ -240,9 +241,10 @@ class BugNominationSet:
filter_args = dict(productseries_id=target.id)
elif ISourcePackage.providedBy(target):
filter_args = dict(distroseries_id=target.series.id)
+ elif IExternalPackageSeries.providedBy(target):
+ filter_args = dict(distroseries_id=target.series.id)
else:
return None
- # IExternalPackageSeries does not support bug nominations
store = IStore(BugNomination)
return store.find(BugNomination, bug=bug, **filter_args).one()
diff --git a/lib/lp/bugs/model/bugtask.py b/lib/lp/bugs/model/bugtask.py
index c3fb364..a903637 100644
--- a/lib/lp/bugs/model/bugtask.py
+++ b/lib/lp/bugs/model/bugtask.py
@@ -907,6 +907,8 @@ class BugTask(StormBase):
if (
bugtask.distroseries is not None
and bugtask.sourcepackagename == self.sourcepackagename
+ and bugtask.packagetype == self.packagetype
+ and bugtask.channel == self.channel
)
]
# Return early, so that we don't have to get currentseries,
@@ -933,6 +935,7 @@ class BugTask(StormBase):
and conjoined_primary.status in self._NON_CONJOINED_STATUSES
):
conjoined_primary = None
+
return conjoined_primary
def _get_shortlisted_bugtasks(self):
@@ -956,6 +959,8 @@ class BugTask(StormBase):
if (
bugtask.distribution == distribution
and bugtask.sourcepackagename == self.sourcepackagename
+ and bugtask.packagetype == self.packagetype
+ and bugtask.channel == self.channel
):
conjoined_replica = bugtask
break
diff --git a/lib/lp/bugs/model/bugtaskflat.py b/lib/lp/bugs/model/bugtaskflat.py
index ea2304b..a500512 100644
--- a/lib/lp/bugs/model/bugtaskflat.py
+++ b/lib/lp/bugs/model/bugtaskflat.py
@@ -1,6 +1,7 @@
# Copyright 2012-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
+from storm.databases.postgres import JSON
from storm.locals import Bool, DateTime, Int, List, Reference
from lp.app.enums import InformationType
@@ -41,6 +42,7 @@ class BugTaskFlat(StormBase):
sourcepackagename_id = Int(name="sourcepackagename")
sourcepackagename = Reference(sourcepackagename_id, "SourcePackageName.id")
packagetype = Int(name="packagetype")
+ channel = JSON(name="channel")
ociproject_id = Int(name="ociproject")
ociproject = Reference(ociproject_id, "OCIProject.id")
status = DBEnum(enum=(BugTaskStatus, BugTaskStatusSearch))
diff --git a/lib/lp/bugs/model/bugtasksearch.py b/lib/lp/bugs/model/bugtasksearch.py
index 2af1ee7..6e9b98d 100644
--- a/lib/lp/bugs/model/bugtasksearch.py
+++ b/lib/lp/bugs/model/bugtasksearch.py
@@ -349,6 +349,9 @@ def _build_query(params):
if where_cond is not None:
extra_clauses.append(where_cond)
+ if params.channel is not None:
+ extra_clauses.append(BugTaskFlat.channel == params.channel)
+
if params.status is not None:
extra_clauses.append(
_build_status_clause(BugTaskFlat.status, params.status)
diff --git a/lib/lp/bugs/model/tests/test_bugtask.py b/lib/lp/bugs/model/tests/test_bugtask.py
index 54a39e2..61901e6 100644
--- a/lib/lp/bugs/model/tests/test_bugtask.py
+++ b/lib/lp/bugs/model/tests/test_bugtask.py
@@ -57,6 +57,7 @@ from lp.registry.interfaces.distributionsourcepackage import (
IDistributionSourcePackage,
)
from lp.registry.interfaces.distroseries import IDistroSeriesSet
+from lp.registry.interfaces.externalpackage import ExternalPackageType
from lp.registry.interfaces.person import IPersonSet
from lp.registry.interfaces.product import IProductSet
from lp.registry.interfaces.projectgroup import IProjectGroupSet
@@ -2169,6 +2170,99 @@ class TestConjoinedBugTasks(TestCaseWithFactory):
self.assertIsNone(gentoo_netapplet_task.conjoined_primary)
self.assertIsNone(gentoo_netapplet_task.conjoined_replica)
+ def test_conjoined_external_package(self):
+ """Test conjoined primary/replica logic for external packages.
+
+ Conjoined tasks must match on sourcepackagename, packagetype,
+ and channel, all within the same distribution.
+ """
+ login("foo.bar@xxxxxxxxxxxxx")
+ launchbag = getUtility(ILaunchBag)
+ user = launchbag.user
+ task_set = getUtility(IBugTaskSet)
+
+ series_older = self.factory.makeDistroSeries(name="older")
+ distribution = removeSecurityProxy(series_older.distribution)
+ series_newer = self.factory.makeDistroSeries(
+ name="newer", distribution=distribution
+ )
+
+ pkg_name = self.factory.makeSourcePackageName()
+ pkg_type_rock = ExternalPackageType.ROCK
+ pkg_type_snap = ExternalPackageType.SNAP
+ channel_stable = (None, "stable", None)
+ channel_edge = (None, "edge", None)
+
+ # 1. Create the bug with a distribution-level task
+ dist_target = distribution.getExternalPackage(
+ pkg_name, pkg_type_rock, channel_stable
+ )
+ params = CreateBugParams(
+ owner=user, title="conjoined bug", comment="test"
+ )
+ bug = dist_target.createBug(params)
+ dist_task = bug.bugtasks[0]
+
+ # Initially, no conjoined tasks
+ self.assertIsNone(dist_task.conjoined_primary)
+ self.assertIsNone(dist_task.conjoined_replica)
+
+ # 2. Add "primary" series task
+ # Adding a task for the newest series. This will become the primary.
+ primary_target = series_newer.getExternalPackageSeries(
+ pkg_name, pkg_type_rock, channel_stable
+ )
+ primary_task = task_set.createTask(bug, user, primary_target)
+
+ # The new series task becomes primary
+ self.assertIsNone(primary_task.conjoined_primary)
+ self.assertEqual(dist_task, primary_task.conjoined_replica)
+ # The original dist task now points to the new primary
+ self.assertEqual(primary_task, dist_task.conjoined_primary)
+ self.assertIsNone(dist_task.conjoined_replica)
+
+ # 3. Test: Negative case (older series)
+ older_target = series_older.getExternalPackageSeries(
+ pkg_name, pkg_type_rock, channel_stable
+ )
+ older_task = task_set.createTask(bug, user, older_target)
+
+ # A series task conjoined_primary is always None
+ self.assertIsNone(older_task.conjoined_primary)
+ # A series task only gets a conjoined_replica if it's the currentseries
+ self.assertIsNone(older_task.conjoined_replica)
+
+ # 4. Test: Negative case (different channel)
+ # This task has a different channel and should NOT be conjoined.
+ edge_target = series_newer.getExternalPackageSeries(
+ pkg_name, pkg_type_rock, channel_edge
+ )
+ edge_task = task_set.createTask(bug, user, edge_target)
+
+ self.assertIsNone(edge_task.conjoined_primary)
+ self.assertIsNone(edge_task.conjoined_replica)
+
+ # 5. Test: Negative case (different packagetype)
+ # This task has a different package type and should NOT be conjoined.
+ snap_target = series_newer.getExternalPackageSeries(
+ pkg_name, pkg_type_snap, channel_stable
+ )
+ snap_task = task_set.createTask(bug, user, snap_target)
+
+ self.assertIsNone(snap_task.conjoined_primary)
+ self.assertIsNone(snap_task.conjoined_replica)
+
+ # 6. Test: Negative case (different distribution)
+ # This task is on a different distribution and should NOT be conjoined.
+ other_distro = self.factory.makeDistribution(name="other")
+ other_target = other_distro.getExternalPackage(
+ pkg_name, pkg_type_rock, channel_stable
+ )
+ other_task = task_set.createTask(bug, user, other_target)
+
+ self.assertIsNone(other_task.conjoined_primary)
+ self.assertIsNone(other_task.conjoined_replica)
+
def test_conjoined_broken_relationship(self):
"""A conjoined relationship can be broken, though.
diff --git a/lib/lp/bugs/model/tests/test_bugtasksearch.py b/lib/lp/bugs/model/tests/test_bugtasksearch.py
index 3dc70a7..75bd9aa 100644
--- a/lib/lp/bugs/model/tests/test_bugtasksearch.py
+++ b/lib/lp/bugs/model/tests/test_bugtasksearch.py
@@ -1667,6 +1667,63 @@ class UpstreamFilterTests:
self.assertSearchFinds(params, self.bugtasks[:1])
+class ExternalPackageSeriesTarget(BugTargetTestBase):
+ """Use an ExternalPackageSeries as the bug target."""
+
+ def setUp(self):
+ super().setUp()
+ # ExternalPackageSeries does not support aggregate search
+ # It requires BugSummary to be implemented
+ self.group_on = None
+ self.searchtarget = removeSecurityProxy(
+ self.factory.makeExternalPackageSeries()
+ )
+ self.owner = self.searchtarget.distroseries.owner
+ self.makeBugTasks()
+
+ def setUpTarget2(self):
+ self.searchtarget2 = removeSecurityProxy(
+ self.factory.makeExternalPackageSeries(
+ distroseries=self.searchtarget.distroseries
+ )
+ )
+ self.bugtasks2 = []
+ self.makeBugTasks(
+ bugtarget=self.searchtarget2,
+ bugtasks=self.bugtasks2,
+ owner=self.searchtarget2.distroseries.owner,
+ )
+ self.makeBugTasks(
+ bugtarget=self.searchtarget2,
+ bugtasks=self.bugtasks2,
+ owner=self.searchtarget2.distroseries.owner,
+ )
+
+ def setBugParamsTarget(self, params, target):
+ params.setExternalPackageSeries(target)
+
+ def subscribeToTarget(self, subscriber):
+ # Subscribe the given person to the search target.
+ # ExternalPackageSeries do not support structural subscriptions,
+ # so we subscribe to the distro series instead.
+ with person_logged_in(subscriber):
+ self.searchtarget.distroseries.addSubscription(
+ subscriber, subscribed_by=subscriber
+ )
+
+ def setUpMilestoneSorting(self):
+ with person_logged_in(self.owner):
+ milestone_1 = self.factory.makeMilestone(
+ distribution=self.searchtarget.distribution, name="1.0"
+ )
+ milestone_2 = self.factory.makeMilestone(
+ distribution=self.searchtarget.distribution, name="2.0"
+ )
+ self.bugtasks[1].transitionToMilestone(milestone_1, self.owner)
+ self.bugtasks[2].transitionToMilestone(milestone_2, self.owner)
+ return self.bugtasks[1:] + self.bugtasks[:1]
+
+
class SourcePackageTarget(BugTargetTestBase, UpstreamFilterTests):
"""Use a source package as the bug target."""
@@ -1721,6 +1778,71 @@ class SourcePackageTarget(BugTargetTestBase, UpstreamFilterTests):
return self.bugtasks[1:] + self.bugtasks[:1]
+class ExternalPackageTarget(
+ BugTargetTestBase,
+ BugTargetWithBugSuperVisor,
+):
+ """Use an ExternalPackage as the bug target."""
+
+ def setUp(self):
+ super().setUp()
+ # ExternalPackage does not support aggregate search
+ # It requires BugSummary to be implemented
+ self.group_on = None
+ self.searchtarget = removeSecurityProxy(
+ self.factory.makeExternalPackage()
+ )
+ self.owner = self.searchtarget.distribution.owner
+ self.makeBugTasks()
+
+ def setUpTarget2(self):
+ self.searchtarget2 = removeSecurityProxy(
+ self.factory.makeExternalPackage(
+ distribution=self.searchtarget.distribution
+ )
+ )
+ self.bugtasks2 = []
+ self.makeBugTasks(
+ bugtarget=self.searchtarget2,
+ bugtasks=self.bugtasks2,
+ owner=self.searchtarget2.distribution.owner,
+ )
+ self.makeBugTasks(
+ bugtarget=self.searchtarget2,
+ bugtasks=self.bugtasks2,
+ owner=self.searchtarget2.distribution.owner,
+ )
+
+ def setBugParamsTarget(self, params, target):
+ params.setExternalPackage(target)
+
+ def subscribeToTarget(self, subscriber):
+ # Subscribe the given person to the search target.
+ # ExternalPackage do not support structural subscriptions,
+ # so we subscribe to the distro instead.
+ with person_logged_in(subscriber):
+ self.searchtarget.distribution.addSubscription(
+ subscriber, subscribed_by=subscriber
+ )
+
+ def setSupervisor(self, supervisor):
+ """Set the bug supervisor for the bug task target."""
+ with person_logged_in(self.owner):
+ self.searchtarget.distribution.bug_supervisor = supervisor
+
+ def setUpMilestoneSorting(self):
+ with person_logged_in(self.owner):
+ milestone_1 = self.factory.makeMilestone(
+ distribution=self.searchtarget.distribution, name="1.0"
+ )
+ milestone_2 = self.factory.makeMilestone(
+ distribution=self.searchtarget.distribution, name="2.0"
+ )
+ self.bugtasks[1].transitionToMilestone(milestone_1, self.owner)
+ self.bugtasks[2].transitionToMilestone(milestone_2, self.owner)
+ return self.bugtasks[1:] + self.bugtasks[:1]
+
+
class DistributionSourcePackageTarget(
BugTargetTestBase, BugTargetWithBugSuperVisor, UpstreamFilterTests
):
@@ -1782,6 +1904,8 @@ bug_targets_mixins = (
ProductTarget,
ProjectGroupTarget,
SourcePackageTarget,
+ ExternalPackageTarget,
+ ExternalPackageSeriesTarget,
)
diff --git a/lib/lp/bugs/tests/test_bugnomination.py b/lib/lp/bugs/tests/test_bugnomination.py
index 19c0a15..8174a30 100644
--- a/lib/lp/bugs/tests/test_bugnomination.py
+++ b/lib/lp/bugs/tests/test_bugnomination.py
@@ -468,3 +468,22 @@ class BugNominationSetTestCase(TestCaseWithFactory):
def test_get_none(self):
bug_nomination_set = getUtility(IBugNominationSet)
self.assertRaises(NotFoundError, bug_nomination_set.get, -1)
+
+ def test_getByBugTarget(self):
+ pkg1 = self.factory.makeExternalPackageSeries()
+ pkg2 = self.factory.makeExternalPackageSeries()
+
+ with person_logged_in(pkg1.distroseries.owner):
+ nomination = self.factory.makeBugNomination(target=pkg1)
+ with person_logged_in(pkg2.distroseries.owner):
+ self.factory.makeBugNomination(target=pkg2)
+
+ bug_nomination_set = getUtility(IBugNominationSet)
+ self.assertEqual(
+ nomination,
+ bug_nomination_set.getByBugTarget(nomination.bug, pkg1),
+ )
+ self.assertNotEqual(
+ nomination,
+ bug_nomination_set.getByBugTarget(nomination.bug, pkg2),
+ )
diff --git a/lib/lp/registry/browser/externalpackage.py b/lib/lp/registry/browser/externalpackage.py
index 6ec7657..1b398d8 100644
--- a/lib/lp/registry/browser/externalpackage.py
+++ b/lib/lp/registry/browser/externalpackage.py
@@ -6,10 +6,12 @@ __all__ = [
"ExternalPackageNavigation",
"ExternalPackageFacets",
"ExternalPackageURL",
+ "ExternalPackageNavigationMixin",
]
from zope.interface import implementer
+from zope.publisher.interfaces import NotFound
from lp.app.interfaces.headings import IHeadingBreadcrumb
from lp.bugs.browser.bugtask import BugTargetTraversalMixin
@@ -17,7 +19,12 @@ from lp.bugs.browser.structuralsubscription import (
StructuralSubscriptionTargetTraversalMixin,
)
from lp.registry.interfaces.externalpackage import IExternalPackage
-from lp.services.webapp import Navigation, StandardLaunchpadFacets, redirection
+from lp.services.webapp import (
+ Navigation,
+ StandardLaunchpadFacets,
+ redirection,
+ stepthrough,
+)
from lp.services.webapp.breadcrumb import Breadcrumb
from lp.services.webapp.interfaces import (
ICanonicalUrlData,
@@ -46,10 +53,56 @@ class ExternalPackageFacets(StandardLaunchpadFacets):
]
+class ExternalPackageNavigationMixin:
+ """Provides traversal for +track, +risk, and +branch."""
+
+ def _get_package_for_channel(self, channel):
+ """
+ Get the external package or package series for a given channel.
+
+ Subclasses must implement this.
+ """
+ raise NotImplementedError
+
+ @stepthrough("+track")
+ def traverse_track(self, track):
+ stack = self.request.getTraversalStack()
+ if len(stack) >= 2 and stack[-1] == "+risk":
+ risk = stack[-2]
+ else:
+ # If no risk provided, default to stable
+ risk = "stable"
+
+ channel = (track, risk)
+ return self._get_package_for_channel(channel)
+
+ @stepthrough("+risk")
+ def traverse_risk(self, risk):
+ if self.context.channel:
+ channel = (self.context.channel[0], risk)
+ else:
+ channel = risk
+
+ return self._get_package_for_channel(channel)
+
+ @stepthrough("+branch")
+ def traverse_branch(self, branch):
+ if self.context.channel:
+ track, risk = self.context.channel[0], self.context.channel[1]
+ else:
+ # If no track/risk provided, default to None/stable
+ track = None
+ risk = "stable"
+
+ channel = (track, risk, branch)
+ return self._get_package_for_channel(channel)
+
+
class ExternalPackageNavigation(
Navigation,
BugTargetTraversalMixin,
StructuralSubscriptionTargetTraversalMixin,
+ ExternalPackageNavigationMixin,
):
usedfor = IExternalPackage
@@ -57,6 +110,14 @@ class ExternalPackageNavigation(
def redirect_editbugcontact(self):
return "+subscribe"
+ def _get_package_for_channel(self, channel):
+ try:
+ return self.context.distribution.getExternalPackage(
+ self.context.name, self.context.packagetype, channel
+ )
+ except ValueError:
+ raise NotFound(self.context, channel)
+
@implementer(ICanonicalUrlData)
class ExternalPackageURL:
@@ -74,4 +135,15 @@ class ExternalPackageURL:
@property
def path(self):
packagetype = self.context.packagetype.name.lower()
- return f"+{packagetype}/{self.context.name}"
+
+ track, risk, branch = self.context.channel or (None, None, None)
+
+ channel_url = ""
+ if track:
+ channel_url = f"/+track/{track}"
+ if risk:
+ channel_url = channel_url + f"/+risk/{risk}"
+ if branch:
+ channel_url = channel_url + f"/+branch/{branch}"
+
+ return f"+{packagetype}/{self.context.name}{channel_url}"
diff --git a/lib/lp/registry/browser/externalpackageseries.py b/lib/lp/registry/browser/externalpackageseries.py
index 4221ef8..a9a54ae 100644
--- a/lib/lp/registry/browser/externalpackageseries.py
+++ b/lib/lp/registry/browser/externalpackageseries.py
@@ -10,12 +10,14 @@ __all__ = [
from zope.interface import implementer
+from zope.publisher.interfaces import NotFound
from lp.app.interfaces.headings import IHeadingBreadcrumb
from lp.bugs.browser.bugtask import BugTargetTraversalMixin
from lp.bugs.browser.structuralsubscription import (
StructuralSubscriptionTargetTraversalMixin,
)
+from lp.registry.browser.externalpackage import ExternalPackageNavigationMixin
from lp.registry.interfaces.externalpackageseries import IExternalPackageSeries
from lp.services.webapp import (
Navigation,
@@ -57,6 +59,7 @@ class ExternalPackageSeriesNavigation(
Navigation,
BugTargetTraversalMixin,
StructuralSubscriptionTargetTraversalMixin,
+ ExternalPackageNavigationMixin,
):
usedfor = IExternalPackageSeries
@@ -74,6 +77,14 @@ class ExternalPackageSeriesNavigation(
redirection_url += "?no-redirect"
return self.redirectSubTree(redirection_url, status=303)
+ def _get_package_for_channel(self, channel):
+ try:
+ return self.context.distroseries.getExternalPackageSeries(
+ self.context.name, self.context.packagetype, channel
+ )
+ except ValueError:
+ raise NotFound(self.context, channel)
+
@implementer(ICanonicalUrlData)
class ExternalPackageSeriesURL:
@@ -91,4 +102,15 @@ class ExternalPackageSeriesURL:
@property
def path(self):
packagetype = self.context.packagetype.name.lower()
- return f"+{packagetype}/{self.context.name}"
+
+ track, risk, branch = self.context.channel or (None, None, None)
+
+ channel_url = ""
+ if track:
+ channel_url = f"/+track/{track}"
+ if risk:
+ channel_url = channel_url + f"/+risk/{risk}"
+ if branch:
+ channel_url = channel_url + f"/+branch/{branch}"
+
+ return f"+{packagetype}/{self.context.name}{channel_url}"
diff --git a/lib/lp/registry/interfaces/externalpackage.py b/lib/lp/registry/interfaces/externalpackage.py
index d0116f7..80ec0a6 100644
--- a/lib/lp/registry/interfaces/externalpackage.py
+++ b/lib/lp/registry/interfaces/externalpackage.py
@@ -4,7 +4,6 @@
"""External package interfaces."""
__all__ = [
- "IExternalURL",
"IExternalPackage",
"ExternalPackageType",
]
@@ -12,7 +11,7 @@ __all__ = [
from lazr.enum import DBEnumeratedType, DBItem
from lazr.restful.declarations import exported, exported_as_webservice_entry
from lazr.restful.fields import Reference
-from zope.interface import Attribute, Interface
+from zope.interface import Attribute
from zope.schema import TextLine
from lp import _
@@ -22,23 +21,12 @@ from lp.registry.interfaces.distribution import IDistribution
from lp.registry.interfaces.role import IHasDrivers
-class IExternalURL(Interface):
- """Uses +external url"""
-
- def isMatching(other):
- """Returns if it matches the other object.
- +external url lacks necessary data, so we only match the necessary
- attributes.
- """
-
-
@exported_as_webservice_entry(as_of="beta")
class IExternalPackageView(
IHeadingContext,
IBugTarget,
IHasOfficialBugTags,
IHasDrivers,
- IExternalURL,
):
"""`IExternalPackage` attributes that require launchpad.View."""
@@ -66,9 +54,6 @@ class IExternalPackageView(
drivers = Attribute("The drivers for the distribution.")
- def isMatching(other):
- """See `IExternalURL`."""
-
def __eq__(other):
"""IExternalPackage comparison method.
diff --git a/lib/lp/registry/interfaces/externalpackageseries.py b/lib/lp/registry/interfaces/externalpackageseries.py
index 63e8d32..c7d5e9b 100644
--- a/lib/lp/registry/interfaces/externalpackageseries.py
+++ b/lib/lp/registry/interfaces/externalpackageseries.py
@@ -17,7 +17,6 @@ from lp.app.interfaces.launchpad import IHeadingContext
from lp.bugs.interfaces.bugtarget import IBugTarget, IHasOfficialBugTags
from lp.registry.interfaces.distribution import IDistribution
from lp.registry.interfaces.distroseries import IDistroSeries
-from lp.registry.interfaces.externalpackage import IExternalURL
from lp.registry.interfaces.role import IHasDrivers
@@ -27,7 +26,6 @@ class IExternalPackageSeriesView(
IBugTarget,
IHasOfficialBugTags,
IHasDrivers,
- IExternalURL,
):
"""`IExternalPackageSeries` attributes that require launchpad.View."""
@@ -62,9 +60,6 @@ class IExternalPackageSeriesView(
drivers = Attribute("The drivers for the distroseries.")
- def isMatching(other):
- """See `IExternalURL`."""
-
def __eq__(other):
"""IExternalPackageSeries comparison method.
diff --git a/lib/lp/registry/model/externalpackage.py b/lib/lp/registry/model/externalpackage.py
index 8327349..c1efca8 100644
--- a/lib/lp/registry/model/externalpackage.py
+++ b/lib/lp/registry/model/externalpackage.py
@@ -111,15 +111,6 @@ class ExternalPackage(
"""See `IExternalPackage`."""
return self.display_name
- def isMatching(self, other) -> bool:
- """See `IExternalURL`."""
- return (
- IExternalPackage.providedBy(other)
- and self.sourcepackagename.id == other.sourcepackagename.id
- and self.distribution.id == other.distribution.id
- and self.packagetype == other.packagetype
- )
-
def __eq__(self, other: "ExternalPackage") -> str:
"""See `IExternalPackage`."""
return (
diff --git a/lib/lp/registry/model/externalpackageseries.py b/lib/lp/registry/model/externalpackageseries.py
index c6c08c7..1fd2695 100644
--- a/lib/lp/registry/model/externalpackageseries.py
+++ b/lib/lp/registry/model/externalpackageseries.py
@@ -133,15 +133,6 @@ class ExternalPackageSeries(
"""See `IExternalPackageSeries`."""
return self.display_name
- def isMatching(self, other) -> bool:
- """See `IExternalURL`."""
- return (
- IExternalPackageSeries.providedBy(other)
- and self.sourcepackagename.id == other.sourcepackagename.id
- and self.distroseries.id == other.distroseries.id
- and self.packagetype == other.packagetype
- )
-
def __eq__(self, other: "ExternalPackageSeries") -> str:
"""See `IExternalPackageSeries`."""
return (
diff --git a/lib/lp/registry/tests/test_externalpackage.py b/lib/lp/registry/tests/test_externalpackage.py
index ab1f715..1fe375e 100644
--- a/lib/lp/registry/tests/test_externalpackage.py
+++ b/lib/lp/registry/tests/test_externalpackage.py
@@ -139,31 +139,6 @@ class TestExternalPackage(TestCaseWithFactory):
self.externalpackage.display_name,
)
- def test_matches(self):
- """Test if two externalpackages matches in sourcepackagename,
- distribution and packagetype.
- """
- self.assertTrue(
- self.externalpackage.isMatching(self.externalpackage_matching)
- )
-
- self.assertFalse(
- self.externalpackage.isMatching(self.externalpackage_maven)
- )
-
- other_spn = self.factory.makeSourcePackageName()
- other_ep_1 = self.factory.makeExternalPackage(
- sourcepackagename=other_spn,
- distribution=self.distribution,
- )
- self.assertFalse(self.externalpackage.isMatching(other_ep_1))
-
- other_distro = self.factory.makeDistribution()
- other_ep_2 = self.factory.makeExternalPackage(
- sourcepackagename=self.sourcepackagename, distribution=other_distro
- )
- self.assertFalse(self.externalpackage.isMatching(other_ep_2))
-
def test_compare(self):
"""Test __eq__ and __neq__"""
self.assertEqual(self.externalpackage, self.externalpackage_copy)
diff --git a/lib/lp/registry/tests/test_externalpackageseries.py b/lib/lp/registry/tests/test_externalpackageseries.py
index 03539d8..1a8dc62 100644
--- a/lib/lp/registry/tests/test_externalpackageseries.py
+++ b/lib/lp/registry/tests/test_externalpackageseries.py
@@ -178,36 +178,6 @@ class TestExternalPackageSeries(TestCaseWithFactory):
)
self.assertEqual(expected, self.externalpackageseries.bugtarget_parent)
- def test_matches(self):
- """Test if two externalpackageseries matches in sourcepackagename,
- distroseries and packagetype.
- """
- self.assertTrue(
- self.externalpackageseries.isMatching(
- self.externalpackageseries_matching
- )
- )
-
- self.assertFalse(
- self.externalpackageseries.isMatching(
- self.externalpackageseries_maven
- )
- )
-
- other_spn = self.factory.makeSourcePackageName()
- other_eps_1 = self.factory.makeExternalPackageSeries(
- sourcepackagename=other_spn,
- distroseries=self.distroseries,
- )
- self.assertFalse(self.externalpackageseries.isMatching(other_eps_1))
-
- other_distroseries = self.factory.makeDistroSeries()
- other_eps_2 = self.factory.makeExternalPackageSeries(
- sourcepackagename=self.sourcepackagename,
- distroseries=other_distroseries,
- )
- self.assertFalse(self.externalpackageseries.isMatching(other_eps_2))
-
def test_compare(self):
"""Test __eq__ and __neq__"""
self.assertEqual(
diff --git a/lib/lp/services/channels.py b/lib/lp/services/channels.py
index 418cd02..e828190 100644
--- a/lib/lp/services/channels.py
+++ b/lib/lp/services/channels.py
@@ -40,7 +40,7 @@ def channel_string_to_list(channel):
# Only 1, 2, or 3 components are allowed
if len(components) > 3:
- raise ValueError("Invalid channel name: %r" % channel)
+ raise ValueError(f"Invalid channel name: {channel}")
track = None
risk = None
@@ -54,16 +54,16 @@ def channel_string_to_list(channel):
elif _is_risk(components[1]):
track, risk = components
else:
- raise ValueError("No valid risk provided: %r" % channel)
+ raise ValueError(f"No valid risk provided: {channel}")
elif len(components) == 1:
risk = components[0]
# Validate risk and branch names
if not _is_risk(risk):
- raise ValueError("No valid risk provided: %r" % channel)
+ raise ValueError(f"No valid risk provided: {channel}")
if branch and _is_risk(branch):
- raise ValueError("Branch name cannot match a risk name: %r" % channel)
+ raise ValueError(f"Branch name cannot match a risk name: {channel}")
return track, risk, branch
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 95ae12e..78bd52b 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -180,6 +180,7 @@ from lp.registry.interfaces.distroseriesdifferencecomment import (
)
from lp.registry.interfaces.distroseriesparent import IDistroSeriesParentSet
from lp.registry.interfaces.externalpackage import ExternalPackageType
+from lp.registry.interfaces.externalpackageseries import IExternalPackageSeries
from lp.registry.interfaces.gpg import IGPGKeySet
from lp.registry.interfaces.mailinglist import (
IMailingListSet,
@@ -2450,7 +2451,9 @@ class LaunchpadObjectFactory(ObjectFactory):
:param target: The `IProductSeries`, `IDistroSeries` or
`ISourcePackage` to nominate for.
"""
- if ISourcePackage.providedBy(target):
+ if ISourcePackage.providedBy(
+ target
+ ) or IExternalPackageSeries.providedBy(target):
non_series = target.distribution_sourcepackage
series = target.distroseries
else:
@@ -5654,8 +5657,10 @@ class LaunchpadObjectFactory(ObjectFactory):
if channel is None:
channel = ("12.1", "stable", None)
- return distribution.getExternalPackage(
- sourcepackagename, packagetype, channel
+ return ProxyFactory(
+ distribution.getExternalPackage(
+ sourcepackagename, packagetype, channel
+ )
)
def makeExternalPackageSeries(
@@ -5676,8 +5681,10 @@ class LaunchpadObjectFactory(ObjectFactory):
if channel is None:
channel = ("12.1", "stable", None)
- return distroseries.getExternalPackageSeries(
- sourcepackagename, packagetype, channel
+ return ProxyFactory(
+ distroseries.getExternalPackageSeries(
+ sourcepackagename, packagetype, channel
+ )
)
def makeEmailMessage(
@@ -5968,7 +5975,7 @@ class LaunchpadObjectFactory(ObjectFactory):
"""Create a new CVE record."""
if sequence is None:
sequence = "2000-%04i" % self.getUniqueInteger()
-
+
if description is None:
description = self.getUniqueUnicode()
Follow ups
