From f4a14422261e0950dbb04a64b1259fb9a44af443 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 11 Mar 2024 16:42:37 +0100 Subject: [PATCH 1/6] Build: remove `append_conf` _magic_ from MkDocs Delete all the magic around MkDocs YAML that processes the `mkdocs.yml`. file and define `readthedocs` theme automatically based on a feature flag, `MKDOCS_THEME_RTD`. This PR removes: - automatically defining `readthedocs` as theme when `MKDOCS_THEME_RTD` feature flag is defined. - processing `mkdocs.yml` to add internal JS and CSS (embed and analytics) files automatically This is another step forward on removing all the magic we perform on behalf the users and being more explicit about how to configure each doctool. Reference: * https://github.com/readthedocs/addons/issues/72#issuecomment-1926647293 Closes #8529 --- readthedocs/config/tests/test_yaml_loader.py | 11 +- readthedocs/doc_builder/backends/mkdocs.py | 246 +-------- readthedocs/projects/tests/mockers.py | 3 - .../rtd_tests/tests/test_doc_builder.py | 506 +----------------- 4 files changed, 17 insertions(+), 749 deletions(-) diff --git a/readthedocs/config/tests/test_yaml_loader.py b/readthedocs/config/tests/test_yaml_loader.py index ff74f2827f9..f5fe980b09b 100644 --- a/readthedocs/config/tests/test_yaml_loader.py +++ b/readthedocs/config/tests/test_yaml_loader.py @@ -1,8 +1,4 @@ -from readthedocs.doc_builder.backends.mkdocs import ( - ProxyPythonName, - yaml_dump_safely, - yaml_load_safely, -) +from readthedocs.doc_builder.backends.mkdocs import ProxyPythonName, yaml_load_safely content = """ int: 3 @@ -30,8 +26,3 @@ def test_yaml_load_safely(): assert type(data["other_function"]) is ProxyPythonName assert data["function"].value == "python_function" assert data["other_function"].value == "module.other.function" - - -def test_yaml_dump_safely(): - data = yaml_load_safely(content) - assert yaml_load_safely(yaml_dump_safely(data)) == data diff --git a/readthedocs/doc_builder/backends/mkdocs.py b/readthedocs/doc_builder/backends/mkdocs.py index cd4a256e0b1..16276a9dcd2 100644 --- a/readthedocs/doc_builder/backends/mkdocs.py +++ b/readthedocs/doc_builder/backends/mkdocs.py @@ -9,14 +9,10 @@ import structlog import yaml from django.conf import settings -from django.template import loader as template_loader from readthedocs.core.utils.filesystem import safe_open from readthedocs.doc_builder.base import BaseBuilder -from readthedocs.doc_builder.exceptions import MkDocsYAMLParseError from readthedocs.projects.constants import MKDOCS, MKDOCS_HTML -from readthedocs.projects.exceptions import UserFileNotFound -from readthedocs.projects.models import Feature log = structlog.get_logger(__name__) @@ -49,31 +45,23 @@ def __init__(self, *args, **kwargs): # This is the *MkDocs* yaml file self.yaml_file = self.get_yaml_config() - # README: historically, the default theme was ``readthedocs`` but in - # https://github.com/rtfd/readthedocs.org/pull/4556 we change it to - # ``mkdocs`` to maintain the same behavior in Read the Docs than - # building locally. Although, we can't apply this into the Corporate - # site. To keep the same default theme there, we created a Feature flag - # for these project that were building with MkDocs in the Corporate - # site. - if self.project.has_feature(Feature.MKDOCS_THEME_RTD): - self.DEFAULT_THEME_NAME = "readthedocs" - log.warning( - "Project using readthedocs theme as default for MkDocs.", - project_slug=self.project.slug, - ) - else: - self.DEFAULT_THEME_NAME = "mkdocs" - def get_final_doctype(self): """ Select a doctype based on the ``use_directory_urls`` setting. https://www.mkdocs.org/user-guide/configuration/#use_directory_urls """ + + # TODO: we should eventually remove this method completely and stop + # relying on "loading the `mkdocs.yml` file in a safe way just to know + # if it's a MKDOCS or MKDOCS_HTML documentation type". + # Allow symlinks, but only the ones that resolve inside the base directory. with safe_open( - self.yaml_file, "r", allow_symlinks=True, base_path=self.project_path + self.yaml_file, + "r", + allow_symlinks=True, + base_path=self.project_path, ) as fh: config = yaml_load_safely(fh) use_directory_urls = config.get("use_directory_urls", True) @@ -89,135 +77,16 @@ def get_yaml_config(self): mkdocs_path, ) - def load_yaml_config(self): - """ - Load a YAML config. - - :raises: ``MkDocsYAMLParseError`` if failed due to syntax errors. - """ - try: - # Allow symlinks, but only the ones that resolve inside the base directory. - result = safe_open( - self.yaml_file, "r", allow_symlinks=True, base_path=self.project_path - ) - if not result: - raise UserFileNotFound( - message_id=UserFileNotFound.FILE_NOT_FOUND, - format_values={ - "filename": self.yaml_file, - }, - ) - - config = yaml_load_safely(result) - - if not config: - raise MkDocsYAMLParseError(MkDocsYAMLParseError.EMPTY_CONFIG) - if not isinstance(config, dict): - raise MkDocsYAMLParseError(MkDocsYAMLParseError.CONFIG_NOT_DICT) - return config - - except IOError: - raise MkDocsYAMLParseError(MkDocsYAMLParseError.NOT_FOUND) - except yaml.YAMLError as exc: - note = "" - if hasattr(exc, "problem_mark"): - mark = exc.problem_mark - note = " (line %d, column %d)" % ( - mark.line + 1, - mark.column + 1, - ) - raise MkDocsYAMLParseError( - MkDocsYAMLParseError.SYNTAX_ERROR, - ) from exc - def append_conf(self): """ - Set mkdocs config values. + Call `cat mkdocs.yaml` only. - :raises: ``MkDocsYAMLParseError`` if failed due to known type errors - (i.e. expecting a list and a string is found). - """ - user_config = self.load_yaml_config() - - # Handle custom docs dirs - docs_dir = user_config.get("docs_dir", "docs") - if not isinstance(docs_dir, (type(None), str)): - raise MkDocsYAMLParseError( - MkDocsYAMLParseError.INVALID_DOCS_DIR_CONFIG, - ) - - user_config["docs_dir"] = docs_dir - static_url = self.project.proxied_static_path - - # Set mkdocs config values. - extra_assets = { - "extra_javascript": [ - "readthedocs-data.js", - f"{static_url}core/js/readthedocs-doc-embed.js", - f"{static_url}javascript/readthedocs-analytics.js", - ], - "extra_css": [ - f"{static_url}css/badge_only.css", - f"{static_url}css/readthedocs-doc-embed.css", - ], - } - - for config, extras in extra_assets.items(): - value = user_config.get(config, []) - if value is None: - value = [] - if not isinstance(value, list): - raise MkDocsYAMLParseError( - message_id=MkDocsYAMLParseError.INVALID_EXTRA_CONFIG, - format_values={ - "extra_config": config, - }, - ) - # Add the static file only if isn't already in the list. - value.extend([extra for extra in extras if extra not in value]) - user_config[config] = value - - # The docs path is relative to the location - # of the mkdocs configuration file. - docs_path = os.path.join( - os.path.dirname(self.yaml_file), - docs_dir, - ) - - # if user puts an invalid `docs_dir` path raise an Exception - if not os.path.exists(docs_path): - raise MkDocsYAMLParseError( - MkDocsYAMLParseError.INVALID_DOCS_DIR_PATH, - ) - - # RTD javascript writing - rtd_data = self.generate_rtd_data( - docs_dir=os.path.relpath(docs_path, self.project_path), - mkdocs_config=user_config, - ) - with safe_open( - os.path.join(docs_path, "readthedocs-data.js"), "w", encoding="utf-8" - ) as f: - f.write(rtd_data) - - # Use Read the Docs' analytics setup rather than mkdocs' - # This supports using RTD's privacy improvements around analytics - user_config["google_analytics"] = None - - # README: make MkDocs to use ``readthedocs`` theme as default if the - # user didn't specify a specific theme manually - if self.project.has_feature(Feature.MKDOCS_THEME_RTD): - if "theme" not in user_config: - # mkdocs<0.17 syntax - user_config["theme"] = self.DEFAULT_THEME_NAME - - # Write the modified mkdocs configuration - with safe_open(self.yaml_file, "w", encoding="utf-8") as f: - yaml_dump_safely( - user_config, - f, - ) + This behavior has changed. We used to parse the YAML file and append + some configs automatically, but we have been removing that magic from + our builders as much as we can. + This method will eventually removed completely. + """ # Write the mkdocs.yml to the build logs self.run( "cat", @@ -225,56 +94,6 @@ def append_conf(self): cwd=self.project_path, ) - def generate_rtd_data(self, docs_dir, mkdocs_config): - """Generate template properties and render readthedocs-data.js.""" - # Use the analytics code from mkdocs.yml - # if it isn't set already by Read the Docs, - analytics_code = self.version.project.analytics_code - if not analytics_code and mkdocs_config.get("google_analytics"): - # http://www.mkdocs.org/user-guide/configuration/#google_analytics - analytics_code = mkdocs_config["google_analytics"][0] - - commit = ( - self.version.project.vcs_repo( - version=self.version.slug, - environment=self.build_env, - ).commit, - ) - - # Will be available in the JavaScript as READTHEDOCS_DATA. - readthedocs_data = { - "project": self.version.project.slug, - "version": self.version.slug, - "language": self.version.project.language, - "programming_language": self.version.project.programming_language, - "page": None, - "theme": self.get_theme_name(mkdocs_config), - "builder": "mkdocs", - "docroot": docs_dir, - "source_suffix": ".md", - "api_host": settings.PUBLIC_API_URL, - "ad_free": not self.project.show_advertising, - "commit": commit, - "global_analytics_code": ( - None - if self.project.analytics_disabled - else settings.GLOBAL_ANALYTICS_CODE - ), - "user_analytics_code": analytics_code, - "proxied_static_path": self.project.proxied_static_path, - "proxied_api_host": self.project.proxied_api_host, - } - - data_ctx = { - "readthedocs_data": readthedocs_data, - "current_version": readthedocs_data["version"], - "slug": readthedocs_data["project"], - "html_theme": readthedocs_data["theme"], - "pagename": None, - } - tmpl = template_loader.get_template("doc_builder/data.js.tmpl") - return tmpl.render(data_ctx) - def build(self): build_command = [ self.python_env.venv_bin(filename="python"), @@ -296,42 +115,12 @@ def build(self): ) return cmd_ret.successful - def get_theme_name(self, mkdocs_config): - """ - Get the theme configuration in the mkdocs_config. - - In v0.17.0, the theme configuration switched - from two separate configs (both optional) to a nested directive. - - :see: http://www.mkdocs.org/about/release-notes/#theme-customization-1164 - :returns: the name of the theme RTD will use - """ - theme_setting = mkdocs_config.get("theme") - if isinstance(theme_setting, dict): - # Full nested theme config (the new configuration) - return theme_setting.get("name") or self.DEFAULT_THEME_NAME - - if theme_setting: - # A string which is the name of the theme - return theme_setting - - theme_dir = mkdocs_config.get("theme_dir") - if theme_dir: - # Use the name of the directory in this project's custom theme directory - return theme_dir.rstrip("/").split("/")[-1] - - return self.DEFAULT_THEME_NAME - class MkdocsHTML(BaseMkdocs): builder = "build" build_dir = "_readthedocs/html" -# TODO: find a better way to integrate with MkDocs. -# See https://github.com/readthedocs/readthedocs.org/issues/7844 - - class ProxyPythonName(yaml.YAMLObject): def __init__(self, value): self.value = value @@ -389,8 +178,3 @@ def yaml_load_safely(content): information loss. """ return yaml.load(content, Loader=SafeLoader) - - -def yaml_dump_safely(content, stream=None): - """Uses ``SafeDumper`` dumper to write YAML contents.""" - return yaml.dump(content, stream=stream, Dumper=SafeDumper) diff --git a/readthedocs/projects/tests/mockers.py b/readthedocs/projects/tests/mockers.py index 4dae7d137de..2f15f3ebb3e 100644 --- a/readthedocs/projects/tests/mockers.py +++ b/readthedocs/projects/tests/mockers.py @@ -108,9 +108,6 @@ def _mock_artifact_builders(self): "readthedocs.doc_builder.backends.sphinx.HtmlBuilder.append_conf", ) - # self.patches['builder.html.mkdocs.yaml_dump_safely'] = mock.patch( - # 'readthedocs.doc_builder.backends.mkdocs.yaml_dump_safely', - # ) # self.patches['builder.html.mkdocs.open'] = mock.patch( # 'readthedocs.doc_builder.backends.mkdocs.builtins.open', # mock.mock_open(read_data='file content'), diff --git a/readthedocs/rtd_tests/tests/test_doc_builder.py b/readthedocs/rtd_tests/tests/test_doc_builder.py index 40367c2cffd..d21d72be8eb 100644 --- a/readthedocs/rtd_tests/tests/test_doc_builder.py +++ b/readthedocs/rtd_tests/tests/test_doc_builder.py @@ -5,24 +5,14 @@ import py import pytest -import yaml from django.test import TestCase from django.test.utils import override_settings -from django_dynamic_fixture import get -from readthedocs.builds.models import Version from readthedocs.config.tests.test_config import get_build_config -from readthedocs.doc_builder.backends.mkdocs import ( - MkdocsHTML, - SafeDumper, - yaml_load_safely, -) from readthedocs.doc_builder.backends.sphinx import BaseSphinx -from readthedocs.doc_builder.environments import LocalBuildEnvironment -from readthedocs.doc_builder.exceptions import MkDocsYAMLParseError from readthedocs.doc_builder.python_environments import Virtualenv from readthedocs.projects.exceptions import ProjectConfigurationError -from readthedocs.projects.models import Feature, Project +from readthedocs.projects.models import Project @override_settings(PRODUCTION_DOMAIN="readthedocs.org") @@ -163,497 +153,3 @@ def test_multiple_conf_py( with pytest.raises(ProjectConfigurationError): with override_settings(DOCROOT=tmp_docs_dir): base_sphinx.append_conf() - - -@override_settings(PRODUCTION_DOMAIN="readthedocs.org") -class MkdocsBuilderTest(TestCase): - def setUp(self): - self.project = get(Project, documentation_type="mkdocs", name="mkdocs") - self.version = get(Version, project=self.project) - - self.build_env = LocalBuildEnvironment(api_client=mock.MagicMock()) - self.build_env.project = self.project - self.build_env.version = self.version - - @patch("readthedocs.projects.models.Project.checkout_path") - def test_get_theme_name(self, checkout_path): - tmpdir = tempfile.mkdtemp() - checkout_path.return_value = tmpdir - python_env = Virtualenv( - version=self.version, - build_env=self.build_env, - config=get_build_config( - {"mkdocs": {"configuration": "mkdocs.yml"}}, validate=True - ), - ) - builder = MkdocsHTML( - build_env=self.build_env, - python_env=python_env, - ) - - # The default theme is mkdocs but in mkdocs>=1.0, theme is required - self.assertEqual(builder.get_theme_name({}), "mkdocs") - - # mkdocs<0.17 syntax - config = { - "theme": "readthedocs", - } - self.assertEqual(builder.get_theme_name(config), "readthedocs") - - # mkdocs>=0.17 syntax - config = { - "theme": { - "name": "test_theme", - }, - } - self.assertEqual(builder.get_theme_name(config), "test_theme") - - # No theme but just a directory - config = { - "theme_dir": "/path/to/mydir", - } - self.assertEqual(builder.get_theme_name(config), "mydir") - config = { - "theme_dir": "/path/to/mydir/", - } - self.assertEqual(builder.get_theme_name(config), "mydir") - - @patch("readthedocs.doc_builder.base.BaseBuilder.run") - @patch("readthedocs.projects.models.Project.checkout_path") - def test_get_theme_name_with_feature_flag(self, checkout_path, run): - tmpdir = tempfile.mkdtemp() - checkout_path.return_value = tmpdir - - feature = get( - Feature, - feature_id=Feature.MKDOCS_THEME_RTD, - ) - feature.projects.add(self.project) - - python_env = Virtualenv( - version=self.version, - build_env=self.build_env, - config=get_build_config( - {"mkdocs": {"configuration": "mkdocs.yml"}}, validate=True - ), - ) - builder = MkdocsHTML( - build_env=self.build_env, - python_env=python_env, - ) - self.assertEqual(builder.get_theme_name({}), "readthedocs") - with patch("readthedocs.doc_builder.backends.mkdocs.yaml") as mock_yaml: - with patch( - "readthedocs.doc_builder.backends.mkdocs.MkdocsHTML.load_yaml_config" - ) as mock_load_yaml_config: - mock_load_yaml_config.return_value = { - "site_name": self.project.name, - "docs_dir": tmpdir, - } - with override_settings(DOCROOT=tmpdir): - builder.append_conf() - - mock_yaml.dump.assert_called_once_with( - { - "site_name": mock.ANY, - "docs_dir": mock.ANY, - "extra_javascript": mock.ANY, - "extra_css": mock.ANY, - "google_analytics": mock.ANY, - "theme": "readthedocs", - }, - stream=mock.ANY, - Dumper=SafeDumper, - ) - mock_yaml.reset_mock() - - config = { - "theme": "customtheme", - } - self.assertEqual(builder.get_theme_name(config), "customtheme") - with patch( - "readthedocs.doc_builder.backends.mkdocs.MkdocsHTML.load_yaml_config" - ) as mock_load_yaml_config: - mock_load_yaml_config.return_value = { - "site_name": self.project.name, - "theme": "customtheme", - "docs_dir": tmpdir, - } - with override_settings(DOCROOT=tmpdir): - builder.append_conf() - - mock_yaml.dump.assert_called_once_with( - { - "site_name": mock.ANY, - "docs_dir": mock.ANY, - "extra_javascript": mock.ANY, - "extra_css": mock.ANY, - "google_analytics": mock.ANY, - "theme": "customtheme", - }, - stream=mock.ANY, - Dumper=SafeDumper, - ) - - @patch("readthedocs.doc_builder.base.BaseBuilder.run") - @patch("readthedocs.projects.models.Project.checkout_path") - def test_append_conf_existing_yaml_on_root(self, checkout_path, run): - tmpdir = tempfile.mkdtemp() - os.mkdir(os.path.join(tmpdir, "docs")) - yaml_file = os.path.join(tmpdir, "mkdocs.yml") - yaml.safe_dump( - { - "site_name": "mkdocs", - "google_analytics": ["UA-1234-5", "mkdocs.org"], - "docs_dir": "docs", - }, - open(yaml_file, "w"), - ) - checkout_path.return_value = tmpdir - - python_env = Virtualenv( - version=self.version, - build_env=self.build_env, - config=get_build_config( - {"mkdocs": {"configuration": "mkdocs.yml"}}, validate=True - ), - ) - self.searchbuilder = MkdocsHTML( - build_env=self.build_env, - python_env=python_env, - ) - with override_settings(DOCROOT=tmpdir): - self.searchbuilder.append_conf() - - run.assert_called_with("cat", "mkdocs.yml", cwd=mock.ANY) - - config = yaml_load_safely(open(yaml_file)) - self.assertEqual( - config["docs_dir"], - "docs", - ) - self.assertEqual( - config["extra_css"], - [ - "/_/static/css/badge_only.css", - "/_/static/css/readthedocs-doc-embed.css", - ], - ) - self.assertEqual( - config["extra_javascript"], - [ - "readthedocs-data.js", - "/_/static/core/js/readthedocs-doc-embed.js", - "/_/static/javascript/readthedocs-analytics.js", - ], - ) - self.assertIsNone( - config["google_analytics"], - ) - self.assertEqual( - config["site_name"], - "mkdocs", - ) - - @patch("readthedocs.doc_builder.base.BaseBuilder.run") - @patch("readthedocs.projects.models.Project.checkout_path") - def test_append_conf_existing_yaml_on_root_with_invalid_setting( - self, checkout_path, run - ): - tmpdir = tempfile.mkdtemp() - os.mkdir(os.path.join(tmpdir, "docs")) - yaml_file = os.path.join(tmpdir, "mkdocs.yml") - checkout_path.return_value = tmpdir - - python_env = Virtualenv( - version=self.version, - build_env=self.build_env, - config=get_build_config( - {"mkdocs": {"configuration": "mkdocs.yml"}}, validate=True - ), - ) - self.searchbuilder = MkdocsHTML( - build_env=self.build_env, - python_env=python_env, - ) - - # We can't use ``@pytest.mark.parametrize`` on a Django test case - yaml_contents = [ - {"docs_dir": ["docs"]}, - {"extra_css": "a string here"}, - {"extra_javascript": ""}, - ] - for content in yaml_contents: - yaml.safe_dump( - content, - open(yaml_file, "w"), - ) - with self.assertRaises(MkDocsYAMLParseError): - with override_settings(DOCROOT=tmpdir): - self.searchbuilder.append_conf() - - @patch("readthedocs.doc_builder.base.BaseBuilder.run") - @patch("readthedocs.projects.models.Project.checkout_path") - def test_append_conf_and_none_values(self, checkout_path, run): - tmpdir = tempfile.mkdtemp() - os.mkdir(os.path.join(tmpdir, "docs")) - yaml_file = os.path.join(tmpdir, "mkdocs.yml") - checkout_path.return_value = tmpdir - - python_env = Virtualenv( - version=self.version, - build_env=self.build_env, - config=get_build_config( - {"mkdocs": {"configuration": "mkdocs.yml"}}, validate=True - ), - ) - builder = MkdocsHTML( - build_env=self.build_env, - python_env=python_env, - ) - - yaml.safe_dump( - { - "extra_css": None, - "extra_javascript": None, - }, - open(yaml_file, "w"), - ) - with override_settings(DOCROOT=tmpdir): - builder.append_conf() - config = yaml_load_safely(open(yaml_file)) - - self.assertEqual( - config["extra_css"], - [ - "/_/static/css/badge_only.css", - "/_/static/css/readthedocs-doc-embed.css", - ], - ) - self.assertEqual( - config["extra_javascript"], - [ - "readthedocs-data.js", - "/_/static/core/js/readthedocs-doc-embed.js", - "/_/static/javascript/readthedocs-analytics.js", - ], - ) - - @patch("readthedocs.doc_builder.base.BaseBuilder.run") - @patch("readthedocs.projects.models.Project.checkout_path") - def test_dont_override_theme(self, checkout_path, run): - tmpdir = tempfile.mkdtemp() - os.mkdir(os.path.join(tmpdir, "docs")) - yaml_file = os.path.join(tmpdir, "mkdocs.yml") - yaml.safe_dump( - { - "theme": "not-readthedocs", - "theme_dir": "not-readthedocs", - "site_name": "mkdocs", - "docs_dir": "docs", - }, - open(yaml_file, "w"), - ) - checkout_path.return_value = tmpdir - - python_env = Virtualenv( - version=self.version, - build_env=self.build_env, - config=get_build_config( - {"mkdocs": {"configuration": "mkdocs.yml"}}, validate=True - ), - ) - self.searchbuilder = MkdocsHTML( - build_env=self.build_env, - python_env=python_env, - ) - with override_settings(DOCROOT=tmpdir): - self.searchbuilder.append_conf() - - run.assert_called_with("cat", "mkdocs.yml", cwd=mock.ANY) - - config = yaml_load_safely(open(yaml_file)) - self.assertEqual( - config["theme_dir"], - "not-readthedocs", - ) - - @patch("readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.generate_rtd_data") - @patch("readthedocs.doc_builder.base.BaseBuilder.run") - @patch("readthedocs.projects.models.Project.checkout_path") - def test_write_js_data_docs_dir(self, checkout_path, run, generate_rtd_data): - tmpdir = tempfile.mkdtemp() - os.mkdir(os.path.join(tmpdir, "docs")) - yaml_file = os.path.join(tmpdir, "mkdocs.yml") - yaml.safe_dump( - { - "site_name": "mkdocs", - "docs_dir": "docs", - }, - open(yaml_file, "w"), - ) - checkout_path.return_value = tmpdir - generate_rtd_data.return_value = "" - - python_env = Virtualenv( - version=self.version, - build_env=self.build_env, - config=get_build_config( - {"mkdocs": {"configuration": "mkdocs.yml"}}, validate=True - ), - ) - self.searchbuilder = MkdocsHTML( - build_env=self.build_env, - python_env=python_env, - ) - with override_settings(DOCROOT=tmpdir): - self.searchbuilder.append_conf() - - generate_rtd_data.assert_called_with( - docs_dir="docs", - mkdocs_config=mock.ANY, - ) - - @patch("readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.generate_rtd_data") - @patch("readthedocs.projects.models.Project.checkout_path") - def test_write_js_data_on_invalid_docs_dir(self, checkout_path, generate_rtd_data): - tmpdir = tempfile.mkdtemp() - os.mkdir(os.path.join(tmpdir, "docs")) - yaml_file = os.path.join(tmpdir, "mkdocs.yml") - yaml.safe_dump( - { - "site_name": "mkdocs", - "google_analytics": ["UA-1234-5", "mkdocs.org"], - "docs_dir": "invalid_docs_dir", - "extra_css": ["http://readthedocs.org/static/css/badge_only.css"], - "extra_javascript": ["readthedocs-data.js"], - }, - open(yaml_file, "w"), - ) - checkout_path.return_value = tmpdir - - python_env = Virtualenv( - version=self.version, - build_env=self.build_env, - config=get_build_config( - {"mkdocs": {"configuration": "mkdocs.yml"}}, validate=True - ), - ) - self.searchbuilder = MkdocsHTML( - build_env=self.build_env, - python_env=python_env, - ) - with self.assertRaises(MkDocsYAMLParseError): - with override_settings(DOCROOT=tmpdir): - self.searchbuilder.append_conf() - - @patch("readthedocs.doc_builder.base.BaseBuilder.run") - @patch("readthedocs.projects.models.Project.checkout_path") - def test_append_conf_existing_yaml_with_extra(self, checkout_path, run): - tmpdir = tempfile.mkdtemp() - os.mkdir(os.path.join(tmpdir, "docs")) - yaml_file = os.path.join(tmpdir, "mkdocs.yml") - yaml.safe_dump( - { - "site_name": "mkdocs", - "google_analytics": ["UA-1234-5", "mkdocs.org"], - "docs_dir": "docs", - "extra_css": ["/_/static/css/badge_only.css"], - "extra_javascript": ["readthedocs-data.js"], - }, - open(yaml_file, "w"), - ) - checkout_path.return_value = tmpdir - - python_env = Virtualenv( - version=self.version, - build_env=self.build_env, - config=get_build_config( - {"mkdocs": {"configuration": "mkdocs.yml"}}, validate=True - ), - ) - self.searchbuilder = MkdocsHTML( - build_env=self.build_env, - python_env=python_env, - ) - with override_settings(DOCROOT=tmpdir): - self.searchbuilder.append_conf() - - run.assert_called_with("cat", "mkdocs.yml", cwd=mock.ANY) - - config = yaml_load_safely(open(yaml_file)) - - self.assertEqual( - config["extra_css"], - [ - "/_/static/css/badge_only.css", - "/_/static/css/readthedocs-doc-embed.css", - ], - ) - self.assertEqual( - config["extra_javascript"], - [ - "readthedocs-data.js", - "/_/static/core/js/readthedocs-doc-embed.js", - "/_/static/javascript/readthedocs-analytics.js", - ], - ) - - @patch("readthedocs.projects.models.Project.checkout_path") - def test_empty_yaml_config(self, checkout_path): - tmpdir = tempfile.mkdtemp() - os.mkdir(os.path.join(tmpdir, "docs")) - yaml_file = os.path.join(tmpdir, "mkdocs.yml") - yaml.safe_dump( - "", - open(yaml_file, "w"), - ) - checkout_path.return_value = tmpdir - - python_env = Virtualenv( - version=self.version, - build_env=self.build_env, - config=get_build_config( - {"mkdocs": {"configuration": "mkdocs.yml"}}, validate=True - ), - ) - self.searchbuilder = MkdocsHTML( - build_env=self.build_env, - python_env=python_env, - ) - - with self.assertRaises(MkDocsYAMLParseError) as exc: - with override_settings(DOCROOT=tmpdir): - self.searchbuilder.append_conf() - self.assertEqual(exc.exception.message_id, MkDocsYAMLParseError.EMPTY_CONFIG) - - @patch("readthedocs.projects.models.Project.checkout_path") - def test_yaml_config_not_returns_dict(self, checkout_path): - tmpdir = tempfile.mkdtemp() - os.mkdir(os.path.join(tmpdir, "docs")) - yaml_file = os.path.join(tmpdir, "mkdocs.yml") - yaml.safe_dump( - "test_string", - open(yaml_file, "w"), - ) - checkout_path.return_value = tmpdir - - python_env = Virtualenv( - version=self.version, - build_env=self.build_env, - config=get_build_config( - {"mkdocs": {"configuration": "mkdocs.yml"}}, validate=True - ), - ) - self.searchbuilder = MkdocsHTML( - build_env=self.build_env, - python_env=python_env, - ) - - with self.assertRaises(MkDocsYAMLParseError) as e: - with override_settings(DOCROOT=tmpdir): - self.searchbuilder.append_conf() - self.assertEqual( - e.exception.message_id, - MkDocsYAMLParseError.CONFIG_NOT_DICT, - ) From e7cbf0e5d32ff958a9a4e512a28f7be87baf4368 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 11 Mar 2024 17:15:21 +0100 Subject: [PATCH 2/6] Enable Addons if project is MkDocs Listen to `Project.post_save` signal and enable Addons if the project was created after a specific date, it's MkDocs and it's the first time the `AddonsConfig` is created. --- readthedocs/projects/apps.py | 1 + readthedocs/projects/models.py | 5 ----- readthedocs/projects/signals.py | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/readthedocs/projects/apps.py b/readthedocs/projects/apps.py index f4a1525c190..45a3eb24e51 100644 --- a/readthedocs/projects/apps.py +++ b/readthedocs/projects/apps.py @@ -9,6 +9,7 @@ class ProjectsConfig(AppConfig): def ready(self): # Load and register notification messages for this application import readthedocs.projects.notifications # noqa + import readthedocs.projects.signals # noqa import readthedocs.projects.tasks.builds # noqa import readthedocs.projects.tasks.search # noqa import readthedocs.projects.tasks.utils # noqa diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 008fd256ad4..ca0e803c7d2 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -1866,7 +1866,6 @@ def add_features(sender, **kwargs): # Feature constants - this is not a exhaustive list of features, features # may be added by other packages - MKDOCS_THEME_RTD = "mkdocs_theme_rtd" API_LARGE_DATA = "api_large_data" CONDA_APPEND_CORE_REQUIREMENTS = "conda_append_core_requirements" ALL_VERSIONS_IN_HTML_CONTEXT = "all_versions_in_html_context" @@ -1897,10 +1896,6 @@ def add_features(sender, **kwargs): SCALE_IN_PROTECTION = "scale_in_prtection" FEATURES = ( - ( - MKDOCS_THEME_RTD, - _("MkDocs: Use Read the Docs theme for MkDocs as default theme"), - ), ( API_LARGE_DATA, _("Build: Try alternative method of posting large data"), diff --git a/readthedocs/projects/signals.py b/readthedocs/projects/signals.py index 0340b3ff05c..788b7af66ba 100644 --- a/readthedocs/projects/signals.py +++ b/readthedocs/projects/signals.py @@ -1,6 +1,13 @@ """Project signals.""" +import datetime + import django.dispatch +from django.db.models.signals import post_save +from django.dispatch import receiver + +from readthedocs.projects.constants import MKDOCS +from readthedocs.projects.models import AddonsConfig, Project before_vcs = django.dispatch.Signal() @@ -11,3 +18,28 @@ # Used to purge files from the CDN files_changed = django.dispatch.Signal() + + +@receiver(post_save, sender=Project) +def enable_addons_on_new_mkdocs_projects(instance, *args, **kwargs): + """ + Enable Addons on projects created after 2024-04-01. + + We removed all the `mkdocs.yml` manipulation that set the `readthedocs` if + undefined and injects JS and CSS files to show the old flyout. + + Now, we are enabling addons by default on this projects to move forward + with this idea of removing the magic executed on behalves the users and + promote addons more. + + Reference https://github.com/readthedocs/addons/issues/72#issuecomment-1926647293 + """ + + if ( + instance.pub_date > datetime.date(2024, 4, 1) + and instance.documentation_type == MKDOCS + ): + created, config = AddonsConfig.objects.get_or_create(project=instance) + if created: + config.enabled = True + config.save() From a72ca029f11b175591459dbfcb093c04d5d469e3 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 11 Mar 2024 18:07:28 +0100 Subject: [PATCH 3/6] Listen to Version instead --- readthedocs/projects/signals.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/readthedocs/projects/signals.py b/readthedocs/projects/signals.py index 788b7af66ba..26ca61748ae 100644 --- a/readthedocs/projects/signals.py +++ b/readthedocs/projects/signals.py @@ -3,11 +3,16 @@ import datetime import django.dispatch +import structlog from django.db.models.signals import post_save from django.dispatch import receiver +from readthedocs.builds.models import Version from readthedocs.projects.constants import MKDOCS -from readthedocs.projects.models import AddonsConfig, Project +from readthedocs.projects.models import AddonsConfig + +log = structlog.get_logger(__name__) + before_vcs = django.dispatch.Signal() @@ -20,7 +25,7 @@ files_changed = django.dispatch.Signal() -@receiver(post_save, sender=Project) +@receiver(post_save, sender=Version) def enable_addons_on_new_mkdocs_projects(instance, *args, **kwargs): """ Enable Addons on projects created after 2024-04-01. @@ -34,12 +39,18 @@ def enable_addons_on_new_mkdocs_projects(instance, *args, **kwargs): Reference https://github.com/readthedocs/addons/issues/72#issuecomment-1926647293 """ + project = instance.project if ( - instance.pub_date > datetime.date(2024, 4, 1) + project.pub_date + > datetime.datetime(2023, 4, 1, 0, 0, 0, tzinfo=datetime.timezone.utc) and instance.documentation_type == MKDOCS ): - created, config = AddonsConfig.objects.get_or_create(project=instance) + created, config = AddonsConfig.objects.get_or_create(project=project) if created: + log.info( + "Creating AddonsConfig automatically for MkDocs project.", + project_slug=project.slug, + ) config.enabled = True config.save() From 2260f2673dffe409a78291000e7d3c8edde99133 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 11 Mar 2024 18:14:11 +0100 Subject: [PATCH 4/6] Typo --- readthedocs/projects/signals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/projects/signals.py b/readthedocs/projects/signals.py index 26ca61748ae..d0652744f45 100644 --- a/readthedocs/projects/signals.py +++ b/readthedocs/projects/signals.py @@ -46,7 +46,7 @@ def enable_addons_on_new_mkdocs_projects(instance, *args, **kwargs): > datetime.datetime(2023, 4, 1, 0, 0, 0, tzinfo=datetime.timezone.utc) and instance.documentation_type == MKDOCS ): - created, config = AddonsConfig.objects.get_or_create(project=project) + config, created = AddonsConfig.objects.get_or_create(project=project) if created: log.info( "Creating AddonsConfig automatically for MkDocs project.", From 90cca9356367240a62ca60821e4b11fbc487731d Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Mon, 11 Mar 2024 18:49:59 +0100 Subject: [PATCH 5/6] Update readthedocs/projects/signals.py Co-authored-by: Santos Gallegos --- readthedocs/projects/signals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/projects/signals.py b/readthedocs/projects/signals.py index d0652744f45..63de226eeda 100644 --- a/readthedocs/projects/signals.py +++ b/readthedocs/projects/signals.py @@ -30,7 +30,7 @@ def enable_addons_on_new_mkdocs_projects(instance, *args, **kwargs): """ Enable Addons on projects created after 2024-04-01. - We removed all the `mkdocs.yml` manipulation that set the `readthedocs` if + We removed all the `mkdocs.yml` manipulation that set the theme to `readthedocs` if undefined and injects JS and CSS files to show the old flyout. Now, we are enabling addons by default on this projects to move forward From 8d78e5cef7d7e0ec7e115864b580b3b36215e86b Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 19 Mar 2024 17:36:18 +0100 Subject: [PATCH 6/6] Remove datetime restriction --- readthedocs/projects/signals.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/readthedocs/projects/signals.py b/readthedocs/projects/signals.py index 63de226eeda..1fb0db17d3c 100644 --- a/readthedocs/projects/signals.py +++ b/readthedocs/projects/signals.py @@ -1,6 +1,5 @@ """Project signals.""" -import datetime import django.dispatch import structlog @@ -28,24 +27,21 @@ @receiver(post_save, sender=Version) def enable_addons_on_new_mkdocs_projects(instance, *args, **kwargs): """ - Enable Addons on projects created after 2024-04-01. + Enable Addons on MkDocs projects. We removed all the `mkdocs.yml` manipulation that set the theme to `readthedocs` if undefined and injects JS and CSS files to show the old flyout. - Now, we are enabling addons by default on this projects to move forward - with this idea of removing the magic executed on behalves the users and + Now, we are enabling addons by default on MkDocs projects to move forward + with the idea of removing the magic executed on behalves the users and promote addons more. Reference https://github.com/readthedocs/addons/issues/72#issuecomment-1926647293 """ + version = instance project = instance.project - if ( - project.pub_date - > datetime.datetime(2023, 4, 1, 0, 0, 0, tzinfo=datetime.timezone.utc) - and instance.documentation_type == MKDOCS - ): + if version.documentation_type == MKDOCS: config, created = AddonsConfig.objects.get_or_create(project=project) if created: log.info(