Merge branch 'int-ua-feature/issue_95' into develop

* int-ua-feature/issue_95:
  fixes #95
  Trying to add tests for #95

CHANGES

@@ -1,6 +1,7 @@
 Release 1.5 (dev)
 =================
 * fixes :ghissue:`116` Fixing ManyToMany merging with intermediary models. (thanks int-ua)
+* fixes :ghissue:`95` Cannot merge models with subclassed ImageField or FileField: "file not sent". (thanks int-ua)
 
 
 Release 1.4

src/adminactions/api.py

@@ -9,6 +9,7 @@
 import xlwt
 from django.conf import settings
 from django.core.exceptions import ObjectDoesNotExist
+from django.db.models import FileField
 from django.db.models.fields import FieldDoesNotExist
 from django.db.models.fields.related import ManyToManyField, OneToOneField
 from django.http import HttpResponse
@@ -100,7 +101,9 @@ def merge(master, other, fields=None, commit=False, m2m=None, related=None):  #
 
         for fieldname in fields:
             f = get_field_by_path(master, fieldname)
-            if f and not f.primary_key:
+            if isinstance(f, FileField):
+                setattr(result, fieldname, getattr(other, fieldname))
+            elif f and not f.primary_key:
                 setattr(result, fieldname, getattr(other, fieldname))
 
         if m2m:
@@ -131,7 +134,6 @@ def merge(master, other, fields=None, commit=False, m2m=None, related=None):  #
                 for rel_fieldname, element in elements:
                     setattr(element, rel_fieldname, master)
                     element.save()
-
             other.delete()
             result.save()
             for fieldname, elements in list(all_m2m.items()):

src/adminactions/mass_update.py

@@ -3,6 +3,8 @@
 import datetime
 import json
 import re
+
+import logging
 import six
 from collections import OrderedDict as SortedDict, defaultdict
 
@@ -15,7 +17,7 @@
 from django.forms import fields as ff
 from django.forms.models import (InlineForeignKeyField,
                                  ModelMultipleChoiceField, construct_instance,
-                                 modelform_factory,)
+                                 modelform_factory, )
 from django.http import HttpResponseRedirect
 from django.shortcuts import render, render_to_response
 from django.template.context import RequestContext

src/adminactions/merge.py

@@ -53,7 +53,10 @@ def clean_dependencies(self):
         return int(self.cleaned_data['dependencies'])
 
     def clean_field_names(self):
-        return self.cleaned_data['field_names'].split(',')
+        if self.cleaned_data['field_names']:
+            return self.cleaned_data['field_names'].split(',')
+        else:
+            return None
 
     def full_clean(self):
         super(MergeForm, self).full_clean()
@@ -84,15 +87,18 @@ def merge(modeladmin, request, queryset):  # noqa
     def raw_widget(field, **kwargs):
         """ force all fields as not required"""
         kwargs['widget'] = TextInput({'class': 'raw-value'})
+        if isinstance(field, models.FileField):
+            kwargs["form_class"] = forms.CharField
+
         return field.formfield(**kwargs)
 
     merge_form = getattr(modeladmin, 'merge_form', MergeForm)
     MForm = modelform_factory(modeladmin.model,
                               form=merge_form,
-                              exclude=('pk', ),
+                              exclude=('pk',),
                               formfield_callback=raw_widget)
     OForm = modelform_factory(modeladmin.model,
-                              exclude=('pk', ),
+                              exclude=('pk',),
                               formfield_callback=raw_widget)
 
     tpl = 'adminactions/merge.html'
@@ -162,7 +168,7 @@ def raw_widget(field, **kwargs):
                                 raw_value.minute,
                                 raw_value.second)
                             setattr(target, field.name, fixed_value)
-        except ValueError:
+        except ValueError as e:
             messages.error(request, _('Please select exactly 2 records'))
             return
 

src/adminactions/templates/adminactions/merge.html

@@ -23,7 +23,6 @@
         {{ adminform.form.non_field_errors }}
         <form method="post">{% csrf_token %}
             {% for f in adminform.form.hidden_fields %}{{ f }}{% endfor %}
-            {{ adminform.form.field_names }}
             <table>
                 <tr>
                     <td>{{ adminform.form.dependencies.label }}</td>

tests/conftest.py

@@ -1,15 +1,25 @@
 import django_webtest
+import logging
 import pytest
 
+logger = logging.getLogger("test")
+
+levelNames = {
+    logging.CRITICAL: "CRITICAL",
+    logging.ERROR: "ERROR",
+    logging.WARNING: "WARNING",
+    logging.INFO: "INFO",
+    logging.DEBUG: "DEBUG",
+    logging.NOTSET: "NOTSET",
+    "CRITICAL": logging.CRITICAL,
+    "ERROR": logging.ERROR,
+    "WARN": logging.WARNING,
+    "WARNING": logging.WARNING,
+    "INFO": logging.INFO,
+    "DEBUG": logging.DEBUG,
+    "NOTSET": logging.NOTSET,
+}
 
