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

Migrate customer view actions #11527

Merged
merged 7 commits into from Dec 11, 2018

Conversation

@sarjon
Copy link
Member

sarjon commented Nov 27, 2018

Questions Answers
Branch? develop
Description? This PR migrates "Save private note" & "Transform guest to customer" actions.
Type? refacto
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? #11010
How to test? Access admin-dev/index.php/sell/customers/1/view and check migrated actions that should work the same as in legacy.

This change is Reviewable

@@ -169,7 +169,7 @@
{% if customerInformation.generalInformation.customerBySameEmailExists %}
<p>{{ 'A registered customer account using the defined email address already exists. '|trans({}, 'Admin.Orderscustomers.Notification') }}</p>

This comment has been minimized.

@LouiseBonnard

LouiseBonnard Nov 27, 2018

Contributor

Watch the space in the end. ;-)

@mickaelandrieu
Copy link
Contributor

mickaelandrieu left a comment

minor changes requested => good job!

@@ -66,6 +66,6 @@ private function setCustomerId($customerId)
));
}
$this->customerId = $customerId;
$this->customerId = (int) $customerId;

This comment has been minimized.

@mickaelandrieu

mickaelandrieu Nov 27, 2018

Contributor

you shouldn't have to do that, improve the conditions of input validity instead.

*/
public function savePrivateNoteAction($customerId, Request $request)
{
$privateNoteForm = $this->getPrivateNoteForm();

This comment has been minimized.

@mickaelandrieu

mickaelandrieu Nov 27, 2018

Contributor

useless function create your form directly.

@matks

This comment has been minimized.

Copy link
Contributor

matks commented Nov 27, 2018

After mickaelandrieu I have nothing else to add 😄

@matks matks added the migration label Nov 27, 2018

@sarjon

This comment has been minimized.

Copy link
Member

sarjon commented Nov 30, 2018

@matks all done. 👍

Changes applied

@matks

This comment has been minimized.

Copy link
Contributor

matks commented Nov 30, 2018

QA it is ! 👏

@marionf

This comment has been minimized.

Copy link
Contributor

marionf commented Nov 30, 2018

@TristanLDD is it ok for you ?

capture d ecran_691

@TristanLDD

This comment has been minimized.

Copy link

TristanLDD commented Dec 10, 2018

Hello ! There is a little modification to do on the text above the blue button: text size should be reduced to 13px

capture d ecran 2018-12-10 a 10 39 41

Otherwise, everything is ok for me !

@TristanLDD TristanLDD added UX ✔️ and removed waiting for UX labels Dec 10, 2018

@sarjon

This comment has been minimized.

Copy link
Member

sarjon commented Dec 10, 2018

@TristanLDD is this size text part of UI Kit? i can find small-text which is 12px or xsmall-text which is 10px. Is there a specific name for this size of text? or should i create new text style (CSS) specific to this page only?

@TristanLDD

This comment has been minimized.

Copy link

TristanLDD commented Dec 10, 2018

@sarjon My bad, you can use small-text ;)

@TristanLDD TristanLDD removed the UX ✔️ label Dec 10, 2018

@sarjon

This comment has been minimized.

Copy link
Member

sarjon commented Dec 10, 2018

and its done. :) you can take a look. https://prnt.sc/lt559u

@TristanLDD TristanLDD added UX ✔️ and removed waiting for UX labels Dec 10, 2018

@TristanLDD

This comment has been minimized.

Copy link

TristanLDD commented Dec 10, 2018

@sarjon Thank you !

@matks

matks approved these changes Dec 10, 2018

@PierreRambaud PierreRambaud merged commit a6c8cc2 into PrestaShop:develop Dec 11, 2018

1 of 2 checks passed

Codacy/PR Quality Review Not up to standards. This pull request quality could be better.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@PierreRambaud

This comment has been minimized.

Copy link
Contributor

PierreRambaud commented Dec 11, 2018

Thanks @sarjon and everyone!

@PierreRambaud PierreRambaud added this to the 1.7.6.0 milestone Dec 11, 2018

@matks matks referenced this pull request Dec 12, 2018

Open

Migrate "Customers > Customers > View customer" page #11010

2 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment