Skip to content

Commit

Permalink
Save dark mode setting in database [SLE-88]
Browse files Browse the repository at this point in the history
  • Loading branch information
samuelgfeller committed Jun 6, 2023
1 parent 93938c7 commit 365c5a8
Show file tree
Hide file tree
Showing 24 changed files with 295 additions and 60 deletions.
113 changes: 113 additions & 0 deletions docs/implementation-examples.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
## Implementation examples

This document functions as a dev journal, documenting some changes made and
the corresponding code modifications.
It may serve as an inspiration for future feature implementation and bug
fixes. Especially for those that are not familiar with the project code, but I will be using
it as a cheatsheet all the time as well probably.

### 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 like expected.
The value can only be added by **update**, so we only need to
change one action.
#### Recap
* `UserTheme.php` create theme enum.
* `UserSubmitUpdateAction.php` add argument to `malformedRequestBodyChecker()` call.
* `UserValidator.php` add validation line in `validateUserUpdate()`.
* `UserAuthorizationChecker.php` add to the `$grantedUpdateKeys` in `isGrantedToUpdate()`.
* `UserUpdater.php` add to the `if(in_array())` in `updateUser()`.
* `UserData.php` add to the instance variables and constructor.
* `UserFinderRepository.php` add to the instance variable `$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:
```php
// Domain/User/Enum/UserTheme.php
enum UserTheme: string
{
case light = 'light';
case dark = 'dark';
}
```
* In the action class `UserSubmitUpdateAction.php`, `'theme'` has to be added to the
`malformedRequestBodyChecker` argument list, so it knows `theme` is an accepted value.
* After it passed the request body keys validator, the service function `updateUser()`
is called which first validates the values `validateUserUpdate()`. As it's a backed enum
the function `validateBackedEnum()` can be used:
```php
// UserValidator.php validateUserUpdate()
// ...
if (array_key_exists('theme', $userValues)) {
$this->validator->validateBackedEnum(
$userValues['theme'],
UserTheme::class,
'theme',
$validationResult
);
}
```
* 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:
```php
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:
```php
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.
```php
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 an instance variable of this file `$fields`:
```php
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`:
```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.
2 changes: 1 addition & 1 deletion public/assets/general/ajax/submit-update-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {handleFail, removeValidationErrorMessages} from "./ajax-util/fail-handle
* On success validation errors are removed and response content returned
*
* @param {object} formFieldsAndValues {field: value} e.g. {[input.name]: input.value}
* @param {string} route after base path
* @param {string} route after base path e.g. clients/1
* @param {boolean|string} redirectToRouteIfUnauthenticated true or redirect route url after base path.
* If true, the redirect url is the same as the given route
*
Expand Down
34 changes: 18 additions & 16 deletions public/assets/general/dark-mode/dark-mode.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
// Get the toggle switch element
import {submitUpdate} from "../ajax/submit-update-data.js?v=0.2.1";
import {displayFlashMessage} from "../page-component/flash-message/flash-message.js?v=0.2.1";

const toggleSwitch = document.querySelector('#dark-mode-toggle-checkbox');

// Retrieve the current theme from localStorage
const currentTheme = localStorage.getItem('theme') ? localStorage.getItem('theme') : null;

// Set the theme based on the stored value from localStorage
if (currentTheme) {
// Set the data-theme attribute on the root element
// Set the data-theme attribute on the html element
document.documentElement.setAttribute('data-theme', currentTheme);

// Check the toggle switch if the current theme is 'dark'
Expand All @@ -18,31 +21,30 @@ if (currentTheme) {
// Add event listener to the toggle switch for theme switching
toggleSwitch.addEventListener('change', switchTheme, false);

// Set the theme on initial load if there is a stored value in localStorage
if (currentTheme) {
// Set the data-theme attribute on the root element
document.documentElement.setAttribute('data-theme', currentTheme);

// If the current theme is 'dark', set the data-theme attribute to 'dark'
if (currentTheme === 'dark') {
document.documentElement.setAttribute('data-theme', 'dark');
}
}

/**
* Handle theme switching with localstorage
*
* @param e
*/
function switchTheme(e) {
let theme;
// Check the current theme and switch to the opposite theme
if (document.documentElement.getAttribute('data-theme') === 'dark') {
document.documentElement.setAttribute('data-theme', 'light');
localStorage.setItem('theme', 'light');
theme = 'light';
} else {
document.documentElement.setAttribute('data-theme', 'dark');
localStorage.setItem('theme', 'dark');
theme = 'dark';
}
// Set html data-attribute and local storage entry
document.documentElement.setAttribute('data-theme', theme);
localStorage.setItem('theme', theme);

// Make ajax call to change value in database
let userId = document.getElementById('user-id').value;
submitUpdate({theme: theme}, `users/${userId}`, true)
.then(r => {})
.catch(r => {
displayFlashMessage('error', 'Failed to change the theme in the database.')
});
}


Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,10 @@
/*width: 200px;*/
/*display: block;*/
}
.flash-message a{
color: black;
font-weight: bold;
}

.flash-close-btn {
position: absolute;
Expand Down
2 changes: 1 addition & 1 deletion public/assets/user/read/user-update-contenteditable.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function saveUserValueAndDisableContentEditable(field) {
submitUpdate(
{[field.dataset.name]: field.textContent.trim()},
`users/${userId}`,
`users/${userId}`
true
).then(responseJson => {
// Field disabled before save request and re enabled on error
}).catch(responseJson => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

class DbChange1316013665647f42494524b extends Phinx\Migration\AbstractMigration
{
public function change()
{
$this->table('user', [
'id' => false,
'primary_key' => ['id'],
'engine' => 'InnoDB',
'encoding' => 'utf8mb4',
'collation' => 'utf8mb4_unicode_ci',
'comment' => '',
'row_format' => 'DYNAMIC',
])
->addColumn('theme', 'enum', [
'null' => true,
'default' => 'light',
'limit' => 5,
'values' => ['light', 'dark'],
'after' => 'password_hash',
])
->changeColumn('updated_at', 'datetime', [
'null' => true,
'default' => 'CURRENT_TIMESTAMP',
'after' => 'theme',
])
->changeColumn('created_at', 'datetime', [
'null' => true,
'default' => 'CURRENT_TIMESTAMP',
'after' => 'updated_at',
])
->changeColumn('deleted_at', 'datetime', [
'null' => true,
'default' => null,
'after' => 'created_at',
])
->save();
}
}
30 changes: 27 additions & 3 deletions resources/schema/schema.php
Original file line number Diff line number Diff line change
Expand Up @@ -1602,12 +1602,36 @@
'IS_GENERATED' => 'NEVER',
'GENERATION_EXPRESSION' => NULL,
),
'theme' =>
array (
'TABLE_CATALOG' => 'def',
'TABLE_NAME' => 'user',
'COLUMN_NAME' => 'theme',
'ORDINAL_POSITION' => 8,
'COLUMN_DEFAULT' => '\'light\'',
'IS_NULLABLE' => 'YES',
'DATA_TYPE' => 'enum',
'CHARACTER_MAXIMUM_LENGTH' => 5,
'CHARACTER_OCTET_LENGTH' => 20,
'NUMERIC_PRECISION' => NULL,
'NUMERIC_SCALE' => NULL,
'DATETIME_PRECISION' => NULL,
'CHARACTER_SET_NAME' => 'utf8mb4',
'COLLATION_NAME' => 'utf8mb4_unicode_ci',
'COLUMN_TYPE' => 'enum(\'light\',\'dark\')',
'COLUMN_KEY' => '',
'EXTRA' => '',
'PRIVILEGES' => 'select,insert,update,references',
'COLUMN_COMMENT' => '',
'IS_GENERATED' => 'NEVER',
'GENERATION_EXPRESSION' => NULL,
),
'updated_at' =>
array (
'TABLE_CATALOG' => 'def',
'TABLE_NAME' => 'user',
'COLUMN_NAME' => 'updated_at',
'ORDINAL_POSITION' => 8,
'ORDINAL_POSITION' => 9,
'COLUMN_DEFAULT' => 'current_timestamp()',
'IS_NULLABLE' => 'YES',
'DATA_TYPE' => 'datetime',
Expand All @@ -1631,7 +1655,7 @@
'TABLE_CATALOG' => 'def',
'TABLE_NAME' => 'user',
'COLUMN_NAME' => 'created_at',
'ORDINAL_POSITION' => 9,
'ORDINAL_POSITION' => 10,
'COLUMN_DEFAULT' => 'current_timestamp()',
'IS_NULLABLE' => 'YES',
'DATA_TYPE' => 'datetime',
Expand All @@ -1655,7 +1679,7 @@
'TABLE_CATALOG' => 'def',
'TABLE_NAME' => 'user',
'COLUMN_NAME' => 'deleted_at',
'ORDINAL_POSITION' => 10,
'ORDINAL_POSITION' => 11,
'COLUMN_DEFAULT' => 'NULL',
'IS_NULLABLE' => 'YES',
'DATA_TYPE' => 'datetime',
Expand Down
1 change: 1 addition & 0 deletions resources/schema/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ CREATE TABLE `user` (
`status` enum('active','locked','unverified','suspended') DEFAULT 'unverified',
`email` varchar(254) NOT NULL,
`password_hash` varchar(300) NOT NULL,
`theme` enum('light','dark') DEFAULT 'light',
`updated_at` datetime DEFAULT current_timestamp(),
`created_at` datetime DEFAULT current_timestamp(),
`deleted_at` datetime DEFAULT NULL,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use App\Domain\Authentication\Service\LoginVerifier;
use App\Domain\Factory\LoggerFactory;
use App\Domain\Security\Exception\SecurityException;
use App\Domain\User\Service\UserFinder;
use App\Domain\Validation\ValidationException;
use Odan\Session\SessionInterface;
use Psr\Http\Message\ResponseInterface as Response;
Expand All @@ -26,6 +27,7 @@ public function __construct(
private readonly LoginVerifier $loginVerifier,
private readonly SessionInterface $session,
private readonly MalformedRequestBodyChecker $malformedRequestBodyChecker,
private readonly UserFinder $userFinder,
) {
$this->logger = $logger->addFileHandler('error.log')->createInstance('auth-login');
}
Expand Down Expand Up @@ -57,12 +59,22 @@ public function __invoke(ServerRequest $request, Response $response): Response
// Add success message to flash
$flash->add('success', 'Login successful');

// Check if user has enabled dark mode and if yes populate var
$themeQueryParams = [];
if ($theme = $this->userFinder->findUserById($userId)->theme) {
$themeQueryParams['theme'] = $theme->value;
}

// After register and login success, check if user should be redirected
if (isset($queryParams['redirect'])) {
return $this->responder->redirectToUrl($response, $request->getQueryParams()['redirect']);
return $this->responder->redirectToUrl(
$response,
$request->getQueryParams()['redirect'],
$themeQueryParams
);
}

return $this->responder->redirectToRouteName($response, 'home-page');
return $this->responder->redirectToRouteName($response, 'home-page', [], $themeQueryParams);
} catch (ValidationException $ve) {
return $this->responder->renderOnValidationError(
$response,
Expand Down
3 changes: 3 additions & 0 deletions src/Application/Actions/User/Ajax/UserSubmitUpdateAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ public function __invoke(
'email',
'status',
'user_role_id',
'theme',
// When adding a new field also add it in updateUser(), validateUserUpdate(), isGrantedToUpdate(),
// UserFinderRepository->fields and don't forget testing
])) {
try {
$updated = $this->userUpdater->updateUser($userIdToChange, $userValuesToChange);
Expand Down
4 changes: 2 additions & 2 deletions src/Application/Middleware/PhpViewExtensionMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ public function process(
'uri' => $request->getUri(),
'basePath' => $this->app->getBasePath(),
'route' => $this->app->getRouteCollector()->getRouteParser(),
'currRouteName' => RouteContext::fromRequest($request)->getRoute()->getName(),
'currRouteName' => RouteContext::fromRequest($request)->getRoute()?->getName(),
'flash' => $this->session->getFlash(),
// Used for public values used by view like company email address
'config' => $this->publicSettings,
// Check if granted to read user that is different from the logged in one (+1)
// Check if granted to read user that is different then the authenticated user itself (+1)
'userListAuthorization' => $this->userAuthorizationChecker->isGrantedToRead(
($this->session->get('user_id') ?? 1) + 1
),
Expand Down
5 changes: 5 additions & 0 deletions src/Domain/User/Authorization/UserAuthorizationChecker.php
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,11 @@ public function isGrantedToUpdate(
if (array_key_exists('password_hash', $userDataToUpdate)) {
$grantedUpdateKeys[] = 'password_hash';
}
if (array_key_exists('theme', $userDataToUpdate)) {
$grantedUpdateKeys[] = 'theme';
}
// If new basic data fiel is added, add it to provider userUpdateAuthorizationCases() $basicDataChanges
// and invalid value to provider invalidUserUpdateCases()

// Things that only managing_advisor and higher privileged are allowed to do with users (and not if own profile)
// If user is managing advisor we know by the parent if-statement that the user to change has not higher
Expand Down
Loading

0 comments on commit 365c5a8

Please sign in to comment.