Merge lp:~developer-ubuntu-com-dev/developer-ubuntu-com/importer-versioning into lp:developer-ubuntu-com

[Daniel Holbach]

[Daniel Holbach (dholbach)]

What's still needed: make the version picker look nice.

[lp:~developer-ubuntu-com-dev/developer-ubuntu-com/importer-versioning]

237. By Daniel Holbach on 2016-05-14

fix dissociation of version information

238. By Daniel Holbach on 2016-05-14

fix urls to not include 'None'

Unmerged revisions

238. By Daniel Holbach on 2016-05-14

fix urls to not include 'None'

237. By Daniel Holbach on 2016-05-14

fix dissociation of version information

236. By Daniel Holbach on 2016-05-12

merge trunk

235. By Daniel Holbach on 2016-05-12

exclude current version

234. By Daniel Holbach on 2016-05-12

make sure we add the alternative url info

233. By Daniel Holbach on 2016-05-12

fix template

232. By Daniel Holbach on 2016-05-12

first cut at adding version/url info to page template

231. By Daniel Holbach on 2016-05-12

fix logic

230. By Daniel Holbach on 2016-05-11

clarify tests, add a new test to check the extension data of the page - currently fails

229. By Daniel Holbach on 2016-05-11

simplify code and make version extension handling actually work

=== modified file 'md_importer/importer/article.py'
--- md_importer/importer/article.py	2016-04-21 12:26:42 +0000
+++ md_importer/importer/article.py	2016-05-14 20:13:14 +0000
@@ -6,7 +6,6 @@
 import sys
 
 from . import (
-    DEFAULT_LANG,
     DEFAULT_TEMPLATE,
     logger,
     MARKDOWN_EXTENSIONS,
@@ -14,6 +13,7 @@
 )
 from .publish import (
     ArticlePage,
+    clean_url,
     ParentNotFoundException,
     slugify,
 )
@@ -25,14 +25,13 @@
 
 
 class Article:
-    def __init__(self, fn, write_to, advertise=True,
+    def __init__(self, fn, full_url, advertise=True,
                  template=DEFAULT_TEMPLATE):
         self.html = None
         self.article_page = None
         self.title = ""
         self.fn = fn
-        self.write_to = slugify(self.fn)
-        self.full_url = write_to
+        self.full_url = full_url
         self.slug = os.path.basename(self.full_url)
         self.links_rewritten = False
         self.local_images = []
@@ -129,9 +128,7 @@
                 in_navigation=self.advertise, template=self.template)
         except ParentNotFoundException:
             return False
-        self.full_url = re.sub(
-            r'^\/None\/', '/{}/'.format(DEFAULT_LANG),
-            self.article_page.draft.get_absolute_url())
+        self.full_url = clean_url(self.article_page.draft.get_absolute_url())
         return True
 
     def publish(self):

=== modified file 'md_importer/importer/process.py'
--- md_importer/importer/process.py	2016-04-27 15:14:30 +0000
+++ md_importer/importer/process.py	2016-05-14 20:13:14 +0000
@@ -3,21 +3,27 @@
 import shutil
 import tempfile
 
-from django.core.management import call_command
-
-from md_importer.importer import logger
-from md_importer.importer.publish import find_page
+from md_importer.importer import (
+    DEFAULT_LANG,
+    logger,
+)
+from md_importer.importer.publish import (
+    find_page,
+    remove_none_from_url,
+)
 from md_importer.importer.repo import Repo
 from md_importer.models import (
+    ExternalDocsBranch,
     ExternalDocsBranchImportDirective,
     ImportedArticle,
+    VersionExtension,
 )
 
 
 def process_branch(branch):
     tempdir = tempfile.mkdtemp()
     repo = Repo(tempdir, branch.origin, branch.branch_name,
-                branch.post_checkout_command)
+                branch.post_checkout_command, branch.version)
     if repo.get() != 0:
         return None
     for directive in ExternalDocsBranchImportDirective.objects.filter(
@@ -46,6 +52,43 @@
                 url=url,
                 last_import=timestamp)
 
