Permalink
Browse files

Merge pull request #242 from lmorchard/simplify-wiki-urls-754534

bug 754534: Simplify wiki URLs
  • Loading branch information...
2 parents d3cf68c + b861a64 commit 2ef4564618c1e18b9e1312e30f6a06b75a950e99 @darkwing committed Jun 5, 2012
View
@@ -77,16 +77,20 @@ def devmo_url(context, path):
Look for a wiki page in the current locale first,
then default to given path
"""
+ if not settings.DEKIWIKI_ENDPOINT:
+ # HACK: If MindTouch is unavailable, skip the rest of this and lean on
+ # locale processing redirects to resolve things. Might be interesting
+ # to resolve some of the redirects first, and come up with the ultimate
+ # real URL. See bug 759356 for followup.
+ path = path.replace('/en', '')
+ return '/%s/docs%s' % (context['request'].locale, path)
+
# HACK: If DEKIWIKI_MOCK is True, just skip hitting the API. This can speed
# up a lot of tests without adding decorators, and should never be true in
# production.
if getattr(settings, 'DEKIWIKI_MOCK', False):
return path
- if not settings.DEKIWIKI_ENDPOINT:
- # If MindTouch is unavailable, skip the rest of this
- return path
-
current_locale = context['request'].locale
m = get_locale_path_match(path)
if not m:
@@ -115,7 +115,7 @@ def test_devmo_url(self):
@attr('current')
@mock.patch('devmo.helpers.check_devmo_local_page')
- def test_devmo_url_midtouch_disabled(self, mock_check_devmo_local_page):
+ def test_devmo_url_mindtouch_disabled(self, mock_check_devmo_local_page):
_old = settings.DEKIWIKI_ENDPOINT
settings.DEKIWIKI_ENDPOINT = False
@@ -132,11 +132,8 @@ def my_check_devmo_local_page(username, password, force=False):
req = test_utils.RequestFactory().get('/')
context = {'request': req}
- # NOTE: This is undesirable behavior, but expected. Since devmo_url can
- # no longer consult MindTouch, it will punt and claim that /en/HTML is
- # the proper path to the de locale page. See also, bug 759356
req.locale = 'de'
- eq_(devmo_url(context, localized_page), '/en/HTML')
+ eq_(devmo_url(context, localized_page), '/de/docs/HTML')
ok_(not trap['was_called'])
settings.DEKIWIKI_ENDPOINT = _old
View
@@ -176,7 +176,7 @@
'tag/[^/]+'
)
-DOCUMENT_LAST_MODIFIED_CACHE_KEY_TMPL = 'kuma:document-last-modified:%s'
+DOCUMENT_LAST_MODIFIED_CACHE_KEY_TMPL = u'kuma:document-last-modified:%s'
class UniqueCollision(Exception):
@@ -425,6 +425,11 @@ class Document(NotificationsMixin, ModelBase):
def natural_key(self):
return (self.locale, self.slug,)
+ @property
+ def natural_cache_key(self):
+ nk = u'/'.join(self.natural_key())
+ return hashlib.md5(nk.encode('utf8')).hexdigest()
+
class Meta(object):
unique_together = (('parent', 'locale'), ('slug', 'locale'))
permissions = (
@@ -549,8 +554,8 @@ def save(self, *args, **kwargs):
super(Document, self).save(*args, **kwargs)
# Delete any cached last-modified timestamp.
- path_hash = hashlib.md5(self.full_path).hexdigest()
- cache_key = DOCUMENT_LAST_MODIFIED_CACHE_KEY_TMPL % path_hash
+ cache_key = (DOCUMENT_LAST_MODIFIED_CACHE_KEY_TMPL %
+ self.natural_cache_key)
cache.delete(cache_key)
# Make redirects if there's an approved revision and title or slug
@@ -620,8 +625,11 @@ def language(self):
@property
def full_path(self):
- """The full path of a document consists of {locale}/{slug}"""
- return '%s/%s' % (self.locale, self.slug)
+ """The full path of a document consists of {slug}"""
+ # TODO: See about removing this and all references to full_path? Once
+ # upon a time, this was composed of {locale}/{slug}, but bug 754534
+ # reverted that.
+ return self.slug
def get_absolute_url(self, ui_locale=None):
"""Build the absolute URL to this document from its full path"""
@@ -632,36 +640,38 @@ def get_absolute_url(self, ui_locale=None):
@staticmethod
def locale_and_slug_from_path(path, request=None):
- """Given a docs URL path, derive locale and slug for model lookup, and
- a suggestion of whether this path deserves a redirect"""
+ """Given a proposed doc path, try to see if there's a legacy MindTouch
+ locale or even a modern Kuma domain in the path. If so, signal for a
+ redirect to a more canonical path. In any case, produce a locale and
+ slug derived from the given path."""
locale, slug, needs_redirect = '', path, False
+ mdn_languages_lower = dict((x.lower(), x)
+ for x in settings.MDN_LANGUAGES)
- # If there's a slash in the path, then the first segment is likely to
- # be the locale.
+ # If there's a slash in the path, then the first segment could be a
+ # locale. And, that locale could even be a legacy MindTouch locale.
if '/' in path:
- locale, slug = path.split('/', 1)
+ maybe_locale, maybe_slug = path.split('/', 1)
+ l_locale = maybe_locale.lower()
- if locale.lower() in settings.MT_TO_KUMA_LOCALE_MAP:
- # If this looks like a MindTouch locale, remap it.
- old_locale = locale
- locale = settings.MT_TO_KUMA_LOCALE_MAP[locale.lower()]
- # But, we only need a redirect if the locale actually changed.
- needs_redirect = (locale != old_locale)
+ if l_locale in settings.MT_TO_KUMA_LOCALE_MAP:
+ # The first segment looks like a MindTouch locale, remap it.
+ needs_redirect = True
+ locale = settings.MT_TO_KUMA_LOCALE_MAP[l_locale]
+ slug = maybe_slug
- if locale not in settings.MDN_LANGUAGES:
- # Oops, that doesn't look like a supported locale, so back out.
+ elif l_locale in mdn_languages_lower:
+ # The first segment looks like an MDN locale, redirect.
needs_redirect = True
- locale, slug = '', path
+ locale = mdn_languages_lower[l_locale]
+ slug = maybe_slug
- # No locale by this point? Go with the locale detected from user agent
- # if we have a request.
+ # No locale yet? Try the locale detected by the request.
if locale == '' and request:
- needs_redirect = True
locale = request.locale
- # Still no locale? Go with the site default locale.
+ # Still no locale? Probably no request. Go with the site default.
if locale == '':
- needs_redirect = True
locale = getattr(settings, 'WIKI_DEFAULT_LANGUAGE', 'en-US')
return (locale, slug, needs_redirect)
@@ -33,7 +33,10 @@
{% if documents.object_list %}
<ul class="documents cols-3">
{% for doc in documents.object_list %}
- <li><a href="{{ doc.get_absolute_url() }}">{{ doc.title }}</a></li>
+ <li>
+ <a href="{{ doc.get_absolute_url() }}">{{ doc.title }}</a> ({{ doc.locale }})<br/>
+ <small>{{ doc.get_absolute_url() }}</small>
+ </li>
{% endfor %}
</ul>
{{ documents|paginator }}
@@ -998,21 +998,21 @@ def setUp(self):
def test_translate_GET_logged_out(self):
"""Try to create a translation while logged out."""
self.client.logout()
- translate_path = 'es/' + self.d.slug
+ translate_path = self.d.slug
translate_uri = urllib.quote(reverse('wiki.translate',
- locale='en-US',
+ locale='es',
args=[translate_path]))
response = self.client.get(translate_uri)
eq_(302, response.status_code)
- expected_url = '%s?next=%s' % (reverse('users.login', locale='en-US'),
+ expected_url = '%s?next=%s' % (reverse('users.login', locale='es'),
translate_uri)
ok_(expected_url in response['Location'])
def test_translate_GET_with_perm(self):
"""HTTP GET to translate URL renders the form."""
- translate_path = 'es/' + self.d.slug
+ translate_path = self.d.slug
translate_uri = urllib.quote(reverse('wiki.translate',
- locale='en-US',
+ locale='es',
args=[translate_path]))
response = self.client.get(translate_uri)
eq_(200, response.status_code)
@@ -1025,18 +1025,18 @@ def test_translate_disallow(self):
"""HTTP GET to translate URL returns 400 when not localizable."""
self.d.is_localizable = False
self.d.save()
- translate_path = 'es/' + self.d.slug
+ translate_path = self.d.slug
translate_uri = urllib.quote(reverse('wiki.translate',
- locale='en-US',
+ locale='es',
args=[translate_path]))
response = self.client.get(translate_uri)
eq_(400, response.status_code)
def test_invalid_document_form(self):
"""Make sure we handle invalid document form without a 500."""
- translate_path = 'es/' + self.d.slug
+ translate_path = self.d.slug
translate_uri = urllib.quote(reverse('wiki.translate',
- locale='en-US',
+ locale='es',
args=[translate_path]))
data = _translation_data()
data['slug'] = '' # Invalid slug
@@ -1046,9 +1046,9 @@ def test_invalid_document_form(self):
def test_invalid_revision_form(self):
"""When creating a new translation, an invalid revision form shouldn't
result in a new Document being created."""
- translate_path = 'es/' + self.d.slug
+ translate_path = self.d.slug
translate_uri = urllib.quote(reverse('wiki.translate',
- locale='en-US',
+ locale='es',
args=[translate_path]))
data = _translation_data()
data['content'] = '' # Content is required
@@ -1064,9 +1064,9 @@ def test_first_translation_to_locale(self, get_current, edited_fire,
"""Create the first translation of a doc to new locale."""
get_current.return_value.domain = 'testserver'
- translate_path = 'es/' + self.d.slug
+ translate_path = self.d.slug
translate_uri = urllib.quote(reverse('wiki.translate',
- locale='en-US',
+ locale='es',
args=[translate_path]))
data = _translation_data()
response = self.client.post(translate_uri, data)
@@ -1110,9 +1110,9 @@ def test_another_translation_to_locale(self, get_current, edited_fire,
rev_enUS.save()
# Verify the form renders with correct content
- translate_path = 'es/' + self.d.slug
+ translate_path = self.d.slug
translate_uri = urllib.quote(reverse('wiki.translate',
- locale='en-US',
+ locale='es',
args=[translate_path]))
response = self.client.get(translate_uri)
doc = pq(response.content)
@@ -1124,7 +1124,7 @@ def test_another_translation_to_locale(self, get_current, edited_fire,
data['content'] = 'loremo ipsumo doloro sito ameto nuevo'
response = self.client.post(translate_uri, data)
eq_(302, response.status_code)
- eq_('http://testserver/en-US/docs/es/un-test-articulo',
+ eq_('http://testserver/es/docs/un-test-articulo',
response['location'])
doc = Document.objects.get(slug=data['slug'])
rev = doc.revisions.filter(content=data['content'])[0]
@@ -1149,17 +1149,17 @@ def test_translate_update_doc_only(self):
"""Submitting the document form should update document. No new
revisions should be created."""
rev_es = self._create_and_approve_first_translation()
- translate_path = 'es/' + self.d.slug
+ translate_path = self.d.slug
translate_uri = urllib.quote(reverse('wiki.translate',
- locale='en-US',
+ locale='es',
args=[translate_path]))
data = _translation_data()
new_title = 'Un nuevo titulo'
data['title'] = new_title
data['form'] = 'doc'
response = self.client.post(translate_uri, data)
eq_(302, response.status_code)
- eq_('http://testserver/en-US/docs/es/un-test-articulo$edit'
+ eq_('http://testserver/es/docs/un-test-articulo$edit'
'?opendescription=1',
response['location'])
revisions = rev_es.document.revisions.all()
@@ -1171,17 +1171,17 @@ def test_translate_update_rev_and_doc(self):
"""Submitting the revision form should create a new revision.
And since Kuma docs default to approved, should update doc too."""
rev_es = self._create_and_approve_first_translation()
- translate_path = 'es/' + self.d.slug
+ translate_path = self.d.slug
translate_uri = urllib.quote(reverse('wiki.translate',
- locale='en-US',
+ locale='es',
args=[translate_path]))
data = _translation_data()
new_title = 'Un nuevo titulo'
data['title'] = new_title
data['form'] = 'rev'
response = self.client.post(translate_uri, data)
eq_(302, response.status_code)
- eq_('http://testserver/en-US/docs/es/un-test-articulo',
+ eq_('http://testserver/es/docs/un-test-articulo',
response['location'])
revisions = rev_es.document.revisions.all()
eq_(2, revisions.count()) # New revision is created
@@ -1192,9 +1192,9 @@ def test_translate_form_content_fallback(self):
"""If there are existing but unapproved translations, prefill
content with latest."""
self.test_first_translation_to_locale()
- translate_path = 'es/' + self.d.slug
+ translate_path = self.d.slug
translate_uri = urllib.quote(reverse('wiki.translate',
- locale='en-US',
+ locale='es',
args=[translate_path]))
response = self.client.get(translate_uri)
doc = pq(response.content)
@@ -1213,9 +1213,9 @@ def test_translate_based_on(self):
d = Document.objects.get(pk=base_rev.document.id)
eq_(r, base_rev.document.current_revision)
- translate_path = 'es/' + d.slug
+ translate_path = d.slug
uri = urllib.quote(reverse('wiki.new_revision_based_on',
- locale='en-US',
+ locale='es',
args=[translate_path,
base_rev.id]))
response = self.client.get(uri)
@@ -1229,9 +1229,9 @@ def test_translate_rejected_parent(self):
en_revision = revision(is_approved=False, save=True, reviewer=user,
reviewed=datetime.now())
- translate_path = 'es/' + en_revision.document.slug
+ translate_path = en_revision.document.slug
translate_uri = urllib.quote(reverse('wiki.translate',
- locale='en-US',
+ locale='es',
args=[translate_path]))
response = self.client.get(translate_uri)
doc = pq(response.content)
@@ -1246,9 +1246,9 @@ def _test_form_maintains_based_on_rev(client, doc, view, post_data,
form was first loaded, even if other revisions have been approved in the
meantime."""
if trans_lang:
- translate_path = trans_lang + '/' + doc.slug
+ translate_path = doc.slug
uri = urllib.quote(reverse('wiki.translate',
- locale='en-US',
+ locale=trans_lang,
args=[translate_path]))
else:
uri = reverse(view, locale=locale, args=[doc.full_path])
Oops, something went wrong.

0 comments on commit 2ef4564

Please sign in to comment.