-# def pytest_configure(config):
-#     try:
-#         from django.apps import AppConfig  # noqa
-#         import django
-#
-#         django.setup()
-#     except ImportError:
-#         pass
 
 def pytest_addoption(parser):
     group = parser.getgroup("selenium", "Selenium Web Browser Automation")
@@ -26,6 +36,14 @@ def pytest_addoption(parser):
              "py.test --selenium-enable --chrome-driver=/home/chromedriver. -k test_name."
     )
 
+    parser.addoption("--log", default=None, action="store",
+                     dest="log_level",
+                     help="enable console log")
+
+    parser.addoption("--log-add", default="", action="store",
+                     dest="log_add",
+                     help="add package to log")
+
 
 def pytest_configure(config):
     # import warnings
@@ -38,6 +56,28 @@ def pytest_configure(config):
         if not config.option.selenium_enable:
             setattr(config.option, 'markexpr', 'not selenium')
 
+    if config.option.log_level:
+        import logging
+        level = config.option.log_level.upper()
+        assert level in levelNames.keys()
+        format = "%(levelname)-7s %(name)-30s %(funcName)-20s:%(lineno)3s %(message)s"
+        formatter = logging.Formatter(format)
+
+        handler = logging.StreamHandler()
+        handler.setLevel(levelNames[level])
+        handler.setFormatter(formatter)
+
+        for app in ["test", "demo", "adminactions"]:
+            l = logging.getLogger(app)
+            l.setLevel(levelNames[level])
+            l.addHandler(handler)
+
+        if config.option.log_add:
+            for pkg in config.option.log_add.split(","):
+                l = logging.getLogger(pkg)
+                l.setLevel(levelNames[level])
+                l.addHandler(handler)
+
 
 @pytest.fixture(scope='function')
 def app(request):

tests/demo/fixtures/demoproject.json

@@ -21,7 +21,9 @@
             "integer": 90000,
             "unique": "unique 1",
             "email": "s.apostolico@gmail.com",
-            "choices": 2
+            "choices": 2,
+            "image": "second.png",
+            "subclassed_image": false
         }
     },
     {
@@ -46,14 +48,16 @@
             "integer": 888888,
             "unique": "unique 2",
             "email": "s.apostolico@gmail.com",
-            "choices": 2
+            "choices": 2,
+            "image": "first.png",
+            "subclassed_image": "subclassed_first.png"
         }
     },
     {
-        "pk": 2,
+        "pk": 3,
         "model": "demo.demomodel",
         "fields": {
-            "nullable": "bbbb",
+            "nullable": "dddd",
             "float": 10.1,
             "generic_ip": "192.168.10.2",
             "url": "https://github.com/saxix/django-adminactions",
@@ -69,9 +73,12 @@
             "logic": false,
             "date": "2013-01-29",
             "integer": 888888,
-            "unique": "unique 2",
+            "unique": "unique 3",
             "email": "s.apostolico@gmail.com",
-            "choices": 2
+            "choices": 2,
+          "image": false,
+
+            "subclassed_image": "subclassed_second.png"
         }
     }
 ]

tests/demo/migrations/0001_initial.py

@@ -5,6 +5,7 @@
 import django.db.models.deletion
 from django.conf import settings
 from django.db import migrations, models
+import demo.models
 
 
 class Migration(migrations.Migration):
@@ -39,6 +40,8 @@ class Migration(migrations.Migration):
                 ('blank', models.CharField(blank=True, max_length=255, null=True)),
                 ('not_editable', models.CharField(blank=True, editable=False, max_length=255, null=True)),
                 ('choices', models.IntegerField(choices=[(1, 'Choice 1'), (2, 'Choice 2'), (3, 'Choice 3')])),
