Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

My account features #4308

Conversation

@maximebiloe
Copy link
Contributor

commented Oct 19, 2015

Add some styles and the following features in my-account :

  • Order history
  • Order details
  • Order return
  • Credit slips
  • Guest tracking
@djfm

View changes

classes/pdf/PDF.php Outdated
@@ -51,13 +51,14 @@ public function __construct($objects, $template, $smarty, $orientation = 'P')
{
$this->pdf_renderer = new PDFGenerator((bool)Configuration::get('PS_PDF_USE_CACHE'), $orientation);
$this->template = $template;
$smarty->escape_html = false;

This comment has been minimized.

Copy link
@djfm

djfm Oct 19, 2015

Contributor

Is $smarty the global smarty object here? If so, I'm afraid you're actually disabling the autoescape for everything and not just the PDF!

@maximebiloe maximebiloe force-pushed the maximebiloe:starter-theme/feat/my-account branch 2 times, most recently Oct 19, 2015

@djfm

View changes

controllers/front/GuestTrackingController.php Outdated
'errors' => $this->errors,
));
$this->setTemplate(_PS_THEME_DIR_.'guest-tracking.tpl');
// $this->context->smarty->assign(array(

This comment has been minimized.

Copy link
@djfm

djfm Oct 19, 2015

Contributor

Leaving a few traps for future developers? :) If commented code is useless go ahead and remove it!

@djfm

View changes

controllers/front/OrderDetailController.php Outdated

$this->context->smarty->assign('message_confirmation', true);
// if (Tools::getValue('ajax') != 'true') {

This comment has been minimized.

Copy link
@djfm

djfm Oct 19, 2015

Contributor

Is this commented code necessary?

@djfm

This comment has been minimized.

Copy link
Contributor

commented Oct 19, 2015

I think there is something weird going on with price conversions: I have 2 currencies installed and when I change the currency then in my order history only the currency unit changes - not the amounts.

In EUR:

image

In USD:

image

I've set up my shop to have 1 EUR = 2 USD so there should be a difference if the amount were converted.

Current behaviour is to display everything in the currency the order was placed with, and I think it makes sense. If I had to guess I'd say the issue is related to the use of Adapter_PricePresenter, which takes the currency from the global context. I think you should use the order's currency everywhere.

@julienbourdeau

This comment has been minimized.

Copy link
Contributor

commented Oct 19, 2015

Here are my thoughts:

  • Nest all order data inside the $order array. Like in the template instead of using $products you'll be using $order.products. For customer for example, you'll avoid conflicts with the customer defined by FrontController::getTemplateVarCustomer()
  • Add parameter to FrontController::getTemplateVarCustomer() so you can use this method here (all customer passed to the template will be consistent).
  • I think some errors should be redirects or 404, like this error or this error.
  • I would remove most title="{l s='xxx'}", it will make theme translation longer and harder and it doesn't help much (example).
  • I just realized we should use a partial for these buttons across the customer area

And agree we should only use the currency of the order, not the one from the context.

@maximebiloe maximebiloe force-pushed the maximebiloe:starter-theme/feat/my-account branch Oct 20, 2015

@maximebiloe maximebiloe force-pushed the maximebiloe:starter-theme/feat/my-account branch to cdc699b Oct 20, 2015

julienbourdeau added a commit that referenced this pull request Oct 20, 2015

@julienbourdeau julienbourdeau merged commit eeaa2e1 into PrestaShop:feat/starter-theme Oct 20, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Maciej-Kaminski

This comment has been minimized.

Copy link

commented Oct 20, 2015

Dear Julien,
From the customer point of view, order history is useless, because people need to open every order too search products. This is very frustrating with virtual ones.
Maybe you could think to add filter, using product ID as key, not order name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.