+    if repo.version:
+        # Updates pages with information about other versions of the article
+        ebs = ExternalDocsBranch.objects.filter(origin=repo.origin)
+        available_versions = [a.version for a in ebs]
+        urls = [remove_none_from_url(i.url)
+                for i in ImportedArticle.objects.all()]
+
+        for page in repo.pages:
+            url = page.get_absolute_url()
+            alternative_urls = [
+                {
+                    'version': v,
+                    'url': remove_none_from_url(url.replace(repo.version, v)),
+                }
+                for v in available_versions if v != repo.version
+            ]
+            # Check which of the alternative versions of the article
+            # actually exist
+            for useless_entry in [x for x in alternative_urls
+                                  if x['url'] not in urls]:
+                alternative_urls.remove(useless_entry)
+            draft = page.get_draft_object()
+            if not hasattr(draft, 'versionextension'):
+                version_ext = VersionExtension(extended_object=draft)
+                version_ext.save()
+                draft.versionextension = version_ext
+            if draft.versionextension.version != repo.version:
+                draft.versionextension.version = repo.version
+            draft.versionextension.update_alternatives(alternative_urls)
+            draft.versionextension.save()
+            draft.publish(DEFAULT_LANG)
+    else:
+        for page in repo.pages:
+            if hasattr(page, 'versionextension'):
+                page.versionextension.delete()
+                page.publish(DEFAULT_LANG)
+
     # Remove old entries
     for imported_article in ImportedArticle.objects.filter(
             branch=branch, last_import__lt=timestamp):
@@ -61,6 +104,7 @@
                          if p.changed_by in ['python-api', 'script']]
     ImportedArticle.objects.filter(
         branch=branch).filter(id__in=imported_page_ids).delete()
+
     shutil.rmtree(tempdir)
 
     return repo

=== modified file 'md_importer/importer/publish.py'
--- md_importer/importer/publish.py	2016-04-21 12:26:42 +0000
+++ md_importer/importer/publish.py	2016-05-14 20:13:14 +0000
@@ -134,12 +134,26 @@
     return os.path.basename(filename).replace('.md', '').replace('.html', '')
 
 
+def remove_none_from_url(url):
+    return re.sub(
+        r'^\/None|{}\/'.format(DEFAULT_LANG),
+        '',
+        url)
+
+
 def clean_url(url):
-    return remove_leading_and_trailing_slash(
-        re.sub(
-            r'^\/None|{}\/'.format(DEFAULT_LANG),
-            '',
-            url))
+    return remove_leading_and_trailing_slash(remove_none_from_url(url))
+
+
+def generate_url(import_to, article_fn, version=None):
+    if not version:
+        if not article_fn:
+            # Unversioned index page
+            return clean_url(import_to)
+        # Unversioned 'regular' article
+        return clean_url(os.path.join(import_to, slugify(article_fn)))
+    # Versioned article
+    return clean_url(os.path.join(import_to, version, slugify(article_fn)))
 
 
 def find_page(url, draft=False):

=== modified file 'md_importer/importer/repo.py'
--- md_importer/importer/repo.py	2016-04-21 12:26:42 +0000
+++ md_importer/importer/repo.py	2016-05-14 20:13:14 +0000
@@ -5,9 +5,9 @@
 )
 from .article import Article
 from .publish import (
+    generate_url,
     IndexPage,
     ParentNotFoundException,
-    slugify,
 )
 from .source import SourceCode
 
@@ -18,7 +18,8 @@
 
 
 class Repo:
-    def __init__(self, tempdir, origin, branch_name, post_checkout_command):
+    def __init__(self, tempdir, origin, branch_name,
+                 post_checkout_command=None, version=None):
         self.directives = []
         self.imported_articles = []
         self.url_map = {}
@@ -31,9 +32,13 @@
         self.origin = origin
         self.branch_name = branch_name
         self.post_checkout_command = post_checkout_command
+        self.version = version
         self.branch_nick = os.path.basename(self.origin.replace('.git', ''))
         self.checkout_location = os.path.join(tempdir, self.branch_nick)
 
+    def __str__(self):
+        return '{} -- {}'.format(self.origin, self.branch_name)
+
     def get(self):
         sourcecode = SourceCode(self.origin, self.checkout_location,
                                 self.branch_name, self.post_checkout_command)