+                ('image', models.ImageField(upload_to='', null=True, blank=True)),
+                ('subclassed_image', demo.models.SubclassedImageField(upload_to='', null=True, blank=True)),
             ],
         ),
         migrations.CreateModel(

tests/demo/models.py

@@ -6,6 +6,10 @@
 from django.db import models
 
 
+class SubclassedImageField(models.ImageField):
+    pass
+
+
 class DemoModel(models.Model):
     char = models.CharField('Chäř', max_length=255)
     integer = models.IntegerField()
@@ -30,8 +34,12 @@ class DemoModel(models.Model):
     not_editable = models.CharField(max_length=255, editable=False, blank=True, null=True)
     choices = models.IntegerField(choices=((1, 'Choice 1'), (2, 'Choice 2'), (3, 'Choice 3')))
 
+    image = models.ImageField(blank=True, null=True)
+    subclassed_image = SubclassedImageField(blank=True, null=True)
+
     class Meta:
         app_label = 'demo'
+        ordering = ('-id',)
 
 
 class UserDetail(models.Model):

tests/test_mass_update.py

@@ -62,17 +62,18 @@ def test_no_permission(self):
 
     def test_validate_on(self):
         self._run_action(**{'_validate': 1})
-        assert DemoModel.objects.filter(char='BBB').exists()
-        assert not DemoModel.objects.filter(char='bbb').exists()
+        assert DemoModel.objects.filter(char='CCCCC').exists()
+        assert not DemoModel.objects.filter(char='ccccc').exists()
 
     def test_validate_off(self):
         self._run_action(**{'_validate': 0})
         self.assertIn("Unable no mass update using operators without", self.app.cookies['messages'])
 
     def test_clean_on(self):
         self._run_action(**{'_clean': 1})
-        assert DemoModel.objects.filter(char='BBB').exists()
-        assert not DemoModel.objects.filter(char='bbb').exists()
+
+        assert DemoModel.objects.filter(char='CCCCC').exists()
+        assert not DemoModel.objects.filter(char='ccccc').exists()
 
     def test_messages(self):
         with user_grant_permission(self.user, ['demo.change_demomodel', 'demo.adminactions_massupdate_demomodel']):

tests/test_merge.py

@@ -1,6 +1,7 @@
 # -*- coding: utf-8 -*-
 from __future__ import absolute_import
 
+import os
 import six
 from six.moves import range
 
@@ -123,6 +124,7 @@ def test_merge_one_to_one_move_single(self):
         master = DemoModel.objects.get(pk=result.pk)  # reload
         self.assertEqual(master.onetoone, related_one)
         self.assertTrue(DemoOneToOne.objects.filter(pk=related_one.pk).exists())
+        self.assertEqual(os.path.basename(master.image.file.name), "first.png")
 
     # @skipIf(not hasattr(settings, 'AUTH_PROFILE_MODULE'), "")
     def test_merge_one_to_one_field(self):
@@ -151,6 +153,24 @@ def test_merge_ignore_related(self):
         self.assertFalse(User.objects.filter(pk=other.pk).exists())
         self.assertFalse(LogEntry.objects.filter(pk=entry.pk).exists())
 
+    def test_merge_image(self):
+        master = DemoModel.objects.get(pk=3)
+        other = DemoModel.objects.get(pk=1)
+        img1 = other.image
+        img2 = other.subclassed_image
+
+        assert master.image != other.image
+        assert master.subclassed_image != other.subclassed_image
+
+        result = merge(master, other,
+                       fields=['image', 'subclassed_image'],
+                       commit=True, related=None)
+
+        master = DemoModel.objects.get(pk=result.pk)  # reload
+        self.assertFalse(DemoModel.objects.filter(pk=other.pk).exists())
+        self.assertEqual(master.image, img1)
+        self.assertEqual(master.subclassed_image, img2)
+
 
 class TestMergeAction(SelectRowsMixin, WebTestMixin, TransactionTestCase):
     csrf_checks = True
@@ -335,3 +355,67 @@ def test_merge_delete_detail(self):
             preserved_after = User.objects.get(pk=self._selected_values[1])
             self.assertEqual(preserved_after.userdetail_set.count(), 1)
             self.assertFalse(User.objects.filter(pk=removed.pk).exists())
+
+
+class TestMergeImageAction(SelectRowsMixin, WebTestMixin, TransactionTestCase):
+    csrf_checks = True
+    fixtures = ['adminactions.json', 'demoproject.json']
+    urls = 'demo.urls'
+    sender_model = User
+    action_name = 'merge'
+    _selected_rows = [0, 2]
+
+    def setUp(self):
+        super(TestMergeImageAction, self).setUp()
+        self.url = reverse('admin:demo_demomodel_changelist')
+        self.user = G(User, username='user', is_staff=True, is_active=True)
+
+    def _run_action(self, steps=3, page_start=None):
+        with user_grant_permission(self.user,
+                                   ['demo.change_demomodel',
+                                    'demo.adminactions_merge_demomodel']):
+            if isinstance(steps, int):
+                steps = list(range(1, steps + 1))
+                res = self.app.get('/', user='user')
+                res = res.click('Demo models')
+            else:
+                res = page_start
+
+            if 1 in steps:
+                form = res.forms['changelist-form']
+                form['action'] = 'merge'
+                self._select_rows(form)
+                res = form.submit()
+                assert not hasattr(res.form, 'errors')
+
+            if 2 in steps:
+                res.form['image'] = res.form['form-1-image'].value
+                res = res.form.submit('preview')
+                assert not hasattr(res.form, 'errors')
+
+            if 3 in steps:
+                res = res.form.submit('apply')
+            return res
+
+    def test_success(self):
+        res = self._run_action(1)
+        preserved = DemoModel.objects.get(pk=self._selected_values[0])
+        removed = DemoModel.objects.get(pk=self._selected_values[1])
+
+        img1 = removed.image
+        img2 = removed.subclassed_image
+
+        assert preserved.image != removed.image  # sanity check
+        assert preserved.subclassed_image != removed.subclassed_image  # sanity check
+        assert preserved.pk == 3  # sanity check
+        assert removed.pk == 1  # sanity check
+
+        self._run_action([2, 3], res)
+
+        self.assertFalse(DemoModel.objects.filter(pk=removed.pk).exists())
+        self.assertTrue(DemoModel.objects.filter(pk=preserved.pk).exists())
+
+        preserved_after = DemoModel.objects.get(pk=preserved.pk)
+
+        assert preserved_after.image == img1
+        assert preserved_after.subclassed_image == img2