Merge 357d69d into 5de86c2

kiwi_lint/__init__.py

@@ -8,6 +8,8 @@
 from .docstring import DocstringChecker
 from .raw_sql import RawSQLChecker
 from .auth_user import AuthUserChecker
+from .bulk_create import BulkCreateChecker
+from .objects_update import ObjectsUpdateChecker
 
 
 def register(linter):
@@ -16,3 +18,5 @@ def register(linter):
     linter.register_checker(DocstringChecker(linter))
     linter.register_checker(RawSQLChecker(linter))
     linter.register_checker(AuthUserChecker(linter))
+    linter.register_checker(BulkCreateChecker(linter))
+    linter.register_checker(ObjectsUpdateChecker(linter))

kiwi_lint/auth_user.py

@@ -12,7 +12,7 @@ class AuthUserChecker(checkers.BaseChecker):
 
     name = 'auth-user-checker'
 
-    msgs = {'R4441': ("Hard-coded 'auth.User'",
+    msgs = {'E4441': ("Hard-coded 'auth.User'",
                       'hard-coded-auth-user',
                       "Don't hard-code the auth.User model. "
                       "Use settings.AUTH_USER_MODEL instead!")}

kiwi_lint/bulk_create.py

@@ -0,0 +1,23 @@
+# Copyright (c) 2018 Alexander Todorov <atodorov@MrSenko.com>
+
+# Licensed under the GPL 2.0: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+
+from pylint import interfaces
+from pylint import checkers
+from pylint.checkers import utils
+
+
+class BulkCreateChecker(checkers.BaseChecker):
+    __implements__ = (interfaces.IAstroidChecker,)
+
+    name = 'bulk-create-checker'
+
+    msgs = {'E4451': ("Use bulk_create_with_history() instead of bulk_create()",
+                      'bulk-create-used',
+                      "bulk_create() will not save model history. "
+                      "Use bulk_create_with_history() instead!")}
+
+    @utils.check_messages('bulk-create-used')
+    def visit_attribute(self, node):
+        if node.attrname == 'bulk_create':
+            self.add_message('bulk-create-used', node=node)

kiwi_lint/objects_update.py

@@ -0,0 +1,36 @@
+# Copyright (c) 2018 Alexander Todorov <atodorov@MrSenko.com>
+
+# Licensed under the GPL 2.0: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+
+from pylint import interfaces
+from pylint import checkers
+from pylint.checkers import utils
+
+
+class ObjectsUpdateChecker(checkers.BaseChecker):
+    __implements__ = (interfaces.IAstroidChecker,)
+
+    name = 'objects-update-checker'
+
+    msgs = {'E4461': ("Model.objects.update() doesn't update history! Use .save() instead.",
+                      'objects-update-used',
+                      "")}
+
+    @utils.check_messages('objects-update-used')
+    def visit_attribute(self, node):
+        """
+            Note: this checker will produce false-positives on
+            dict.update() or anything else that is named .update().
+
+            These should be white-listed on a line by line basis
+            b/c there can be many situations where .update() is used
+            after filtering or directly on top of another variable which
+            itself is a query set.
+        """
+        if node.attrname == 'update':
+            # white-list
+            if node.as_string() == 'context.update':
+                return
+            if node.as_string() == 'data.update':
+                return
+            self.add_message('objects-update-used', node=node)

kiwi_lint/raw_sql.py

@@ -14,7 +14,7 @@ class RawSQLChecker(checkers.BaseChecker):
 
     name = 'raw-sql-checker'
 
-    msgs = {'R4431': ('Avoid using raw SQL',
+    msgs = {'E4431': ('Avoid using raw SQL',
                       'avoid-raw-sql',
                       'Avoid raw SQL, use Django ORM queries instead')}
 

requirements/base.txt

@@ -2,6 +2,7 @@ Django==2.0.7
 django-attachments>=1.3
 django-glrm
 django-grappelli
+django-simple-history
 django-vinaigrette
 django-uuslug
 odfpy

tcms/core/admin.py

@@ -12,7 +12,6 @@
 from django.contrib.auth.forms import UserChangeForm
 from django.utils.translation import ugettext_lazy as _
 
-
 from django_comments.models import Comment
 
 
@@ -54,6 +53,15 @@ class KiwiUserAdmin(UserAdmin):
     # even when adding users via admin panel
     form = MyUserChangeForm
 
+    def change_view(self, request, object_id, extra_context=None):
+        if request.user.is_superuser:
+            return super().change_view(request, object_id, extra_context)
+
+        # object history view link to admin_user_change so we redirect
+        # to the user profile instead
+        user = User.objects.get(pk=object_id)
+        return HttpResponseRedirect(reverse('tcms-profile', args=[user.username]))
+
     def _modifying_myself(self, request, object_id):
         return request.user.pk == int(object_id)
 

tcms/core/history.py

@@ -0,0 +1,64 @@
+from django.db.models import signals
+from simple_history.models import HistoricalRecords
+from simple_history.admin import SimpleHistoryAdmin
+
+
+class KiwiHistoricalRecords(HistoricalRecords):
+    """
+        This class will keep track of what fields were changed
+        inside of the ``history_change_reason`` field. This gives us
+        a crude changelog until upstream introduces their new interface.
+    """
+
+    def pre_save(self, instance, **kwargs):
+        """
+            Signal handlers don't have access to the previous version of
+            an object so we have to load it from the database!
+        """
+        if instance.pk:
+            try:
+                instance.previous = instance.__class__.objects.get(pk=instance.pk)
+            except:  # noqa: bare-except
+                # raises for tcms.settings.test tcms.xmlrpc.tests.test_category.TestCategory
+                # b/c there are no statuses in the DB for some reason.
+                # todo: not sure if this can raise in production
+                pass
+
+    def post_save(self, instance, created, **kwargs):
+        """
+            Calculate the changelog and call the inherited method to
+            write the data into the database.
+        """
+        change_reason = []
+
+        if hasattr(instance, 'previous'):
+            for field in self.fields_included(instance):
+                old_value = getattr(instance.previous, field.attname)
+                new_value = getattr(instance, field.attname)
+                if old_value != new_value:
+                    change_reason.append("%s: %s -> %s" % (field.attname, old_value, new_value))
+        instance.changeReason = "; ".join(change_reason)
+        super().post_save(instance, created, **kwargs)
+
+    def finalize(self, sender, **kwargs):
+        """
+            Connect the pre_save signal handler after calling the inherited method.
+        """
+        super().finalize(sender, **kwargs)
+        signals.pre_save.connect(self.pre_save, sender=sender, weak=False)
+
+
+class ReadOnlyHistoryAdmin(SimpleHistoryAdmin):
+    """
+        Custom history admin which shows all fields
+        as read-only.
+    """
+    history_list_display = ['history_change_reason']
+
+    def get_readonly_fields(self, request, obj=None):
+        # make all fields readonly
+        readonly_fields = list(set(
+            [field.name for field in self.opts.local_fields] +
+            [field.name for field in self.opts.local_many_to_many]
+        ))
+        return readonly_fields

tcms/core/models/fields.py

@@ -3,6 +3,6 @@
     # may not be running under MySQL
     from MySQLdb.constants import FIELD_TYPE
     from django.db.backends.mysql.base import django_conversions
-    django_conversions.update({FIELD_TYPE.TIME: None})
+    django_conversions.update({FIELD_TYPE.TIME: None})  # pylint: disable=objects-update-used
 except ImportError:
     pass

tcms/core/utils/checksum.py

@@ -6,5 +6,5 @@ def checksum(value):
     if value is None:
         return ''
     _checksum = hashlib.sha256()
-    _checksum.update(value.encode("UTF-8"))
+    _checksum.update(value.encode("UTF-8"))  # pylint: disable=objects-update-used
     return _checksum.hexdigest()

tcms/settings/common.py

@@ -113,6 +113,7 @@
     'django.contrib.messages.middleware.MessageMiddleware',
     'global_login_required.GlobalLoginRequiredMiddleware',
     'dj_pagination.middleware.PaginationMiddleware',
+    'simple_history.middleware.HistoryRequestMiddleware',
 ]
 
 
@@ -241,6 +242,7 @@
     'django_comments',
     'dj_pagination',
     'modernrpc',
+    'simple_history',
     'tinymce',
 
     'tcms.core',

tcms/settings/test/mysql.py

@@ -1,4 +1,4 @@
-# pylint: disable=wildcard-import, unused-wildcard-import
+# pylint: disable=wildcard-import, unused-wildcard-import, objects-update-used
 
 from tcms.settings.test import *  # noqa: F403
 

tcms/settings/test/postgresql.py

@@ -1,4 +1,4 @@
-# pylint: disable=wildcard-import, unused-wildcard-import
+# pylint: disable=wildcard-import, unused-wildcard-import, objects-update-used
 
 from tcms.settings.test import *  # noqa: F403
 

tcms/static/js/testplan_actions.js

@@ -870,19 +870,6 @@ Nitrate.TestPlans.Details = {
         Nitrate.TestPlans.Details.reviewingCasesTabOpened = true;
       }
     });
-
-    // Initial the enable/disble btns
-    if (jQ('#btn_disable').length) {
-      jQ('#btn_disable').bind('click', function(e){
-        updateObject('testplans.testplan', plan_id, 'is_active', 'False', 'bool', reloadWindow);
-      });
-    }
-
-    if (jQ('#btn_enable').length) {
-      jQ('#btn_enable').bind('click', function(e) {
-        updateObject('testplans.testplan', plan_id, 'is_active', 'True', 'bool', reloadWindow);
-      });
-    }
   },
   'reopenCasesTabThen': function() {
     Nitrate.TestPlans.Details.testcasesTabOpened = false;
@@ -1070,26 +1057,6 @@ Nitrate.TestPlans.Clone.on_load = function() {
   });
 };
 
