← Back to team overview

widelands-dev team mailing list archive

[Merge] lp:~widelands-dev/widelands-website/bug-1399461_error_on_renaming_article into lp:widelands-website

 

kaputtnik has proposed merging lp:~widelands-dev/widelands-website/bug-1399461_error_on_renaming_article into lp:widelands-website.

Commit message:
Prevent double names of wiki articles. Add a 'What links here' page.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1399461 in Widelands Website: "Renaming an article triggers an error, if article exists"
  https://bugs.launchpad.net/widelands-website/+bug/1399461

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands-website/bug-1399461_error_on_renaming_article/+merge/314033

This fixes bug 1399461 'Renaming an article triggers an error, if article exists' by making the name of an article a unique value on database level. Also prevents reverting an article if the reverting implies renaming and the name to which the article wants to be renamed exists in the meantime. An example (which shows also some other difficulties), imagine some month/years left between the steps:

1. Article "A_1" was created
2. Article "B_1" was created
3. "A_1" get renamed to "A" -> All old links called "A_1" redirects then to article "A"
4. "B_1" get renamed to "A_1" -> All old links called "A_1" open now article "A_1" (not "A" anymore)
5. "A" wants to be reverted to a revision where its name was "A_1" -> This is now prevented, because "A_1" already exists (got renamed from "B_1")

I wanted to prevent step 4 (renaming an article to a name where a redirect exists) but i couldn't fiddle this out. In general i think redirects are not really a good thing at all, because it may lead into many redirects after some time. So i implemented a 'backlinks' page ('What links here'). This page shows following information:

1. All articles which have links to the current name of this article
2. All articles which have links which points to (one of) the old name(s) of this article (redirects)
3. If this article is not linked at all, show a message with a request to link it somewhere

The 2. point has also a request to change the links in all listed articles to point to the current name. Generally speaking: Please remove the redirects
Point 3. has a disadvantage: If an article is only linked over the main menu, it shows also the message to link this article somewhere. Not the best, but i have no idea how to fix this in a convenient manner.

Changes to admin page of wiki:
1. Add a link to show all changesets of this article

If this branch get deployed, one has to run './manage.py migrate' to apply the changes.

-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands-website/bug-1399461_error_on_renaming_article into lp:widelands-website.
=== added directory 'templates/admin'
=== added directory 'templates/admin/wiki'
=== added directory 'templates/admin/wiki/article'
=== added file 'templates/admin/wiki/article/change_form.html'
--- templates/admin/wiki/article/change_form.html	1970-01-01 00:00:00 +0000
+++ templates/admin/wiki/article/change_form.html	2017-01-03 20:32:57 +0000
@@ -0,0 +1,9 @@
+{% extends "admin/change_form.html" %}
+{% load i18n admin_urls %}
+
+{% block object-tools-items %}
+    <li>
+        <a href="/admin/wiki/changeset/?article__title={{ original.title }}" class="historylink">{% trans "Changesets" %}</a>
+    </li>
+    {% if has_absolute_url %}<li><a href="{{ absolute_url }}" class="viewsitelink">{% trans "View on site" %}</a></li>{% endif %}
+{% endblock %}

=== added file 'templates/wiki/backlinks.html'
--- templates/wiki/backlinks.html	1970-01-01 00:00:00 +0000
+++ templates/wiki/backlinks.html	2017-01-03 20:32:57 +0000
@@ -0,0 +1,39 @@
+{% extends 'wiki/base.html' %}
+{% load wiki_extras %}
+{% load i18n %}
+{% block title %}
+{{ name }} - Backlinks - {{block.super}}
+{% endblock %}
+
+{% block content %}
+<div class="posRight small">
+	<a class="invertedColor" href="{% url 'wiki_article' name %}">Back to article</a>
+</div>
+<h1>{% trans "Backlinks of " %} {{ name }}</h1>
+<div class="blogEntry">
+	{% if found_links or found_old_links %}
+		{% if found_links %}
+		<h3>Following pages have link(s) which points to this article:</h3>
+		<ul>
+		{% for article in found_links %}
+				<li><a href="{% url 'wiki_article' article.title %}">{{ article.title }}</a></li>
+		{% endfor %}
+		</ul>
+		{% endif %}
+		{% if found_old_links %}
+		<h3> Following articles have links which points to one of the old name(s) of this article </h3>
+		<p>Just an easy thing to contribute: Edit those pages and change the links to let them point to '{{ name }}'
+		<img src="/wlmedia/img/smileys/face-smile.png" alt="face-smile.png"> See <a href="/wiki/WikiSyntax/#links">Wiki Syntax</a> for explanations.</p>
+		<ul>
+		{% for article in found_old_links %}
+			<li><a href="{% url 'wiki_article' article.title %}">{{ article.title }}</a> ({{ article.old_title }})</li>		
+		{% endfor %}
+		</ul>
+		{% endif %}
+	{% else %}
+		<p><span class="errormessage">Every Wikipage must at least be linked from within another page.</span>
+		Please link it <img src="/wlmedia/img/smileys/face-smile.png" alt="face-smile.png"> See <a href="/wiki/WikiSyntax/#links">Wiki Syntax</a> for explanations.</p></p>
+	{% endif %}
+</div>
+{% endblock %}
+

