Company account with multiple users by Miroslav-Stopka · Pull Request #15 · shopsys/demoshop

Miroslav-Stopka · 2018-09-06T21:06:00Z

Q	A
Description, reason for the PR
The administrator needs to be able to create user accounts grouped under one company account. Individual company users have separate login credentials but they will share some attributes - for example a billing address.
New feature	Yes
BC breaks	No
Fixes issues
Standards and tests pass	Yes
Have you read and signed our License Agreement for contributions?	Yes

vitek-rostislav

Hi, the review is done 🙂 Hats off to you! 🥇

vitek-rostislav · 2018-09-07T05:22:57Z

+     *
+     * New state:
+     *  - Mapping of child class property is overriden only if:
+     *      - or mapping of child class property is equal to mapping of parent class property


I would remove the first "or" 🙂

Will be change to either.

vitek-rostislav · 2018-09-07T05:45:31Z

+                                    $overriddingClassReflection = new \ReflectionClass($class);
+                                    $overiddingClassProperties = $overriddingClassReflection->getProperties();
+
+                                    $isOverridenPropertyInChildClass = $this->checkIsOverridenPropertyInChildClass($overiddingClassProperties, $name, $class);


the word is correctly spelled "overridden" - both in the variable and the private method name...
#nitpick 😇

...and in the comment above the method

vitek-rostislav · 2018-09-07T10:50:54Z

+     * @param \Shopsys\ShopBundle\Model\Customer\User[] $users
+     * @return \Shopsys\ShopBundle\Model\Customer\UserData[]
+     */
+    public function createMultipleUserDataFromUsers(array $users)


this method is never used

Right and removed

Wrong and returned :D It is used in CustomerController :D

vitek-rostislav · 2018-09-07T12:20:31Z

 use Shopsys\FrameworkBundle\Model\Order\Item\OrderProduct;
 use Shopsys\FrameworkBundle\Model\Order\Item\OrderTransport;
 use Shopsys\FrameworkBundle\Model\Order\Order;
+use Shopsys\FrameworkBundle\Model\Order\Order as ExtendedOrder;


was it not working before?

Im thinking, both ways are functional. Should I change to ShopBundle?

vitek-rostislav

Hi, just a few final notes from CR, please check them out.

I also tested it and:

when dynamically adding new company users in admin, javascript validation does not work for the fields (Shopsys.register.registerCallback and Shopsys.register.registerNewContent should help you here 🙂 )
there is an unhandled exception (UniqueConstraintViolationException) when trying to add new company user with already registered email
company users created on company edit page are not visible in the customers list admin/customer/list/ - is this intended? How can I get on an admin detail page of those customers?
is it ok that a particular company customer sees all company orders (even those that were not created by himself?) (on FE customer/orders/)

vitek-rostislav · 2018-09-10T10:19:57Z


                if (!$('.js-is-company-with-multiple-users').prop('checked')) {
-                    Shopsys.companyData.changeSetUpCompanyWithMultipleUsers(false);
+                    Shopsys.companyData.unsetUpCompanyWithMultipleUsers();


could you please rename to unsetCompanyWithMultipleUsers? That "Up" is weird there 😇

vitek-rostislav · 2018-09-10T10:26:39Z


  Shopsys\FrameworkBundle\Model\Customer\CustomerFacade: '@Shopsys\ShopBundle\Model\Customer\CustomerFacade'

-  Shopsys\ShopBundle\Model\Customer\UserRepository: ~


you can remove OrderRepository as well 🙂

- after the change it is possible to overwrite the already existing property of an entity even with the possibility of overwriting the orm associations of this property - before the change the final associations of the new property were automatically taken over from the original property

…t is a company with multiple users - customer with a billing address that belongs to a company with multiple users, can not edit the billing address in a personal data or in the order

- billing address represents a company, so multiple customers of the same billing address mean multiple users of the same company

…des all the orders of the customers from the same company

- customer section in admin displays list of billing addresses instead of users because on billing address can have multiple users - customers list in admin is simplified because of company accounts with multiple users - in the case of request for standard account - isCompanyWithMultipleUsers === false - is displayed standard edit form for customer - in the case of request for multiple users companz account - isCompanyWithMultipleUsers === true - is displayed company edit form for with list of company users - UserData contains new property id to allow efective and clear manipulation with entries of users in the list of company users - in the case of cancellation of setup of company with multiple users, they are removed all company users except one for preserving company account - js code is part of twig templates because of problems with adding new custom js for admin

- EntityExtensionTest merge original extension map with tested custom extended entities

…le users

…rs() to isCompanyWithMultipleUsers()

…h autodiscover

…th autodiscover

Miroslav-Stopka force-pushed the msa-company-with-users branch from 8c443e2 to 7f584b4 Compare September 6, 2018 21:09

vitek-rostislav reviewed Sep 7, 2018

View reviewed changes

vitek-rostislav reviewed Sep 10, 2018

View reviewed changes

Miroslav-Stopka added 12 commits September 10, 2018 18:04

billing address now has an attribute with information about whether i…

e81690b

…t is a company with multiple users - customer with a billing address that belongs to a company with multiple users, can not edit the billing address in a personal data or in the order

multiple customers may be assigned to the same billing address

f388436

- billing address represents a company, so multiple customers of the same billing address mean multiple users of the same company

customer from the company with multiple users sees in a section my or…

940fa02

…des all the orders of the customers from the same company

tests are functional after customer company account with multiple users

0d663ea

- EntityExtensionTest merge original extension map with tested custom extended entities

CHANGELOG.md added note about new feature company account with multip…

f27cce1

…le users

theme.html.twig is loaded for overriden admin controllers too

d0e0eaa

BillingAddress entity renamed method from getIsCompanyWithMultipleUse…

33bc568

…rs() to isCompanyWithMultipleUsers()

services.yml: removed UserRepository service because it is loaded wit…

30488b3

…h autodiscover

check the uniqueness of the emails of the company users

456833e

services.yml: removed OrderRepository service because it is loaded wi…

91e7938

…th autodiscover

Miroslav-Stopka force-pushed the msa-company-with-users branch from 1712d09 to 91e7938 Compare September 10, 2018 16:33

Miroslav-Stopka merged commit e7c2d84 into master Sep 10, 2018

Miroslav-Stopka deleted the msa-company-with-users branch December 11, 2018 15:28


		Shopsys\FrameworkBundle\Model\Customer\CustomerFacade: '@Shopsys\ShopBundle\Model\Customer\CustomerFacade'

		Shopsys\ShopBundle\Model\Customer\UserRepository: ~

Conversation

Miroslav-Stopka commented Sep 6, 2018

Uh oh!

vitek-rostislav left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vitek-rostislav left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants