payment_mean: add test for source reference generation

bt5/erp5_payment_mean/TestTemplateItem/portal_components/test.erp5.testPaymentTransactionGroup.py

+##############################################################################
+#
+# Copyright (c) 2002-2017 Nexedi SA and Contributors. All Rights Reserved.
+#
+# WARNING: This program as such is intended to be used by professional
+# programmers who take the whole responsibility of assessing all potential
+# consequences resulting from its eventual inadequacies and bugs
+# End users who are looking for a ready-to-use solution with commercial
+# guarantees and support are strongly adviced to contract a Free Software
+# Service Company
+#
+# This program is Free Software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+#
+##############################################################################
+
+from Products.ERP5Type.tests.ERP5TypeTestCase import ERP5TypeTestCase
+
+class TestPaymentTransactionGroup(ERP5TypeTestCase):
+  def test_source_reference_generated(self):
+    ptg = self.portal.payment_transaction_group_module.newContent(
+        portal_type='Payment Transaction Group')
+    self.assertTrue(ptg.getSourceReference())
+
+  def test_source_reference_reset_on_clone(self):
+    ptg = self.portal.payment_transaction_group_module.newContent(
+        portal_type='Payment Transaction Group')
+    cloned_ptg = ptg.Base_createCloneDocument(batch_mode=True)
+    # after clone, payment transaction group has a source reference
+    self.assertTrue(cloned_ptg.getSourceReference())
+    # a new source reference
+    self.assertNotEqual(
+        ptg.getSourceReference(), cloned_ptg.getSourceReference())

bt5/erp5_payment_mean/TestTemplateItem/portal_components/test.erp5.testPaymentTransactionGroup.xml

+<?xml version="1.0"?>
+<ZopeData>
+  <record id="1" aka="AAAAAAAAAAE=">
+    <pickle>
+      <global name="Test Component" module="erp5.portal_type"/>
+    </pickle>
+    <pickle>
+      <dictionary>
+        <item>
+            <key> <string>_recorded_property_dict</string> </key>
+            <value>
+              <persistent> <string encoding="base64">AAAAAAAAAAI=</string> </persistent>
+            </value>
+        </item>
+        <item>
+            <key> <string>default_reference</string> </key>
+            <value> <string>testPaymentTransactionGroup</string> </value>
+        </item>
+        <item>
+            <key> <string>description</string> </key>
+            <value>
+              <none/>
+            </value>
+        </item>
+        <item>
+            <key> <string>id</string> </key>
+            <value> <string>test.erp5.testPaymentTransactionGroup</string> </value>
+        </item>
+        <item>
+            <key> <string>portal_type</string> </key>
+            <value> <string>Test Component</string> </value>
+        </item>
+        <item>
+            <key> <string>sid</string> </key>
+            <value>
+              <none/>
+            </value>
+        </item>
+        <item>
+            <key> <string>text_content_error_message</string> </key>
+            <value>
+              <tuple/>
+            </value>
+        </item>
+        <item>
+            <key> <string>text_content_warning_message</string> </key>
+            <value>
+              <tuple/>
+            </value>
+        </item>
+        <item>
+            <key> <string>version</string> </key>
+            <value> <string>erp5</string> </value>
+        </item>
+        <item>
+            <key> <string>workflow_history</string> </key>
+            <value>
+              <persistent> <string encoding="base64">AAAAAAAAAAM=</string> </persistent>
+            </value>
+        </item>
+      </dictionary>
+    </pickle>
+  </record>
+  <record id="2" aka="AAAAAAAAAAI=">
+    <pickle>
+      <global name="PersistentMapping" module="Persistence.mapping"/>
+    </pickle>
+    <pickle>
+      <dictionary>
+        <item>
+            <key> <string>data</string> </key>
+            <value>
+              <dictionary/>
+            </value>
+        </item>
+      </dictionary>
+    </pickle>
+  </record>
+  <record id="3" aka="AAAAAAAAAAM=">
+    <pickle>
+      <global name="PersistentMapping" module="Persistence.mapping"/>
+    </pickle>
+    <pickle>
+      <dictionary>
+        <item>
+            <key> <string>data</string> </key>
+            <value>
+              <dictionary>
+                <item>
+                    <key> <string>component_validation_workflow</string> </key>
+                    <value>
+                      <persistent> <string encoding="base64">AAAAAAAAAAQ=</string> </persistent>
+                    </value>
+                </item>
+              </dictionary>
+            </value>
+        </item>
+      </dictionary>
+    </pickle>
+  </record>
+  <record id="4" aka="AAAAAAAAAAQ=">
+    <pickle>
+      <global name="WorkflowHistoryList" module="Products.ERP5Type.patches.WorkflowTool"/>
+    </pickle>
+    <pickle>
+      <tuple>
+        <none/>
+        <list>
+          <dictionary>
+            <item>
+                <key> <string>action</string> </key>
+                <value> <string>validate</string> </value>
+            </item>
+            <item>
+                <key> <string>validation_state</string> </key>
+                <value> <string>validated</string> </value>
+            </item>
+          </dictionary>
+        </list>
+      </tuple>
+    </pickle>
+  </record>
+</ZopeData>