=== modified file 'templates/wiki/view.html'
--- templates/wiki/view.html	2016-04-26 16:10:04 +0000
+++ templates/wiki/view.html	2017-01-03 20:32:57 +0000
@@ -18,6 +18,8 @@
 		<a class="invertedColor" href="{% url 'wiki_edit' article.title %}">{% trans "Edit this article" %}</a>
 		|
 		<a class="invertedColor" href="{% url 'wiki_article_history' article.title %}">{% trans "Editing history" %}</a>
+		|
+		<a class="invertedColor" href="{% url 'backlinks' article.title %}">{% trans "Backlinks" %}</a>
 		{% if can_observe %}
 			| 
 			{% if is_observing %} 
@@ -29,6 +31,13 @@
 	{% endif %}
 </div>
 <h1>{{ article.title }}</h1>
+
+	{% if messages %}
+	    {% for message in messages %}
+	    <p class="errormessage">{{ message }}</p>
+	    {% endfor %}
+	{% endif %}
+
 <div class="blogEntry">
 	{% if not article.id %}
 		<p>

=== modified file 'wiki/admin.py'
--- wiki/admin.py	2016-11-22 22:17:23 +0000
+++ wiki/admin.py	2017-01-03 20:32:57 +0000
@@ -15,7 +15,7 @@
 
 class ArticleAdmin(admin.ModelAdmin):
     search_fields = ['title']
-    list_display = ('title', 'markup', 'created_at', 'last_update',)
+    list_display = ('title', 'creator', 'last_update',)
     list_filter = ('title',)
     ordering = ['-last_update']
     fieldsets = (
@@ -32,11 +32,11 @@
 
 
 class ChangeSetAdmin(admin.ModelAdmin):
-    search_fields = ['article__title']
-    list_display = ('article', 'revision', 'old_title', 'old_markup',
-                    'editor', 'editor_ip', 'reverted', 'modified',
+    search_fields = ['old_title']
+    list_display = ('article', 'old_title', 'old_markup',
+                    'editor', 'reverted', 'modified',
                     'comment')
-    list_filter = ('old_title', 'content_diff')
+    list_filter = ('article__title',)
     ordering = ('-modified',)
     fieldsets = (
         ('Article', {'fields': ('article',)}),

=== added file 'wiki/migrations/0002_auto_20161218_1056.py'
--- wiki/migrations/0002_auto_20161218_1056.py	1970-01-01 00:00:00 +0000
+++ wiki/migrations/0002_auto_20161218_1056.py	2017-01-03 20:32:57 +0000
@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('wiki', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='article',
+            name='title',
+            field=models.CharField(unique=True, max_length=50, verbose_name='Title'),
+        ),
+    ]

=== modified file 'wiki/models.py'
--- wiki/models.py	2016-12-13 18:28:51 +0000
+++ wiki/models.py	2017-01-03 20:32:57 +0000
@@ -48,8 +48,8 @@
 
 
 class Article(models.Model):
-    """A wiki page."""
-    title = models.CharField(_(u"Title"), max_length=50)
+    """A wiki page reflecting the actual revision."""
+    title = models.CharField(_(u"Title"), max_length=50, unique=True)
     content = models.TextField(_(u"Content"))
     summary = models.CharField(_(u"Summary"), max_length=150,
                                null=True, blank=True)

=== modified file 'wiki/urls.py'
--- wiki/urls.py	2016-12-13 18:28:51 +0000
+++ wiki/urls.py	2017-01-03 20:32:57 +0000
@@ -56,4 +56,7 @@
 
     url(r'^history/(?P<title>' + WIKI_URL_RE + r')/revert/$', views.revert_to_revision,
         name='wiki_revert_to_revision'),
+    
+    url(r'^backlinks/(?P<title>' + WIKI_URL_RE + r')/$', views.backlinks,
+        name='backlinks'),
 ]

=== modified file 'wiki/views.py'
--- wiki/views.py	2016-12-13 18:28:51 +0000
+++ wiki/views.py	2017-01-03 20:32:57 +0000
@@ -10,15 +10,16 @@
                          HttpResponseNotAllowed, HttpResponse, HttpResponseForbidden)
 from django.shortcuts import get_object_or_404, render_to_response, redirect
 from django.contrib.contenttypes.models import ContentType
