Migrate navigation table

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## [Unreleased]
+
+### Added
+
+- Support for migrating the navigation table to the contents index
+
 ## [v0.7.0] - 2023-10-09
 
 ### Added

src-docs/migration.py.md

@@ -12,7 +12,7 @@ Module for migrating remote documentation into local git repository.
 
 ---
 
-<a href="../src/migration.py#L160"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/migration.py#L219"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ## <kbd>function</kbd> `make_parent`
 
@@ -37,7 +37,7 @@ Construct path leading to document to be created.
 
 ---
 
-<a href="../src/migration.py#L309"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/migration.py#L371"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ## <kbd>function</kbd> `run`
 

src-docs/navigation_table.py.md

@@ -8,7 +8,7 @@ Module for parsing and rendering a navigation table.
 
 ---
 
-<a href="../src/navigation_table.py#L122"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/navigation_table.py#L123"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ## <kbd>function</kbd> `from_page`
 
@@ -35,7 +35,7 @@ Algorithm:  1.  Extract the table based on a regular expression looking for a 3
 
 ---
 
-<a href="../src/navigation_table.py#L152"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
+<a href="../src/navigation_table.py#L153"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
 
 ## <kbd>function</kbd> `generate_table_row`
 

src/migration.py

@@ -65,6 +65,19 @@ def _validate_table_rows(
         current_group_level = row.level if row.is_group else row.level - 1
 
 
+def _generate_document_path(row: types_.TableRow) -> Path:
+    """Generate the path to a document from the table row.
+
+    Args:
+        row: Row containing link to document and path information.
+
+    Returns:
+        The path to the documentation file.
+
+    """
+    return Path(*row.path[:-1]) / f"{row.path[-1]}.md"
+
+
 def _create_document_meta(row: types_.TableRow) -> types_.DocumentMeta:
     """Create document meta file for migration from table row.
 
@@ -84,7 +97,7 @@ def _create_document_meta(row: types_.TableRow) -> types_.DocumentMeta:
             "Internal error, no implementation for creating document meta with missing link in row."
         )
     return types_.DocumentMeta(
-        path=Path(*row.path[:-1]) / f"{row.path[-1]}.md", link=row.navlink.link, table_row=row
+        path=_generate_document_path(row), link=row.navlink.link, table_row=row
     )
 
 
@@ -145,16 +158,62 @@ def _extract_docs_from_table_rows(
         yield _create_gitkeep_meta(row=previous_row)
 
 
-def _index_file_from_content(content: str) -> types_.IndexDocumentMeta:
+def _table_row_to_contents_index_line(row: types_.TableRow, discourse: Discourse) -> str:
+    """Generate a line of the contents index from a row of the navigation table.
+
+    Args:
+        row: the row of the navigation table.
+        discourse: Client to the documentation server.
+
+    Returns:
+        The contents index line.
+
+    """
+    leader = f"{(len(row.path) - 1) * '  '}1. "
+    if row.is_external(server_hostname=discourse.host):
+        return f"{leader}[{row.navlink.title}]({row.navlink.link})"
+    if row.is_group:
+        return f"{leader}[{row.navlink.title}]({Path(*row.path)})"
+    return f"{leader}[{row.navlink.title}]({_generate_document_path(row)})"
+
+
+def _migrate_navigation_table(rows: typing.Iterable[types_.TableRow], discourse: Discourse) -> str:
+    """Generate the contents index from the table rows of the navigation table.
+
+    Args:
+        rows: the rows of the navigation table.
+        discourse: Client to the documentation server.
+
+    Returns:
+        The contents index.
+
+    """
+    contents_index_lines = (
+        _table_row_to_contents_index_line(row=row, discourse=discourse) for row in rows
+    )
+    contents_index_body = "\n".join(contents_index_lines)
+    return f"# Contents\n\n{contents_index_body}".strip()
+
+
+def _index_file_from_content(
+    content: str,
+    table_rows: typing.Iterable[types_.TableRow],
+    discourse: Discourse,
+) -> types_.IndexDocumentMeta:
     """Get index file document metadata.
 
     Args:
         content: Index file content.
+        table_rows: Table rows from the index table.
+        discourse: Client to the documentation server.
 
     Returns:
         Index file document metadata.
     """
-    return types_.IndexDocumentMeta(path=Path("index.md"), content=content)
+    contents_index = _migrate_navigation_table(rows=table_rows, discourse=discourse)
+    return types_.IndexDocumentMeta(
+        path=Path("index.md"), content=f"{content}\n\n{contents_index}"
+    )
 
 
 def make_parent(docs_path: Path, document_meta: types_.MigrationFileMeta) -> Path:
@@ -301,7 +360,10 @@ def _get_docs_metadata(
     Returns:
         Metadata of files to be migrated.
     """
-    index_doc = _index_file_from_content(content=index_content)
+    table_rows, table_rows_for_index = itertools.tee(table_rows, 2)
+    index_doc = _index_file_from_content(
+        content=index_content, table_rows=table_rows_for_index, discourse=discourse
+    )
     table_docs = _extract_docs_from_table_rows(table_rows=table_rows, discourse=discourse)
     return itertools.chain((index_doc,), table_docs)
 

src/navigation_table.py

@@ -31,7 +31,8 @@
 _PATH_REGEX = rf"{_WHITESPACE}([${constants.PATH_CHARS}]+){_WHITESPACE}"
 _PUNCTUATION = string.punctuation.replace("/", "\\/")
 _NAVLINK_TITLE_REGEX = rf"[\w\- {_PUNCTUATION}]+?"
-_NAVLINK_LINK_REGEX = r"[\w\/-]*"
+# Link characters according to https://www.rfc-editor.org/rfc/rfc3986#section-2
+_NAVLINK_LINK_REGEX = r"[\w\/._~:/?#\[\]@!$&'*+,;-]*"
 _NAVLINK_REGEX = (
     rf"{_WHITESPACE}\[{_WHITESPACE}({_NAVLINK_TITLE_REGEX}){_WHITESPACE}\]{_WHITESPACE}"
     rf"\({_WHITESPACE}({_NAVLINK_LINK_REGEX}){_WHITESPACE}\){_WHITESPACE}"
@@ -106,10 +107,10 @@ def _check_table_row_write_permission(
         PagePermissionError: The user does not have write permission for the linked topic.
         ServerError: The interaction with discourse failed.
     """
-    if table_row.navlink.link is None:
+    if table_row.is_group or table_row.is_external(server_hostname=discourse.host):
         return table_row
 
-    url = table_row.navlink.link
+    url = typing.cast(str, table_row.navlink.link)
     try:
         if discourse.check_topic_write_permission(url=url):
             return table_row

tests/integration/test___init__run_migrate.py

@@ -79,22 +79,24 @@ async def test_run_migrate(
     ).removeprefix(discourse_prefix)
     index_page_content = f"""Testing index page.
 
-    Testing index page content.
-
-    # Navigation
-
-    | Level | Path | Navlink |
-    | -- | -- | -- |
-    | 1 | https-canonical-com | [Canonical](https://canonical.com) |
-    | 1 | group-1 | [Group 1]() |
-    | 1 | group-2 | [Group 2]() |
-    | 2 | group-2-content-1 | [{content_page_1.content}]({content_page_1_url}) |
-    | 2 | group-2-content-2 | [{content_page_2.content}]({content_page_2_url}) |
-    | 1 | group-3 | [Group 3]() |
-    | 2 | group-3-group-4 | [Group 4]() |
-    | 3 | group-3-group-4-content-3 | [{content_page_3.content}]({content_page_3_url}) |
-    | 2 | group-3-content-4 | [{content_page_4.content}]({content_page_4_url}) |
-    | 1 | group-5 | [Group 5]() |"""
+Testing index page content.
+
+# Navigation
+
+| Level | Path | Navlink |
+| -- | -- | -- |
+| 1 | https-canonical-com-1 | [Canonical 1](https://canonical.com/1) |
+| 1 | group-1 | [Group 1]() |
+| 1 | group-2 | [Group 2]() |
+| 2 | group-2-content-1 | [{content_page_1.content}]({content_page_1_url}) |
+| 2 | group-2-content-2 | [{content_page_2.content}]({content_page_2_url}) |
+| 2 | https-canonical-com-2 | [Canonical 2](https://canonical.com/2) |
+| 1 | group-3 | [Group 3]() |
+| 2 | group-3-group-4 | [Group 4]() |
+| 3 | group-3-group-4-content-3 | [{content_page_3.content}]({content_page_3_url}) |
+| 3 | https-canonical-com-3 | [Canonical 3](https://canonical.com/3) |
+| 2 | group-3-content-4 | [{content_page_4.content}]({content_page_4_url}) |
+| 1 | group-5 | [Group 5]() |"""
     index_url = discourse_api.create_topic(
         title=f"{document_name.replace('-', ' ').title()} Documentation Overview",
         content=index_page_content,
@@ -128,6 +130,29 @@ async def test_run_migrate(
     assert output_migrate is not None
     assert output_migrate.pull_request_url == mock_pull_request.html_url
     assert output_migrate.action == PullRequestAction.OPENED
+    assert (upstream_doc_dir / "index.md").is_file()
+    assert (
+        (upstream_doc_dir / "index.md").read_text(encoding="utf-8")
+        == f"""Testing index page.
+
+Testing index page content.
+
+
+# Contents
+
+1. [Canonical 1](https://canonical.com/1)
+1. [Group 1](group-1)
+1. [Group 2](group-2)
+  1. [{content_page_1.content}](group-2/content-1.md)
+  1. [{content_page_2.content}](group-2/content-2.md)
+  1. [Canonical 2](https://canonical.com/2)
+1. [Group 3](group-3)
+  1. [Group 4](group-3/group-4)
+    1. [{content_page_3.content}](group-3/group-4/content-3.md)
+    1. [Canonical 3](https://canonical.com/3)
+  1. [{content_page_4.content}](group-3/content-4.md)
+1. [Group 5](group-5)"""
+    )
     assert (group_1_path := upstream_doc_dir / "group-1").is_dir()
     assert (group_1_path / migration.GITKEEP_FILENAME).is_file()
     assert (group_2_path := upstream_doc_dir / "group-2").is_dir()

tests/unit/test___init__.py

@@ -707,7 +707,8 @@ def test__run_migrate(
 
     Content body."""
     index_table = f"""{constants.NAVIGATION_TABLE_START}
-    | 1 | path-1 | [Tutorials](link-1) |"""
+| 1 | path-1 | [Tutorials](link-1) |
+| 1 | https-canonical-com | [Canonical](https://canonical.com/) |"""
     index_page = f"{index_content}{index_table}"
 
     mocked_clients.discourse.retrieve_topic.side_effect = [
@@ -732,7 +733,12 @@ def test__run_migrate(
     assert (
         path_file := upstream_repository_path / DOCUMENTATION_FOLDER_NAME / "path-1.md"
     ).is_file()
-    assert index_file.read_text(encoding="utf-8") == index_content
+    assert index_file.read_text(encoding="utf-8") == (
+        f"{index_content}\n\n"
+        "# Contents\n\n"
+        "1. [Tutorials](path-1.md)\n"
+        "1. [Canonical](https://canonical.com/)"
+    )
     assert path_file.read_text(encoding="utf-8") == link_content
 
 
@@ -831,7 +837,12 @@ def test__run_migrate_with_pull_request_no_modification(
 
     # Set up remote repository with content
     (docs_folder := upstream_repository_path / "docs").mkdir()
-    (docs_folder / "index.md").write_text(index_content)
+    (docs_folder / "index.md").write_text(
+        f"{index_content}\n\n"
+        "# Contents\n\n"
+        "1. [empty-navlink](path-1)\n"
+        "  1. [file-navlink](path-1/file-1.md)"
+    )
     (docs_folder / "path-1").mkdir()
     (docs_folder / "path-1" / "file-1.md").write_text(navlink_page)
 
@@ -941,7 +952,12 @@ def test_run_no_docs_dir(
         / "my-path-1"
         / "my-file-1.md"
     ).is_file()
-    assert index_file.read_text(encoding="utf-8") == index_content
+    assert index_file.read_text(encoding="utf-8") == (
+        f"{index_content}\n\n"
+        "# Contents\n\n"
+        "1. [empty-navlink](my-path-1)\n"
+        "  1. [file-navlink](my-path-1/my-file-1.md)"
+    )
     assert path_file.read_text(encoding="utf-8") == navlink_page
 
 
@@ -997,7 +1013,12 @@ def test_run_no_docs_dir_no_tag(
         / "t-path-1"
         / "t-file-1.md"
     ).is_file()
-    assert index_file.read_text(encoding="utf-8") == index_content
+    assert index_file.read_text(encoding="utf-8") == (
+        f"{index_content}\n\n"
+        "# Contents\n\n"
+        "1. [empty-navlink](t-path-1)\n"
+        "  1. [file-navlink](t-path-1/t-file-1.md)"
+    )
     assert path_file.read_text(encoding="utf-8") == navlink_page
 
 
@@ -1026,7 +1047,12 @@ def test_run_migrate_same_content_local_and_server(mock_edit_pull_request, caplo
     mocked_clients.discourse.retrieve_topic.side_effect = [index_page, navlink_page]
 
     (docs_folder := mocked_clients.repository.base_path / "docs").mkdir()
-    (docs_folder / "index.md").write_text(index_content)
+    (docs_folder / "index.md").write_text(
+        f"{index_content}\n\n"
+        "# Contents\n\n"
+        "1. [empty-navlink](their-path-1)\n"
+        "  1. [file-navlink](their-path-1/their-file-1.md)"
+    )
     (docs_folder / "their-path-1").mkdir()
     (docs_folder / "their-path-1" / "their-file-1.md").write_text(navlink_page)
 
@@ -1084,7 +1110,12 @@ def test_run_migrate_same_content_local_and_server_open_pr(
     mocked_clients.discourse.retrieve_topic.side_effect = [index_page, navlink_page]
 
     (docs_folder := mocked_clients.repository.base_path / "docs").mkdir()
-    (docs_folder / "index.md").write_text(index_content)
+    (docs_folder / "index.md").write_text(
+        f"{index_content}\n\n"
+        "# Contents\n\n"
+        "1. [empty-navlink](their-path-1)\n"
+        "  1. [file-navlink](their-path-1/their-file-1.md)"
+    )
     (docs_folder / "their-path-1").mkdir()
     (docs_folder / "their-path-1" / "their-file-1.md").write_text(navlink_page)
 

tests/unit/test_download.py

@@ -5,15 +5,12 @@
 
 import pytest
 
-from src import DOCUMENTATION_FOLDER_NAME, DOCUMENTATION_TAG, constants  # GETTING_STARTED,
+from src import DOCUMENTATION_FOLDER_NAME, DOCUMENTATION_TAG, constants
 from src.download import recreate_docs
 from src.metadata import METADATA_DOCS_KEY, METADATA_NAME_KEY
 
 from .helpers import create_metadata_yaml
 
-# Need access to protected functions for testing
-# pylint: disable=protected-access
-
 
 @pytest.mark.usefixtures("patch_create_repository_client")
 def test_recreate_docs(
@@ -47,5 +44,10 @@ def test_recreate_docs(
     assert (
         path_file := repository_path / DOCUMENTATION_FOLDER_NAME / "page-path-1" / "page-file-1.md"
     ).is_file()
-    assert index_file.read_text(encoding="utf-8") == index_content
+    assert index_file.read_text(encoding="utf-8") == (
+        f"{index_content}\n\n"
+        "# Contents\n\n"
+        "1. [empty-navlink](page-path-1)\n"
+        "  1. [file-navlink](page-path-1/page-file-1.md)"
+    )
     assert path_file.read_text(encoding="utf-8") == navlink_page

tests/unit/test_migration/__init__.py

@@ -0,0 +1,4 @@
+# Copyright 2023 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+"""Unit tests for migartion module."""