Merge pull request #3848 from addgene/mw/fix-publish-user

Fix #3845
Set Page.changed_by correctly in cms.api.publish_page()

cms/api.py

@@ -32,7 +32,7 @@
 from cms.utils import copy_plugins
 from cms.utils.conf import get_cms_setting
 from cms.utils.i18n import get_language_list
-from cms.utils.permissions import _thread_locals
+from cms.utils.permissions import _thread_locals, current_user
 from menus.menu_pool import menu_pool
 
 
@@ -448,7 +448,11 @@ def __init__(self, user):
     request = FakeRequest(user)
     if not page.has_publish_permission(request):
         raise PermissionDenied()
-    page.publish(language)
+    # Set the current_user to have the page's changed_by
+    # attribute set correctly.
+    # 'user' is a user object, but current_user() just wants the username (a string).
+    with current_user(user.get_username()):
+        page.publish(language)
     return page.reload()
 
 

cms/models/pagemodel.py

@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-from logging import Logger
+from logging import getLogger
 from os.path import join
 
 from django.conf import settings
@@ -30,6 +30,8 @@
 from treebeard.mp_tree import MP_Node
 
 
+logger = getLogger(__name__)
+
 @python_2_unicode_compatible
 class Page(six.with_metaclass(PageMetaClass, MP_Node)):
     """
@@ -180,7 +182,7 @@ def move_page(self, target, position='first-child'):
                 if target.get_root().get_next_sibling().pk == public.get_root().pk:
                     target = target.publisher_public
                 else:
-                    Logger.warning('tree may need rebuilding: run manage.py cms fix-tree')
+                    logger.warning('tree may need rebuilding: run manage.py cms fix-tree')
         if position == 'first-child' or position == 'last-child':
             self.parent_id = target.pk
         else:

cms/tests/api.py

@@ -5,16 +5,18 @@
 from django.contrib.auth.models import Permission
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import FieldError
+from django.core.exceptions import PermissionDenied
 from django.template import TemplateDoesNotExist
 from django.test.testcases import TestCase
 from djangocms_text_ckeditor.cms_plugins import TextPlugin
 from djangocms_text_ckeditor.models import Text
 from menus.menu_pool import menu_pool
 
-from cms.api import _generate_valid_slug, create_page, _verify_plugin_type, assign_user_to_page
+from cms.api import _generate_valid_slug, create_page, _verify_plugin_type, assign_user_to_page, publish_page
 from cms.apphook_pool import apphook_pool
 from cms.constants import TEMPLATE_INHERITANCE_MAGIC
 from cms.models.pagemodel import Page
+from cms.models.permissionmodels import GlobalPagePermission
 from cms.plugin_base import CMSPluginBase
 from cms.test_utils.util.menu_extender import TestMenu
 from cms.test_utils.util.mock import AttributeObject
@@ -215,3 +217,27 @@ def test_create_reverse_id_collision(self):
         create_page('home', 'nav_playground.html', 'en', published=True, reverse_id="foo")
         self.assertRaises(FieldError, create_page, 'foo', 'nav_playground.html', 'en', published=True, reverse_id="foo")
         self.assertTrue(Page.objects.count(), 2)
+
+    def test_publish_page(self):
+        page_attrs = self._get_default_create_page_arguments()
+        page_attrs['language'] = 'en'
+        page_attrs['published'] = False
+        page = create_page(**page_attrs)
+        self.assertFalse(page.is_published('en'))
+        self.assertEqual(page.changed_by, 'script')
+        user = get_user_model().objects.create_user(username='user', email='user@django-cms.org',
+                                                    password='user')
+        # Initially no permission
+        self.assertRaises(PermissionDenied, publish_page, page, user, 'en')
+        user.is_staff = True
+        user.save()
+        # Permissions are cached on user instances, so create a new one.
+        user = get_user_model().objects.get(pk=user.pk)
+        _grant_page_permission(user, 'publish')
+        gpp = GlobalPagePermission.objects.create(user=user, can_publish=True)
+        gpp.sites.add(page.site)
+        publish_page(page, user, 'en')
+        # Reload the page to get updates.
+        page = page.reload()
+        self.assertTrue(page.is_published('en'))
+        self.assertEqual(page.changed_by, user.get_username())

cms/utils/permissions.py

@@ -1,4 +1,5 @@
 # -*- coding: utf-8 -*-
+from contextlib import contextmanager
 from threading import local
 
 from django.contrib.auth import get_permission_codename, get_user_model
@@ -30,6 +31,17 @@ def get_current_user():
     return getattr(_thread_locals, 'user', None)
 
 
+@contextmanager
+def current_user(user):
+    """
+    Changes the current user just within a context.
+    """
+    old_user = get_current_user()
+    set_current_user(user)
+    yield
+    set_current_user(old_user)
+
+
 def has_page_add_permission(request):
     """
     Return true if the current user has permission to add a new page. This is