Dev Journal

Table of contents

Introduction

This dev journal is intended for documenting some changes made and the corresponding code modifications.
It may serve as an inspiration for future feature implementation and bug fixes.

Splitting ClientResult into use case specific DTOs

The ClientResultData object was being used for the client read and the client list request. This was done for convenience because the result for client list has the same attributes as for client read it just doesn't need all of them. I wanted to save time and not create an extra object with its own specificities.

The issue

The problem is that it goes against the Single Responsibility Principle. For every little change, all the use cases where the result DTO is being used are affected, which makes it more prone to lead to unexpected side effects.

Probably the risk isn't huge yet with two use-cases and few attributes, but in the future each use-case will probably have more specific attributes.
This template application is intended to be as "clean" as possible for it to be easily scalable. Better do it right from the start instead of having more work refactoring later when it's getting bigger.

What does have to be changed

The ClientResultData should be split into two separate DTOs, one for each use-case.

Change steps

Commit: d4a7ac2f484839a8f2b7dd5159c59fafca6d2371

1. Rename ClientResultData into ClientReadResult make a copy ClientListResult.
2. Check where the instance ClientReadResult is used and if it is for the client list use-case, replace with the new ClientListResult.
3. The next step is to find out which attributes are needed for both use cases
4. Check where ClientReadResult is used and what attributes are needed
   • For this, ideally we can click on the attributes with the mouse middle wheel or Ctrl + click but this only works if the DTO is being used by a PHP template and not if it's requested via an AJAX request.
   • To reliably find out where the attributes are being used, we have to check where the ClientResultData instance is being used.
   • In the end, the values are used by the frontend so the Action that returns the read result data is most relevant. Actions get their data from a domain service, and there is only one service function that returns the ClientReadResult: ClientFinder::findClientReadAggregate(). This function is used by the ClientReadPageAction which renders the client-read.html.php template.
   • There we can look for the attributes that are being used. It's a rendered template, and we now know that it's the only place where ClientReadResult is being used, so we can Ctrl + click on the attributes in the DTO directly to check if they're needed in the template or not.
5. All are needed except the array $notes which is removed.
6. Check where ClientListResult is used
   • Steps are similar, but this time the DTO is serialized into JSON and returned via an AJAX request.
   • The ClientListResult is used by the ClientFinderRepository::findClientsWithResultAggregate() and following the path backwards to the Action we find ClientFetchListAction which returns the ClientListResult as JSON. This action is invoked by the route name GET /clients.
   • Searching for 'clients' in the JS-files associated with a GET request, we find the client-list-loading.js fetchClients() function and the JSON response is used in fetchAndLoadClients().
   • The client collection of this JSON response is used in getClientProfileCardHtml() of the client-list-profile-card.html.js file.
7. We can see that appart from the client DTO item values (i.e. first_name, last_name, etc.), only the assignedUserPrivilege and clientStatusPrivilege are used.
8. So every attribute from ClientListResult can be removed except these two.
9. Now the place where the ClientListResult is populated (ClientFinderRepository::findClientsWithResultAggregate()) can be adapted to only populate it with the needed attributes.
10. In the ClientFinderRepository I tried to avoid having to use the same field strings twice in each select query for both the client read and client list find function. I made them to be attributes of the class so I could re-used them, but it made everything so much more complex and rigid. Now both functions select exactly their needed fields and the fact that the findClientsWithResultAggregate for client list contains some of the exact same names than for client read does not bother me any more. It's the opposite, I find it so much better because each one can be updated without having to worry about the other use case at all. And if there is a change in field name that affects both they can easily be changed with Ctrl + Shift + R (search and replace) in PHPStorm.
11. The ClientListActionTest has to be modified to assert only the needed attributes.

Adding a simple new field theme

The user can choose a theme that should be saved in the database.
There are a few places where the code has to be modified, and the field added so that it works as expected.
The value can only be added by update, so we only need to change one action.

Recap

• UserTheme.php create theme enum.
• UserValidator.php add validation line in validateUserValues().
• UserAuthorizationChecker.php add to the $grantedUpdateKeys in isGrantedToUpdate().
• UserUpdater.php add theme to the authorized update keys in updateUser().
• UserData.php add instance variable and populate in constructor.
• UserFinderRepository.php add to the class attribute $fields.
• UserUpdateProvider.php add theme change to $basicDataChanges var in userUpdateAuthorizationCases().
• UserUpdateProvider.php add theme change to request body and error message to response in invalidUserUpdateCases().

Implementation

• The field is stored as enum in the database, and it is possible that one day there will be other themes than just light and dark, so it makes sense to create a php enum:

  // Domain/User/Enum/UserTheme.php
  enum UserTheme: string
  {
      use EnumToArray;
  
      case light = 'light';
      case dark = 'dark';
  }

  • The service function updateUser() is called which first validates the values validateUserUpdate(). As it's a backed enum the function validateBackedEnum() can be used:

    // UserValidator.php  
    $validator
    // ...
    ->requirePresence('theme', false, __('Key is required'))
    ->add('theme', 'themeIsAvailable', [
      'rule' => function ($value, $context) {
          // Check if given user status is one of the enum cases values
          return in_array($value, UserTheme::values(), true);
      },
      'message' => __('Invalid option'),
    ]);

• When value validation is done, authorization is tested with isGrantedToUpdate(). It checks if authenticated user is allowed to change given field. The way it works is by adding the field name to a $grantedUpdateKeys array if the permissions match.
  theme has the same authorization rules as the other "general user data" so it's added right below the list:

  if (array_key_exists('theme', $userDataToUpdate)) {
      $grantedUpdateKeys[] = 'theme';
  }

• The service function updateUser() creates an array of the fields that may be updated to be certain that only the fields that are designed to be updated actually get changed:

  if (in_array($column, ['first_name', '...', 'theme'], true)) {
      $validUpdateData[$column] = $value;
  }

• Now to be able to read this value and so the app knows that theme is now a user value, add it to UserData.php instance variables and also the constructor.

  class UserData implements \JsonSerializable
  {
      /* ... */    
      public ?UserTheme $theme = null;
  
      public function __construct(array $userData = [])
      {
          $this->theme = $userData['theme'] ?? null ? UserTheme::tryFrom($userData['theme']) : null;
      }
  }

• The file that retrieves the user values from the database is the repository UserFinderRepository.php and it retrieves only the requested values, not automatically all fields. The list of values that should be selected are in the class attribute $fields:

  class UserFinderRepository
  {
      private array $fields = ['id', '...', 'theme',];
      // ...
  }

  Testing

• User theme value is an extremely simple, value therefore not much has to be done especially when there are good existing tests.
  The test itself doesn't even has to be changed, only the data provider that feeds it values UserUpdateProvider.php:

  public static function userUpdateAuthorizationCases(): array
  {
    // ...
    $basicDataChanges = ['first_name' => 'NewFirstName', '...', 'theme' => 'dark'];
    // ...
  }

• There is a validation being made so an invalid value should be tested as well. For this the function
  invalidUserUpdateCases() can be extended to also include 'theme' => 'invalid', in the request body and ['field' => 'theme', 'message' => 'theme not existing'], in the response validation errors.