Some feedback

[kevinpapst]

Hi!

Really nice idea and a great start. I'd like to leave some initial feedback:

• Kimai 1.1 and higher has some changes in the repositories. The method findByQuery() was removed from the CustomerRepository and replaced with getCustomersForQuery()
• If you give view_readonly_customer to eg. the user role, every user can access the new screen. But then every user has access even if no readOnlyAccessCustomer preference exist for that user. That leads to a 500 The identifier id is missing for a query of App\Entity\Customer. You should check if the customerid preference and the customer itself is really existing/not null.
• What do you do with the ROLE_READONLYACCESS_USER? This seems not to be used besides in the frontend. Some ideas follow:

Just from a first look I'd say: replace the isGranted('view_readonly_customer') call in the MenuSubscriber and in the ReadOnlyAccessUserController with isGranted('ROLE_READONLYACCESS_USER'). Then you can remove the permission view_readonly_customer altogether.

Or you could even make it simpler: remove the role and the permission and inside the controller and MenuSubscriber you simply check if currentUser->getPreferenceValue("readOnlyAccessCustomer") !== null and use that to decide whether the menu should be displayed. Inside the ReadOnlyAccessUserController you could replace the isGranted with:

$customerId = $this->getUser()->getPreferenceValue("readOnlyAccessCustomer");
if (null === $customerId) {
    throw $this->createAccessDeniedException();
}

Just in case someone knows the URL.

Oh and finally: add some tags to your repository description like kimai2, kimai-plugins ... just check which ones are offered in the autocomplete dropdown. Then it can be found easier by other people.

[fungus75]

Kevin, thank you for that ideas. I have adjusted it and it looks good... Still going to do some tests...

[fungus75]

Kevin, from my point of view, this is fixed by now.

[kevinpapst]

Like that 👍 only two minor points: Customer shouldn't be deleted, but if ... you still get a 500. You should add in line 40, after this one:

    if ($customer === null) {
        throw $this->createNotFoundException();
    }

The menu icon ist still from the CustomCSSBundle, what about https://fontawesome.com/icons/book?style=solid ?

[fungus75]

Also fixed and icon adjusted. Thanks for all that testing...

[kevinpapst]

Great work, really like it! Closing, thanks for having a look at my suggestions