bt5/erp5_payment_mean/bt/template_test_id_list

+test.erp5.testPaymentTransactionGroup
\ No newline at end of file

bt5/erp5_payment_mean/bt/test_dependency_list

+erp5_full_text_mroonga_catalog
\ No newline at end of file

[Georgios Dagkakis]

cloned_ptg.getSourceReference() is used twice, so better to save it in a new variable not to re-call the getter

[Georgios Dagkakis]

plus a warning for the use of ptg as abbreviation. It is short test code and understandable enough, also cloned_payment_transaction_group is kind of long, so not a strong objection, just saying I would use the full.

[Jérome Perrin]

Is it for readability ? or for the few milliseconds of calling the method again ?

I don't have strong preference, but I thought it was a bit more concise like this, as using a variable would need one more line. I don't mind changing, if others also prefer a variable.

[Georgios Dagkakis]

Is it for readability ? or for the few milliseconds of calling the method again ?

both for me. For the performance gain I have grown a habit of doing it always in such cases, so that it would not be missed in a more important case, e.g. a loop. So now it feels more readable to my eyes (e.g.. I instinctively checked if there is some reason to change and you need to get it again). But it is of course a detail, especially in test code, so not an objection

[Jérome Perrin]

(my previous comment was for the twice usage)

... about ptg you are right, this is not ok with the guideline.

I have been writing this so many times in project code that I decided to allow myself an exception for this one and I know type ptg without even realizing that.

[Jérome Perrin]

I remembered what made me use ptg and not payment_transaction_group.

A simple assignment like this:

payment_transaction_group = self.portal.payment_transaction_group_module.newContent(

is already 84 characters long, which exceeds the limit of 79 coming from pep 8 https://www.python.org/dev/peps/pep-0008/#maximum-line-length

Using

ptg = self.portal.payment_transaction_group_module.newContent(

seems a bit less ugly than:

payment_transaction_group =  \
   self.portal.payment_transaction_group_module.newContent(

but I'm not sure this is a valid exception to the rule.

[Jérome Perrin]

Also, when I started to use ptg, I was using scripts with very long names because of thePaymentTransactionGroup_ prefix. Sometimes just the script name was almost 80 characters long.

With a shorter variable name we can do

ptg.PaymentTransactionGroup_exportToSomeThirdPartyInterface()

and don't have to do

payment_transaction_group\
    .PaymentTransactionGroup_exportToSomeThirdPartyInterface()

That was the reasons that lead me to consider PTG has an "acceptable acronym" when working with Payment Transaction Groups, but unlike other accepted acronyms (HTTP, PDF etc, this is not universally recognized)

[Georgios Dagkakis]

I cannot understand why this MR introduces the need of this dependency, can you please explain?

[Jérome Perrin]

Note that it's a test dependency.

erp5_base depends on the virtual erp5_full_text_catalog which is provided by erp5_full_text_mroonga_catalog or other full text bt.

In a real instance you will have to select a bt providing erp5_full_text_catalog, so this is a hint for the test runner that just knows it has to run erp5_payment_mean:testPaymentTransactionGroup.

[Georgios Dagkakis]

Ok, thanks