Code suggestions

[Honza]

Hi,

I have to hack VM after every update to work properly, so I tried to contribute on the source code. At this page:

http://virtuemart.net/community/support-the-project

is written:

Help speed up the development of the project by taking care of bug reports at the Feature & Bug Tracker.

with link to:

http://dev.virtuemart.net/login?back_url=http%3A%2F%2Fdev.virtuemart.net%2Fprojects%2Fvirtuemart%2Fissues

where is login form but I couldn't find where to register. Anybody know where can I register?

I'll describe my hacks / suggestions here:

1) /administrator/components/com_virtuemart/helpers/config.php function vmInfo

Instead of

Code Select Expand

$app ->enqueueMessage('Info: '.JText::_($publicdescr),'info');

use this line

Code Select Expand

$app ->enqueueMessage(JText::_($publicdescr),'info');

Why is there string 'Info: ' hardcoded? It happens that messages are showing only with this string. It is useless.

2) /components/com_virtuemart/controllers/cart.php function setpayment

2.1) there is $app variable used in this method, but not defined. Place

Code Select Expand

$app = JFactory::getApplication();

at the begining of this method.

2.2) at the end of this method there is redirect like this

Code Select Expand

parent::display();

What it does is that when you change payment method from the cart and save it, it will redirect you to homepage. That means you lost itemId and the whole page can be destroyed. And it does in our case. It is better to use

Code Select Expand

$app->redirect(JRoute::_('index.php?option=com_virtuemart&view=cart'));

3) /components/com_virtuemart/controllers/user.php

There is so many unimportant messages in the user controller. Checkout process is very delicate procedure where we don't want to disturb the customer. Why do we have to tell him things like "User data stored"? What can he possibly take from this message?

3.1) function saveCheckoutUser

instead of

Code Select Expand

$msg = $this->saveData(true);

we are using

Code Select Expand

$msg = $this->saveData(true, true);

What it does is that it will automatically register AND LOG IN the user during checkout process. Another big improvement for seamlessness of the checkout process. Of course there is problem when you want to confirm email address before. But if not, why bother customer with login?

3.2) Redirect without message or only if the message is error.

instead of

Code Select Expand

$this->setRedirect( JRoute::_('index.php?option=com_virtuemart&view=cart&task=checkout',$this->useXHTML,$this->useSSL), $msg );

easy solution is

Code Select Expand

$this->setRedirect( JRoute::_('index.php?option=com_virtuemart&view=cart&task=checkout',$this->useXHTML,$this->useSSL));

3.3) function saveCartUser

This method does the same thing as saveCheckoutUser, but with different redirect. I don't understand the reason for that. I just call the

Code Select Expand

$this->saveCheckoutUser();

method again instead of code inside this method.

3.4) function saveData

There is line

Code Select Expand

$msg = (is_array($ret)) ? $ret['message'] : $ret;

again with useless information for the customer. Should we get rid of it?



Please, let me know if you can use some of these suggestions to the next release and if not, why not.

[Milbo]

where is login form but I couldn't find where to register. Anybody know where can I register?

I'll describe my hacks / suggestions here:

In fact misleading, the link should point to this forum. Yes, write them here.

Why is there string 'Info: ' hardcoded? It happens that messages are showing only with this string. It is useless.

The Info was hardcoded to understand the exactly the problem with the empty messages. It was also there to differ the different types of messages, which all use the "info" message of joomla. Is removed now and if there is an empty message it is directly doing a stackTrace (only with debug activated, just activate it for administrators). Please observe this and help us to find empty messages.
Btw empty messages are not written empty!. They just use a generic error variable which is empty then.

2) /components/com_virtuemart/controllers/cart.php function setpayment

2.1) there is $app variable used in this method, but not defined. Place

Code Select Expand

$app = JFactory::getApplication();

at the begining of this method.

Strange I have it, directly before the redirects

2.2) at the end of this method there is redirect like this

Code Select Expand

parent::display();

What it does is that when you change payment method from the cart and save it, it will redirect you to homepage. That means you lost itemId and the whole page can be destroyed. And it does in our case. It is better to use

Code Select Expand

$app->redirect(JRoute::_('index.php?option=com_virtuemart&view=cart'));

We are already in the controller of the cart. There is no reason to redirect to the same controller. I just investigate why you have this effect. I added an own display function. Checkout the next version, which I release in the forum today.

3) /components/com_virtuemart/controllers/user.php

There is so many unimportant messages in the user controller. Checkout process is very delicate procedure where we don't want to disturb the customer. Why do we have to tell him things like "User data stored"? What can he possibly take from this message?

Next version you can override the langkey with empty string and then there shouldnt be any message, anylonger.

3.1) function saveCheckoutUser

instead of

Code Select Expand

$msg = $this->saveData(true);

we are using

Code Select Expand

$msg = $this->saveData(true, true);

