Merge lp:~widelands-dev/widelands-website/move_maps_minimap into lp:widelands-website

[kaputtnik]

Move minimaps to wlmaps/minimaps
Implemented the new value needs_version_after
Delete map files (.wmf and .png) when deleting a map from the database

The maps minimaps are now moved to wlmaps/minimaps. This fixes also creating a faulty path in the database for the minimaps (beginning with a slash).

Deleting a map does now delete the related files.

The new value of 'needs_version_after' produced by wl_map_info is now considered. There is now an inline html file for this, so the logic is in one file.

The changes related to minimaps need manual intervention regarding the files and the database entries:

- All png files in MEDIA_ROOT/wlmaps/maps/ needs moving to MEDIA_ROOT/wlmaps/minimaps/
- Database entries for minimaps need to be changed from
  '/wlmaps/maps/NAME.png'
  to
  'wlmaps/minimaps/NAME.png'

To get this in:
- set maintenance
- backup the database
- backup the files from MEDIA_ROOT/wlmaps/
- run ./manage.py migrate
- move the png files
- correct the path(s) in the database. I want to change also some bad entries for the mapfiles. With activated virtualenvironment:

./manage.py shell
from wlmaps.models import Map
maps = Map.objects.all()
for m in maps:
    if m.minimap.name.startswith('/'):
        new_name = m.minimap.name.rpartition('/')[2]
        m.minimap = 'wlmaps/minimaps/{}'.format(new_name)
        m.save()

maps = Map.objects.all()
for m in maps:
    if m.file.name.startswith('/var/www'):
        new_name = m.file.name.rpartition('/')[2]
        m.file = 'wlmaps/maps/{}'.format(new_name)
        m.save()

This can maybe done also in one loop (changing minimap and file), but i fear saving an object twice in one loop can be problematic, although at home it works nicely.

[GunChleoc (gunchleoc)]

LGTM :)

review: Approve

[lp:~widelands-dev/widelands-website/move_maps_minimap]

548. By kaputtnik on 2019-07-14

merged with trunk

[kaputtnik (franku)]

I have to test a bit more before merging this. On my second computer i get some errors after merging this branch.

review: Needs Fixing

[lp:~widelands-dev/widelands-website/move_maps_minimap]

549. By kaputtnik on 2019-07-14

use a custom delete function instead of signals

[kaputtnik (franku)]

A better solution: Using signals, especially when deleting files, can lead into some sideeffects. So i have implemented a custom delete() function which is much better, imho.

review: Needs Resubmitting

[lp:~widelands-dev/widelands-website/move_maps_minimap]

550. By kaputtnik on 2019-07-15

merged with trunk

[GunChleoc (gunchleoc)]

LGTM :)

[kaputtnik (franku)]

Thanks :-)

Merged and deployed.

=== modified file 'wlmaps/admin.py'
--- wlmaps/admin.py	2019-04-11 15:06:09 +0000
+++ wlmaps/admin.py	2019-07-15 06:06:57 +0000
@@ -5,8 +5,20 @@
 from .models import Map
 from django.contrib import admin
 
+def delete_selected(modeladmin, request, queryset):
+    """Overwritten Django's default action to delete a map.
+
+    This action uses the delete() method of the map model. This ensures
+    also deleting the corresponding files (.wmf and .png) of a map.
+
+    """
+    for obj in queryset:
+        obj.delete()
+delete_selected.short_description = 'Delete selected maps'
+
 
 class MapAdmin(admin.ModelAdmin):
+    actions = [delete_selected,]
     list_display = ['name', 'author', 'pub_date']
     prepopulated_fields = {'slug': ('name',)}
     search_fields = ['name', 'author']
