Community merge conflicts

.github/workflows/e2e_test.yaml

@@ -56,6 +56,7 @@ jobs:
           discourse_host: discourse.charmhub.io
           discourse_api_username: ${{ secrets.DISCOURSE_API_USERNAME }}
           discourse_api_key: ${{ secrets.DISCOURSE_API_KEY }}
+          github_token: ${{ secrets.GITHUB_TOKEN }}
           dry_run: true
       - name: Show pages
         run: echo '${{ steps.e2e-test-draft.outputs.urls_with_actions }}'
@@ -71,6 +72,7 @@ jobs:
           discourse_host: discourse.charmhub.io
           discourse_api_username: ${{ secrets.DISCOURSE_API_USERNAME }}
           discourse_api_key: ${{ secrets.DISCOURSE_API_KEY }}
+          github_token: ${{ secrets.GITHUB_TOKEN }}
       - name: Show pages
         run: echo '${{ steps.e2e-test-create.outputs.urls_with_actions }}'
       - name: Check create
@@ -90,6 +92,7 @@ jobs:
           discourse_host: discourse.charmhub.io
           discourse_api_username: ${{ secrets.DISCOURSE_API_USERNAME }}
           discourse_api_key: ${{ secrets.DISCOURSE_API_KEY }}
+          github_token: ${{ secrets.GITHUB_TOKEN }}
       - name: Show pages
         run: echo '${{ steps.e2e-test-delete-topics.outputs.urls_with_actions }}'
       - name: Check delete topics disabled
@@ -104,6 +107,7 @@ jobs:
           discourse_host: discourse.charmhub.io
           discourse_api_username: ${{ secrets.DISCOURSE_API_USERNAME }}
           discourse_api_key: ${{ secrets.DISCOURSE_API_KEY }}
+          github_token: ${{ secrets.GITHUB_TOKEN }}
       - name: Show pages
         run: echo '${{ steps.e2e-test-delete.outputs.urls_with_actions }}'
       - name: Check delete topics enabled

.github/workflows/integration_test.yaml

@@ -8,4 +8,4 @@ jobs:
     uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main
     secrets: inherit
     with:
-      modules: '["discourse", "reconcile", "migrate"]'
+      modules: '["discourse", "reconcile", "reconcile_conflict", "migrate"]'

.github/workflows/test.yaml

@@ -18,5 +18,5 @@ jobs:
       - name: Check for any code improvements
         run: |
           PYTHON_FILES=$(find . -name '*.py')
-          pyupgrade --py310-plus $PYTHON_FILES
+          pyupgrade --py311-plus $PYTHON_FILES
           refurb $PYTHON_FILES

README.md