What it does is that it will automatically register AND LOG IN the user during checkout process. Another big improvement for seamlessness of the checkout process. Of course there is problem when you want to confirm email address before. But if not, why bother customer with login?

Yes, I wrote it that way. What you are missing is the silent registration. Philosophic, juristic and ethic considerations see silent registrations as something which can be easily misused. There are enough features for non registered. But I add now a VmConfig::get('reg_silent',0),... but you need to activate it via the config file. Add to your virtuemart.cfg reg_silent = 1 and use in the tools the option "renew config by file". Be aware that your actual config is lost then.

easy solution is

Code Select Expand

$this->setRedirect( JRoute::_('index.php?option=com_virtuemart&view=cart&task=checkout',$this->useXHTML,$this->useSSL));

Yes but nothing for the core. You can do a list of the messages which annoys you. Not just the text, also with the action you do to see them (controller function name). Then lets do a poll for a group of messages which should be configurable or so. So long use the language override which should work with the new core 2.0.21, then.

3.3) function saveCartUser
This method does the same thing as saveCheckoutUser, but with different redirect. I don't understand the reason for that. I just call the

Code Select Expand

$this->saveCheckoutUser();
method again instead of code inside this method.

The difference is the following task. You can change the user address being in the checkout process or just coming from the cart.

Have a good day.

[Honza]

Hi Milbo,

thank you very much for your fast and detailed reply. I will write more suggestions like this one then

ad 2.1) You are right. It is there. I had to overlook it. But it could be written better instead of initializing the JApplication twice inside one method or instead of

Code Select Expand


$app = JFactory::getApplication();
$app->redirect(JRoute::_('index.php?option=com_virtuemart&view=cart&task=editpayment',$this->useXHTML,$this->useSSL), $msg);


use just this

Code Select Expand


JFactory::getApplication()->redirect(JRoute::_('index.php?option=com_virtuemart&view=cart&task=editpayment',$this->useXHTML,$this->useSSL), $msg);


but that's just about programmer's taste.

ad 2.2) We are using JoomSEF. Maybe this causes the bad redirect.

-------------------------------------------

I will test your new release. Hopefully it will work for us better.

Thanks again, take care and have a nice day too.

[Milbo]

We instance it only if it is needed, the other solution may instanciate it, but not needed.

Yepp, then it is joomsef. I would use virtuemart without any extra SEF solution.

[Honza]

I totally agree. But default Joomla / VM SEF URL to product detail looks like this:

/component/virtuemart/[VM_catebory_name]/[VM_product_name]-detail?Itemid=0

With JoomSEF and it's VM SEF plugin, the URL looks like this

/[Joomla_menu_item_name]/[VM_product_name]

Is there way how to have these SEF URLs without any additional extension?

[lindapowers]

Check our website, we use SEF urls only with joomla 2.5 core SEF and VM2.

http://comenaranjas.com/es/tienda/naranjas/naranjas-de-zumo-10-kg-detail.html

or in english so you understand better:

http://comenaranjas.com/en/shop/oranges/juice-oranges-10-kg-detail.html

VM1 was horrible for SEF and we used sh404sef but I'll say VM2 does pretty well for SEF urls so no need for 3rd party component.

You can get rid of the html too, but we like using it always.

[atrus]

It all depends on how deep u want to go with SEF.

VM2 SEF is doing the job ok, can't blame it; it is not fair to compare it with Joomsef (which is what we are using) which is a powerful BEAST in terms of sef optimization.

There are quite a few basic things that VM2 still fails in SEF I'm afraid, i.e. duplicate urls for products that belong in more than 1 categories, etc..

Sent from my BlackBerry 9360 using Tapatalk

[Milbo]

the canonical is fixed in vm2.0.21c

[Honza]

Hello Milbo,

PayPal plugin at our eshops started to refuse payments, so I'm forced to update VirtueMart and it's extensions again. I will continue with our previous communication about hacks I have to make after every VM update.


/components/com_virtuemart/controllers/user.php, method saveCheckoutUser:

There is parameter for silent registration as you promised:

Code Select Expand

$msg = $this->saveData(true,VmConfig::get('reg_silent',0));

But as you said, I would have to reset whole VM config to make it work. That isn't exactly what I want to do. I mean that is quite user unfriendly. Also I don't want register user silently. He filled in register form, so I suppose that he/she knows about registration. All I want to do is to log him/her in at the same time he/she register himself/herself. There is no philosophic, juristic or ethic problem. I just want our customers to save time.

So I still have to hack this line to:

Code Select Expand

$msg = $this->saveData(true, true);


I tested whole checkout process and I didn't find any other problem. Except those annoying info messages and small problem that at our checkout pricelist template there were language constants instead of translated text (I'm not sure whose fault is that ). So thank you. It is big improvement from last time. I hope you will consider feature "Automatic login after registration" for next version.