Wagtail/Smartling robustness improvements

bedrock/cms/management/commands/run_smartling_sync.py

@@ -9,6 +9,7 @@
 from django.core.management.base import BaseCommand
 
 import requests
+from sentry_sdk import capture_exception
 
 from bedrock.base.config_manager import config
 
@@ -32,3 +33,4 @@ def handle(self, *args, **kwargs):
                 sys.stdout.write("Snitch pinged\n")
         except Exception as ex:
             sys.stderr.write(f"\nsync_smartling did not execute successfully: {ex}\n")
+            capture_exception(ex)

bedrock/cms/tests/test_callbacks.py

@@ -11,7 +11,7 @@
 from wagtaildraftsharing.models import WagtaildraftsharingLink
 
 from bedrock.cms.tests.factories import SimpleRichTextPageFactory, WagtailUserFactory
-from bedrock.cms.wagtail_localize_smartling.callbacks import visual_context
+from bedrock.cms.wagtail_localize_smartling.callbacks import _get_html_for_sharing_link, visual_context
 
 
 @pytest.mark.django_db
@@ -54,4 +54,53 @@ def test_visual_context__for_inviable_object(client):
     with pytest.raises(IncapableVisualContextCallback) as exc:
         url, html = visual_context(smartling_job=mock_job)
 
-    assert exc.value.args[0] == "Object was not visually previewable"
+    assert exc.value.args[0] == "Object was not visually previewable (i.e. not a Page)"
+
+
+@pytest.mark.django_db
+@mock.patch("bedrock.cms.wagtail_localize_smartling.callbacks.capture_message")
+def test_visual_context__for_page__with_no_revision(mock_capture_message, client):
+    top_level_page = SimpleRichTextPageFactory()
+
+    page = SimpleRichTextPageFactory(parent=top_level_page, slug="visual-context-text-page")
+    page.save_revision()
+
+    wagtail_factories.SiteFactory(
+        root_page=top_level_page,
+        is_default_site=True,
+        hostname=client._base_environ()["SERVER_NAME"],
+    )
+
+    user = WagtailUserFactory()
+
+    mock_job = mock.Mock()
+    mock_job.translation_source.get_source_instance.return_value = page
+    mock_job.user = user
+
+    page.latest_revision = None
+    page.save()
+    with pytest.raises(IncapableVisualContextCallback) as ctx:
+        visual_context(smartling_job=mock_job)
+
+    assert ctx.value.args[0] == (
+        "Object was not visually previewable because it didn't have a saved revision. Are you a developer with a local export?"
+    )
+    mock_capture_message.assert_called_once_with(f"Unable to get a latest_revision for {page} so unable to send visual context.")
+
+
+# The happy path is implicitly tested in test_visual_context__*, above
+@mock.patch("bedrock.cms.wagtail_localize_smartling.callbacks.capture_exception")
+@mock.patch("bedrock.cms.wagtail_localize_smartling.callbacks.SharingLinkView.as_view")
+def test__get_html_for_sharing_link__unhappy_path(mock_sharing_link_as_view, mock_capture_exception):
+    test_exception = Exception("Boom!")
+    mock_sharing_link_as_view.return_value = mock.Mock(side_effect=test_exception)
+
+    sharing_link = mock.Mock()
+    sharing_link.url = "https://example.com/1231313211/link"
+
+    with pytest.raises(IncapableVisualContextCallback) as ctx:
+        _get_html_for_sharing_link(sharing_link)
+
+    assert ctx.value.args[0] == "Was not able to get a HTML export from the sharing link"
+
+    mock_capture_exception.assert_called_once_with(test_exception)

bedrock/cms/tests/test_commands.py

@@ -7,7 +7,7 @@
 
 from django.contrib.auth.models import User
 from django.core.management import call_command
-from django.test import TransactionTestCase
+from django.test import TestCase, TransactionTestCase
 
 import everett
 
@@ -175,3 +175,24 @@ def test_multiple_emails_available_but_exist_already_somehow(self, mock_write):
                 call("User test3@mozilla.com already exists - not creating\n"),
             ],
         )
+
+
+class SmartlingSyncTests(TestCase):
+    @patch("bedrock.cms.management.commands.run_smartling_sync.call_command")
+    def test_sentry_logging_for_run_smartling_sync_command(self, mock_call_command):
+        test_exception = Exception("Boom!")
+        mock_call_command.side_effect = test_exception
+        with patch("bedrock.cms.management.commands.run_smartling_sync.capture_exception") as mock_capture_exception:
+            call_command("run_smartling_sync")
+        mock_capture_exception.assert_called_once_with(test_exception)
+
+    @patch("bedrock.cms.management.commands.bootstrap_local_admin.sys.stderr.write")
+    @patch("bedrock.cms.management.commands.run_smartling_sync.call_command")
+    def test_error_messaging_for_run_smartling_sync_command(self, mock_call_command, mock_stderr_write):
+        test_exception = Exception("Boom!")
+        mock_call_command.side_effect = test_exception
+        call_command("run_smartling_sync")
+
+        expected_output = "\nsync_smartling did not execute successfully: Boom!\n"
+        output = mock_stderr_write.call_args_list[0][0][0]
+        self.assertEqual(output, expected_output)

