widelands-dev team mailing list archive
-
widelands-dev team
-
Mailing list archive
-
Message #08891
[Merge] lp:~widelands-dev/widelands-website/wlimages_rework into lp:widelands-website
kaputtnik has proposed merging lp:~widelands-dev/widelands-website/wlimages_rework into lp:widelands-website.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1523256 in Widelands Website: "Uploading images with uppercase letters cause server error"
https://bugs.launchpad.net/widelands-website/+bug/1523256
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands-website/wlimages_rework/+merge/311985
Fixes the bug 1523256 (Uploading images with uppercase letters cause server error) with additional changes:
Admin pages:
wiki/articles:
- Sort List by 'last_update' as default
- Added search for article titles
wiki/changeset
- Added search for article titles
wiki/article/<article_title>
- Removed list of changesets -> We have revert functions in the wiki and a changeset list in admin
- Instead show a list of images attached to this article
- commented the 'Group'-section. We currently do not use this and Django grumble on save if this
is shown in the admin page
wlimages/images/image
- content_type, object_id are 'read only' now, since it makes no sense to make this changeable
- Show also the content_object (e.g. the name of article this image is attached to)
- Add an admin action giving the possibility to remove the file from disk when deleting it
Stripped the 'image_list' from wiki/edit.html to an own template in wlimages
Changes to wiki:
- Trying to edit a wikipage which has an image attached which couldn't be found on disk causes formerly an IOError and the wiki article couldn't be edited anymore. To prevent this Error a filter is added and a hint on the missing file is shown in the list of images.
- Using of the url property of Django's default FieldFile object instead of providing an own model
field for that. The Django url() method returns automatically percent encoded strings (e.g. 'Br%C3%BCcke' instead of 'Brücke') See https://docs.djangoproject.com/en/1.8/ref/models/fields/#django.db.models.fields.files.FieldFile.url
Formerly uploading an image may cause several errors (e.g.: 'File already exists...'). Those errors redirected to an ugly page with a form wich didn't work for uploading an image. This is changed as follows:
- If an error occurs the redirect still happens but the page shows only the error (no form anymore) and provide a button to go back to the previous page.
Filehandling in general:
Until all Form validation is passed (e.g. 'File name too long') the get_valid_name() method of
Django's storage class is used to get a filename that is 'suitable for use on the target storage system'. See https://launchpadlibrarian.net/295298758/valid_name.jpg
More example image of changes:
- Redirect after error: https://launchpadlibrarian.net/295144720/upload_failure.jpg (top=old, bottom=new)
- Missing file and Url escaping: https://launchpadlibrarian.net/295145198/image_list.jpg
After merging './manage.py migrate' has to be run because i removed the url field in wlimages.Image
--
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands-website/wlimages_rework into lp:widelands-website.
=== modified file 'templates/wiki/edit.html'
--- templates/wiki/edit.html 2016-03-02 21:02:38 +0000
+++ templates/wiki/edit.html 2016-11-28 21:46:58 +0000
@@ -20,7 +20,7 @@
$("#preview").html("<h3>Preview</h3>\n<hr>\n<div class=\"wiki_article\" id=\"content_preview\">Loading...</div>\n<hr>");
$("#content_preview").load( "{% url 'wiki_preview' %}",
{"body": $("#id_content").val()});
- })
+ });
{% ifequal new_article 0 %}
// Insert diff
@@ -29,7 +29,7 @@
$("#diff").html("<h3>Diff</h3>\n<hr>\n<div id=\"content_diff\">Loading...</div>\n<hr>");
$("#content_diff").load( "{% url 'wiki_preview_diff' %}",
{"body": $("#id_content").val(), "article": {{ object_id }} });
- })
+ });
{%endifequal%}
});
</script>
@@ -95,55 +95,10 @@
{% else %}
{% if images.count %}
Images attached to this article:
-
- {% for img in images %}
- <table width="100%" class="images">
- <thead>
- <tr>
- <th class="first-column" style="text-align: right;">Name:</th>
- <th>{{ img.name}}</th>
- </tr>
- </thead>
- <tbody>
- <tr>
- <td>
- <span>Properties:</span><br />
- <span class="grey">Width:</span><br />
- {{ img.image.width }} px<br />
- <span class="grey">Height:</span><br />
- {{ img.image.height }} px<br />
- <span class="grey">Filesize:</span><br />
- {{ img.image.size|filesizeformat }}
- <td style="vertical-align: middle; text-align: center;"><img src="{{ img.url }}"></td>
- </tr>
- <tr>
- <td class="grey">Code to use in article:</td>
- {# Show only one code snippet if the image is greater than 700px #}
- {% if img.image.width > 700 %}
- <td style="text-align: center;">[]({{ img.url }})</td>
- </tr>
-
- {# otherwise provide all options #}
- {% else %}
- <td style="text-align: center;"></td>
- </tr>
-
- <tr>
- <td class="grey">Right aligned:</td>
- <td style="text-align: center;">{: .right}</td>
- </tr>
- <tr>
- <td class="grey">Left aligned:</td>
- <td style="text-align: center;">{: .left}</td>
- </tr>
- {% endif %}
- </tbody>
- </table>
- {% endfor %}
{% endif %}
-
+ {% include 'wlimages/inlines/image_list.html' %}
{% get_upload_form as iform %}
- <form enctype="multipart/form-data" action="/images/upload/{{content_type}}/{{object_id}}/{{request.path|iriencode}}" method="POST">
+ <form enctype="multipart/form-data" action="/images/upload/{{content_type}}/{{object_id}}/{{ request.path|iriencode }}" method="POST">
{{ iform.as_p }}
<div>
<input type="submit" value="{% trans "Upload Image" %}" />
=== added directory 'templates/wlimages/inlines'
=== added file 'templates/wlimages/inlines/image_list.html'
--- templates/wlimages/inlines/image_list.html 1970-01-01 00:00:00 +0000
+++ templates/wlimages/inlines/image_list.html 2016-11-28 21:46:58 +0000
@@ -0,0 +1,55 @@
+
+{% load wlimages_extras %}
+
+{% for img in images %}
+<table width="100%" class="images">
+ <thead>
+ <tr>
+ <th class="first-column" style="text-align: right;">Name:</th>
+ <th>{{ img.name}}</th>
+ </tr>
+ </thead>
+ <tbody>
+ {% if img|has_file %}
+ <tr>
+ <td>
+ <span>Properties:</span><br />
+ <span class="grey">Width:</span><br />
+ {{ img.image.width }} px<br />
+ <span class="grey">Height:</span><br />
+ {{ img.image.height }} px<br />
+ <span class="grey">Filesize:</span><br />
+ {{ img.image.size|filesizeformat }}
+ <td style="vertical-align: middle; text-align: center;"><img src="{{ img.image.url }}"></td>
+ </tr>
+ <tr>
+ <td class="grey">Code to use in article:</td>
+ {# Show only one code snippet if the image is greater than 700px #}
+ {% if img.image.width > 700 %}
+ <td style="text-align: center;">[]({{ img.image.url }})</td>
+ </tr>
+ {# provide all options #}
+ {% else %}
+ <td style="text-align: center;"></td>
+ </tr>
+ <tr>
+ <td class="grey">Right aligned:</td>
+ <td style="text-align: center;">{: .right}</td>
+ </tr>
+ <tr>
+ <td class="grey">Left aligned:</td>
+ <td style="text-align: center;">{: .left}</td>
+ </tr>
+ {% endif %}
+ {% else %}
+ <tr>
+ <td colspan=2 align="center">
+ !! Haven't found a file to this image !!<br />
+ {{ img.image.name}}<br />
+ Please inform an admin about this issue
+ </td>
+ </tr>
+ {% endif %}
+ </tbody>
+</table>
+{% endfor %}
=== modified file 'templates/wlimages/upload.html'
--- templates/wlimages/upload.html 2011-07-04 01:14:23 +0000
+++ templates/wlimages/upload.html 2016-11-28 21:46:58 +0000
@@ -4,9 +4,12 @@
{% endcomment %}
{% block content %}
-<form enctype="multipart/form-data" action="/images/upload/" method="POST">
-{{ upload_form.as_p }}
-<input type="submit" value="Submit" />
-{% csrf_token %}
-</form>
+ <div class="blogEntry"
+ <form enctype="multipart/form-data" action="/images/upload/" method="POST">
+ {{ upload_form.imagename.errors }}
+ {% csrf_token %}
+ </form>
+ <div class="button"><a href="{% url 'wiki_edit' referer %}">Back</a></div>
+ </div>
+
{% endblock %}
=== modified file 'wiki/admin.py'
--- wiki/admin.py 2009-02-20 12:25:18 +0000
+++ wiki/admin.py 2016-11-28 21:46:58 +0000
@@ -1,38 +1,43 @@
# -*- coding: utf-8 -*-
from django.contrib import admin
-
from wiki.models import Article, ChangeSet
-
-
-class InlineChangeSet(admin.TabularInline):
- model = ChangeSet
+from wlimages.models import Image
+from django.contrib.contenttypes.admin import GenericTabularInline
+
+
+class InlineImages(GenericTabularInline):
+ model = Image
extra = 0
- raw_id_fields = ('editor',)
+ fields = ('name', 'image', 'user', 'date_submitted')
+ raw_id_fields = ('user',)
+
class ArticleAdmin(admin.ModelAdmin):
- list_display = ('title', 'markup', 'created_at')
+ search_fields = ['title']
+ list_display = ('title', 'markup', 'created_at', 'last_update',)
list_filter = ('title',)
- ordering = ('last_update',)
+ ordering = ['-last_update']
fieldsets = (
(None, {'fields': ('title', 'content', 'markup')}),
('Creator', {'fields': ('creator', 'creator_ip'),
'classes': ('collapse', 'wide')}),
- ('Group', {'fields': ('object_id', 'content_type'),
- 'classes': ('collapse', 'wide')}),
+ # ('Group', {'fields': ('object_id', 'content_type'),
+ # 'classes': ('collapse', 'wide')}),
)
raw_id_fields = ('creator',)
- inlines = [InlineChangeSet]
+ inlines = [InlineImages]
admin.site.register(Article, ArticleAdmin)
class ChangeSetAdmin(admin.ModelAdmin):
+ search_fields = ['article__title']
list_display = ('article', 'revision', 'old_title', 'old_markup',
'editor', 'editor_ip', 'reverted', 'modified',
'comment')
list_filter = ('old_title', 'content_diff')
- ordering = ('modified',)
+ ordering = ('-modified',)
fieldsets = (
('Article', {'fields': ('article',)}),
('Differences', {'fields': ('old_title', 'old_markup',
=== modified file 'wiki/models.py'
--- wiki/models.py 2016-06-18 18:51:06 +0000
+++ wiki/models.py 2016-11-28 21:46:58 +0000
@@ -84,7 +84,7 @@
class Meta:
verbose_name = _(u'Article')
verbose_name_plural = _(u'Articles')
- app_label = 'wiki'
+ app_label = 'wiki'
def get_absolute_url(self):
if self.group is None:
@@ -134,7 +134,6 @@
changeset = self.changeset_set.get(revision=to_revision)
return changeset.compare_to(from_revision)
-
def __unicode__(self):
return self.title
=== modified file 'wiki/views.py'
--- wiki/views.py 2016-10-12 20:42:21 +0000
+++ wiki/views.py 2016-11-28 21:46:58 +0000
@@ -277,7 +277,7 @@
# 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
@@ -308,7 +308,6 @@
initial['action'] = 'edit'
form = ArticleFormClass(instance=article,
initial=initial)
-
if not article:
template_params = {'form': form, "new_article": True }
else:
=== modified file 'wl_utils.py'
--- wl_utils.py 2016-11-15 22:47:15 +0000
+++ wl_utils.py 2016-11-28 21:46:58 +0000
@@ -1,4 +1,5 @@
+# Get the real IP when the Django server runs behind a proxy
def get_real_ip(request):
"""Returns the real user IP, even if behind a proxy."""
for key in ('HTTP_X_FORWARDED_FOR', 'REMOTE_ADDR'):
=== modified file 'wlimages/admin.py'
--- wlimages/admin.py 2016-02-03 08:21:08 +0000
+++ wlimages/admin.py 2016-11-28 21:46:58 +0000
@@ -13,15 +13,25 @@
from django.utils.translation import ugettext_lazy as _
from models import Image
+
+def delete_with_file(modeladmin, request, queryset):
+ for obj in queryset:
+ storage = obj.image.storage
+ storage.delete(obj.image)
+ obj.delete()
+delete_with_file.short_description = 'Delete Image including File'
+
class ImageAdmin(admin.ModelAdmin):
+ readonly_fields = ('content_object', 'content_type', 'object_id')
list_display = ( '__unicode__', 'date_submitted', 'content_type', 'user')
list_filter = ('date_submitted',)
date_hierarchy = 'date_submitted'
search_fields = ('image', 'user__username')
+ actions = [delete_with_file]
fieldsets = (
- (None, {'fields': ( ('image', 'name'), 'date_submitted', 'url','revision')}),
+ (None, {'fields': ( 'image', 'name', 'date_submitted','revision')}),
(_('Upload data:'), { 'fields': ( 'user', 'editor_ip')}),
- (_('Content object:'), { 'fields': ( 'content_type', 'object_id')}),
+ (_('Content object:'), { 'fields': ( ('content_type', 'content_object'), 'object_id')}),
)
admin.site.register(Image, ImageAdmin)
=== modified file 'wlimages/forms.py'
--- wlimages/forms.py 2013-06-14 19:23:53 +0000
+++ wlimages/forms.py 2016-11-28 21:46:58 +0000
@@ -4,12 +4,18 @@
import os
class UploadImageForm(forms.Form):
- imagename = forms.ImageField()
+ # max_length = 90 because the length of 'upload_to =' has to be considered
+ imagename = forms.ImageField(max_length=90)
def clean_imagename( self ):
- name = self.cleaned_data["imagename"]
-
- if Image.objects.has_image(name.name.lower()):
- raise forms.ValidationError, "An Image with this name already exists. Please rename and upload again."
-
+ in_mem_img = self.cleaned_data["imagename"]
+
+ if Image.objects.has_image(in_mem_img.name):
+ # Get the existing image to show it's properties
+ exist_img = Image.objects.get(name = in_mem_img.name)
+ raise forms.ValidationError(
+ ("An Image with name '%(name)s' is already attached to %(object)s '%(obj_name)s'. \
+ Use the existing one or rename your file and upload gain."),
+ params={'name': exist_img.name, 'object': exist_img.content_type, 'obj_name': exist_img.content_object}
+ )
=== added file 'wlimages/migrations/0002_remove_image_url.py'
--- wlimages/migrations/0002_remove_image_url.py 1970-01-01 00:00:00 +0000
+++ wlimages/migrations/0002_remove_image_url.py 2016-11-28 21:46:58 +0000
@@ -0,0 +1,18 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('wlimages', '0001_initial'),
+ ]
+
+ operations = [
+ migrations.RemoveField(
+ model_name='image',
+ name='url',
+ ),
+ ]
=== modified file 'wlimages/models.py'
--- wlimages/models.py 2016-02-26 19:00:22 +0000
+++ wlimages/models.py 2016-11-28 21:46:58 +0000
@@ -6,8 +6,8 @@
from django.utils.translation import ugettext_lazy as _
from django.db import IntegrityError
from datetime import datetime
-
from settings import MEDIA_ROOT, MEDIA_URL
+from django.core.files.storage import FileSystemStorage
class ImageManager(models.Manager):
"""
@@ -17,7 +17,6 @@
"""
def has_image(self,image_name):
return bool(self.filter(name=image_name).count())
-
def create(self,**keyw):
"""
@@ -28,25 +27,22 @@
if self.filter(name=keyw["name"],revision=keyw["revision"]).count():
raise Image.AlreadyExisting()
-
+
return super(ImageManager,self).create(**keyw)
- def create_and_save_image(self,user,image, content_type, object_id, ip):
- # if self.has_image(name):
- # raise RuntimeError,"Image with name %s already exists. This is likely an Error" % name
- name = image.name.lower()
+ def create_and_save_image(self, user, image, content_type, object_id, ip):
+ # Use Django default's for a safe filename
+ storage = FileSystemStorage()
+ safe_filename = storage.get_valid_name(image.name)
im = self.create(content_type=content_type, object_id=object_id,
- user=user,revision=1,name=name, editor_ip = ip)
-
- path = "%swlimages/%s" % (MEDIA_ROOT,image.name)
- url = "%swlimages/%s" % (MEDIA_URL,image.name)
+ user=user, revision=1, name=image.name, editor_ip = ip)
+ path = "%swlimages/%s" % (MEDIA_ROOT,safe_filename)
destination = open(path,"wb")
for chunk in image.chunks():
destination.write(chunk)
- im.image = "wlimages/%s" % (name)
- im.url = url
+ im.image = "wlimages/%s" % (safe_filename)
im.save()
@@ -74,7 +70,6 @@
# Date Fields
date_submitted = models.DateTimeField(_('date/time submitted'), default = datetime.now)
image = models.ImageField(upload_to="wlimages/")
- url = models.CharField(max_length=250)
objects = ImageManager()
=== modified file 'wlimages/templatetags/wlimages_extras.py'
--- wlimages/templatetags/wlimages_extras.py 2016-03-07 20:38:39 +0000
+++ wlimages/templatetags/wlimages_extras.py 2016-11-28 21:46:58 +0000
@@ -8,6 +8,7 @@
# import re
from django import template
from wlimages.forms import UploadImageForm
+import os.path
register = template.Library()
@@ -33,6 +34,12 @@
form = UploadImageForm()
context[self.context_name] = form
return ''
-
register.tag('get_upload_form', do_get_upload_image_form)
+def has_file(obj):
+ try:
+ os.path.isfile(obj.image.file.name)
+ return True
+ except IOError:
+ return False
+register.filter('has_file', has_file)
=== modified file 'wlimages/views.py'
--- wlimages/views.py 2016-10-12 20:42:21 +0000
+++ wlimages/views.py 2016-11-28 21:46:58 +0000
@@ -30,12 +30,17 @@
if form.is_valid(): # All validation rules pass
Image.objects.create_and_save_image(user=request.user,image=request.FILES["imagename"],
content_type=ContentType.objects.get(pk=content_type),object_id=object_id, ip=get_real_ip(request))
-
return HttpResponseRedirect(next) # Redirect after POST
else:
form = UploadImageForm() # An unbound form
+
+ # Get the App (model) to which this image belongs to:
+ app = ContentType.objects.get(id=content_type)
+ # Get the current objects name (provided by __unicode__()) from this model
+ name = app.get_object_for_this_type(id=object_id)
return render_to_response('wlimages/upload.html', {
'upload_form': form,
+ 'referer': name,
}, context_instance=RequestContext(request))
Follow ups
