From 3fbb0c1a7c94bcc979f54d67fad8dc94f74854c8 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 24 Oct 2018 16:08:45 +0200 Subject: [PATCH 1/2] Feature flag to make `readthedocs` theme default on MkDocs docs Historically, we were using `readthedocs` as default theme for MkDocs but in https://github.com/rtfd/readthedocs.org/pull/4556 we decided to change it to get the same behavior when building locally than in Read the Docs. This commit adds a Feature flag to keep having the old behavior for some particular projects so we can add these project and do not break their documentation (change the theme without asking/reason). --- readthedocs/doc_builder/backends/mkdocs.py | 25 ++++++++ .../0028_feature-flag-mkdocs-theme.py | 35 +++++++++++ readthedocs/projects/models.py | 2 + .../rtd_tests/tests/test_doc_builder.py | 63 ++++++++++++++++++- 4 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 readthedocs/projects/migrations/0028_feature-flag-mkdocs-theme.py diff --git a/readthedocs/doc_builder/backends/mkdocs.py b/readthedocs/doc_builder/backends/mkdocs.py index 2164a850104..f88b7dd88fe 100644 --- a/readthedocs/doc_builder/backends/mkdocs.py +++ b/readthedocs/doc_builder/backends/mkdocs.py @@ -16,6 +16,7 @@ from readthedocs.doc_builder.base import BaseBuilder from readthedocs.doc_builder.exceptions import BuildEnvironmentError +from readthedocs.projects.models import Feature log = logging.getLogger(__name__) @@ -50,6 +51,23 @@ def __init__(self, *args, **kwargs): self.root_path = self.version.project.checkout_path(self.version.slug) 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 the 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: slug=%s', + self.project.slug, + ) + else: + self.DEFAULT_THEME_NAME = 'mkdocs' + + def get_yaml_config(self): """Find the ``mkdocs.yml`` file in the project root.""" mkdoc_path = self.config.mkdocs.configuration @@ -130,6 +148,13 @@ def append_conf(self, **__): # 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 yaml.safe_dump( user_config, diff --git a/readthedocs/projects/migrations/0028_feature-flag-mkdocs-theme.py b/readthedocs/projects/migrations/0028_feature-flag-mkdocs-theme.py new file mode 100644 index 00000000000..6f53d27443c --- /dev/null +++ b/readthedocs/projects/migrations/0028_feature-flag-mkdocs-theme.py @@ -0,0 +1,35 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.16 on 2018-10-24 07:43 +from __future__ import unicode_literals + +from django.db import migrations + + +FEATURE_ID = 'mkdocs_theme_rtd' + + +def forward_add_feature(apps, schema_editor): + Feature = apps.get_model('projects', 'Feature') + Feature.objects.create( + feature_id=FEATURE_ID, + # Not using ``default_true=True`` because we will do this manually in + # the database from the Corporate site only, since this is not required + # in the Community site + # default_true=True, + ) + + +def reverse_add_feature(apps, schema_editor): + Feature = apps.get_model('projects', 'Feature') + Feature.objects.filter(feature_id=FEATURE_ID).delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('projects', '0027_remove_json_with_html_feature'), + ] + + operations = [ + migrations.RunPython(forward_add_feature, reverse_add_feature), + ] diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 29cbe5f4d6e..0bb641b8a9f 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -1061,6 +1061,7 @@ def add_features(sender, **kwargs): SKIP_SUBMODULES = 'skip_submodules' DONT_OVERWRITE_SPHINX_CONTEXT = 'dont_overwrite_sphinx_context' ALLOW_V2_CONFIG_FILE = 'allow_v2_config_file' + MKDOCS_THEME_RTD = 'mkdocs_theme_rtd' FEATURES = ( (USE_SPHINX_LATEST, _('Use latest version of Sphinx')), @@ -1072,6 +1073,7 @@ def add_features(sender, **kwargs): 'Do not overwrite context vars in conf.py with Read the Docs context',)), (ALLOW_V2_CONFIG_FILE, _( 'Allow to use the v2 of the configuration file')), + (MKDOCS_THEME_RTD, _('Use Read the Docs theme for MkDocs as default theme')), ) projects = models.ManyToManyField( diff --git a/readthedocs/rtd_tests/tests/test_doc_builder.py b/readthedocs/rtd_tests/tests/test_doc_builder.py index 6a453c4837e..5da0fccc4fe 100644 --- a/readthedocs/rtd_tests/tests/test_doc_builder.py +++ b/readthedocs/rtd_tests/tests/test_doc_builder.py @@ -20,7 +20,7 @@ from readthedocs.doc_builder.backends.sphinx import BaseSphinx from readthedocs.doc_builder.python_environments import Virtualenv from readthedocs.projects.exceptions import ProjectConfigurationError -from readthedocs.projects.models import Project +from readthedocs.projects.models import Feature, Project class SphinxBuilderTest(TestCase): @@ -224,6 +224,67 @@ def test_get_theme_name(self, checkout_path): } 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.objects.get( + feature_id=Feature.MKDOCS_THEME_RTD, + ).projects.add(self.project) + + python_env = Virtualenv( + version=self.version, + build_env=self.build_env, + config=None, + ) + 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} + builder.append_conf() + + mock_yaml.safe_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', + }, + mock.ANY, + ) + 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', + } + builder.append_conf() + + mock_yaml.safe_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', + }, + mock.ANY, + ) + + @patch('readthedocs.doc_builder.base.BaseBuilder.run') @patch('readthedocs.projects.models.Project.checkout_path') def test_append_conf_create_yaml(self, checkout_path, run): From d5ae01273d1613c6a7b634a16293b856f0c4bd6d Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 24 Oct 2018 18:03:34 +0200 Subject: [PATCH 2/2] Typo --- readthedocs/doc_builder/backends/mkdocs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/doc_builder/backends/mkdocs.py b/readthedocs/doc_builder/backends/mkdocs.py index f88b7dd88fe..eaa388cad7b 100644 --- a/readthedocs/doc_builder/backends/mkdocs.py +++ b/readthedocs/doc_builder/backends/mkdocs.py @@ -54,7 +54,7 @@ def __init__(self, *args, **kwargs): # 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 the this into the Corporate + # 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.