diff --git a/docs/trailhead/tools.md b/docs/trailhead/tools.md index 493c971e..9a03d0dd 100644 --- a/docs/trailhead/tools.md +++ b/docs/trailhead/tools.md @@ -432,8 +432,8 @@ confirm nothing went stale. ## repair_instance_page_slugs -Realigns an instance's **root page slug** with its identifier. Run it after renaming an -instance — otherwise every subpage of it 404s. +Realigns an instance's **root page slug** with its identifier, and carries the page +revisions along with it. Run it after renaming an instance. ```bash python manage.py repair_instance_page_slugs longmont longmont-2021 # plan @@ -443,30 +443,35 @@ python manage.py repair_instance_page_slugs --all # au Nothing is written without `--apply`. -### Why a rename breaks the subpages +### Why a rename used to break the subpages -Page routing keys off the *identifier*, not off `InstanceConfig.root_page`. -`PathsPage.resolve_url_path` strips `^/(-[0-9]+)?/` from the page's -`url_path`, and `Query.resolve_page` then prepends the translated root page's `url_path` -to the path it is handed. They agree only while the root slug equals the identifier — and -nothing keeps them in step, because the slug is set once at page creation -(`_create_default_pages`) and never revisited. +Page routing used to key off the *identifier*. `PathsPage.resolve_url_path` stripped +`^/(-[0-9]+)?/` from the page's `url_path`, and `Query.resolve_page` prepended +the translated root page's `url_path` to the path it was handed. They agreed only while the +root slug equalled the identifier — and nothing kept them in step, because the slug was set +once at page creation (`_create_default_pages`) and never revisited. -The failure is asymmetric, which is what makes it confusing: **the front page keeps -working** because its path is empty and survives the mismatch, while every child 404s. -With identifier `longmont` against slug `longmont-dev`, the strip does not match, so -`resolve_url_path` returns `/longmont-dev/actions` and `resolve_page` looks up +The failure was asymmetric, which is what made it confusing: **the front page kept +working** because its path is empty and survives the mismatch, while every child 404ed. +With identifier `longmont` against slug `longmont-dev`, the strip did not match, so +`resolve_url_path` returned `/longmont-dev/actions` and `resolve_page` looked up `/longmont-dev/longmont-dev/actions/`. -It has bitten twice: `augsburg-bisko` in July 2026 (an admin slug edit) and both Longmont +It bit twice: `augsburg-bisko` in July 2026 (an admin slug edit) and both Longmont instances in September 2026 (a rename). +Both resolvers now derive the prefix from the root page's own `url_path` (`pages/url_paths.py`), +so a mismatched slug no longer breaks routing. This command is now hygiene rather than a +fix — with one exception, the revision pass below, which prevents a *different* regression. + ### What it does - Sets the primary root page's slug to the identifier, and each **translated** root page's - to `-`. Translations matter: that `(-[0-9]+)?` in the regex is what makes - the Spanish site work, and it fails by the same rule. Translations resolve by - `translation_key` and locale, never by slug, so renaming them moves no references. + to `-`. The suffix is no longer a routing requirement, but Wagtail still + needs it: `Page._slug_is_available` compares against all siblings regardless of locale, + and translated roots are siblings under the Wagtail root, so they cannot share the primary + page's slug. Translations resolve by `translation_key` and locale, never by slug, so + renaming them moves no references. - Moves every page through a **temporary slug first**. Root pages are siblings and Wagtail requires the slug to be unique among siblings, so a rename that swaps two identifiers cannot be applied in one pass — the retired instance still holds the slug the new one diff --git a/src/nodes/management/commands/repair_instance_page_slugs.py b/src/nodes/management/commands/repair_instance_page_slugs.py index 32894e56..e066adae 100644 --- a/src/nodes/management/commands/repair_instance_page_slugs.py +++ b/src/nodes/management/commands/repair_instance_page_slugs.py @@ -7,31 +7,31 @@ Nothing is written without ``--apply``. -## Why this is needed at all - -Page routing keys off the *identifier*, not off ``InstanceConfig.root_page``. -``PathsPage.resolve_url_path`` (``pages/page_interface.py``) strips -``^/(-[0-9]+)?/`` from the page's ``url_path``, and ``Query.resolve_page`` -(``pages/schema.py``) then prepends the translated root page's ``url_path`` to the path it -is given. The two only agree while the root page's slug equals the identifier. - -Nothing keeps them in step. The slug is set from the identifier once, when the pages are -created (``InstanceConfig._create_default_pages``), and never revisited -- so **renaming an -instance silently breaks every subpage of it**, and so does editing the root page's slug in -the Wagtail admin. The failure is a 404 on the children while the front page keeps working, -because the front page's path is empty and survives the mismatch: with identifier -``longmont`` and slug ``longmont-dev``, the strip does not match, ``resolve_url_path`` -returns ``/longmont-dev/actions``, and ``resolve_page`` looks up -``/longmont-dev/longmont-dev/actions/``, which exists nowhere. - -This has now happened twice -- ``augsburg-bisko`` in July 2026 via an admin slug edit, and -both Longmont instances in September 2026 via a rename -- which is why it is a command and -not a third one-off script. +## What this was for, and what it is for now + +Page routing used to key off the *identifier*. ``PathsPage.resolve_url_path`` stripped +``^/(-[0-9]+)?/`` from a page's ``url_path`` while ``Query.resolve_page`` +prepended the translated root page's ``url_path``, so the two agreed only while the root +page's slug equalled the identifier -- which nothing maintained. A rename, or a slug edited +in the Wagtail admin, silently 404ed every subpage while the front page kept working, twice: +``augsburg-bisko`` in July 2026 and both Longmont instances in September 2026. + +Both resolvers now take the prefix from the root page's own ``url_path`` (see +``pages/url_paths.py``), so **a mismatched slug no longer breaks routing**. What is left is +hygiene, and it is still worth having: the slug is what the Wagtail admin shows, it is what +``_create_default_pages`` falls back to when ``root_page`` was never populated, and a +drifted slug is a reliable sign that an instance was renamed without anything else being +looked at. Run it with no ``--apply`` as an audit for that. + +The revision pass is the part that is not cosmetic -- see below. ## Translations are part of the fix -A translated root page carries a numeric suffix (``longmont-1``), which is what the -``(-[0-9]+)?`` in the regex is for, so the Spanish site breaks and heals by the same rule. +A translated root page carries a numeric suffix (``longmont-1``). Routing no longer needs +that suffix, but Wagtail still does: ``Page._slug_is_available`` compares against all +siblings regardless of locale, and translated roots are siblings under the Wagtail root, so +they cannot share the primary page's slug. + Translations are resolved by ``translation_key`` and locale (``InstanceConfig.get_translated_root_page``), never by slug, so renaming them moves no references. Every tree the instance serves is repaired the same way: slug, descendant diff --git a/src/nodes/models.py b/src/nodes/models.py index b5b2da0a..9fdfb792 100644 --- a/src/nodes/models.py +++ b/src/nodes/models.py @@ -1685,10 +1685,33 @@ def get_outcome_nodes(self) -> list[NodeConfig]: pks = [node.database_id for node in root_nodes if node.database_id is not None] return list(self.nodes.filter(pk__in=pks)) + def _find_existing_root_page(self, candidates: models.QuerySet[Page]) -> Page | None: + """ + Find this instance's existing root page among the Wagtail root's children. + + Keyed on ``root_page`` first, because that is the authoritative link and the only + one that survives a rename. Looking the page up by ``slug=self.identifier`` alone + used to be the whole of it, which meant that once the slug and the identifier had + drifted apart -- a rename, or a slug edited in the Wagtail admin -- this found + nothing and the caller built a *second* root page beside the real one. That orphan + is what left ``augsburg-bisko`` with a duplicate in July 2026, and what put a stale + page in a position to squat on the slug ``longmont`` needed in September 2026. + + The slug fallback is kept for instances whose ``root_page`` was never populated. + """ + if self.root_page_id is not None: + page = candidates.filter(pk=self.root_page_id).first() + if page is not None: + return page + return candidates.filter(slug=self.identifier).first() + def _create_instance_root_page(self) -> Page: from pages.models import InstanceRootPage root_node: Page = cast('Page', Page.get_first_root_node()) + existing = self._find_existing_root_page(root_node.get_children()) + if existing is not None: + return existing with override(self.primary_language): locale, _ = Locale.objects.get_or_create(language_code=self.primary_language) page = root_node.add_child( @@ -1734,9 +1757,8 @@ def _create_default_pages(self) -> Page: # noqa: C901 root_node: Page = cast('Page', Page.get_first_root_node()) with override(self.primary_language): locale, _ = Locale.objects.get_or_create(language_code=self.primary_language) - try: - home_page = home_pages.get(slug=self.identifier) - except Page.DoesNotExist: + home_page = self._find_existing_root_page(home_pages) + if home_page is None: home_page = root_node.add_child( instance=OutcomePage( locale=locale, diff --git a/src/nodes/tests/test_default_root_page.py b/src/nodes/tests/test_default_root_page.py new file mode 100644 index 00000000..a2eae700 --- /dev/null +++ b/src/nodes/tests/test_default_root_page.py @@ -0,0 +1,94 @@ +""" +Tests for finding an instance's existing root page rather than building a second one. + +The lookup used to be ``home_pages.get(slug=self.identifier)``, so once the slug and the +identifier had drifted apart it found nothing and its caller created a duplicate root page +beside the real one. That orphan is half of what made both page-routing incidents hard to +unpick: ``augsburg-bisko`` ended up with a duplicate in July 2026, and a stale page was left +holding the slug ``longmont`` needed in September 2026. +""" + +from __future__ import annotations + +from wagtail.models import Page + +import pytest + +from nodes.models import InstanceConfig +from nodes.tests.factories import InstanceConfigFactory + +pytestmark = pytest.mark.django_db + + +def _root_page(ic: InstanceConfig, slug: str) -> Page: + from pages.models import InstanceRootPage + + wagtail_root = Page.get_first_root_node() + assert wagtail_root is not None + page = wagtail_root.add_child(instance=InstanceRootPage(title=ic.name, slug=slug, url_path='')) + InstanceConfig.objects.filter(pk=ic.pk).update(root_page=page) + ic.refresh_from_db() + return page + + +def test_the_root_page_is_found_by_fk_when_the_slug_has_drifted() -> None: + """A rename leaves the slug behind but never moves the ``root_page`` link.""" + ic = InstanceConfigFactory.create(identifier='longmont', name='Longmont') + root = _root_page(ic, slug='longmont-dev') + + wagtail_root = Page.get_first_root_node() + assert wagtail_root is not None + found = ic._find_existing_root_page(wagtail_root.get_children()) + + assert found is not None + assert found.pk == root.pk + + +def test_the_slug_fallback_still_works_without_a_root_page_link() -> None: + """Instances predating the FK being populated have only the slug to go on.""" + ic = InstanceConfigFactory.create(identifier='longmont', name='Longmont') + root = _root_page(ic, slug='longmont') + InstanceConfig.objects.filter(pk=ic.pk).update(root_page=None) + ic.refresh_from_db() + + wagtail_root = Page.get_first_root_node() + assert wagtail_root is not None + found = ic._find_existing_root_page(wagtail_root.get_children()) + + assert found is not None + assert found.pk == root.pk + + +def test_a_sibling_instances_root_page_is_not_claimed() -> None: + """ + The lookup must not reach outside the instance it is called on. + + With the slug fallback in play, an instance with no ``root_page`` and no page of its own + must come back empty rather than adopting a neighbour's. + """ + ic = InstanceConfigFactory.create(identifier='longmont', name='Longmont') + other = InstanceConfigFactory.create(identifier='longmont-2021', name='Longmont 2021') + _root_page(other, slug='longmont-2021') + + wagtail_root = Page.get_first_root_node() + assert wagtail_root is not None + assert ic._find_existing_root_page(wagtail_root.get_children()) is None + + +def test_creating_the_instance_root_page_twice_does_not_duplicate_it() -> None: + """ + ``_create_instance_root_page`` is reached on every sync, not only at creation. + + It used to ``add_child`` unconditionally, so a drifted slug meant a fresh orphan on each + call. + """ + ic = InstanceConfigFactory.create(identifier='longmont', name='Longmont') + root = _root_page(ic, slug='longmont-dev') + + returned = ic._create_instance_root_page() + + assert returned.pk == root.pk + wagtail_root = Page.get_first_root_node() + assert wagtail_root is not None + # Scoped to this instance's own page: the Wagtail default "Welcome" page is a sibling. + assert wagtail_root.get_children().filter(title=ic.name).count() == 1 diff --git a/src/pages/page_interface.py b/src/pages/page_interface.py index 46ae541e..9c2f695d 100644 --- a/src/pages/page_interface.py +++ b/src/pages/page_interface.py @@ -1,6 +1,5 @@ from __future__ import annotations -import re from dataclasses import dataclass from typing import TYPE_CHECKING, Self @@ -8,6 +7,8 @@ from grapple.types.interfaces import PageInterface as BasePageInterface, get_page_interface +from pages.url_paths import to_instance_path + if TYPE_CHECKING: from wagtail.models import Page @@ -75,12 +76,20 @@ def resolve_ancestors(root: Page, info: GQLInfo) -> list[PathsPage]: return specific.page.get_visible_ancestors(specific.cache) @staticmethod - def resolve_url_path(root, info: GQLInstanceInfo) -> str: - url_path = root.url_path - # FIXME: This is a dirty way to work around the issue of the slug having the form -1 or so for translated - # pages. - # Replace instance ID, optionally followed by a `-` and a number, if it is surrounded by slashes, by a single slash - url_path = re.sub('^/%s(-[0-9]+)?/' % re.escape(info.context.instance.id), '/', root.url_path) - if len(url_path) > 1: - url_path = url_path.rstrip('/') - return url_path + def resolve_url_path(root: Page, info: GQLInstanceInfo) -> str: + """ + Expose the page's path relative to its instance's root page. + + The prefix comes from the root page's own ``url_path``, which is the same value + ``Query.resolve_page`` prepends when the front end hands the result back, so the + round trip holds whatever the root page's slug happens to be. See + ``pages.url_paths`` for why that matters. + + ``for_page`` locates the instance by treebeard ``path`` prefix rather than by slug, + so it cannot drift either. + """ + cache = info.context.cache.for_page(root) + root_page = cache.translated_root_page if cache is not None else None + if root_page is None: + return root.url_path + return to_instance_path(root.url_path, root_page.url_path) diff --git a/src/pages/schema.py b/src/pages/schema.py index ac1c5222..7fd18e3c 100644 --- a/src/pages/schema.py +++ b/src/pages/schema.py @@ -11,6 +11,7 @@ from nodes.models import InstanceConfig from nodes.schema import NodeType from pages.page_interface import PageInterface +from pages.url_paths import to_url_path from .models import OutcomePage, PathsPage from .perms import PagePermissionPolicy @@ -85,14 +86,13 @@ def resolve_pages( @staticmethod def resolve_page(query, info: GQLInstanceInfo, path: str, **kwargs) -> Page | None: qs = Query.resolve_pages(query, info, **kwargs) - if not path.endswith('/'): - path = path + '/' - # Prepend the url_path of the translated root page instance_config = InstanceConfig.objects.get(identifier=info.context.instance.id) root_page = instance_config.get_translated_root_page() - path = root_page.url_path.rstrip('/') + path + # Inverse of `PathsPage.resolve_url_path`, which handed this path out; both take the + # prefix from the root page's `url_path`. See `pages.url_paths`. + url_path = to_url_path(path, root_page.url_path) for page in qs: - if page.url_path == path: + if page.url_path == url_path: return page return None diff --git a/src/pages/tests/test_page_routing.py b/src/pages/tests/test_page_routing.py new file mode 100644 index 00000000..392fa7d3 --- /dev/null +++ b/src/pages/tests/test_page_routing.py @@ -0,0 +1,98 @@ +""" +End-to-end check that the page API's two directions agree over GraphQL. + +``pages`` hands out ``urlPath`` and ``page(path:)`` takes it back. Those are resolved by +different code in different modules, and the bug they caused twice was precisely that they +disagreed -- so the test that matters is the round trip through the real schema, with a root +page slug that does not match the instance identifier. +""" + +from __future__ import annotations + +from wagtail.models import Page + +import pytest + +from nodes.models import InstanceConfig + +pytestmark = pytest.mark.django_db + +PAGES_QUERY = """ + query { + pages { + title + urlPath + } + } +""" + +PAGE_QUERY = """ + query($path: String!) { + page(path: $path) { + title + urlPath + } + } +""" + + +def _give_root_page(ic: InstanceConfig, slug: str) -> Page: + """Attach a two-level page tree to the instance, rooted on a page with the given slug.""" + from pages.models import InstanceRootPage + + wagtail_root = Page.get_first_root_node() + assert wagtail_root is not None + root = wagtail_root.add_child(instance=InstanceRootPage(title='Emissions', slug=slug, url_path='')) + root.add_child(instance=InstanceRootPage(title='Actions', slug='actions')) + InstanceConfig.objects.filter(pk=ic.pk).update(root_page=root) + ic.refresh_from_db() + return root + + +@pytest.mark.parametrize('slug_suffix', ['', '-dev', '-2021']) +def test_every_page_the_api_exposes_can_be_looked_back_up( + graphql_client_query_data, instance_config: InstanceConfig, slug_suffix: str +) -> None: + """ + The contract the front end relies on, over the real schema. + + Two of these already worked before the prefix was unified: ``''`` because the slug + matched the identifier, and ``'-2021'`` because ``-[0-9]+`` is exactly what the old + regex's optional numeric-suffix branch stripped. They are kept as no-regression cases. + + ``'-dev'`` is the one that broke -- a non-numeric suffix the regex did not match -- and + it 404ed every child while the front page kept resolving. + """ + _give_root_page(instance_config, slug=f'{instance_config.identifier}{slug_suffix}') + + exposed = graphql_client_query_data(PAGES_QUERY)['pages'] + assert {page['title'] for page in exposed} == {'Emissions', 'Actions'} + + for page in exposed: + looked_up = graphql_client_query_data(PAGE_QUERY, variables={'path': page['urlPath']})['page'] + assert looked_up is not None, f'{page["title"]!r} was handed out at {page["urlPath"]!r} but does not resolve' + assert looked_up['title'] == page['title'] + + +def test_the_exposed_paths_are_relative_to_the_instance(graphql_client_query_data, instance_config: InstanceConfig) -> None: + """ + The slug must not leak into what the API hands out. + + This is the observable symptom the front end reported: a nav link to + ``/longmont-dev`` instead of ``/``. + """ + _give_root_page(instance_config, slug=f'{instance_config.identifier}-dev') + + exposed = graphql_client_query_data(PAGES_QUERY)['pages'] + by_title = {page['title']: page['urlPath'] for page in exposed} + assert by_title == {'Emissions': '/', 'Actions': '/actions'} + + +def test_a_path_that_does_not_exist_still_resolves_to_null(graphql_client_query_data, instance_config: InstanceConfig) -> None: + """The fix must not turn a genuine miss into a match.""" + _give_root_page(instance_config, slug=f'{instance_config.identifier}-dev') + + assert graphql_client_query_data(PAGE_QUERY, variables={'path': '/nope'})['page'] is None + # The absolute Wagtail path is not a valid input and must not resolve either. + absolute = graphql_client_query_data(PAGE_QUERY, variables={'path': f'/{instance_config.identifier}-dev/actions'}) + assert absolute['page'] is None diff --git a/src/pages/tests/test_url_paths.py b/src/pages/tests/test_url_paths.py new file mode 100644 index 00000000..3fa7c586 --- /dev/null +++ b/src/pages/tests/test_url_paths.py @@ -0,0 +1,91 @@ +""" +Tests for the ``url_path`` conversions the page API round-trips through. + +The point of these is the round trip, not either direction on its own. The front end takes +a page's ``urlPath`` and hands it straight back to ``page(path:)``, so the two functions +have to remain inverses -- and specifically have to remain inverses when the root page's +slug does *not* match the instance identifier, which is the case that broke twice before +the prefix was taken from one place. +""" + +from __future__ import annotations + +import pytest + +from pages.url_paths import to_instance_path, to_url_path + +# These are pure functions, but conftest's autouse `instance_config` fixture writes to +# the database, so the marker is required regardless. +pytestmark = pytest.mark.django_db + +# Root page paths that all have to behave identically. Only the first matches the shape the +# old regex assumed; the rest are what a rename or an admin slug edit actually leaves. +ROOTS = [ + '/longmont/', + '/longmont-dev/', + '/longmont-1/', + '/longmont-dev-1/', + '/klimabilanz/', + '/longmont-2021/', +] + +RELATIVE_PATHS = ['/', '/actions', '/avoided', '/actions/deep', '/a-b_c'] + + +@pytest.mark.parametrize('root_url_path', ROOTS) +@pytest.mark.parametrize('relative', RELATIVE_PATHS) +def test_round_trip_is_lossless_whatever_the_root_slug(root_url_path: str, relative: str) -> None: + """What the API hands out has to be what the API can look back up.""" + url_path = to_url_path(relative, root_url_path) + assert url_path.startswith(root_url_path) + assert url_path.endswith('/') + assert to_instance_path(url_path, root_url_path) == relative + + +@pytest.mark.parametrize('root_url_path', ROOTS) +def test_the_root_page_itself_is_the_bare_slash(root_url_path: str) -> None: + """ + The front page is the case that hid both incidents. + + Its relative path is ``/`` regardless of the root slug, so it kept resolving while + every child 404ed -- which is why the failure looked like a content problem rather than + a routing one. + """ + assert to_instance_path(root_url_path, root_url_path) == '/' + assert to_url_path('/', root_url_path) == root_url_path + + +@pytest.mark.parametrize( + ('url_path', 'root_url_path', 'expected'), + [ + # A slug that disagrees with the identifier is no longer special. + ('/longmont-dev/actions/', '/longmont-dev/', '/actions'), + # The old regex stripped `-` but not `-dev`, so a translated root + # whose slug had drifted was mangled twice over. + ('/longmont-dev-1/avoided/', '/longmont-dev-1/', '/avoided'), + # A root page whose slug happens to prefix another's must not over-strip. + ('/longmont-2021/actions/', '/longmont-2021/', '/actions'), + ], +) +def test_paths_the_old_regex_got_wrong(url_path: str, root_url_path: str, expected: str) -> None: + assert to_instance_path(url_path, root_url_path) == expected + + +def test_a_sibling_root_is_not_treated_as_a_descendant() -> None: + """ + ``/longmont-2021/`` is not under ``/longmont/`` and must not be read as if it were. + + A prefix test on the bare slug rather than on the full segment would strip ``/longmont`` + off the front of ``/longmont-2021/`` and invent a page. + """ + assert to_instance_path('/longmont-2021/actions/', '/longmont/') == '/longmont-2021/actions' + + +def test_a_path_outside_the_instance_is_left_alone() -> None: + assert to_instance_path('/elsewhere/actions/', '/longmont/') == '/elsewhere/actions' + + +@pytest.mark.parametrize('relative', ['/actions', 'actions', '/actions/']) +def test_inbound_paths_are_normalised(relative: str) -> None: + """The front end is not required to be careful about leading or trailing slashes.""" + assert to_url_path(relative, '/longmont/') == '/longmont/actions/' diff --git a/src/pages/url_paths.py b/src/pages/url_paths.py new file mode 100644 index 00000000..f8f34cbf --- /dev/null +++ b/src/pages/url_paths.py @@ -0,0 +1,62 @@ +""" +Conversion between Wagtail ``url_path`` values and the paths the API speaks. + +Wagtail stores an absolute path per page, rooted at the Wagtail root and built out of +slugs: ``/longmont/actions/``. The API speaks paths relative to the instance's root page: +``/actions``. Every page the API hands out goes through :func:`to_instance_path`, and every +path the API is given comes back through :func:`to_url_path`. + +The two are inverses, and that round trip is the whole contract -- the front end takes a +page's ``urlPath`` and hands it straight back to ``page(path:)``. + +They used to be implemented apart. The outbound side stripped +``^/(-[0-9]+)?/`` by regex while the inbound side prepended the root page's +``url_path``, so the two agreed only while the root page's slug happened to equal the +instance identifier -- something nothing in the codebase maintained. A rename or an admin +slug edit broke the assumption silently, and every subpage of the instance 404ed while the +front page kept working, because the front page's relative path is empty and survives the +mismatch either way. That cost two incidents (``augsburg-bisko`` in July 2026, both +Longmont instances in September 2026). + +Both directions now take the prefix from the same place -- the root page's own +``url_path`` -- so a slug that disagrees with the identifier no longer separates them. Keep +it that way: if one of these functions changes, the other has to change with it, and +``test_url_paths.py`` round-trips them against slugs that do not match their identifier for +exactly that reason. +""" + +from __future__ import annotations + + +def to_instance_path(url_path: str, root_url_path: str) -> str: + """ + Convert a Wagtail ``url_path`` into the instance-relative path the API exposes. + + The instance's own root page becomes ``/``; anything below it keeps the remainder with + no trailing slash. A page that does not sit under ``root_url_path`` is returned + unchanged -- the API only ever exposes pages from the requested instance's tree, so + that case means something upstream is already wrong and is not this function's to + paper over. + """ + prefix = root_url_path.rstrip('/') + if prefix and url_path.startswith(prefix + '/'): + url_path = url_path[len(prefix) :] + if not url_path.startswith('/'): + url_path = '/' + url_path + if len(url_path) > 1: + url_path = url_path.rstrip('/') + return url_path + + +def to_url_path(instance_path: str, root_url_path: str) -> str: + """ + Convert an instance-relative path into the Wagtail ``url_path`` to look up. + + Inverse of :func:`to_instance_path`. Wagtail's ``url_path`` always carries a trailing + slash, so the result does too. + """ + if not instance_path.startswith('/'): + instance_path = '/' + instance_path + if not instance_path.endswith('/'): + instance_path += '/' + return root_url_path.rstrip('/') + instance_path