@@ -79,29 +84,33 @@
             for fn in glob.glob('{}/*'.format(directive['import_from'])):
                 if fn not in [a[0] for a in import_list]:
                     import_list += [
-                        (fn, os.path.join(directive['write_to'], slugify(fn)),
+                        (fn,
+                         generate_url(directive['write_to'], fn, self.version),
                          directive['advertise'], directive['template'])
                     ]
             # If we import into a namespace and don't have an index doc,
             # we need to write one.
             # XXX: Right now we just create one index doc, this will change in
             # the near future.
-            if directive['write_to'] not in [x[1] for x in import_list] and \
+            index_url = generate_url(directive['write_to'], '', self.version)
+            if index_url not in [x[1] for x in import_list] and \
                not self.index_page:
                 try:
                     self.index_page = IndexPage(
-                        title=self.branch_nick, full_url=directive['write_to'],
+                        title=self.branch_nick, full_url=index_url,
                         in_navigation=directive['advertise'], html='',
                         menu_title=None, template=DEFAULT_TEMPLATE)
-                    self.pages.extend([self.index_page.page])
                 except ParentNotFoundException:
                     return self._abort_import(
                         'Could not create fake index page at {}'.format(
-                            directive['write_to']))
+                            index_url))
+                self.pages.extend([self.index_page.page])
+
         # The actual import
         for entry in import_list:
             article = self._read_article(
-                entry[0], entry[1], entry[2], entry[3])
+                fn=entry[0], full_url=entry[1], advertise=entry[2],
+                template=entry[3])
             if article:
                 self.imported_articles += [article]
                 self.titles[article.fn] = article.title
@@ -121,8 +130,8 @@
         logger.error('Importing of {} aborted: {}.'.format(self.origin, msg))
         return False
 
-    def _read_article(self, fn, write_to, advertise, template):
-        article = Article(fn, write_to, advertise, template)
+    def _read_article(self, fn, full_url, advertise, template):
+        article = Article(fn, full_url, advertise, template)
         if article.read():
             return article
         return None

=== added file 'md_importer/migrations/0005_add_versioning.py'
--- md_importer/migrations/0005_add_versioning.py	1970-01-01 00:00:00 +0000
+++ md_importer/migrations/0005_add_versioning.py	2016-05-14 20:13:14 +0000
@@ -0,0 +1,41 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('cms', '0014_auto_20160404_1908'),
+        ('md_importer', '0004_move_from_page_object_to_url_string'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='VersionedURL',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+                ('version', models.CharField(max_length=10)),
+                ('url', models.URLField(max_length=1000)),
+            ],
+        ),
+        migrations.CreateModel(
+            name='VersionExtension',
+            fields=[
+                ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
+                ('version', models.CharField(max_length=10, blank=True)),
+                ('alternative_urls', models.ManyToManyField(to='md_importer.VersionedURL', blank=True)),
+                ('extended_object', models.OneToOneField(editable=False, to='cms.Page')),
+                ('public_extension', models.OneToOneField(related_name='draft_extension', null=True, editable=False, to='md_importer.VersionExtension')),
+            ],
+            options={
+                'abstract': False,
+            },
+        ),
+        migrations.AddField(
+            model_name='externaldocsbranch',
+            name='version',
+            field=models.CharField(help_text='If this is a versioned import, set the version, e.g. "15.04" here.', max_length=10, blank=True),
+        ),
+    ]

=== modified file 'md_importer/models.py'
--- md_importer/models.py	2016-04-18 08:56:51 +0000
+++ md_importer/models.py	2016-05-14 20:13:14 +0000
@@ -2,6 +2,9 @@
 from django.db import models
 from django.utils.translation import ugettext_lazy as _
 
+from cms.extensions import PageExtension
+from cms.extensions.extension_pool import extension_pool
+
 
 if settings.CMS_TEMPLATES:
     cms_templates = settings.CMS_TEMPLATES
@@ -26,6 +29,11 @@
         help_text=_('Command to run after checkout of the branch.'),
         blank=True)
     active = models.BooleanField(default=True)
+    version = models.CharField(
+        max_length=10,
+        help_text=_('If this is a versioned import, set the version, e.g. '
+                    '"15.04" here.'),
+        blank=True)
 
     def __str__(self):
         if self.branch_name:
@@ -80,3 +88,32 @@
     def __str__(self):
         return '{} -- {} -- {}'.format(
             self.url, self.branch, self.last_import)
