diff --git a/bedrock/cms/management/commands/run_smartling_sync.py b/bedrock/cms/management/commands/run_smartling_sync.py index 15beb235e04..ba6b7433828 100644 --- a/bedrock/cms/management/commands/run_smartling_sync.py +++ b/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) diff --git a/bedrock/cms/tests/test_callbacks.py b/bedrock/cms/tests/test_callbacks.py index d34c5ba9016..d4c815565bd 100644 --- a/bedrock/cms/tests/test_callbacks.py +++ b/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) diff --git a/bedrock/cms/tests/test_commands.py b/bedrock/cms/tests/test_commands.py index 69bc1e111b9..58d0705b46d 100644 --- a/bedrock/cms/tests/test_commands.py +++ b/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) diff --git a/bedrock/cms/wagtail_localize_smartling/callbacks.py b/bedrock/cms/wagtail_localize_smartling/callbacks.py index 57e5ba3c812..faf80e2c9f1 100644 --- a/bedrock/cms/wagtail_localize_smartling/callbacks.py +++ b/bedrock/cms/wagtail_localize_smartling/callbacks.py @@ -18,7 +18,7 @@ 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 @@ -26,11 +26,16 @@ 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) diff --git a/bedrock/settings/base.py b/bedrock/settings/base.py index 63947ed3d07..c2580825d1c 100644 --- a/bedrock/settings/base.py +++ b/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 = [ diff --git a/docs/cms.rst b/docs/cms.rst index 9d7ead46e19..f6f51e38f7c 100644 --- a/docs/cms.rst +++ b/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 =========================== diff --git a/requirements/dev.txt b/requirements/dev.txt index 4caf69d99bf..7c94119b872 100644 --- a/requirements/dev.txt +++ b/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 \ diff --git a/requirements/prod.in b/requirements/prod.in index c8c1fcd4d68..131a861b160 100644 --- a/requirements/prod.in +++ b/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 diff --git a/requirements/prod.txt b/requirements/prod.txt index 59c5626914e..b9548b5062b 100644 --- a/requirements/prod.txt +++ b/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 \