@@ -101,10 +101,10 @@ charmhub.
               github_token: ${{ secrets.GITHUB_TOKEN }}
     ```
 
-    a branch name with `upload-charm-docs/migrate` will be created and a pull 
-    request named `[upload-charm-docs] Migrate charm docs` will be created 
-    towards the working branch the workflow was triggered with. 
-    In order to ensure that the branches can be created successfully, please 
+    a branch name with `upload-charm-docs/migrate` will be created and a pull
+    request named `[upload-charm-docs] Migrate charm docs` will be created
+    towards the working branch the workflow was triggered with.
+    In order to ensure that the branches can be created successfully, please
     make sure that there are no existing branches clashing with the name above.
     Please note that `dry_run` parameter has no effect on migrate mode.
 

action.yaml

@@ -40,6 +40,12 @@ inputs:
       Required if running in migration mode.
     required: false
     type: string
+  base_branch:
+    description: |
+      The name of the branch that contains the documentation after it has been uploaded to
+      discourse. Defaults to the default branch of the repository.
+    required: false
+    type: string
 outputs:
   urls_with_actions:
     description: |

dev-requirements.txt

@@ -1,2 +1,6 @@
 factory_boy>=3.2,<3.3
+ops>=2.1,<2.2
+pytest-asyncio>=0.21,<0.22
+pytest-operator>=0.26,<0.27
+pytest>=7.2,<7.3
 juju==3.0.4

main.py

@@ -36,6 +36,7 @@ def _parse_env_vars() -> types_.UserInputs:
     delete_topics = os.getenv("INPUT_DELETE_TOPICS") == "true"
     dry_run = os.getenv("INPUT_DRY_RUN") == "true"
     github_access_token = os.getenv("INPUT_GITHUB_TOKEN")
+    base_branch = os.getenv("INPUT_BASE_BRANCH")
 
     return types_.UserInputs(
         discourse=types_.UserInputsDiscourse(
@@ -47,6 +48,7 @@ def _parse_env_vars() -> types_.UserInputs:
         delete_pages=delete_topics,
         dry_run=dry_run,
         github_access_token=github_access_token,
+        base_branch=base_branch,
     )
 
 

prepare_check_cleanup/migration.py

@@ -18,15 +18,15 @@
 from github.Repository import Repository
 
 from prepare_check_cleanup import exit_
+from src.constants import (
+    DOCUMENTATION_FOLDER_NAME,
+    DOCUMENTATION_INDEX_FILENAME,
+    NAVIGATION_TABLE_START,
+)
 from src.discourse import create_discourse
 from src.exceptions import DiscourseError
-from src.index import DOCUMENTATION_FOLDER_NAME, DOCUMENTATION_INDEX_FILENAME
-from src.pull_request import (
-    ACTIONS_PULL_REQUEST_TITLE,
-    DEFAULT_BRANCH_NAME,
-    create_repository_client,
-)
-from src.reconcile import NAVIGATION_TABLE_START
+from src.pull_request import DEFAULT_BRANCH_NAME
+from src.repository import ACTIONS_PULL_REQUEST_TITLE, create_repository_client
 
 
 class Action(str, Enum):

pyproject.toml

@@ -18,7 +18,7 @@ show_missing = true
 [tool.pytest.ini_options]
 minversion = "6.0"
 log_cli_level = "INFO"
-markers = ["reconcile", "migrate", "discourse"]
+markers = ["reconcile", "reconcile_conflict", "migrate", "discourse"]
 
 # Formatting tools configuration
 [tool.black]

src/__init__.py

@@ -3,21 +3,26 @@
 
 """Library for uploading docs to charmhub."""
 
+from itertools import tee
 from pathlib import Path
 
 from .action import DRY_RUN_NAVLINK_LINK, FAIL_NAVLINK_LINK
 from .action import run_all as run_all_actions
+from .check import conflicts as check_conflicts
+from .constants import DOCUMENTATION_FOLDER_NAME
 from .discourse import Discourse
 from .docs_directory import has_docs_directory
 from .docs_directory import read as read_docs_directory
 from .exceptions import InputError
-from .index import DOCUMENTATION_FOLDER_NAME, contents_from_page
+from .index import contents_from_page
 from .index import get as get_index
 from .metadata import get as get_metadata
 from .migration import run as migrate_contents
 from .navigation_table import from_page as navigation_table_from_page
-from .pull_request import RepositoryClient, create_pull_request, create_repository_client
+from .pull_request import create_pull_request
 from .reconcile import run as run_reconcile
+from .repository import Client as RepositoryClient
+from .repository import create_repository_client
 from .types_ import ActionResult, Metadata, UserInputs
 
 GETTING_STARTED = (
@@ -30,35 +35,55 @@ def _run_reconcile(
     base_path: Path,
     metadata: Metadata,
     discourse: Discourse,
-    dry_run: bool,
-    delete_pages: bool,
+    repository: RepositoryClient,
+    user_inputs: UserInputs,
 ) -> dict[str, str]:
     """Upload the documentation to charmhub.
 
     Args:
         base_path: The base path of the repository.
         metadata: Information about the charm.
         discourse: A client to the documentation server.
-        dry_run: If enabled, only log the action that would be taken.
-        delete_pages: Whether to delete pages that are no longer needed.
+        repository: Repository client for managing both local and remote git repositories.
+        user_inputs: Configurable inputs for running upload-charm-docs.
 
     Returns:
         All the URLs that had an action with the result of that action.
 
+    Raises:
+        InputError: if there are any problems with executing any of the actions.
+
     """
     index = get_index(metadata=metadata, base_path=base_path, server_client=discourse)
     path_infos = read_docs_directory(docs_path=base_path / DOCUMENTATION_FOLDER_NAME)
     server_content = (
         index.server.content if index.server is not None and index.server.content else ""
     )
     table_rows = navigation_table_from_page(page=server_content, discourse=discourse)
-    actions = run_reconcile(path_infos=path_infos, table_rows=table_rows, discourse=discourse)
+    actions = run_reconcile(
+        path_infos=path_infos,
+        table_rows=table_rows,
+        discourse=discourse,
+        repository=repository,
+        base_path=base_path,
+        user_inputs=user_inputs,
+    )
+
+    # tee creates a copy of the iterator which is needed as check_conflicts consumes the iterator
+    # it is passed
+    actions, check_actions = tee(actions, 2)
+    problems = tuple(check_conflicts(actions=check_actions))
+    if problems:
+        raise InputError(
+            "One or more of the required actions could not be executed, see the log for details"
+        )
+
     reports = run_all_actions(
         actions=actions,
         index=index,
         discourse=discourse,
-        dry_run=dry_run,
-        delete_pages=delete_pages,
+        dry_run=user_inputs.dry_run,
+        delete_pages=user_inputs.delete_pages,
     )
     return {
         str(report.location): report.result
@@ -121,10 +146,10 @@ def run(base_path: Path, discourse: Discourse, user_inputs: UserInputs) -> dict[
     """
     metadata = get_metadata(base_path)
     has_docs_dir = has_docs_directory(base_path=base_path)
+    repository = create_repository_client(
+        access_token=user_inputs.github_access_token, base_path=base_path
+    )
     if metadata.docs and not has_docs_dir:
-        repository = create_repository_client(
-            access_token=user_inputs.github_access_token, base_path=base_path
-        )
         return _run_migrate(
             base_path=base_path,
             metadata=metadata,
@@ -136,7 +161,7 @@ def run(base_path: Path, discourse: Discourse, user_inputs: UserInputs) -> dict[
             base_path=base_path,
             metadata=metadata,
             discourse=discourse,
-            dry_run=user_inputs.dry_run,
-            delete_pages=user_inputs.delete_pages,
+            repository=repository,
+            user_inputs=user_inputs,
         )
     raise InputError(GETTING_STARTED)

src/action.py

@@ -3,15 +3,17 @@
 
 """Module for taking the required actions to match the server state with the local state."""
 
-import difflib
 import logging
 import typing
 
 from . import exceptions, reconcile, types_
+from .content import diff as content_diff
+from .content import merge as content_merge
 from .discourse import Discourse
 
 DRY_RUN_NAVLINK_LINK = "<not created due to dry run>"
 DRY_RUN_REASON = "dry run"
+BASE_MISSING_REASON = "no base for the content to be automatically merged"
 FAIL_NAVLINK_LINK = "<not created due to error>"
 NOT_DELETE_REASON = "delete_topics is false"
 
@@ -29,23 +31,19 @@ def _absolute_url(url: types_.Url | None, discourse: Discourse) -> types_.Url |
     return discourse.absolute_url(url=url) if url is not None else None
 
 
-def _log_content_change(old: str, new: str) -> None:
-    """Log the difference between the old and new content, if any.
+def _log_content_change(base: str, new: str) -> None:
+    """Log the difference between the base and new content, if any.
 
     Args:
-        old: The previous content.
+        base: The previous content.
         new: The current content.
     """
-    old = f"{old}\n" if not old.endswith("\n") else old
+    old = f"{base}\n" if not base.endswith("\n") else base
     new = f"{new}\n" if not new.endswith("\n") else new
     if new != old:
         logging.info(
             "content change:\n%s",
-            "".join(
-                difflib.Differ().compare(
-                    old.splitlines(keepends=True), new.splitlines(keepends=True)
-                )
-            ),
+            content_diff(old, new),
         )
 
 
@@ -140,39 +138,55 @@ def _update(
         raise exceptions.ActionError(
             f"internal error, content change for page is None, {action=!r}"
         )
-    if (
-        action.navlink_change.new.link is not None
-        and action.content_change is not None
-        and action.content_change.new is None
-    ):
-        raise exceptions.ActionError(f"internal error, new content for page is None, {action=!r}")
 
     if (
         action.content_change is not None
         and action.content_change.old is not None
         and action.content_change.new is not None
     ):
-        _log_content_change(old=action.content_change.old, new=action.content_change.new)
+        _log_content_change(
+            base=action.content_change.base or action.content_change.old,
+            new=action.content_change.new,
+        )
 
-    if (
+    # All of these are needed to ensure types are as expected
+    if (  # pylint: disable=too-many-boolean-expressions
         not dry_run
         and action.navlink_change.new.link is not None
         and action.content_change is not None
+        and action.content_change.base is not None
         and action.content_change.new is not None
         and action.content_change.new != action.content_change.old
     ):
         try:
-            discourse.update_topic(
-                url=action.navlink_change.new.link, content=action.content_change.new
+            merged_content = content_merge(
+                base=action.content_change.base,
+                theirs=action.content_change.old,
+                ours=action.content_change.new,
             )
+            discourse.update_topic(url=action.navlink_change.new.link, content=merged_content)
             result = types_.ActionResult.SUCCESS
             reason = None
         except exceptions.DiscourseError as exc:
             result = types_.ActionResult.FAIL
             reason = str(exc)
+        except exceptions.ContentError as exc:
+            result = types_.ActionResult.FAIL
+            reason = str(exc)
     else:
-        result = types_.ActionResult.SKIP if dry_run else types_.ActionResult.SUCCESS
-        reason = DRY_RUN_REASON if dry_run else None
+        if dry_run:
+            result = types_.ActionResult.SKIP
+            reason = DRY_RUN_REASON
+        elif (
+            action.content_change is not None
+            and action.content_change.base is None
+            and action.content_change.new != action.content_change.old
+        ):
+            result = types_.ActionResult.FAIL
+            reason = BASE_MISSING_REASON
+        else:
+            result = types_.ActionResult.SUCCESS
+            reason = None
 
     url = _absolute_url(action.navlink_change.new.link, discourse=discourse)
     table_row = types_.TableRow(
@@ -343,7 +357,7 @@ def _run_index(
         case types_.UpdateIndexAction:
             try:
                 assert isinstance(action, types_.UpdateIndexAction)  # nosec
-                _log_content_change(old=action.content_change.old, new=action.content_change.new)
+                _log_content_change(base=action.content_change.old, new=action.content_change.new)
                 discourse.update_topic(url=action.url, content=action.content_change.new)
                 report = types_.ActionReport(
                     table_row=None,