bedrock/cms/wagtail_localize_smartling/callbacks.py

@@ -18,19 +18,24 @@
 
 if TYPE_CHECKING:
     from wagtail_localize_smartling.models import Job
-
+from sentry_sdk import capture_exception, capture_message
 from wagtaildraftsharing.models import WagtaildraftsharingLink
 from wagtaildraftsharing.views import SharingLinkView
 
 
 def _get_html_for_sharing_link(sharing_link: WagtaildraftsharingLink) -> str:
     request = RequestFactory().get(sharing_link.url)
     view_func = SharingLinkView.as_view()
-    resp = view_func(
-        request=request,
-        key=sharing_link.key,
-    )
-    return resp.content.decode("utf-8")
+    try:
+        resp = view_func(
+            request=request,
+            key=sharing_link.key,
+        )
+        return resp.content.decode("utf-8")
+    except Exception as ex:
+        # Ensure Sentry gets any problem with turning the sharing link into HTML
+        capture_exception(ex)
+        raise IncapableVisualContextCallback("Was not able to get a HTML export from the sharing link")
 
 
 def _get_full_url_for_sharing_link(sharing_link: WagtaildraftsharingLink, page: "Page") -> str:
@@ -49,16 +54,25 @@ def visual_context(smartling_job: "Job") -> tuple[str, str]:
         # We can currently only supply visual context for Pages, but not for
         # other things like Snippets, so return early and show there's nothing
         # to be processed
-        raise IncapableVisualContextCallback("Object was not visually previewable")
+        raise IncapableVisualContextCallback("Object was not visually previewable (i.e. not a Page)")
 
     revision = content_obj.latest_revision
-
-    sharing_link = WagtaildraftsharingLink.objects.get_or_create_for_revision(
+    if revision is None:
+        # The only time we'll have a situation like this is when someone is using
+        # a DB export from dev/stage/prod, which has all of its revision history
+        # excluded from the export.
+        capture_message(f"Unable to get a latest_revision for {content_obj} so unable to send visual context.")
+        raise IncapableVisualContextCallback(
+            "Object was not visually previewable because it didn't have a saved revision. Are you a developer with a local export?"
+        )
+
+    # Always use `create_for_revision` to ensure max lifespan of the link
+    sharing_link = WagtaildraftsharingLink.objects.create_for_revision(
         revision=revision,
         user=smartling_job.user,
+        max_ttl=-1,  # -1 signifies "No expiry". If we pass None we get the default TTL
     )
 
     url = _get_full_url_for_sharing_link(sharing_link=sharing_link, page=content_obj)
     html = _get_html_for_sharing_link(sharing_link=sharing_link)
-
     return (url, html)

bedrock/settings/base.py

@@ -2325,7 +2325,7 @@ def lazy_wagtail_langs():
 WAGTAIL_CONTENT_LANGUAGES = lazy(lazy_wagtail_langs, list)()
 
 # Don't automatically make a page for a non-default locale availble in the default locale
-WAGTAILLOCALIZE_SYNC_LIVE_STATUS_ON_TRANSLATE = False
+WAGTAILLOCALIZE_SYNC_LIVE_STATUS_ON_TRANSLATE = False  # note that WAGTAILLOCALIZE is correct without the _
 
 # Settings for https://github.com/mozilla/wagtail-localize-smartling
 WAGTAIL_LOCALIZE_SMARTLING = {
@@ -2366,16 +2366,14 @@ def lazy_wagtail_langs():
     "VISUAL_CONTEXT_CALLBACK": "bedrock.cms.wagtail_localize_smartling.callbacks.visual_context",
 }
 
-WAGTAIL_DRAFTSHARING_ADMIN_MENU_POSITION = 9000
-# WAGTAIL_DRAFTSHARING_VERBOSE_NAME = "Internal Share"
-# WAGTAIL_DRAFTSHARING_VERBOSE_NAME_PLURAL = "Internal Shares"
-# WAGTAIL_DRAFTSHARING_MENU_ITEM_LABEL = "Create internal sharing link"
+WAGTAILDRAFTSHARING = {
+    "ADMIN_MENU_POSITION": 9000,
+    # MAX_TTL: 14 * 24 * 60 * 60
+    # VERBOSE_NAME: "Internal Share"
+    # VERBOSE_NAME_PLURAL: "Internal Shares"
+    # MENU_ITEM_LABEL: "Create internal sharing link"
+}
 
-# At the moment, wagtaildraftsharing's expiry is only set via settings. We're using
-# 21 days here because the links are used to send a preview of the source page to
-# Smartling translators, and we want to give them plenty of time. In the future
-# we can bring this back down to 7 days and set a Smartling-specific one of 21
-WAGTAILDRAFTSHARING_MAX_AGE = 21 * 24 * 60 * 60
 
 # Custom settings, not a core Wagtail ones, to scope out RichText options
 WAGTAIL_RICHTEXT_FEATURES_FULL = [

docs/cms.rst

@@ -109,7 +109,8 @@ sync down any images you don't already have.
   or ``AWS_DB_S3_BUCKET=bedrock-db-prod``.
 
   ``AWS_DB_S3_BUCKET=bedrock-db-stage make preflight``
-  ``AWS_DB_S3_BUCKET=bedrock-db-stage python manage.py download_media_to_local``
+
+  ``python manage.py download_media_to_local --environment=stage``
 
 Adding new content surfaces
 ===========================

requirements/dev.txt

@@ -2306,12 +2306,12 @@ wagtail-localize==1.10 \
     # via
     #   -r requirements/prod.txt
     #   wagtail-localize-smartling
-wagtail-localize-smartling==0.7.0 \
-    --hash=sha256:b9551683f5865dce45576ebfc00b5ad8eba3589afce231f74c4b57d62494be17 \
-    --hash=sha256:d9fe19ae24b025b773a27b6f43ac891b836b3e36e032271f6005c8e580cbfd19
+wagtail-localize-smartling==0.8.0 \
+    --hash=sha256:9a4842e51805614f22a12cb46f2f35379993680c71c66ca391a8068aa17ef210 \
+    --hash=sha256:e6ca356ce2310344a288cc1e85c1a280e696064d9c3a4559f0a2d12704083477
     # via -r requirements/prod.txt
-wagtaildraftsharing @ https://github.com/mozmeao/wagtaildraftsharing/archive/refs/tags/mozmeao-0.1.3.tar.gz#egg=wagtaildraftsharing \
-    --hash=sha256:3ffe076b0c3b99e71bd1f0d9a02831f4413577f60b38fc258633fc3602b8409d
+wagtaildraftsharing @ https://github.com/mozmeao/wagtaildraftsharing/archive/refs/tags/mozmeao-0.2.0.tar.gz#egg=wagtaildraftsharing \
+    --hash=sha256:7a3399c4d621628e6458f1ee3236b7d1d5fdd9577ee385d52e69edf7379f7562
     # via -r requirements/prod.txt
 wand==0.6.13 \
     --hash=sha256:e5dda0ac2204a40c29ef5c4cb310770c95d3d05c37b1379e69c94ea79d7d19c0 \

requirements/prod.in

@@ -56,8 +56,8 @@ sentry-processor==0.0.1
 sentry-sdk==2.18.0
 supervisor==4.2.5
 timeago==1.0.16
-https://github.com/mozmeao/wagtaildraftsharing/archive/refs/tags/mozmeao-0.1.3.tar.gz#egg=wagtaildraftsharing
-wagtail-localize-smartling==0.7.0
+https://github.com/mozmeao/wagtaildraftsharing/archive/refs/tags/mozmeao-0.2.0.tar.gz#egg=wagtaildraftsharing
+wagtail-localize-smartling==0.8.0
 wagtail-localize==1.10
 Wand==0.6.13  # For animated GIF support
 Wagtail==6.1.3

requirements/prod.txt

@@ -1627,12 +1627,12 @@ wagtail-localize==1.10 \
     # via
     #   -r requirements/prod.in
     #   wagtail-localize-smartling
-wagtail-localize-smartling==0.7.0 \
-    --hash=sha256:b9551683f5865dce45576ebfc00b5ad8eba3589afce231f74c4b57d62494be17 \
-    --hash=sha256:d9fe19ae24b025b773a27b6f43ac891b836b3e36e032271f6005c8e580cbfd19
+wagtail-localize-smartling==0.8.0 \
+    --hash=sha256:9a4842e51805614f22a12cb46f2f35379993680c71c66ca391a8068aa17ef210 \
+    --hash=sha256:e6ca356ce2310344a288cc1e85c1a280e696064d9c3a4559f0a2d12704083477
     # via -r requirements/prod.in
-wagtaildraftsharing @ https://github.com/mozmeao/wagtaildraftsharing/archive/refs/tags/mozmeao-0.1.3.tar.gz#egg=wagtaildraftsharing \
-    --hash=sha256:3ffe076b0c3b99e71bd1f0d9a02831f4413577f60b38fc258633fc3602b8409d
+wagtaildraftsharing @ https://github.com/mozmeao/wagtaildraftsharing/archive/refs/tags/mozmeao-0.2.0.tar.gz#egg=wagtaildraftsharing \
+    --hash=sha256:7a3399c4d621628e6458f1ee3236b7d1d5fdd9577ee385d52e69edf7379f7562
     # via -r requirements/prod.in
 wand==0.6.13 \
     --hash=sha256:e5dda0ac2204a40c29ef5c4cb310770c95d3d05c37b1379e69c94ea79d7d19c0 \