Jump to content

Set currency and markup on a per product basis


Recommended Posts

  • 4 weeks later...
  • 2 weeks later...

Have you ever tested the addon if it works before releasing it? I had to edit the code all over, fixing bugs, just to make it show properly. Really, testing before releasing would be a good idea, try it.

 

List of bugs that I found so far (version 1.2):

1. Missing ' in the SQL query on line 84 in cron_Products.php

2. CurrencyPriceUpdater.php: a lot of SQL queries refer to column mod_id instead of id in the mod_products_currency table

3. The Deactivate Module function will not work properly becaus the SQL query contains wrong table name - mod_CurrencyPriceUpdater instead of mod_products_currency

 

I really appreciate the free addon (thank you for your efforts) but it should at least work properly, shouldn't it?

 

Best regards

Link to comment
Share on other sites

1. Missing ' in the SQL query on line 84 in cron_Products.php

 

Works without the ' as well. Not sure I would call it a bug. Feel free to put them in if you wish. The same line is elsewhere in the code as well.

 

2. CurrencyPriceUpdater.php: a lot of SQL queries refer to column mod_id instead of id in the mod_products_currency table

 

Both `id` and `mod_id` are valid columns. I am not sure where you think I should be referring to `id` that I am referring to `mod_id`.

 

3. The Deactivate Module function will not work properly becaus the SQL query contains wrong table name - mod_CurrencyPriceUpdater instead of mod_products_currency

 

That is definitely a bug. Thank you for letting me know. It has been fixed in Ver 1.3

Link to comment
Share on other sites

Works without the ' as well. Not sure I would call it a bug. Feel free to put them in if you wish. The same line is elsewhere in the code as well.

cron_Products.php (v1.3): On line 83:

$sql3 = "INSERT INTO `mod_products_currency` (`id`,`gid`,`name`,`base_currency`,`cost`,`hidden`,`order`) VALUES ('$id','$gid','$name','$default_currency','0.00','$hidden',$order');";

Look at the missing single-quote at the end, before the $order variable. This definitely produces an invalid SQL statement. I don't know how you manage to run an invalid SQL at your end but it doesn't work with me.

 

Both `id` and `mod_id` are valid columns. I am not sure where you think I should be referring to `id` that I am referring to `mod_id`.

Your CREATE TABLE statement is:

CREATE TABLE IF NOT EXISTS `mod_products_currency` (
         `id` int(10) NOT NULL,
         `gid` int(10) NOT NULL,
         `name` text NOT NULL,
         `hidden` text NOT NULL,
         `order` int(1) NOT NULL DEFAULT '0',
         `base_currency` text NOT NULL,
         `cost` decimal(10,2) NOT NULL,
         `markup_type` set('f','p') NOT NULL DEFAULT 'f',
         `markup` decimal(10,2) NOT NULL DEFAULT '0',
         `discount_type` set('f','p') NOT NULL DEFAULT 'f',
         `mdiscount` decimal(10,2) NOT NULL,
         `qdiscount` decimal(10,2) NOT NULL,
         `sdiscount` decimal(10,2) NOT NULL,
         `adiscount` decimal(10,2) NOT NULL,
         `bdiscount` decimal(10,2) NOT NULL,
         `tdiscount` decimal(10,2) NOT NULL,
         `msetupfee` decimal(10,2) NOT NULL,
         `qsetupfee` decimal(10,2) NOT NULL,
         `ssetupfee` decimal(10,2) NOT NULL,
         `asetupfee` decimal(10,2) NOT NULL,
         `bsetupfee` decimal(10,2) NOT NULL,
         `tsetupfee` decimal(10,2) NOT NULL,
         `monthly` decimal(10,2) NOT NULL,
         `quarterly` decimal(10,2) NOT NULL,
         `semiannually` decimal(10,2) NOT NULL,
         `annually` decimal(10,2) NOT NULL,
         `biennially` decimal(10,2) NOT NULL,
         `triennially` decimal(10,2) NOT NULL
   );

I don't see anywhere a mod_id column, so your statement is completely false. And just search for "mod_id" in CurrencyPriceUpdater.php and you will see. All occurrences of "mod_id" should be replaced with "id" because there is no column named "mod_id" in mod_products_currency table.

 

Something else - you stated in the forum that the addon will ignore products that have values "-1" for their prices in the default currency and will not update their prices. I have a product with "-1" for all the billing periods with the default currency and when I set the cost for that product in the addon it updated the prices in WHMCS even though it shouldn't, according to your statement. Is this supposed to be like that or am I missing something? Thank you.

 

Best regards

Link to comment
Share on other sites

Something else. Currently the addon has a problem working with UTF-8 databases - it outputs product name as question marks (??????). This is easily solvable by adding this statement immediately after the mysql_connect() call:

