From 7bceb65d232c20a77338d10e9b27fdfe2a16f7b6 Mon Sep 17 00:00:00 2001 From: Anthony Date: Tue, 12 Nov 2024 04:59:22 -0800 Subject: [PATCH] Fix site-wide new dashboard notification (#11543) Notification on new dashboard: ![image](https://github.com/user-attachments/assets/1cc4c608-94c3-4a2f-bab1-6e4622fe6c72) Notification on old dashboard: ![image](https://github.com/user-attachments/assets/3dbe24a1-9597-418c-82b7-c7962f2c969b) ---- - Fixes #11209 - Requires https://github.com/readthedocs/ext-theme/pull/524 - Refs https://github.com/readthedocs/website/pull/335 --- readthedocs/builds/views.py | 24 ++------- readthedocs/core/context_processors.py | 3 ++ readthedocs/core/notifications.py | 50 ++++++++++++------- readthedocs/projects/views/private.py | 11 +++- .../rtd_tests/tests/test_notifications.py | 1 + readthedocs/settings/base.py | 6 +++ 6 files changed, 55 insertions(+), 40 deletions(-) diff --git a/readthedocs/builds/views.py b/readthedocs/builds/views.py index f88c3320d68..5a77fb3064e 100644 --- a/readthedocs/builds/views.py +++ b/readthedocs/builds/views.py @@ -192,27 +192,9 @@ def get_context_data(self, **kwargs): build = self.get_object() - # Temporary notification to point to the same page on the new dashboard - # - # To support readthedocs.com, we have to point to the login view. We - # can't point directly to the build view on the new dashboard as this - # will give the users a 404 because they aren't logged in. - # - # On community, we _don't want this_ as this requires the user to have - # a login to view the new dashboard. - url_domain = settings.PRODUCTION_DOMAIN - if url_domain.startswith("app."): - url_domain = url_domain[4:] - else: - url_domain = f"app.{url_domain}" - url_build = build.get_absolute_url() - # Point to the login view with the build as ?next. We are expecting - # users to have accounts to view this. - if settings.RTD_ALLOW_ORGANIZATIONS: - url_build = reverse("account_login") + f"?next={url_build}" - context["url_switch_dashboard"] = f"https://{url_domain}{url_build}" - - context["notifications"] = build.notifications.all() + # We consume these notifications through the API in the new dashboard + if not settings.RTD_EXT_THEME_ENABLED: + context["notifications"] = build.notifications.all() if not build.notifications.filter( message_id=BuildAppError.GENERIC_WITH_BUILD_ID ).exists(): diff --git a/readthedocs/core/context_processors.py b/readthedocs/core/context_processors.py index 34bb53042c4..43b48299d9c 100644 --- a/readthedocs/core/context_processors.py +++ b/readthedocs/core/context_processors.py @@ -13,9 +13,12 @@ def readthedocs_processor(request): If you need to add something that depends on the request, create a new context processor. """ + exports = { "PUBLIC_DOMAIN": settings.PUBLIC_DOMAIN, "PRODUCTION_DOMAIN": settings.PRODUCTION_DOMAIN, + # TODO this can be removed with RTD_EXT_THEME_ENABLED + "SWITCH_PRODUCTION_DOMAIN": settings.SWITCH_PRODUCTION_DOMAIN, "GLOBAL_ANALYTICS_CODE": settings.GLOBAL_ANALYTICS_CODE, "DASHBOARD_ANALYTICS_CODE": settings.DASHBOARD_ANALYTICS_CODE, "SITE_ROOT": settings.SITE_ROOT + "/", diff --git a/readthedocs/core/notifications.py b/readthedocs/core/notifications.py index 70405f3249f..d946633538e 100644 --- a/readthedocs/core/notifications.py +++ b/readthedocs/core/notifications.py @@ -4,11 +4,11 @@ from django.utils.translation import gettext_lazy as _ -from readthedocs.notifications.constants import INFO, WARNING +from readthedocs.notifications.constants import TIP, WARNING from readthedocs.notifications.messages import Message, registry MESSAGE_EMAIL_VALIDATION_PENDING = "core:email:validation-pending" -MESSAGE_BETA_DASHBOARD_AVAILABLE = "core:dashboard:beta-available" +MESSAGE_NEW_DASHBOARD = "core:dashboard:new" messages = [ Message( id=MESSAGE_EMAIL_VALIDATION_PENDING, @@ -23,24 +23,38 @@ ), type=WARNING, ), + # This message looks quite odd because we need to show different content in + # the notification depending on which instance the user is on -- if the user + # is on our legacy dashboard, we don't want a notification "Welcome to our + # new dashboard!". + # + # Localization is avoided because the body has template logic inside and we + # don't want to push that to our translations sources. Message( - id=MESSAGE_BETA_DASHBOARD_AVAILABLE, - header=_("New beta dashboard"), - body=_( - textwrap.dedent( - """ - {% if RTD_EXT_THEME_ENABLED %} - This dashboard is currently in beta, - you can return to the legacy dashboard if you encounter any problems. - Feel free to report any feedback you may have. - {% else %} - Our new beta dashboard is now available for testing. - Give it a try and send us feedback. - {% endif %} + id=MESSAGE_NEW_DASHBOARD, + header=textwrap.dedent( """ - ).strip(), - ), - type=INFO, + {% if RTD_EXT_THEME_ENABLED %} + Welcome to our new dashboard! + {% else %} + Our new dashboard is ready! + {% endif %} + """ + ).strip(), + body=textwrap.dedent( + """ + {% if RTD_EXT_THEME_ENABLED %} + We are beginning to direct users to our new dashboard as we work to retire our legacy dashboard. + {% else %} + You are currently using our legacy dashboard, which will be retired on . + You should switch to our new dashboard before then. + {% endif %} + For more information on this change and what to expect, + read our blog post. + """ + ).strip(), + type=TIP, + icon_classes="fad fa-sparkles", ), ] diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 98b9a8eeeda..9efa0db13af 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -123,7 +123,16 @@ def get_context_data(self, **kwargs): template_name = None projects = AdminPermission.projects(user=self.request.user, admin=True) n_projects = projects.count() - if n_projects == 0 or ( + + # TODO remove this with RTD_EXT_THEME_ENABLED + # This is going to try hard to show the new dashboard announcement. + # We can't yet back down to another announcement as we don't have + # the ability to evaluate local storage. Until we add the ability to + # dynamically change the announcement, this is going to be the only + # announcement shown. + if True: # pylint: disable=using-constant-test + template_name = "new-dashboard.html" + elif n_projects == 0 or ( n_projects < 3 and (timezone.now() - projects.first().pub_date).days < 7 ): template_name = "example-projects.html" diff --git a/readthedocs/rtd_tests/tests/test_notifications.py b/readthedocs/rtd_tests/tests/test_notifications.py index 8727767e680..cb1d3912e2a 100644 --- a/readthedocs/rtd_tests/tests/test_notifications.py +++ b/readthedocs/rtd_tests/tests/test_notifications.py @@ -54,6 +54,7 @@ class TestNotification(EmailNotification): "DO_NOT_TRACK_ENABLED": mock.ANY, "GLOBAL_ANALYTICS_CODE": mock.ANY, "PRODUCTION_DOMAIN": "readthedocs.org", + "SWITCH_PRODUCTION_DOMAIN": "app.readthedocs.org", "PUBLIC_DOMAIN": mock.ANY, "PUBLIC_API_URL": mock.ANY, "RTD_EXT_THEME_ENABLED": mock.ANY, diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index de5b7723ef7..6fde3d02f53 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -86,6 +86,12 @@ def SHOW_DEBUG_TOOLBAR(self): RTD_INTERSPHINX_URL = "https://{}".format(PRODUCTION_DOMAIN) RTD_EXTERNAL_VERSION_DOMAIN = "external-builds.readthedocs.io" + @property + def SWITCH_PRODUCTION_DOMAIN(self): + if self.RTD_EXT_THEME_ENABLED: + return self.PRODUCTION_DOMAIN.removeprefix("app.") + return f"app.{self.PRODUCTION_DOMAIN}" + # Doc Builder Backends MKDOCS_BACKEND = "readthedocs.doc_builder.backends.mkdocs" SPHINX_BACKEND = "readthedocs.doc_builder.backends.sphinx"