-function showMoreSummary() {
-  jQ('#display_summary').show();
-  if (jQ('#display_summary_short').length) {
-    jQ('#id_link_show_more').hide();
-    jQ('#id_link_show_short').show();
-    jQ('#display_summary_short').hide();
-  }
-}
-
-function showShortSummary() {
-  jQ('#id_link_show_more').show();
-  jQ('#display_summary').hide();
-  if (jQ('#display_summary_short').length) {
-    jQ('#id_link_show_short').hide();
-    jQ('#display_summary_short').show();
-  }
-
-  window.scrollTo(0, 0);
-}
-
 /*
  * Unlink selected cases from current TestPlan.
  *

tcms/templates/case/printable.html

@@ -38,7 +38,7 @@ <h3>Contents</h3>
         <div>
             <h2 id="plan_document">Test Plan Document</h2>
             <div class="thick-line"></div>
-            {{ test_plan.latest_text.plan_text|safe }}
+            {{ test_plan.text|safe }}
         </div>
     </div>
     {% endif %}

tcms/templates/plan/clone.html

@@ -79,7 +79,6 @@ <h2>Clone Test Plan</h2>
 						<ul class="ul-no-format">
 							<li>{{ clone_form.set_parent }}{{ clone_form.set_parent.label }} ({{ clone_form.set_parent.help_text }})</li>
 							<li>{{ clone_form.keep_orignal_author }}{{ clone_form.keep_orignal_author.label }} ({{ clone_form.keep_orignal_author.help_text }})</li>
-							<li>{{ clone_form.copy_texts }}{{ clone_form.copy_texts.label }} ({{ clone_form.copy_texts.help_text }})</li>
 							<li>{{ clone_form.copy_environment_group }}{{ clone_form.copy_environment_group.label }} ({{ clone_form.copy_environment_group.help_text }})</li>
 							<li>
 								<fieldset class="choose">

tcms/templates/plan/edit.html

@@ -11,9 +11,6 @@
 <style>
 #tab_testcases{ padding-left:5px;}
 #tab_testruns {padding-left:5px;}
-.display_summary textarea{
-	background:red;
-}
 </style>
 {% endblock %}
 
@@ -42,13 +39,6 @@
 		>> Edit
 	</div>
 	<input id="id_plan_id" type="hidden" name="plan_id" value="{{ test_plan.plan_id }}">
-	<div class="control">
-		<span class="right-bar">
-			<a href="{% url "plan-text_history" test_plan.plan_id %}" class="historylink">
-				Edit History
-			</a>
-		</span>
-	</div>
 	<form action="{% url "plan-edit" test_plan.plan_id %}" method="post" enctype="multipart/form-data">
 		<div id="" class="Detailform border-1">
 			<table class="editor" cellspacing="0">
@@ -119,7 +109,7 @@
 				<tr>
 					<td><label class="lab strong">Plan Document</label></td>
 					<td>
-						<div id="display_summary"  class="mec">
+						<div>
 							{{ form.text }}
 						</div>
                         <div >{{ form.text.errors }}</div>

tcms/templates/plan/get.html

@@ -39,7 +39,6 @@
 	<input type="hidden" name="content_type" value="testplans.testplan" />
 	<input type="hidden" name="object_pk" value="{{ test_plan.pk }}" />
 	<input type="hidden" name="name" value="{{ test_plan.name }}" />
-	{# <input type="hidden" name="description" value="{{ test_plan.latest_text.plan_text }}" /> #}
 {% endblock %}
 
 {% block contents %}
@@ -54,11 +53,6 @@
 		<span id="id_buttons" class="button">
 			<input id="btn_edit" type="button" value="Edit Plan " title="Edit test plan" data-param="{% url 'plan-edit' test_plan.plan_id %}" {% if perms.testplans.change_testplan %}{% else%}disabled{% endif %}>
 			<input id="btn_clone" type="button" value="Clone Plan" title="Clone this plan to other product" data-params='["{% url "plans-clone" %}", {{ test_plan.plan_id }}]' {% if perms.testplans.add_testplan %}{% else %}disabled{% endif %}>
-			{% if test_plan.is_active %}
-			<input id="btn_disable" type="button" value="Disable Plan " title="Disabled this plan" {% if not perms.testplans.change_testplan %}disabled="true"{% endif %}>
-			{% else %}
-			<input id="btn_enable" type="button" value="Enable Plan " title="Enabled this plan" {% if not perms.testplans.change_testplan %}disabled="true"{% endif %}>
-			{% endif %}
 			<input id="btn_print" type="button" value="Print Plan "
                    title="Print Plan"
                    data-params='["{% url "plans-printable" %}", {{ test_plan.pk }}]'>
@@ -107,7 +101,7 @@ <h2 id="display_title" class="{% ifequal test_plan.is_active 0 %}line-through{%
 					<span class="blue strong"><a href="{% url "plans-all" %}?action=search&name__icontains=&author__email__startswith=&owner__username__startswith=&type=&tag__name__in=&case__default_tester__username__startswith=&product=&product_version=&env_group={{ env_group.id }}&create_date__gte=&create_date__lte=" title="Search plans of use {{ env_group.name }} ">{{ env_group.name }}</a></span>
 					{% endfor %}
 				</div>
-				<div id="display_summary" >
+				<div>
 					{% for env_property in env_properties %}
 					{{ env_property.property.name }},
 					{% endfor %}
@@ -138,6 +132,9 @@ <h2 id="display_title" class="{% ifequal test_plan.is_active 0 %}line-through{%
 				{% endif %}
 				</div>
 			</div>
+			<div class="listinfo">
+				<div class="title"><a href="/admin/testplans/testplan/{{ test_plan.pk }}/history/">View Edit History</a></div>
+			</div>
 		</div>
 		<div class="clear"></div>
 	</div>
@@ -170,7 +167,15 @@ <h2 id="display_title" class="{% ifequal test_plan.is_active 0 %}line-through{%
 				</li>
 			</ul>
 			<div id="document" class="tab_list" style="display:none">
-				{% include "plan/get_docs.html" %}
+                            <div class="listinfo_doc_content">
+                                <div id="display_summary" class="listinfo_doc">
+                                {% if test_plan.text %}
+                                    {{ test_plan.text|safe }}
+                                {% else %}
+                                    <span style="color: #666">The document for this plan hasn't been written yet.</span>
+                                {% endif %}
+                                </div>
+                            </div>
 			</div>
 			<div id="attachment" class="tab_list" style="display:none">
 				{% include "plan/get_attachments.html" %}