Currencies are to be stored in the database and converted to Ctools exportables. This allows for Views integration and other modules to update currency information.

CommentFileSizeAuthor
#5 currency_1531974_01.patch50.73 KBxano
#1 currency_1531974_00.patch47.34 KBxano

Comments

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Assigned: Unassigned » xano
Status: Active » Needs review
StatusFileSize
new currency_1531974_00.patch47.34 KB
amateescu’s picture

amateescu commented
Status: Needs review » Needs work 
+++ b/currency.api.php
@@ -0,0 +1,36 @@
+    'minorUnit' => 2,

I don't think I like this one.. why not 'minor_unit'?

+++ b/currency.install
@@ -0,0 +1,55 @@
+      'minorUnit' => array(
+        'type' => 'int',
+        'size' => 'tiny',
+      ),

Same here, I'd prefer the schema to not be turned into camelCase.

+++ b/currency.api.php
@@ -0,0 +1,36 @@
\ No newline at end of file

There are a couple of these throughout this patch :)

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

The coding standards specify properties are supposed to be camelcased. Since the standards don't mention column names, IMHO this is the way to comply with them, also because Ctools does not differentiate betwee schema and property names.

amateescu’s picture

amateescu commented

Currently in D8 we do non-camelCase for properties that are also column names, so that's what I'd like to follow.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Status: Needs work » Needs review
StatusFileSize
new currency_1531974_01.patch50.73 KB

Added tests, two missing DB columns, renamed minorUnit to minor_unit, and fixed the "no newline" errors.

Status: Needs review » Needs work

The last submitted patch, currency_1531974_01.patch, failed testing.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Status: Needs work » Needs review

The testbot fails, because it does not checkout Ctools as a dependency. This is a known bug in the testbot. Tests pass locally.

amateescu’s picture

amateescu commented
Status: Needs review » Fixed

Committed to 7.x-2.x. Thanks a lot for picking this up again :)

http://drupalcode.org/project/currency.git/commit/9d2d9e5

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Assigned: xano » Unassigned