mysql_query("SET NAMES 'utf8'", $conn);

 

Best regards

Link to comment
Share on other sites

Yet another bug.

 

In cron_Products.php, line 174:

$sql3 = "SELECT * FROM `tblpricing` WHERE `relid`='$id' and `currency`='$default_currency_id';";

This will get all the prices, INCLUDING the prices for configurable options, domains, etc., because you did not restrict the query by type. And the problem is that after that you go and update the prices in tblpricing without restricting by 'type' column, thus updating the prices of config options, domains, etc. This leads to a complete mess in prices. Check the tblpricing table, there is a column named 'type'. You need to update only the rows that have the type set to 'product', not all. But since you are updating only by relid, this leads to a mess in pricing.

 

And since the above select gets multiple rows, the mysql_fetch_array() will get only the first row and all the manipulations that you do after that will be applied to the wrong set of data. And that's why even if I set my prices to '-1.00', the addon does not respect that and overwrites these values (reported in a previous post by me).

 

Also this addon does not take care of the prices set for configurable options and product addons (will not take care after the bugs above are fixed :-) ). What if I have a product with configurable options which have prices set? It will update the prices of the product but will leave the prices of the config options unattended. The same goes for product addons.

 

For me this addon doesn't work as advertised in it's current state. Even worse it can turn into a mess the pricing for a production site. It is good that I put it on a test environment and I have to fix the prices only for several products, config options and domains. But for a production installation where people could have hundreds of prices it could lead to serious problems.

 

That's why I would advise anyone with a production site to delay the installation of the addon until all bugs are fixed.

 

Best regards

Edited by ddenev
Link to comment
Share on other sites

cron_Products.php (v1.3): On line 83:

$sql3 = "INSERT INTO `mod_products_currency` (`id`,`gid`,`name`,`base_currency`,`cost`,`hidden`,`order`) VALUES ('$id','$gid','$name','$default_currency','0.00','$hidden',$order');";

Look at the missing single-quote at the end, before the $order variable. This definitely produces an invalid SQL statement. I don't know how you manage to run an invalid SQL at your end but it doesn't work with me.

 

Not sure why I don't get the error but I just fixed this in V1.4

 

 

cron_Products.php (v1.3): On line 83:

Your CREATE TABLE statement is:

CREATE TABLE IF NOT EXISTS `mod_products_currency` (
         `id` int(10) NOT NULL,
         `gid` int(10) NOT NULL,
         `name` text NOT NULL,
         `hidden` text NOT NULL,
         `order` int(1) NOT NULL DEFAULT '0',
         `base_currency` text NOT NULL,
         `cost` decimal(10,2) NOT NULL,
         `markup_type` set('f','p') NOT NULL DEFAULT 'f',
         `markup` decimal(10,2) NOT NULL DEFAULT '0',
         `discount_type` set('f','p') NOT NULL DEFAULT 'f',
         `mdiscount` decimal(10,2) NOT NULL,
         `qdiscount` decimal(10,2) NOT NULL,
         `sdiscount` decimal(10,2) NOT NULL,
         `adiscount` decimal(10,2) NOT NULL,
         `bdiscount` decimal(10,2) NOT NULL,
         `tdiscount` decimal(10,2) NOT NULL,
         `msetupfee` decimal(10,2) NOT NULL,
         `qsetupfee` decimal(10,2) NOT NULL,
         `ssetupfee` decimal(10,2) NOT NULL,
         `asetupfee` decimal(10,2) NOT NULL,
         `bsetupfee` decimal(10,2) NOT NULL,
         `tsetupfee` decimal(10,2) NOT NULL,
         `monthly` decimal(10,2) NOT NULL,
         `quarterly` decimal(10,2) NOT NULL,
         `semiannually` decimal(10,2) NOT NULL,
         `annually` decimal(10,2) NOT NULL,
         `biennially` decimal(10,2) NOT NULL,
         `triennially` decimal(10,2) NOT NULL
   );

I don't see anywhere a mod_id column, so your statement is completely false. And just search for "mod_id" in CurrencyPriceUpdater.php and you will see. All occurrences of "mod_id" should be replaced with "id" because there is no column named "mod_id" in mod_products_currency table.

 

Fixed in V1.4

 

 

Sorry for the issues - Guess it should have been listed as BETA

Link to comment
Share on other sites