@@ -14,7 +26,7 @@
     readonly_fields = ('uploader', 'nr_players', 'w', 'h', 'minimap', 'file', 'world_name')
     fieldsets = (
         (None, {
-            'fields': (('name', 'author'), 'uploader', 'uploader_comment')
+            'fields': (('name', 'author'), 'uploader', 'uploader_comment', 'wl_version_after')
         }),
         ('Map properties', {
             'classes': ('collapse',),

=== modified file 'wlmaps/forms.py'
--- wlmaps/forms.py	2019-03-31 11:08:21 +0000
+++ wlmaps/forms.py	2019-07-15 06:06:57 +0000
@@ -11,6 +11,8 @@
 
 from wlmaps.models import Map
 import os
+import shutil
+
 
 class UploadMapForm(ModelForm):
     """
@@ -59,8 +61,8 @@
             os.chdir(settings.WIDELANDS_SVN_DIR)
             check_call(['wl_map_info', saved_file])
 
-            # TODO(shevonar): delete file because it will be saved again when
-            # the model is saved. File should not be saved twice
+            # Deleting the file because it will be saved again when
+            # the model is saved.
             default_storage.delete(saved_file)
             os.chdir(old_cwd)
         except CalledProcessError:
@@ -86,11 +88,18 @@
         self.instance.descr = mapinfo['description']
         self.instance.hint = mapinfo['hint']
         self.instance.world_name = mapinfo['world_name']
+        self.instance.wl_version_after = mapinfo['needs_widelands_version_after']
 
-        # mapinfo["minimap"] is an absolute path.
-        # We partition it to get the correct file path
-        minimap_path = mapinfo['minimap'].partition(settings.MEDIA_ROOT)[2]
-        self.instance.minimap = '/' + minimap_path
+        # mapinfo["minimap"] is the absolute path containing the path where it
+        # is saved, extract the name
+        minimap_name = mapinfo['minimap'].rpartition('/')[2]
+        minimap_upload_to = self.instance._meta.get_field('minimap').upload_to
+        # Set the destination relative to MEDIA_ROOT
+        minimap_path = os.path.join(minimap_upload_to, minimap_name)
+        self.instance.minimap = minimap_path
+        # Move the minimap file (.png) from wlmaps/maps to wlmaps/minimaps
+        shutil.move(mapinfo['minimap'], os.path.join(
+            settings.MEDIA_ROOT, minimap_path))
 
         # the json file is no longer needed
         default_storage.delete(saved_file + '.json')

=== added file 'wlmaps/migrations/0003_auto_20190712_0928.py'
--- wlmaps/migrations/0003_auto_20190712_0928.py	1970-01-01 00:00:00 +0000
+++ wlmaps/migrations/0003_auto_20190712_0928.py	2019-07-15 06:06:57 +0000
@@ -0,0 +1,65 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.22 on 2019-07-12 09:28
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('wlmaps', '0002_auto_20181119_1855'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='map',
+            name='wl_version_after',
+            field=models.PositiveIntegerField(blank=True, null=True, verbose_name='WL version after'),
+        ),
+        migrations.AlterField(
+            model_name='map',
+            name='descr',
+            field=models.TextField(verbose_name='Description'),
+        ),
+        migrations.AlterField(
+            model_name='map',
+            name='file',
+            field=models.FileField(upload_to='wlmaps/maps', verbose_name='Mapfile'),
+        ),
+        migrations.AlterField(
+            model_name='map',
+            name='h',
+            field=models.PositiveIntegerField(verbose_name='Height'),
+        ),
+        migrations.AlterField(
+            model_name='map',
+            name='hint',
+            field=models.TextField(blank=True, verbose_name='Hint'),
+        ),
+        migrations.AlterField(
+            model_name='map',
+            name='minimap',
+            field=models.ImageField(upload_to='wlmaps/minimaps', verbose_name='Minimap'),
+        ),
+        migrations.AlterField(
+            model_name='map',
+            name='nr_downloads',
+            field=models.PositiveIntegerField(default=0, verbose_name='Download count'),
+        ),
+        migrations.AlterField(
+            model_name='map',
+            name='nr_players',
+            field=models.PositiveIntegerField(verbose_name='Max Players'),
+        ),
+        migrations.AlterField(
+            model_name='map',
+            name='uploader_comment',
+            field=models.TextField(blank=True, verbose_name='Uploader comment'),
+        ),
+        migrations.AlterField(
+            model_name='map',
+            name='w',
+            field=models.PositiveIntegerField(verbose_name='Width'),
+        ),
+    ]

=== modified file 'wlmaps/models.py'
--- wlmaps/models.py	2019-04-11 15:20:33 +0000
+++ wlmaps/models.py	2019-07-15 06:06:57 +0000
@@ -5,7 +5,11 @@
 from django.contrib.auth.models import User
 from django.template.defaultfilters import slugify
 from django.urls import reverse
+from django.db.models.signals import pre_delete
+from django.conf import settings
+
 import datetime
+import os
 try:
     from notification import models as notification
 except ImportError:
@@ -35,7 +39,10 @@
     uploader = models.ForeignKey(User)
     nr_downloads = models.PositiveIntegerField(
         verbose_name='Download count', default=0)
-
+    wl_version_after = models.PositiveIntegerField(
+        verbose_name='WL version after',
+        null=True,
+        blank=True)
 
     class Meta:
         ordering = ('-pub_date',)
@@ -72,3 +79,11 @@
                               )
 
         return map
+
+    def delete(self, *args, **kwargs):
+        """Delete also corresponding map files."""
+
+        # For some reason this throws no error if a file didn't exist
+        self.minimap.delete()
+        self.file.delete()
+        super(Map, self).delete(*args, **kwargs)

=== modified file 'wlmaps/templates/wlmaps/index.html'
--- wlmaps/templates/wlmaps/index.html	2018-11-22 11:08:51 +0000
+++ wlmaps/templates/wlmaps/index.html	2019-07-15 06:06:57 +0000
@@ -69,13 +69,9 @@
 					</tr>
 					{% if not map.world_name %}
 					<tr>
-						<td colspan="5"><strong>This map requires a version of Widelands newer than build
-						{% if map.nr_players > 8 %}
-							19!
-						{% else %}
-							18!
-						{% endif %}
-						</strong></td>
+						<td colspan="5">
+						{% include 'wlmaps/inlines/version_info.html' %}
+						</td>
 					</tr>
 					{% endif %}
 					<tr>

=== added directory 'wlmaps/templates/wlmaps/inlines'
=== added file 'wlmaps/templates/wlmaps/inlines/version_info.html'
--- wlmaps/templates/wlmaps/inlines/version_info.html	1970-01-01 00:00:00 +0000
+++ wlmaps/templates/wlmaps/inlines/version_info.html	2019-07-15 06:06:57 +0000
@@ -0,0 +1,11 @@
+<strong>This map requires a version of Widelands newer than build
+    {% if map.wl_version_after %}
+    	{{ map.wl_version_after }}!
+    {% else %}
+    	{% if map.nr_players > 8 %}
+    		19!
+    	{% else %}
+    		18!
+    	{% endif %}
+    {% endif %}
+</strong>

=== modified file 'wlmaps/templates/wlmaps/map_detail.html'
--- wlmaps/templates/wlmaps/map_detail.html	2018-11-22 11:08:51 +0000
+++ wlmaps/templates/wlmaps/map_detail.html	2019-07-15 06:06:57 +0000
@@ -90,13 +90,7 @@
 		
 		{% if not map.world_name %}
 		<div>
-			<strong>This map requires a version of Widelands newer than build
-            {% if map.nr_players > 8 %}
-                19!
-            {% else %}
-                18!
-            {% endif %}
-            </strong>
+			{% include 'wlmaps/inlines/version_info.html'%}
 		</div>
 		{% endif %}
 	</div>