-
+from django.contrib import messages
+from django.core.exceptions import ObjectDoesNotExist
 from wiki.forms import ArticleForm
 from wiki.models import Article, ChangeSet, dmp
 
 from wiki.utils import get_ct
 from django.contrib.auth.decorators import login_required
-from mainpage.templatetags.wl_markdown import do_wl_markdown
 
 from wl_utils import get_real_ip
+import re
 
 # Settings
 #  lock duration in minutes
@@ -271,18 +272,8 @@
         form.cache_old_content()
         if form.is_valid():
 
-            # NOCOMM Franku: This has never worked as i know and is IMHO
-            # useless. This code works with django 1.8 but misses some code
-            # in template. See
-            # https://docs.djangoproject.com/en/1.8/ref/contrib/messages/#module-django.contrib.messages
-
             if request.user.is_authenticated():
                 form.editor = request.user
-            #     if article is None:
-            #         user_message = u"Your article was created successfully."
-            #     else:
-            #         user_message = u"Your article was edited successfully."
-            #     messages.success(request, user_message)
 
             if ((article is None) and (group_slug is not None)):
                 form.group = group
@@ -478,18 +469,23 @@
 
         article = get_object_or_404(article_qs, **article_args)
 
-        if request.user.is_authenticated():
-            article.revert_to(revision, get_real_ip(request), request.user)
-        else:
-            article.revert_to(revision, get_real_ip(request))
-
-        # NOCOMM Franku: This has never worked as i know and is IMHO
-        # useless. If we want this it has to be fixed for django 1.8
-        # See comment in edit_article()
-        # if request.user.is_authenticated():
-        #     request.user.message_set.create(
-        #         message=u"The article was reverted successfully.")
-
+        
+        # Check whether there is another Article with the same name to which this article
+        # want's to be reverted to. If so: prevent it and show a message.
+        old_title = article.changeset_set.filter(
+            revision=revision+1).get().old_title
+        try:
+            art = Article.objects.exclude(pk=article.pk).get(title=old_title)
+        except ObjectDoesNotExist:
+            # No existing article found -> reverting possible
+            if request.user.is_authenticated():
+                article.revert_to(revision, get_real_ip(request), request.user)
+            else:
+                article.revert_to(revision, get_real_ip(request))
+            return redirect(article)
+        # An article with this name exists
+        messages.error(
+            request, 'Reverting not possible because an article with name \'%s\' already exists' % old_title)
         return redirect(article)
 
     return HttpResponseNotAllowed(['POST'])
@@ -622,3 +618,50 @@
     dmp.diff_cleanupSemantic(diffs)
 
     return HttpResponse(dmp.diff_prettyHtml(diffs), content_type='text/html')
+
+
+def backlinks(request, title):
+    """Simple text search for links in other wiki articles pointing to the
+    current article.
+
+    If we convert WikiWords to markdown wikilinks syntax, this view
+    should be changed to use '[[title]]' for searching.
+
+    """
+
+    # Find old title(s) of this article
+    this_article = Article.objects.get(title=title)
+    changesets = this_article.changeset_set.all()
+    old_titles = []
+    for cs in changesets:
+        if cs.old_title and cs.old_title != title and cs.old_title not in old_titles:
+            old_titles.append(cs.old_title)
+
+    # Differentiate between WikiWords and other
+    m = re.match(r"(!?)(\b[A-Z][a-z]+[A-Z]\w+\b)", title)
+    if m:
+        # title is a 'WikiWord' -> This catches also 'MingW' but we have no such title
+        search_title = re.compile(r"%s" % title)
+    else:
+        # Others must be written like links: '[Wiki Page](/wiki/Wiki Page)'
+        search_title = re.compile(r"\/%s\)" % title)
+    
+    # Search for current and previous titles
+    found_old_links = []
+    found_links = []
+    articles_all = Article.objects.all().exclude(title=title)
+    for article in articles_all:
+        match = search_title.search(article.content)
+        if match:
+            found_links.append({'title': article.title})
+        
+        for old_title in old_titles:
+            if old_title in article.content:
+                found_old_links.append({'old_title': old_title, 'title': article.title })
+
+    context = {'found_links': found_links,
+               'found_old_links': found_old_links,
+               'name': title}
+    return render_to_response('wiki/backlinks.html',
+                              context,
+                              context_instance=RequestContext(request))


Follow ups