I checked the last version 1.5 but the bug that I reported in the previous post (#8) hasn't been fixed.

 

Best regards

 

Checking the "type" will not work because the type listed in the products table does not match the type listed in the price table. However, since the ID number of the product is unique, you should only get one row returned.

 

I was unable to figure out why there was an issue.... It has worked as expected in my test environment. I can take another look at the logic in the code to see if I can see something. Do you have the modified code that you used to make it work for you?

 

Also, I do not think it will take much to add the additional functionality of addons etc. but I won't go there until I know you are satisfied that the current code is solid.

Link to comment
Share on other sites

Checking the "type" will not work because the type listed in the products table does not match the type listed in the price table.

I am not talking about TYPE in the tblproducts table but in the tblpricing table. They have nothing in common. You must use the "type" column in your WHERE clause in order to restrict the resultset to one row.

 

However, since the ID number of the product is unique, you should only get one row returned.

The relation between tblproducts and tblpricing is one-to -many with the relid field being the foreign key in the tblpricing table which points to the id field in the tblproducts table. That's why the query:

SELECT * FROM `tblpricing` WHERE `relid`='$id' and `currency`='$default_currency_id';

will always return more than one row because you may have many rows in tblpricing which have the same relid value. This is simple SQL common knowledge.

 

This is only the part of the problem. The second part is with the later UPDATE statements (one for each billing period):

UPDATE `tblpricing` SET `monthly` = '$monthly' WHERE `relid`='$id' and `currency`='$default_currency_id';

The above will update several rows for the same reason as in the SELECT query - you do not restrict the UPDATE by the "type" column of the tblpricing table. This leads to updating all rows that have for example ID=1. And there are many with ID=1 but they all have different TYPE.

 

Please study the WHMCS table structure in more depth and you will get what I'm talking about.

 

Best regards

Link to comment
Share on other sites

Ok - I fully understand what you are getting at...

 

My test environment doesn't have any addons etc. and I knew the mod didn't yet support them so I did not test to see how they were affected. When I looked at the relid for my test environment, the values were all unique. I now see that in the live environment that the relid is not unique. I will modify the code to take the "type" into consideration as you have suggested.

Link to comment
Share on other sites

v1.6 has the bugs you referred to worked out. As always the addon is available at... http://www.whmcs.com/members/communityaddons.php?action=viewmod&id=255

 

If the fine folks at WHMCS have been working too hard and haven't gotten the time yet to put the latest version on that page, simply download the latest version listed. Inside the module you will be given the chance to upgrade to the latest version.

 

PLEASE DO NOT USE ANY VERSION BELOW v1.6 - UPGRADE PREVIOUS VERSIONS TO v1.6

 

There are still some new features to be added but what is already provided, should be stable.

Link to comment
Share on other sites

  • 4 weeks later...

Hello,

I have installed v1.6, but if i try to access the main page of CurrencyPriceUpdater

 

DOMAINNAME/addonmodules.php?module=CurrencyPriceUpdater

 

Everytime I'm getting Connection Time Out Error, Can you please guide inorder for me to resolve this error.

 

Regards,

Ritesh Jain

Link to comment
Share on other sites

  • 2 months later...

Dose this module updates the Recurring Amount for monthly payments and so on... ?

 

I installed v1.6 but after invoice generated it still has original price that my client paid at the time he signed up. There are no changes for the prices to the current customers.

 

Did I miss something during the module set up?

 

Thanks,

Sergey

Link to comment
Share on other sites

  • 2 months later...
  • 4 weeks later...

It does work but I will warn you that somehow after I installed it all my prices were set to $0 for my products.

Don't actually know what happened but it made me panic.

 

I also find it a little confusing as there are no documents for it, and the tables that it uses are a little confusing since there is no documentation.

 

It is really great though as you can use it to set all the prices and terms all at one time without switching back and forth between each product.

 

I am using it on WHMCS 5.0.3 without any problem.

Only thing I have noticed is that it takes a while for the module to respond. I haven't actually counted the seconds but it feels like at least 30 seconds.

 

Hope this helps.

Link to comment
Share on other sites

  • 3 months later...

supernix... You are correct... Documentation is lacking. Your prices likely went to $0 because the prices were either not sent "inside the add-on" or because not all the cron jobs were run. In any event, I am glad you got it working am glad you enjoy it.

 

What I would caution others on is that a domain pricing script has been supplied along with the add-on even though it is not part of the add-on. That script my be buggy but is being redeveloped so it might be best to wait for the new version of it. This add-on is quite stable for managing products/services though.

Link to comment
Share on other sites

It should not ever wipe out existing prices though. No script should alter other data than what is being worked on.

 

Worded like that it sounds bad; however, the premise of this script is for it to be where you set your pricing. It is assumed that if you are installing the add-on that you will use it to set your pricing. Out of the box, all prices are $0 until they are set. Maybe some extra code should be added to copy current pricing to the add-on at the point of install to avoid the problem. In the meantime, if you fully configure the add-on immediately after install (before the cron job runs), you'll have no problem. I guess the simple thing would be to not enable the cron until it is fully configured.

 

In any event, I agree that the current pricing should be copied over.

Link to comment
Share on other sites

  • 3 weeks later...

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use & Guidelines and understand your posts will initially be pre-moderated