diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4c59c093..6c6f8635 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/src-docs/migration.py.md b/src-docs/migration.py.md
index 5f28353e..972280bf 100644
--- a/src-docs/migration.py.md
+++ b/src-docs/migration.py.md
@@ -12,7 +12,7 @@ Module for migrating remote documentation into local git repository.
---
-
+
## function `make_parent`
@@ -37,7 +37,7 @@ Construct path leading to document to be created.
---
-
+
## function `run`
diff --git a/src-docs/navigation_table.py.md b/src-docs/navigation_table.py.md
index 88979ef6..1d2956e4 100644
--- a/src-docs/navigation_table.py.md
+++ b/src-docs/navigation_table.py.md
@@ -8,7 +8,7 @@ Module for parsing and rendering a navigation table.
---
-
+
## function `from_page`
@@ -35,7 +35,7 @@ Algorithm: 1. Extract the table based on a regular expression looking for a 3
---
-
+
## function `generate_table_row`
diff --git a/src/migration.py b/src/migration.py
index 2f944627..777b99bc 100644
--- a/src/migration.py
+++ b/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)
diff --git a/src/navigation_table.py b/src/navigation_table.py
index 49a7d320..0cd9f72b 100644
--- a/src/navigation_table.py
+++ b/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
diff --git a/tests/integration/test___init__run_migrate.py b/tests/integration/test___init__run_migrate.py
index 14560db5..ce699cc5 100644
--- a/tests/integration/test___init__run_migrate.py
+++ b/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()
diff --git a/tests/unit/test___init__.py b/tests/unit/test___init__.py
index a740845c..d6b0b86e 100644
--- a/tests/unit/test___init__.py
+++ b/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)
diff --git a/tests/unit/test_download.py b/tests/unit/test_download.py
index 261003ed..9c326296 100644
--- a/tests/unit/test_download.py
+++ b/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
diff --git a/tests/unit/test_migration/__init__.py b/tests/unit/test_migration/__init__.py
new file mode 100644
index 00000000..6b3fa463
--- /dev/null
+++ b/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."""
diff --git a/tests/unit/test_migration.py b/tests/unit/test_migration/test_private.py
similarity index 86%
rename from tests/unit/test_migration.py
rename to tests/unit/test_migration/test_private.py
index 6c0b7a7d..b77ae3f0 100644
--- a/tests/unit/test_migration.py
+++ b/tests/unit/test_migration/test_private.py
@@ -1,7 +1,7 @@
# Copyright 2023 Canonical Ltd.
# See LICENSE file for licensing details.
-"""Unit tests for migration module."""
+"""Unit tests for private functions of migration module."""
# Need access to protected functions for testing
# pylint: disable=protected-access
@@ -15,8 +15,8 @@
from src import discourse, exceptions, migration, types_
-from .. import factories
-from .helpers import assert_substrings_in_string
+from ... import factories
+from ..helpers import assert_substrings_in_string
@pytest.mark.parametrize(
@@ -230,22 +230,24 @@ def test__create_gitkeep_meta(row: types_.TableRow, expected_meta: types_.Gitkee
assert migration._create_gitkeep_meta(row=row) == expected_meta
-@pytest.mark.parametrize(
- "content, expected_meta",
- [
- pytest.param(
- content := "content-1",
- types_.IndexDocumentMeta(path=Path("index.md"), content=content),
- ),
- ],
-)
-def test__index_file_from_content(content: str, expected_meta: types_.IndexDocumentMeta):
+def test__index_file_from_content(mocked_clients):
"""
arrange: given an index file content
act: when _index_file_from_content is called
assert: expected index document metadata is returned.
"""
- assert migration._index_file_from_content(content) == expected_meta
+ content = "content 1"
+ table_row = factories.TableRowFactory(is_document=True)
+
+ returned_index_file = migration._index_file_from_content(
+ content, table_rows=(table_row,), discourse=mocked_clients.discourse
+ )
+
+ assert returned_index_file.path == Path("index.md")
+ assert (
+ returned_index_file.content
+ == f"{content}\n\n# Contents\n\n1. [{table_row.navlink.title}]({table_row.path[0]}.md)"
+ )
@pytest.mark.parametrize(
@@ -646,6 +648,133 @@ def test__migrate_gitkeep(meta: types_.GitkeepMeta, tmp_path: Path):
assert (tmp_path / meta.path).is_file()
+def _test__table_row_to_contents_index_line_parameters():
+ """Generate parameters for the test__table_row_to_contents_index_line test.
+
+ Returns:
+ The tests.
+ """
+ return [
+ pytest.param(
+ row := factories.TableRowFactory(is_document=True),
+ f"1. [{row.navlink.title}]({row.path[0]}.md)",
+ id="page top level path",
+ ),
+ pytest.param(
+ row := factories.TableRowFactory(is_document=True, path=("dir_1", "file")),
+ f" 1. [{row.navlink.title}]({row.path[0]}/{row.path[1]}.md)",
+ id="page nested path path",
+ ),
+ pytest.param(
+ row := factories.TableRowFactory(is_document=True, path=("dir_1", "dir_2", "file")),
+ f" 1. [{row.navlink.title}]({row.path[0]}/{row.path[1]}/{row.path[2]}.md)",
+ id="page deeply nested path",
+ ),
+ pytest.param(
+ row := factories.TableRowFactory(is_group=True),
+ f"1. [{row.navlink.title}]({row.path[0]})",
+ id="group top level path",
+ ),
+ pytest.param(
+ row := factories.TableRowFactory(is_group=True, path=("dir_1", "dir_2")),
+ f" 1. [{row.navlink.title}]({row.path[0]}/{row.path[1]})",
+ id="group nested path path",
+ ),
+ pytest.param(
+ row := factories.TableRowFactory(is_group=True, path=("dir_1", "dir_2", "dir_3")),
+ f" 1. [{row.navlink.title}]({row.path[0]}/{row.path[1]}/{row.path[2]})",
+ id="group deeply nested path",
+ ),
+ pytest.param(
+ row := factories.TableRowFactory(is_external=True),
+ f"1. [{row.navlink.title}]({row.navlink.link})",
+ id="page top level path",
+ ),
+ pytest.param(
+ row := factories.TableRowFactory(
+ is_external=True, path=("dir_1", "https-canonical-com")
+ ),
+ f" 1. [{row.navlink.title}]({row.navlink.link})",
+ id="page nested path path",
+ ),
+ pytest.param(
+ row := factories.TableRowFactory(
+ is_external=True, path=("dir_1", "dir_2", "https-canonical-com")
+ ),
+ f" 1. [{row.navlink.title}]({row.navlink.link})",
+ id="page deeply nested path",
+ ),
+ ]
+
+
+@pytest.mark.parametrize(
+ "row, expected_line", _test__table_row_to_contents_index_line_parameters()
+)
+def test__table_row_to_contents_index_line(
+ row: types_.TableRow, expected_line: str, mocked_clients
+):
+ """
+ arrange: given table row
+ act: when _table_row_to_contents_index_line is called with the row
+ assert: then the expected contents index line is returned.
+ """
+ returned_line = migration._table_row_to_contents_index_line(
+ row=row, discourse=mocked_clients.discourse
+ )
+
+ assert returned_line == expected_line
+
+
+def _test__migrate_navigation_table_parameters():
+ """Generate parameters for the test__migrate_navigation_table test.
+
+ Returns:
+ The tests.
+ """
+ return [
+ pytest.param(
+ (),
+ "# Contents",
+ id="empty rows",
+ ),
+ pytest.param(
+ (row_1 := factories.TableRowFactory(is_document=True),),
+ f"# Contents\n\n1. [{row_1.navlink.title}]({row_1.path[0]}.md)",
+ id="single row",
+ ),
+ pytest.param(
+ (
+ row_1 := factories.TableRowFactory(is_document=True),
+ row_2 := factories.TableRowFactory(is_document=True),
+ ),
+ (
+ "# Contents\n\n"
+ f"1. [{row_1.navlink.title}]({row_1.path[0]}.md)\n"
+ f"1. [{row_2.navlink.title}]({row_2.path[0]}.md)"
+ ),
+ id="multiple rows",
+ ),
+ ]
+
+
+@pytest.mark.parametrize(
+ "rows, expected_contents_index", _test__migrate_navigation_table_parameters()
+)
+def test__migrate_navigation_table(
+ rows: tuple[types_.TableRow, ...], expected_contents_index: str, mocked_clients
+):
+ """
+ arrange: given table rows
+ act: when _migrate_navigation_table is called with the rows
+ assert: then the expected contents index is returned.
+ """
+ returned_contents_index = migration._migrate_navigation_table(
+ rows=rows, discourse=mocked_clients.discourse
+ )
+
+ assert returned_contents_index == expected_contents_index
+
+
def test__migrate_document_fail(tmp_path: Path, mocked_clients):
"""
arrange: given valid document metadata and mocked discourse that raises an error
@@ -821,78 +950,3 @@ def test__get_docs_metadata(mocked_clients):
assert len(returned_docs_metadata) == 2
assert isinstance(returned_docs_metadata[0], types_.IndexDocumentMeta)
assert isinstance(returned_docs_metadata[1], types_.MigrationFileMeta)
-
-
-def test_run_error(tmp_path: Path):
- """
- arrange: given table rows, index content, mocked discourse that throws an exception and a
- temporary docs path
- act: when run is called
- assert: table rows are successfully migrated
- """
- mocked_discourse = mock.MagicMock(spec=discourse.Discourse)
- mocked_discourse.retrieve_topic.side_effect = exceptions.DiscourseError
- table_rows = (factories.TableRowFactory(level=1),)
-
- with pytest.raises(exceptions.MigrationError):
- migration.run(
- table_rows=table_rows,
- index_content="content-1",
- discourse=mocked_discourse,
- docs_path=tmp_path,
- )
-
-
-@pytest.mark.parametrize(
- "table_rows, index_content, expected_files",
- [
- pytest.param(
- (factories.TableRowFactory(is_document=True, path=("doc-1",), level=1),),
- "content-1",
- (Path("doc-1.md"),),
- id="single doc",
- ),
- pytest.param(
- (
- factories.TableRowFactory(is_group=True, path=("group-1",), level=1),
- factories.TableRowFactory(is_document=True, path=("group-1", "doc-1"), level=2),
- ),
- "content-1",
- (Path("group-1/doc-1.md"),),
- id="nested doc",
- ),
- pytest.param(
- (
- factories.TableRowFactory(is_group=True, path=("group-1",), level=1),
- factories.TableRowFactory(is_group=True, path=("group-1", "group-2"), level=2),
- ),
- "content-1",
- (Path("group-1/group-2/.gitkeep"),),
- id="nested group no docs",
- ),
- ],
-)
-def test_run(
- table_rows: tuple[types_.TableRow, ...],
- index_content: str,
- tmp_path: Path,
- expected_files: Iterable[Path],
-):
- """
- arrange: given table rows, index content, mocked discourse and a temporary docs path
- act: when run is called
- assert: table rows are successfully migrated
- """
- mocked_discourse = mock.MagicMock(spec=discourse.Discourse)
- mocked_discourse.retrieve_topic.return_value = "document-content"
-
- migration.run(
- table_rows=table_rows,
- index_content=index_content,
- discourse=mocked_discourse,
- docs_path=tmp_path,
- )
-
- assert (tmp_path / "index.md").read_text() == index_content
- for path in expected_files:
- assert (tmp_path / path).is_file()
diff --git a/tests/unit/test_migration/test_public.py b/tests/unit/test_migration/test_public.py
new file mode 100644
index 00000000..94475435
--- /dev/null
+++ b/tests/unit/test_migration/test_public.py
@@ -0,0 +1,116 @@
+# Copyright 2023 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+"""Unit tests for public functions in migration module."""
+
+from collections.abc import Iterable
+from pathlib import Path
+from unittest import mock
+
+import pytest
+
+from src import discourse, exceptions, migration, types_
+
+from ... import factories
+
+
+def test_run_error(tmp_path: Path):
+ """
+ arrange: given table rows, index content, mocked discourse that throws an exception and a
+ temporary docs path
+ act: when run is called
+ assert: table rows are successfully migrated
+ """
+ mocked_discourse = mock.MagicMock(spec=discourse.Discourse)
+ mocked_discourse.retrieve_topic.side_effect = exceptions.DiscourseError
+ table_rows = (factories.TableRowFactory(level=1),)
+
+ with pytest.raises(exceptions.MigrationError):
+ migration.run(
+ table_rows=table_rows,
+ index_content="content-1",
+ discourse=mocked_discourse,
+ docs_path=tmp_path,
+ )
+
+
+def _test_run_parameters():
+ """Generate parameters for the test_run test.
+
+ Returns:
+ The tests.
+ """
+ return [
+ pytest.param(
+ (row_1 := factories.TableRowFactory(is_document=True, path=("doc-1",), level=1),),
+ (index_content := "content-1"),
+ (path_1 := Path(f"{row_1.path[0]}.md"),),
+ f"{index_content}\n\n# Contents\n\n1. [{row_1.navlink.title}]({path_1})",
+ id="single doc",
+ ),
+ pytest.param(
+ (
+ row_1 := factories.TableRowFactory(is_group=True, path=("group-1",), level=1),
+ row_2 := factories.TableRowFactory(
+ is_document=True, path=("group-1", "doc-1"), level=2
+ ),
+ ),
+ (index_content := "content-1"),
+ (path_2 := Path(f"{row_2.path[0]}/{row_2.path[1]}.md"),),
+ (
+ f"{index_content}\n\n"
+ "# Contents\n\n"
+ f"1. [{row_1.navlink.title}]({row_1.path[0]})\n"
+ f" 1. [{row_2.navlink.title}]({path_2})"
+ ),
+ id="nested doc",
+ ),
+ pytest.param(
+ (
+ row_1 := factories.TableRowFactory(is_group=True, path=("group-1",), level=1),
+ row_2 := factories.TableRowFactory(
+ is_group=True, path=("group-1", "group-2"), level=2
+ ),
+ ),
+ (index_content := "content-1"),
+ (Path(f"{row_2.path[0]}/{row_2.path[1]}/.gitkeep"),),
+ (
+ f"{index_content}\n\n"
+ "# Contents\n\n"
+ f"1. [{row_1.navlink.title}]({row_1.path[0]})\n"
+ f" 1. [{row_2.navlink.title}]({row_2.path[0]}/{row_2.path[1]})"
+ ),
+ id="nested group no docs",
+ ),
+ ]
+
+
+@pytest.mark.parametrize(
+ "table_rows, index_content, expected_files, expected_index_content",
+ _test_run_parameters(),
+)
+def test_run(
+ table_rows: tuple[types_.TableRow, ...],
+ index_content: str,
+ tmp_path: Path,
+ expected_files: Iterable[Path],
+ expected_index_content: str,
+):
+ """
+ arrange: given table rows, index content, mocked discourse and a temporary docs path
+ act: when run is called
+ assert: table rows are successfully migrated
+ """
+ mocked_discourse = mock.MagicMock(spec=discourse.Discourse)
+ mocked_discourse.retrieve_topic.return_value = "document-content"
+
+ migration.run(
+ table_rows=table_rows,
+ index_content=index_content,
+ discourse=mocked_discourse,
+ docs_path=tmp_path,
+ )
+
+ assert (tmp_path / "index.md").read_text() == expected_index_content
+ for path in expected_files:
+ assert (tmp_path / path).is_file()
diff --git a/tests/unit/test_navigation_table.py b/tests/unit/test_navigation_table.py
index 728ff60e..c233c897 100644
--- a/tests/unit/test_navigation_table.py
+++ b/tests/unit/test_navigation_table.py
@@ -59,6 +59,9 @@
pytest.param("|1|a|[a()|", True, id="third column closing square bracket missing"),
pytest.param("|1|a|[a])|", True, id="third column opening link bracket missing"),
pytest.param(r"|1|a|[a](\)|", True, id="third column link includes backslash"),
+ pytest.param(
+ r"|1|a|[a](https://canonical.com/1?2#3)|", False, id="third column link is external"
+ ),
pytest.param("|1|a|[a](|", True, id="third column closing link bracket missing"),
pytest.param("|1|a|[a]()|", False, id="matches row"),
pytest.param("||a|[a]()|", False, id="matches hidden row"),
@@ -420,10 +423,25 @@ def test__check_table_row_write_permission_group(mocked_clients):
assert: then the table row is returned.
"""
mocked_discourse = mocked_clients.discourse
- table_row = factories.TableRowFactory(
- level=1, path=("path 1",), navlink=factories.NavlinkFactory(title="title 1", link=None)
+ table_row = factories.TableRowFactory(is_group=True)
+
+ returned_table_row = navigation_table._check_table_row_write_permission(
+ table_row=table_row, discourse=mocked_discourse
)
+ assert returned_table_row == table_row
+ mocked_discourse.check_topic_write_permission.assert_not_called()
+
+
+def test__check_table_row_write_permission_external(mocked_clients):
+ """
+ arrange: given mocked discourse and table row for a external reference
+ act: when _check_table_row_write_permission is called with the table row and mocked discourse
+ assert: then the table row is returned.
+ """
+ mocked_discourse = mocked_clients.discourse
+ table_row = factories.TableRowFactory(is_external=True)
+
returned_table_row = navigation_table._check_table_row_write_permission(
table_row=table_row, discourse=mocked_discourse
)
@@ -440,18 +458,16 @@ def test__check_table_row_write_permission_page_error(mocked_clients):
"""
mocked_discourse = mocked_clients.discourse
mocked_discourse.check_topic_write_permission.side_effect = exceptions.DiscourseError
+ table_row = factories.TableRowFactory(is_document=True)
with pytest.raises(exceptions.ServerError) as exc_info:
navigation_table._check_table_row_write_permission(
- table_row=factories.TableRowFactory(
- level=1,
- path=("path 1",),
- navlink=factories.NavlinkFactory(title="title 1", link=(link := "link 1")),
- ),
+ table_row=table_row,
discourse=mocked_discourse,
)
- assert link in str(exc_info.value)
+ assert table_row.navlink.link
+ assert table_row.navlink.link in str(exc_info.value)
def test__check_table_row_write_permission_page_false(mocked_clients):
@@ -463,18 +479,18 @@ def test__check_table_row_write_permission_page_false(mocked_clients):
"""
mocked_discourse = mocked_clients.discourse
mocked_discourse.check_topic_write_permission.return_value = False
+ table_row = factories.TableRowFactory(is_document=True)
with pytest.raises(exceptions.PagePermissionError) as exc_info:
navigation_table._check_table_row_write_permission(
- table_row=factories.TableRowFactory(
- level=1,
- path=("path 1",),
- navlink=factories.NavlinkFactory(title="title 1", link=(link := "link 1")),
- ),
+ table_row=table_row,
discourse=mocked_discourse,
)
- assert_substrings_in_string((link, "write", "permission"), str(exc_info.value))
+ assert table_row.navlink.link
+ assert_substrings_in_string(
+ (table_row.navlink.link, "write", "permission"), str(exc_info.value)
+ )
def test__check_table_row_write_permission_page_true(mocked_clients):
@@ -485,18 +501,17 @@ def test__check_table_row_write_permission_page_true(mocked_clients):
"""
mocked_discourse = mocked_clients.discourse
mocked_discourse.check_topic_write_permission.return_value = True
- table_row = factories.TableRowFactory(
- level=1,
- path=("path 1",),
- navlink=factories.NavlinkFactory(title="title 1", link=(link := "link 1")),
- )
+ table_row = factories.TableRowFactory(is_document=True)
returned_table_row = navigation_table._check_table_row_write_permission(
table_row=table_row, discourse=mocked_discourse
)
assert returned_table_row == table_row
- mocked_discourse.check_topic_write_permission.assert_called_once_with(url=link)
+ assert table_row.navlink.link
+ mocked_discourse.check_topic_write_permission.assert_called_once_with(
+ url=table_row.navlink.link
+ )
def test_from_page_missing_write_permission(mocked_clients):