+
+
+class VersionedURL(models.Model):
+    version = models.CharField(max_length=10)
+    url = models.URLField(max_length=1000)
+
+
+class VersionExtension(PageExtension):
+    version = models.CharField(
+        max_length=10,
+        blank=True)
+    alternative_urls = models.ManyToManyField(VersionedURL, blank=True)
+
+    def update_alternatives(self, alternative_urls):
+        for alternative in alternative_urls:
+            versioned_url, created = VersionedURL.objects.get_or_create(
+                version=alternative['version'],
+                url=alternative['url'])
+            if versioned_url not in self.alternative_urls.all():
+                self.alternative_urls.add(versioned_url)
+        for listed_url in self.alternative_urls.all():
+            if not [a for a in alternative_urls
+                    if a['url'] == listed_url.url]:
+                self.alternative_urls.remove(listed_url)
+
+    def copy_relations(self, oldinstance, language):
+        self.alternative_urls = oldinstance.alternative_urls.all()
+
+extension_pool.register(VersionExtension)

=== renamed file 'md_importer/tests/test_import_process.py' => 'md_importer/tests/test_process.py'
--- md_importer/tests/test_import_process.py	2016-04-18 10:30:14 +0000
+++ md_importer/tests/test_process.py	2016-05-14 20:13:14 +0000
@@ -1,4 +1,5 @@
 import os
+import re
 
 from django.test import TestCase
 from cms.api import publish_pages
@@ -328,3 +329,97 @@
         self.assertTrue(published_pages.has_at_least_size(20))
         for imported_article in ImportedArticle.objects.all():
             self.assertTrue(check_imported_article(imported_article))
+
+
+class TestVersionedImportProcessPasses(TestCase):
+    def runTest(self):
+        db_empty_page_list()
+        root = db_create_root_page()
+        snappy_page = db_add_empty_page('Snappy', root)
+        guides = db_add_empty_page('Guides', snappy_page)
+        publish_pages([snappy_page, guides])
+        ExternalDocsBranch.objects.all().delete()
+        ExternalDocsBranchImportDirective.objects.all().delete()
+        ImportedArticle.objects.all().delete()
+        branch, created = ExternalDocsBranch.objects.get_or_create(
+            origin=os.path.join(
+                os.path.dirname(__file__), 'data/snappy-test'),
+            branch_name='',
+            version='16.04')
+        a, created = ExternalDocsBranchImportDirective.objects.get_or_create(
+            import_from='docs', write_to='snappy/guides',
+            external_docs_branch=branch)
+        self.assertIsNotNone(process_branch(branch))
+
+
+class TestTwoVersionedImportProcessPass(TestCase):
+    def runTest(self):
+        db_empty_page_list()
+        root = db_create_root_page()
+        snappy_page = db_add_empty_page('Snappy', root)
+        guides = db_add_empty_page('Guides', snappy_page)
+        publish_pages([snappy_page, guides])
+        ExternalDocsBranch.objects.all().delete()
+        ExternalDocsBranchImportDirective.objects.all().delete()
+        ImportedArticle.objects.all().delete()
+        branch, created = ExternalDocsBranch.objects.get_or_create(
+            origin=os.path.join(
+                os.path.dirname(__file__), 'data/snappy-test'),
+            branch_name='',
+            version='16.04')
+        a, created = ExternalDocsBranchImportDirective.objects.get_or_create(
+            import_from='docs', write_to='snappy/guides',
+            external_docs_branch=branch)
+        self.assertIsNotNone(process_branch(branch))
+        branch2, created = ExternalDocsBranch.objects.get_or_create(
+            origin=os.path.join(
+                os.path.dirname(__file__), 'data/snappy-test'),
+            branch_name='',
+            version='15.04')
+        b, created = ExternalDocsBranchImportDirective.objects.get_or_create(
+            import_from='docs', write_to='snappy/guides',
+            external_docs_branch=branch2)
+        self.assertIsNotNone(process_branch(branch2))
+
+
+class TestTwoVersionedImportsValidExtensionData(TestCase):
+    def runTest(self):
+        db_empty_page_list()
+        root = db_create_root_page()
+        snappy_page = db_add_empty_page('Snappy', root)
+        guides = db_add_empty_page('Guides', snappy_page)
+        publish_pages([snappy_page, guides])
+        ExternalDocsBranch.objects.all().delete()
+        ExternalDocsBranchImportDirective.objects.all().delete()
+        ImportedArticle.objects.all().delete()
+        branch, created = ExternalDocsBranch.objects.get_or_create(
+            origin=os.path.join(
+                os.path.dirname(__file__), 'data/snappy-test'),
+            branch_name='',
+            version='16.04')
+        a, created = ExternalDocsBranchImportDirective.objects.get_or_create(
+            import_from='docs', write_to='snappy/guides',
+            external_docs_branch=branch)
+        self.assertIsNotNone(process_branch(branch))
+        branch2, created = ExternalDocsBranch.objects.get_or_create(
+            origin=os.path.join(
+                os.path.dirname(__file__), 'data/snappy-test'),
+            branch_name='',
+            version='15.04')
+        b, created = ExternalDocsBranchImportDirective.objects.get_or_create(
+            import_from='docs', write_to='snappy/guides',
+            external_docs_branch=branch2)
+        self.assertIsNotNone(process_branch(branch2))
+
+        # Check if the REST article was imported into 15.04 and 16.04 namespace
+        published_pages = PublishedPages()
+        self.assertTrue(published_pages.contains_url(
+            '/{}/snappy/guides/15.04/rest/'.format(DEFAULT_LANG)))
+        self.assertTrue(published_pages.contains_url(
+            '/{}/snappy/guides/16.04/rest/'.format(DEFAULT_LANG)))
+
+        for page in published_pages.pages:
+            if re.findall(
+                '\/{}\/snappy\/guides\/1\d\.04\/(\S+?)\/'.format(DEFAULT_LANG),
+                    page.get_absolute_url()):
+                self.assertTrue(hasattr(page, 'versionextension'))

=== modified file 'md_importer/tests/test_publish.py'
--- md_importer/tests/test_publish.py	2016-04-21 10:39:58 +0000
+++ md_importer/tests/test_publish.py	2016-05-14 20:13:14 +0000
@@ -5,7 +5,10 @@
 from developer_portal.models import RawHtml
 
 from md_importer.importer import DEFAULT_LANG
-from md_importer.importer.publish import ArticlePage
+from md_importer.importer.publish import (
+    ArticlePage,
+    generate_url,
+)
 
 from .utils import (
     db_empty_page_list,
@@ -75,3 +78,19 @@
         plugins = placeholder.get_plugins()
         self.assertEqual(plugins.count(), 1)
         self.assertIsInstance(plugins[0].get_plugin_instance()[0], RawHtml)
+
+
+class TestGenerateURL(TestCase):
+    def runTest(self):
+        self.assertEqual(
+            generate_url('snappy/guides', ''),
+            'snappy/guides')
+        self.assertEqual(
+            generate_url('snappy/guides', 'interfaces.md'),
+            'snappy/guides/interfaces')
+        self.assertEqual(
+            generate_url('snappy/guides', '', '16.04'),
+            'snappy/guides/16.04')
+        self.assertEqual(
+            generate_url('snappy/guides', 'interfaces.md', '16.04'),
+            'snappy/guides/16.04/interfaces')

=== modified file 'md_importer/tests/utils.py'
--- md_importer/tests/utils.py	2016-04-18 10:30:14 +0000
+++ md_importer/tests/utils.py	2016-05-14 20:13:14 +0000
@@ -52,10 +52,10 @@
             self.root.get_absolute_url(),
             '/{}/'.format(DEFAULT_LANG))
 
-    def create_repo(self, docs_path):
+    def create_repo(self, docs_path, version=None):
         origin = os.path.join(os.path.dirname(__file__), docs_path)
         self.assertTrue(os.path.exists(origin))
-        self.repo = Repo(self.tempdir, origin, '', '')
+        self.repo = Repo(self.tempdir, origin, '', '', version=version)
         self.fetch_retcode = self.repo.get()
         self.assertEqual(self.fetch_retcode, 0)
 

=== modified file 'templates/cms_page_base.html'
--- templates/cms_page_base.html	2015-10-21 14:41:23 +0000
+++ templates/cms_page_base.html	2016-05-14 20:13:14 +0000
@@ -9,5 +9,19 @@
 {% endblock extrameta %}
 
 {% block content %}
+{% if request.current_page.versionextension %}
+    <div>
+        <p>Current version {{ request.current_page.versionextension.version }}</p>
+        {% if request.current_page.versionextension.alternative_urls %}
+        <p>Other versions:
+            <ul>
+                {% for alternative in request.current_page.versionextension.alternative_urls.all %}
+                <li><a href="{{ alternative.url }}">{{ alternative.version }}</a></li>
+                {% endfor %}
+            </ul>
+        </p>
+        {% endif %}
+    </div>
+{% endif %}
   {% placeholder page_content %}
 {% endblock %}