Чат Telegram
Группа ВКонтакте
Новый комментарий

Sparkqy 02.10.2018 в 00:26

UsersController

    public function activate(int $userId, string $activationCode)
    {
        try
        {
            $user = User::getById($userId);
        } catch (UserActivationException $e)
        {
            $this->view->renderHtml('mail/userActivationError.php', [
                'error' => $e->getMessage()
            ],422);
            return;
        }

        try
        {
            $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
        } catch (UserActivationException $e)
        {
            $this->view->renderHtml('mail/userActivationError.php', [
                'error' => $e->getMessage()
            ],422);
            return;
        }

        if ($isCodeValid) {
            $user->activate();
            echo 'OK!';
            UserActivationService::deleteActivationCode($userId, $activationCode);
        }
    }

UserActivationService

public static function checkActivationCode(User $user, string $code): bool
    {
        $db = Db::getInstance();
        $result = $db->query(
            'SELECT * FROM `' . self::TABLE_NAME . '` WHERE `user_id` = :user_id AND `code` = :code',
            [
                'user_id' => $user->getId(),
                'code' => $code
            ]
        );

        if (!$result)
        {
            throw new UserActivationException('Ошибка активации. Проверочный код не валидный.');
        }

        return !empty($result);
    }

    public static function deleteActivationCode(int $userId, string $activationCode): void
    {
        $db = Db::getInstance();
        $db->query(
            'DELETE FROM `' . self::TABLE_NAME . '` WHERE `user_id` = :userId AND `code` = :activationCode;', [
                ':userId' => $userId,
                ':activationCode' => $activationCode,
            ]
        );
    }

Так же для проверки на "если в ссылку активации подставить несуществующего пользователя" в методе getById бросаю исключение при получении пустого массива после запроса в БД. \
Все верно?

ivashkevich 02.10.2018 в 22:45
    public function activate(int $userId, string $activationCode)
    {
        try {
            $user = User::getById($userId);
        } catch (UserActivationException $e)

Неправильно - в процессе получения пользователя по id никак не должно бросаться исключение с типом UserActivationException. UserActivationException - только для ошибок, связанных с активацией. Само по себе получение пользователя из базы никак не связано с активацией.

echo 'OK!';

а где шаблон для успешного случая?

if (!$result)
        {
            throw new UserActivationException('Ошибка активации. Проверочный код не валидный.');
        }

        return !empty($result);

конкретно здесь !$result и !empty($result) - дадут просто противоположные значения. Не имеет смыла здесь кидать исключение - достаточно только вернуть true или false. Этот метод должен сказать, является ли код валидным или нет. То есть если он невалидный - это не исключительная ситуация, а вполне себе штатная. Поэтому исключение здесь не нужно.

Таким образом, в контроллере вам не нужно ничего ловить, нужно просто проверять пользователя на !== null и то что метод проверки кода вернул true.

Sparkqy 02.10.2018 в 23:13

Согласен, еще не до конца понимаю где кидать и ловить нужно, с простым рендерингом ошибок было легче. Буду разбираться

ivashkevich 03.10.2018 в 10:57

Ничего, я тоже поначалу не понимал, даже когда устраивался на работу - это придет с опытом.

Kirill.K 10.10.2018 в 23:43

UsersActivationService:

    public static function existCode($userId): bool
    {
        $db = Db::getInstance();
        $code = $db->query(
            'SELECT code FROM ' . self::TABLE_NAME . ' WHERE user_id = :user_id',
            [
                'user_id' => $userId
            ]

        );
        return !empty($code);
    }

    public static function deleteActivationCode(int $userId): void
    {
        $db = Db::getInstance();
        $db->query(
            'DELETE FROM ' . self::TABLE_NAME . ' WHERE user_id = :user_id',
            [
                'user_id' => $userId
            ]
        );
    }

Функцию delete провожу без сверки с кодом, так как мы запускаем её непосредственно после проверки кода на валидность, или это открывает возможность появления каких-либо ошибок?

UsrsController:

public function activate(int $userId, string $activationCode)
    {
        try {
            $user = User::getById($userId);
            if ($user === null) {
                throw new ActivateException('Нет такого пользователя');
            }
            if ($user->getIsConfirmed() == 1) {
                throw new ActivateException('Пользователь уже активирован');
            }

            if (!UserActivationService::existCode($userId)) {
                throw new ActivateException('Не создан код активации');
            }

            $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
            if ($isCodeValid) {
                $user->activate();
                UserActivationService::deleteActivationCode($userId);
                echo 'OK!';
            } else {
                echo 'Код активации не верен';
            }
        } catch (ActivateException $e) {
            $this->view->renderHtml('errors/noId.php', ['error' => $e->getMessage()]);
            return;
        }
    }

Я пытался учесть замечание Sparky, про то, что Activation-исключение бросается в процессе получения пользователя, но ума не приложу, как сделать правильно... нужно бросать другое исключение? или выносить catch в index?
Ещё не сделал шаблоны, ибо это не сложно, но времяёмко, а уже поздно и пора спать) Поэтому проторопился и сделал просто вывод месаджами(

ivashkevich 12.10.2018 в 09:10

1) Удаление кода без проверки кода в модели - норм. Ошибки вряд ли тут будут.
2) Логику, отвечающую за проверку существования кода можно перенести внутрь UserActivationService::checkActivationCode(). Если кода нет - просто возвращать false, исключение тут не нужно.
3) Исключения вы бросаете в нужном месте. Не надо ничего усложнять, у Вас все просто и понятно.
4) Когда код активации неверен - тоже можно бросить исключение.
5) Структуру кода можно переделать так, что сначала проверяются все исключительные ситуации и бросаются исключения, а затем, если все хорошо, просто работает код для успешного исхода. Суть такая:

if (что-то плохо) {
    бросаем исключение
}

if (что-то другое плохо) {
    бросаем исключение
}

если дошли до сюда, то просто рисуем шаблон, никаких if-ов уже не нужно.
dnldcode 06.03.2019 в 23:11
    public function activate(int $userId, string $activationCode): void
    {
        try {
            $user = User::getById($userId);

            if ($user === null) {
                throw new ActivationException('User was not found');
            }
            if ($user->isActivated()) {
                throw new ActivationException('User is already activated');
            }
            $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
            if (!$isCodeValid) {
                throw new ActivationException('Code is not valid');
            }

            $user->activate();
            UserActivationService::removeActivationCode($user);
            $this->view->renderHtml('users/activationSuccess.php');
        } catch (ActivationException $e) {
            $this->view->renderHtml('errors/activationError.php', ['error' => $e->getMessage()], 422);
        }
    }

Вопросы:

  1. Обязательно делать проверку на то что код уже активирован? В нашем случае это же невозможно и делается в виде подстраховки. Лишний код? Хотелось бы детальнее узнать, нужны ли эти дополнительный проверки на каждую функцию делать в будущем?
  2. В методе checkActivationCode обязательно делать проверку на то, существует ли данный код? Ведь если запрос сам не найдет - значит не существует.
ivashkevich 09.03.2019 в 19:21
  1. Такая проверка позволит узнать о возможных ошибках в логике работы программы в других местах. Если пользователь уже активирован, и его пытаются активировать еще раз, говорит о том, что где-то косяк.
  2. А какая там проверка. Там только смотрим, что запрос вернул непустой результат. Как вы и написали.
alepawka 23.03.2019 в 22:18

UserController

public function activate(int $userId, string $activationCode)
    {
        try {
            $user = User::getById($userId);
            if ($user->getConfirmed() === 1) {
                throw new InvalidActivationException('Пользователь уже авторизован');
            }
            $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);

            if ($isCodeValid) {
                $user->activate();
                UserActivationService::deleteCode($user->getId());

                $this->view->renderHTML('users/successValidation.php');
            } else {
                throw new InvalidActivationException('Произошла ошибка авторизации. Попробуйте снова');
            }
        } catch (InvalidActivationException $e) {
            $this->view->renderHTML('users/UserError.php', ['error' => $e->getMessage()]);
        }
    }

далее, шаблон с ошибками(идентичен 500.php)

<h1>Что то пошло не так...</h1>
<?= $error; ?>

Ну и шаблон успешной авторизации

<?php include __DIR__ . '/../header.php'; ?>
<h1>Вы успешно подтвердили свою почту</h1>
<a href="/">Перейти к просмотру всех статей</a>
<?php include __DIR__ . '/../footer.php'; ?>
ivashkevich 23.03.2019 в 22:35
throw new InvalidActivationException('Пользователь уже авторизован');

В условии проверяется другое.

В остальной логике все ок.

excent63 03.04.2019 в 11:26

Добрый день! Решение д/з:
UserActivationService

    public static function deleteActivationCode(User $user, string $code)
    {
        $db = Db::getInstance();
        $db->query(
            'DELETE FROM ' . self::TABLE_NAME . ' WHERE `user_id` = :user_id AND `code` = :code;',
            [
                ':user_id' => $user->getId(),
                ':code' => $code
            ]
        );
    }

UsersController

...
    public function activate(int $userId, string $activationCode): void
    {
        try {
            $user = User::getById($userId);

            if ($user === null) {
                throw new ActivationException('Пользователь не найден.');
            }

            if ($user->IsConfirmed()) {
                throw new ActivationException('Пользователь уже активирован');
            }

            $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);

            if (!$isCodeValid) {
                throw new ActivationException('Неверный код активации');
            }

            if ($isCodeValid) {
                $user->activate();
                $this->view->renderHtml('users/successfulActivation.php');
                UserActivationService::deleteActivationCode($user, $activationCode);
                return;
            }

        } catch (ActivationException $e) {
            $this->view->renderHtml('users/nonexistentCode.php', ['error' => $e->getMessage()]);
        }
    }
...

nonexistensCode

<?php include __DIR__ . '/../header.php'; ?>
    <div style="text-align: center;">
        <h1>Не удалось активировать пользователя.</h1><br>
        <?= $error ?>
    </div>
<?php include __DIR__ . '/../footer.php'; ?>

successfulActivation

<?php include __DIR__ . '/../header.php'; ?>
<div style="text-align: center;">
    <h1>Ваша учётная запись успешно активирована!</h1>
    Вы можете перейти на <a href="/">главную страницу</a>.
</div>
<?php include __DIR__ . '/../footer.php'; ?>

Получается вот так?

ivashkevich 03.04.2019 в 22:59

getIsConfirmed() лучше просто isConfirmed(), и возвращаться должно булево значение, а не единичка.
В остальном - отличная домашка!

polvanovv 11.06.2019 в 11:37

Подскажите, в чем проблема? Мой код соответствует коду урока но после перехода по ссылке в письме выдает ошибку:

( ! ) Fatal error: Uncaught TypeError: Argument 1 passed to MyProject\Models\Users\UserActivationService::checkActivationCode() must be an instance of MyProject\Models\Users\User, null given, called in W:\domains\newProject\src\MyProject\Controllers\UserController.php on line 26 and defined in W:\domains\newProject\src\MyProject\Models\Users\UserActivationService.php on line 31

( ! ) TypeError: Argument 1 passed to MyProject\Models\Users\UserActivationService::checkActivationCode() must be an instance of MyProject\Models\Users\User, null given, called in W:\domains\newProject\src\MyProject\Controllers\UserController.php on line 26 in W:\domains\newProject\src\MyProject\Models\Users\UserActivationService.php on line 31

Никак не могу разобраться.

ivashkevich 11.06.2019 в 11:47

Ожидается, что в метод прилетит объект класса User, а прилетел null. Используйте Xdebug, и учитесь читать ошибки, на этом уроке не должно возникать подобных вопросов.

polvanovv 11.06.2019 в 17:00

Проблема была в двух опечатках в sql запросе. Невнимательность = минус день.

ivashkevich 11.06.2019 в 20:33

Со временем научитесь разбираться быстрее)

polvanovv 12.06.2019 в 14:21

1

public static function deleteActivationCode(int $userId, string $code): void
    {
        $db = Db::getInstance();
        $result = $db->query(
            'DELETE FROM `' . self::TABLE_NAME . '` WHERE `code` = :code AND `user_id` = :user_id',
        [
            'user_id' => $userId,
            'code' => $code
        ]
        );
    }

2

public function activate(int $userId, string $activationCode)
    {
        try {
            $user = User::getById($userId);
            if ($user == null){
                throw new UserActivationException('Такого пользователя не существует!');
            }
            if($user->isConfirmed()){
                throw new UserActivationException('Аккаунт уже активирован!');
            }
            if (empty($activationCode)){
                throw new UserActivationException('Время активации аккаунта истекло');
            }
            $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
            if ($isCodeValid) {
                $user->activate();
                $this->view->renderHtml('mail/UserActivationSuccessful.php');
                UserActivationService::deleteActivationCode($userId, $activationCode);
            }else{
                throw new UserActivationException('Неверный код активации аккаунта!');
            }
        }catch (UserActivationException $e){
            $this->view->renderHtml('errors/userActivationError.php', ['error' => $e->getMessage(), 422]);
            return;
        }
    }

3

<?php include __DIR__ . '/../header.php'; ?>
    <div style="text-align: center">
        <h2>Поздравляем! Активация аккаунта прошла успешно!</h2>
    </div>
<?php include __DIR__ . '/../footer.php'; ?>
<?php include __DIR__ . '/../header.php'; ?>
<div style="text-align: center">
<h1>Ошибка активации аккаунта: </h1>
<?= $error ?>
</div>
<?php include __DIR__ . '/../footer.php'; ?>

Правильно что надо два шаблона?

ivashkevich 13.06.2019 в 19:39
            if (empty($activationCode)){
                throw new UserActivationException('Время активации аккаунта истекло');
            }

Тут этого быть не должно. Должно быть в вызывающем этот метод коде.

Исключения бросать и тут же ловить в этом же методе - бесполезное занятие. Можно сразу вызывать рисование шаблона с ошибкой и делать return.

2 шаблона - это ок

sirserik 28.06.2019 в 14:23

Доброго времени суток автор, скажите а как все это проверить и настроить на linux ubuntu 19.04 я установил апач c php mysql но письма не отправляются. При поиске в интернет нашел статьи но они очень старые не подскажешь куда смортеть и как фиксить?

ivashkevich 30.06.2019 в 14:57

Погуглите, готовой инструкции на этот случай у меня нет.

jimholder37@gmail.com Patron 12.05.2020 в 00:12

Поднимай теперь почтовый сервис с помощью sendmail, либо, еще вариант PEAR Mail и Net_SMTP. Reply от гугл почты можно использовать, как и в уроке по OpenServer.
Но, честно, не знаю зачем с локалки отправлять письма на реальный сервер, можно фейковый сервер поднять на файлах и также в почтовом клиенте они будут появляться. У меня конфиг xampp + Evolution. Погугли про xampp на линуксе, мануалы даже на русском есть.

Metey 23.07.2019 в 19:01
public function activate(int $userId, string $activationCode)
    {
        $user = User::getById($userId); 
        if ($user === null){ //если пользователя нет - исключение
            throw new \MyProject\Exceptions\UserNotFoundException();
        }
        $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
        $isConfirmed = $user->getConfirmed(); // активирован пользователь? - добавил метод в User
        if ($isCodeValid) { //если код есть в базе, то активируем и ок
            $user->activate();
            echo 'OK!';
        }
        elseif ($isConfirmed == 1){ //если кода нет в базе(удален после активации??), но есть актив юзер
            $this->view->renderHtml('users/alreadyActivated.php');//
        }
        else { //если юзер не активный и кода вб азе нет - исключение!
            throw new \MyProject\Exceptions\CodeNotFoundException();
        }
        UserActivationService::deleteActivationCode($user);  //добавил в UserActivationService Метод deleteActivationCode

    }

а это метод deleteActivationCode :

 public static function deleteActivationCode(User $user): void
    {
        $db = Db::getInstance();
        $db->query(
            'DELETE FROM `' . self::TABLE_NAME . '` WHERE user_id = :user_id',
            [':user_id' => $user->getId() ]
        );
    }

вот добавил ловлю исключений в index :

........
$controller = new $controllerName();  
$controller->$actionName(...$matches);
} catch (\MyProject\Exceptions\DbException $e) {
    $view = new \MyProject\View\View(__DIR__ . '/../templates/errors');
    $view->renderHtml('500.php', ['error' => $e->getMessage()], 500);
} catch (\MyProject\Exceptions\NotFoundException $e) {
    $view = new \MyProject\View\View(__DIR__ . '/../templates/errors');
    $view->renderHtml('404.php', ['error' => $e->getMessage()], 404);
} catch (\MyProject\Exceptions\UserNotFoundException $e) {
    $view = new \MyProject\View\View(__DIR__ . '/../templates/errors');
    $view->renderHtml('UserNotFound.php', ['error' => $e->getMessage()], 404);
} catch (\MyProject\Exceptions\CodeNotFoundException $e) {
    $view = new \MyProject\View\View(__DIR__ . '/../templates/errors');
    $view->renderHtml('CodeNotFound.php', ['error' => $e->getMessage()], 404);
}

и вот такие шаблоны добавил для каждого случая :

alreadyActivated.php
<?php include __DIR__ . '/../header.php'; ?>
    <div style="text-align: center;">
        <h1>Пользователь уже активирован!</h1>
    </div>

CodeNotFound.php
<?php include __DIR__ . '/../header.php'; ?>
    <div style="text-align: center;">
        <h1>Код активации не найден!</h1>

    </div>
<?php include __DIR__ . '/../footer.php'; ?>

UserNotFound.php

<?php include __DIR__ . '/../header.php'; ?>
    <div style="text-align: center;">
        <h1>Такого пользователя не существует!</h1>

    </div>
<?php include __DIR__ . '/../footer.php'; ?>
ivashkevich 27.07.2019 в 18:34

В целом хорошо. Шаблоны ошибок все одинаковые, за исключением текста ошибки. Для такого случая можно сделать один шаблон, в котором выводить текст исключения.

Metey 27.07.2019 в 18:57

Спасибо, понял, исправлю

Iliusha99 30.07.2019 в 19:37

Вот так я доработал метод, если ID юзера нет то будет ошибка и соответственно если код не правильный будет тоже ошибка. Шаблон ошибок тот же и в зависимости от ошибки будет меняться в переменную $error.

public function activate(int $userId, string $activationCode)
    {
        try {
            $user = User::getById($userId);

            if ($user === null) {
                throw new UserNotFoundException('Пользователь не найден');
            }

            $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
            if ($isCodeValid) {
                $user->activate();

                echo 'OK!';
                return;
            } else {
                throw new IncorrectUserActivationCode('Некорректный код активации');
            }
        } catch (UserNotFoundException $e) {
            $this->view->renderHtml('errors/404.php', ['error' => $e->getMessage()], 404);
        } catch(IncorrectUserActivationCode $e) {
            $this->view->renderHtml('errors/404.php', ['error' => $e->getMessage()], 404);
        }
    }

Удаление производил сразу после активации пользователя при условии что активация произошла успешно:

public static function checkActivationCode(User $user, string $code): bool
    {
        $db = Db::getInstance();
        $result = $db->query(
            'SELECT * FROM ' . self::TABLE_NAME . ' WHERE user_id = :user_id AND code = :code',
            [
                'user_id' => $user->getId(),
                'code' => $code
            ]
        );

        if (!empty($result)) {
            $db->query(
                'DELETE FROM ' . self::TABLE_NAME . ' WHERE user_id = :user_id AND code = :code',
                [
                    'user_id' => $user->getId(),
                    'code' => $code
                ]
            );
        }
        return !empty($result);
    } 
ivashkevich 30.07.2019 в 19:47
            if ($isCodeValid) {
                $user->activate();

                echo 'OK!';
                return;
            } else {
                throw new IncorrectUserActivationCode('Некорректный код активации');
            }

Тут можно инвертировать условие, и код станет с меньшей вложенностью:

            if (!$isCodeValid) {
                throw new IncorrectUserActivationCode('Некорректный код активации');
            }
            $user->activate();
            echo 'OK!';
Iliusha99 30.07.2019 в 19:51

Почему сразу не подумал, спасибо))

ivashkevich 30.07.2019 в 19:52

Придет с практикой, не за что

Iliusha99 30.07.2019 в 19:42

Сразу подумал и сделал отдельный метод для удаления

public static function DeleteUserActivationCode(int $userId, string $activationCode): void
{
    $db = Db::getInstance();
    $db->query(
        'DELETE FROM ' . self::TABLE_NAME . ' WHERE user_id = :user_id AND code = :code',
        [
            'user_id' => $userId,
            'code' => $activationCode
        ]
    );
}

но не знал как будет правильнее, ведь здесь должны тоже получить какое нибудь булевое значение, и подумал что логичнее будет сразу же удалить если активация уже прошла, как прислал выше. Поступил ли я правильно? Если нет, хотелось бы совет в этом направлении. Спасибо))

ivashkevich 30.07.2019 в 19:49

Всё ок, удалять можно сразу после активации. А вот имена методов пишутся с маленькой буквы.

artemship 28.08.2019 в 13:14

UserActivationService.php:

    public static function deleteActivationCode(User $user): void
    {
        $db = Db::getInstance();
        $db->query('DELETE FROM ' . self::TABLE_NAME . ' WHERE user_id = :user_id',
            ['user_id' => $user->getId()]
        );
    }

UsersController.php:

    public function activate(int $userId, string $activationCode): void
    {
        try {
            $user = User::getById($userId);
            if ($user === null) {
                throw new UserActivationException('Пользователь не найден');
            }
            if ($user->isConfirmed()) {
                throw new UserActivationException('Пользователь уже активирован');
            }
            $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
            if (!$isCodeValid) {
                throw new UserActivationException('Неверный код активации');
            }
        } catch (UserActivationException $e) {
            $this->view->renderHtml('users/activationFailed.php', ['error' => $e->getMessage()]);
            return;
        }

        $user->activate();
        UserActivationService::deleteActivationCode($user);
        $this->view->renderHtml('users/activationSuccessful.php');
    }

Шаблон activationFailed.php:

<?php include __DIR__ . '/../header.php'; ?>
    <div style="text-align: center;">
        <h1>Не удалось активировать аккаунт</h1>
        <?= $error ?>
    </div>
<?php include __DIR__ . '/../footer.php'; ?>

Шаблон activationSuccessful.php:

<?php include __DIR__ . '/../header.php'; ?>
    <div style="text-align: center;">
        <h1>Активация аккаунта прошла успешно!</h1>
    </div>
<?php include __DIR__ . '/../footer.php'; ?>
ivashkevich 28.08.2019 в 17:55

Нет смысла бросать исключение и тут же его ловить в том же методе. В остальном всё ок

artemship 29.08.2019 в 11:11

Понял, спасибо большое! Вынес catch в index.php:

} catch (UserActivationException $e) {
    $view = new View(__DIR__ . '/../templates/users');
    $view->renderHtml('activation.php', ['error' => $e->getMessage()]);
}

Экшн activate приобрел следующий вид:

    public function activate(int $userId, string $activationCode): void
    {
        $user = User::getById($userId);
        if ($user === null) {
            throw new UserActivationException('Пользователь не найден');
        }
        if ($user->isConfirmed()) {
            throw new UserActivationException('Пользователь уже активирован');
        }
        $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
        if (!$isCodeValid) {
            throw new UserActivationException('Неверный код активации');
        }

        $user->activate();
        UserActivationService::deleteActivationCode($user);
        $this->view->renderHtml('users/activation.php');
    }

Ну и объединил в один шаблон activation.php:

<?php include __DIR__ . '/../header.php'; ?>
    <div style="text-align: center;">
        <?php if (!empty($error)): ?>
            <h1>Не удалось активировать аккаунт</h1>
            <?= $error ?>
        <?php else: ?>
            <h1>Активация аккаунта прошла успешно!</h1>
        <?php endif; ?>

    </div>
<?php include __DIR__ . '/../footer.php'; ?>
ivashkevich 30.08.2019 в 04:37

Нет. В index.html его выносить тоже не имело смысла. Такого рода исключения должны бросаться в слое модели и обрабатываться в слое контроллера. Но конкретно здесь не нужно вообще исключений, достаточно if-else использовать для рендеринга разных шаблонов.

artemship 03.09.2019 в 15:01

Ладно, спасибо за ответы! Видимо я не до конца понимаю когда надо использовать исключения, а когда нет. Позже попытаюсь разобраться)

ivashkevich 03.09.2019 в 18:54

Ну, в принципе, первый вариант допустим. Исключения бросаются в исключительных ситуациях - когда дальнейшее выполнение кода не имеет смысла.

jimholder37@gmail.com Patron 12.05.2020 в 00:21

Так допустим или недопустим?)

ivashkevich 12.05.2020 в 08:22

Допустим, хоть и коряв, потому что проще будет смотреться конструкция с if-else.

artemship 04.09.2019 в 17:29

Попробую еще раз) Убрал исключения, пользовался только if-else. Так норм?

UsersController.php:

public function activate(int $userId, string $activationCode): void
{
    $error = null;
    $user = User::getById($userId);

    if ($user === null) {
        $error = 'Пользователь не найден';
    } elseif ($user->isConfirmed()) {
        $error = 'Пользователь уже активирован';
    } elseif (!UserActivationService::checkActivationCode($user, $activationCode)) {
        $error = 'Неверный код активации';
    } else {
        $user->activate();
        UserActivationService::deleteActivationCode($user);
    }

    $this->view->renderHtml('users/activation.php', ['error' => $error]);
}

UserActivationService.php:

public static function deleteActivationCode(User $user): void
{
    $db = Db::getInstance();
    $db->query('DELETE FROM ' . self::TABLE_NAME . ' WHERE user_id = :user_id',
        ['user_id' => $user->getId()]
    );
}

Шаблон activation.php:

<?php include __DIR__ . '/../header.php'; ?>
    <div style="text-align: center;">
        <?php if (!empty($error)): ?>
            <h1>Не удалось активировать аккаунт</h1>
            <?= $error ?>
        <?php else: ?>
            <h1>Активация аккаунта прошла успешно!</h1>
        <?php endif; ?>
    </div>
<?php include __DIR__ . '/../footer.php'; ?>
ivashkevich 04.09.2019 в 21:24

Отлично)

XXX 09.01.2020 в 21:26

Добрый день. По технологии ORM мы не должны были создать класс users_activation_codes со свойствами как у таблици? Или если это не нужно, то не обязательно создавать класс под каждую таблицу в БД.

ivashkevich 11.01.2020 в 10:56

Можно было сделать и так. Но здесь была настолько простая бизнес-логика, что слой ORM не добавит простоты, а только всё усложнит.

andreskrip 14.02.2020 в 15:12

Спасибо за урок!

UsersController.php

    public function activate(int $userId, string $activationCode): void
    {
        try {
            $user = User::getById($userId);

            if ($user === null) {
                throw new ActivationException('Пользователь не найден');
            }
            if ($user->getConfirmed() === true) {
                throw new ActivationException('Пользователь уже подтвержден');
            }

            $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);

            if (!$isCodeValid) {
                throw new ActivationException('Неверный код активации');
            } else {
                $user->activate();
                $this->view->renderHtml('users/activationSuccessful.php');
                UserActivationService::deleteActivationCode($userId);
            }
        } catch (ActivationException $error) {
            $this->view->renderHtml('users/activationError.php', ['error' => $error->getMessage()]);
        }
    }

UserActivationService.php

    // удаление кода активации из бд
    public static function deleteActivationCode(int $userId): void
    {
        $db = Db::getInstance();
        $delete = $db->query(
            'DELETE FROM `' . self::TABLE_NAME . '` WHERE user_id = :user_id',
            [':user_id' => $userId]
        );
    }

templates/users/activationError.php

<?php include __DIR__ . '/../header.php'; ?>
    <div style="text-align: center;">
        <h1>Не удалось активировать учетную запись!</h1>
        <?= $error ?>
    </div>
<?php include __DIR__ . '/../footer.php'; ?>

templates/users/activationSuccessful.php

<?php include __DIR__ . '/../header.php'; ?>
    <div style="text-align: center;">
        <h1>Учетная запись успешно активирована!</h1>
    </div>
<?php include __DIR__ . '/../footer.php'; ?><?php
ivashkevich 18.02.2020 в 07:50
if ($user->getConfirmed() === true) {

Просто

if ($user->getConfirmed()) {

Не имеет смысла сравнивать true с true, чтобы получить true :)

if (!$isCodeValid) {
                throw new ActivationException('Неверный код активации');
            } else {
                $user->activate();
                $this->view->renderHtml('users/activationSuccessful.php');
                UserActivationService::deleteActivationCode($userId);
            }

Если уж бросили исключение, зачем тогда писать else? Код после выбрасывания исключения уже не будет дальше выполняться. А если исключение не возникло, то он просто будет выполняться строчка за строчкой и оборачивать его в блок else опять-таки не имеет смысла, только усложняет код за счёт лишней вложенности.

Не имеет смысла также бросать исключения в контроллере и тут же их ловить. Суть в том чтобы их бросать в одном слое, а ловить в другом.

babls2332@gmail.com 14.03.2020 в 19:55

UsersController.php


        public function activate(int $userId, string $activationCode)
    {
        $user = User::getById($userId);
        try {
            if (empty($user)) {
                throw new Exception('Пользователя не существует');
            }
            if ($user->getIsConfirmed() === 1) {
                throw new Exception('Пользователь уже авторизован');
            }
                $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
                if ($isCodeValid) {
                    $user->activate();
                    UserActivationService::deleteCode($userId);
                    $this->view->renderHtml('errors/success.php', ['msg' => 'Пользователь успешно подтверждён'], 200);
                } else {
                    throw new Exception('Не валидный код');
                }
            } catch (Exception $e) {
                $this->view->renderHtml('errors/somethingWrong.php', ['error' => $e->getMessage()] , 500);
        }
    }

UserActivationService.php

public static function deleteCode(int $userId) {
        $db = Db::getInstance();
        $db->query('DELETE FROM '.self::TABLE_NAME.' WHERE user_id = :userId',
        [
            'userId' => $userId
        ]
        );
    }

somethingWrong.php

Возникла ошибка: <?=$error?>

success.php

<?=$msg?>
ivashkevich 15.03.2020 в 07:13
if ($user->getIsConfirmed() === 1) {

метод следует назвать isConfirmed, он должен возвращать булево значение.

Писать блок try и прямо внутри него бросать исключения не имеет смысла. Их имеет смысл бросать и кидать в разных слоях приложения.

$this->view->renderHtml('errors/success.php', ['msg' => 'Пользователь успешно подтверждён'], 200);

Ошибка с названием success - это что-то невероятное :)
Для чего в шаблон передается это сообщение, если можно его просто в шаблоне захардкодить?
Следует создать в шаблонах папку activation, а в ней файл success.php. В нём прописать текст об успешной активации. При рендеринге передавать пустой массив с параметрами. По-хорошему метод renderHtml должен иметь второй и третий аргументы по-умолчанию: пустой массив и 200 соответственно.

В целом по логике неплохо.

babls2332@gmail.com 15.03.2020 в 10:30

Файл success.php должен быть в другой папке. Это моя не внимательность. А сообщение передаётся,потому что думал, что это страница успеха общая. Для всех случаев жизни. И чтоб можно было передать соответствующее сообщение

ivashkevich 15.03.2020 в 11:54

Нуу, в целом, не вижу препятствий сделать так.

OneMoreTime 19.03.2020 в 12:33

1.

return !empty($result);

Вместо такой формы, корректно ли будет использовать разжеванную, типа:

return !empty($result)? true : false;

?
А то первую как то неохотно мозг воспринимает..

2.
Из комментария, относительно размещения в коде исключений и места их отлова:

Такого рода исключения должны бросаться в слое модели и обрабатываться в слое контроллера. Но конкретно здесь не нужно вообще исключений, достаточно if-else использовать для рендеринга разных шаблонов.

Почему тогда в уроке про статьи в качестве примера, как раз так и было - ислючения бросали в контроллере, а ловили в индексе?

ivashkevich 19.03.2020 в 16:12
  1. Переучивайте свой мозг, значит) Вы написали по факту - если true, то true, а иначе false. Лишняя работа.
  2. Речь была именно о том, что не надо в одном и том же методе и кидать исключение, и тут же его ловить.
Dimitry 06.04.2020 в 07:37

//UsersController
<?php

namespace MyProject\Controllers;

use MyProject\View\View;
use MyProject\Models\Users\User;
use MyProject\Exceptions\InvalidArgumentException;
use MyProject\Models\Users\UserActivationService;
use MyProject\Services\EmailSender;

class UsersController
{
    /** @var View */
    private $view;

    public function __construct()
    {
        $this->view = new View(__DIR__ . '/../../../templates');
    }

    public function signUp()
    {
        if (!empty($_POST)) {
            try {
                $user = User::signUp($_POST);
            } catch (InvalidArgumentException $e) {
                $this->view->renderHtml('users/signUp.php', ['error' => $e->getMessage()]);
                return;
            }

            if ($user instanceof User) {
                $code = UserActivationService::createActivationCode($user);

                EmailSender::send($user, 'Активация', 'userActivation.php', [
                    'userId' => $user->getId(),
                    'code' => $code
                ]);

                $this->view->renderHtml('users/signUpSuccessful.php');
                return;
            }
        }

        $this->view->renderHtml('users/signUp.php');
    }
    public function activate(int $userId, string $activationCode)
    {
        $user = User::getById($userId);
        if($user===null){
            $this->view->renderHtml('mail/activationError.php', ['error'=>"Пользователь с таким именем не найден"]);
            return;
        }
        $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
        if ($isCodeValid) {
            $user->activate();
            $this->view->renderHtml('mail/activationSucsess.php');
        }else{
            $this->view->renderHtml('mail/activationError.php', ['error'=>"Неверный ключ активации"]);
        }
    }
}

Домашка

ivashkevich 06.04.2020 в 15:46

Красота)

Ed 17.04.2020 в 12:05

UsersController

public function activate(int $userId, string $activationCode)
  {
    try{
      $user = User::getById($userId);
      if($user === null) {
        throw new \MyProject\Exceptions\NotFoundException('Такого пользователя не существует');
      }
      $isCodeValid = UserActivationService::checkAutorisationCode($user, $activationCode);
      if($user->getIsConfirmed()) {
        throw new InvalidArgumentException('Пользователь уже активирован, ничего не могу поделать!');
      }
      if(!$user->getIsConfirmed() && !$isCodeValid) {
        throw new InvalidArgumentException('Пользователя действительно нужно активировать, но код активации подкачал! Попробуйте перейти по ссылке из сообщения, отправленного Вам на почту.');
      }
      if($isCodeValid){
        $user->activate();
        $this->view->renderHtml('users/activationSuccessful.php');
        UserActivationService::cleanCodeFromDb($userId);
      } else {
        throw new InvalidArgumentException('Данная ссылка неактивна. Вероятно, вы попали сюда случайно.');
      }
    } catch(\MyProject\Exceptions\NotFoundException $e) {
      $view = new \MyProject\View\View(__DIR__ . '/../../../templates/errors');
      $view->renderHtml('404.php', ['error' => $e->getMessage()], 404);
    } catch(\MyProject\Exceptions\InvalidArgumentException $e) {
      $view = new \MyProject\View\View(__DIR__ . '/../../../templates/errors');
      $view->renderHtml('404.php', ['error' => $e->getMessage()], 404);
    }
  }

activationSuccessful

<h1>Ну, добро пожаловать! Будем рады!</h1>
<a href="/">Вернуться на главную</a>
ivashkevich 17.04.2020 в 20:27

Нет смысла бросать исключения в том же слое, что и ловить их. Проще сразу рендерить шаблон с ошибкой.

Alexann Patron 18.04.2020 в 17:55

Файл UsersController.php:

...
public function activate(int $userId, string $activationCode)
    {
        try {
            $user = User::getById($userId);
            if ($user === null) {
                throw new ActivationException('Такого пользователя не существует');
            }

            if ($user->isConfirmed()) {
                throw new ActivationException('Пользователь уже активирован');
            }
            $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
            if (!$isCodeValid) {
                throw new ActivationException('Код подтверждения авторизации не верен.');
            }
            $user->activate();
            UserActivationService::deleteActivationCode($userId);
            $this->view->renderHtml('users/activationSuccessful.php',
                ['nick' => $user->getNickname()]);
        } catch (ActivationException $e) {
            $this->view->renderHtml('errors/activationError.php',
                ['error' => $e->getMessage()], 422);
        }
    }
...

Файл UserActivationService.php:

...
public static function deleteActivationCode(int $userId): void
    {
        $db = Db::getInstance();
        $db->query(
            'DELETE FROM ' . self::TABLE_NAME . ' WHERE user_id = :user_id',
            [
                'user_id' => $userId,
            ]
        );
    }
...

activationError.php:

<h2>Ошибка при активации пользователя</h2>
<?= $error ?>
<br><br>
<a href="/">Вернуться на главную страницу</a>

activationSeccessful.php:

<?php include __DIR__ . '/../header.php'; ?>
<div style="text-align: center;">
    <h2>Добро пожаловать <?= $nick ?>. Ваша учетная запись успешно активирована!</h2>
    <a href="/">Вернуться на главную страницу</a>
</div>
<?php include __DIR__ . '/../footer.php'; ?>

У меня вопрос:
Надо ли после удаления кода из базы сбрасывать счетчик - автоматический индекс?

ivashkevich 18.04.2020 в 20:18

Нет смысла бросать исключения в том же слое, что и ловить их. Проще сразу рендерить шаблон с ошибкой.

Надо ли после удаления кода из базы сбрасывать счетчик - автоматический индекс?

Нет, не рекомендую вообще когда-либо его трогать

Alexann Patron 18.04.2020 в 20:49

Значит так должно быть?
UsersController.php:

...
 public function activate(int $userId, string $activationCode)
    {
        $user = User::getById($userId);
        if ($user === null) {
            $this->view->renderHtml('errors/activationError.php',
                ['error' => 'Такого пользователя не существует'], 422);
            return;
        }
        if ($user->isConfirmed()) {
            $this->view->renderHtml('errors/activationError.php',
                ['error' => 'Пользователь уже активирован'], 422);
            return;
        }
        $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
        if (!$isCodeValid) {
            $this->view->renderHtml('errors/activationError.php',
                ['error' => 'Код подтверждения авторизации не верен'], 422);
            return;
        }
        $user->activate();
        UserActivationService::deleteActivationCode($userId);
        $this->view->renderHtml('users/activationSuccessful.php',
            ['nick' => $user->getNickname()]);
    }
...

Или все-таки с исключениями, только ловить их в index.php:

ivashkevich 19.04.2020 в 07:41

Ловить их в index.php, конечно. У вас же там уже есть обработка их даже.

Alexann Patron 19.04.2020 в 12:11

Исправил. UsersController.php:

...
 public function activate(int $userId, string $activationCode)
    {
        $user = User::getById($userId);
        if ($user === null) {
            throw new ActivationException('Такого пользователя не существует');
        }
        if ($user->isConfirmed()) {
            throw new ActivationException('Пользователь уже активирован');
        }
        $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
        if (!$isCodeValid) {
            throw new ActivationException('Код подтверждения авторизации не верен');
        }
        $user->activate();
        UserActivationService::deleteActivationCode($userId);
        $this->view->renderHtml('users/activationSuccessful.php',
            ['nick' => $user->getNickname()]);
    }
...

index.php:

...
catch (\MyProject\Exceptions\ActivationException $e) {
    $view = new \MyProject\View\View(__DIR__ . '/../templates/errors');
    $view->renderHtml('activationError.php', ['error' => $e->getMessage()], 422);
}
ivashkevich 19.04.2020 в 15:52

Отлично!

Fill Patron 07.05.2020 в 19:02

src/MyProject/Controllers/UserController.php

...
    public function activate(int $userId, string $activationCode)
    {
        $user = User::getById($userId);

        if ($user === null) {
            throw new ActivationException('Пользователя не существует');
        }

        if ($user->getIsConfirmed()) {
            throw new ActivationException('Пользователь уже активирован');
        }

        $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
        if (!$isCodeValid) {
            throw new ActivationException('Неверный код активации');
        }
        $user->activate();
        UserActivationService::deleteActivationCode($userId);
        $this->view->renderHtml('users/activationSuccessful.php', ['userName' => $user->getNickname()]);
    }
...

src/MyProject/Models/Users/UserActivationService.php

...
    public static function deleteActivationCode (int $userId): void
    {
        $db = Db::getInstance();
        $db->query('DELETE FROM ' .self::TABLE_NAME . ' WHERE user_id = :user_id', ['user_id' => $userId]);
    }
...

templates/errors/ActivationError.php

<h1>Ошибка активации!</h1>
<?= $error ?>

templates/users/activationSuccessful.php

<?php include __DIR__ . '/../header.php'; ?>
    <div style="text-align: center;">
        <h1>Пользователь <?= $userName?> успешно активирован</h1>
    </div>
<?php include __DIR__ . '/../footer.php'; ?>
ivashkevich 08.05.2020 в 20:17

getIsConfirmed() стоит переименовать в isConfirmed

Fill Patron 08.05.2020 в 20:42

Тогда придется поменять область видимости для isConfirmed в User.php, сейчас стоит protected.
Сделал по аналогии с другими геттерами в User.php.
Чем руководствоваться при определении области видимости для свойств?

ivashkevich 09.05.2020 в 06:24

Не не, метод с таким именем сделать. Свойство по-прежнему private. Методы, возвращающие булево значение стоит именовать с is... has... are и т.п.

Fill Patron 09.05.2020 в 07:03

Понятно, спасибо!

jimholder37@gmail.com Patron 12.05.2020 в 00:43
public function activate(int $userId, string $activationCode)
{
    $user = User::getById($userId);
    if ($user === null) {
        $this->renderActivate('Такой пользователь не существует');
        return;
    }

    if ( $user->getConfirmed() ) {
        $this->renderActivate('Подтверждение по email уже получено, учетная запить активирована');
        return;
    }

    $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);

    if (!$isCodeValid) {
        $this->renderActivate('Код подтверждения не совпадает, перейдите по ссылке из письма снова');
        return;
    }

    $user->activate();
    $this->renderActivate();
    UserActivationService::deleteActivationCode($user, $activationCode);
}

private function renderActivate($msg = '')
{
    $this->view->renderHtml('users/activation.php', [
        'error' => $msg
    ]);
}
ivashkevich 12.05.2020 в 08:22

Отлично!

OneMoreTime 19.05.2020 в 12:03

1.
почему шаблон для активации нового пользователя находится в отдельной папке mail? Это же относится к юзерам, можно было его в папку users положить.

2.
Для чего нужен отдельный класс UserActivationService? Нельзя методы этого класса было оформить в классе User? Это же так же относится к юзеру. При этом метод activate находится в классе User. Почему тогда и этот метод не в классе UserActivationService? Или как вариант - разместить в папке сервисов, раз в названии присутствует слово Service. Запутанной структура воспринимается...
Я так понимаю, сама структуризация в этом плане - субъективна?

3.
В классе EmailSender мы передаем в письме готовую страничку-шаблон с линком. А в чем преимущество/недостаток, да и вообще принципиальные отличия от варианта - передать в письме просто текст со ссылкой. Мне второй вариант кажется намного проще и компактнее. Недостатков в нем по сравнению с первым не вижу.


По домашке:
Оставил всю логику в классе UserActivationService:

/**
 * Class UserActivationService
 * @package MyProject\Models\Users
 */
class UserActivationService
{
    /**
     *
     */
    private const TABLE_NAME = 'users_activation_codes';

    /**
     * @param User $user
     * @return string
     * @throws DbException
     */
    public static function activationCodeCreate(User $user): string
    {
        $code = bin2hex(random_bytes(16));
        $db = Db::getDb();
        $sql = 'INSERT INTO `'.self::TABLE_NAME.'`  SET user_id=:user_id, code=:code;';
        $db->queryWithoutGettingData($sql, [':user_id' => $user->getId(), ':code' => $code]);
        return $code;
    }

    /**
     * @param int $userId
     * @param string $code
     * @return bool
     * @throws DbException
     */
    public static function activationCodeCheck(int $userId, string $code): bool
    {
        $db = Db::getDb();
        $sql = 'SELECT * FROM `' . self::TABLE_NAME . '`  WHERE user_id=:user_id AND code=:code;';
        $result = $db->queryWithGettingData($sql, [':user_id' => $userId, ':code' => $code]);
        return !empty($result);
    }

    /**
     * @param int $userId
     * @throws DbException
     */
    public static function activationCodeDelete(int $userId)
    {
        $db = Db::getDb();
        $sql = 'DELETE  FROM `' . self::TABLE_NAME . '`  WHERE user_id=:user_id;';
        $db->queryWithoutGettingData($sql, [':user_id' => $userId]);
    }

    /**
     * @param int $userId
     * @throws DbException
     */
    public static function activate(int $userId): void
    {
        $user = User::getById($userId);
        $user->setIsConfirmed(1);
        $user->save();
    }
}

Вызываю из метода activate в классе UsersController:

/**
     * @param int $userId
     * @param string $code
     * @throws DbException
     */
    public function activate(int $userId, string $code)
    {
        if (UserActivationService::activationCodeCheck($userId, $code)) {
            UserActivationService::activate($userId);
            $message = 'Your account is activated now';
            UserActivationService::activationCodeDelete($userId);
            $this->view->renderHtml('successful/successful.php', ['message' => $message]);
            return;
        } else {
            $message = 'Something wrong with activation data. Check your activation link.';
            $this->view->renderHtml('unsuccessful/unsuccessful.php', ['message' => $message]);
            return;
        }

    }

Шаблон самый простой, с возможностью дальнейшего расширения:

<?php include_once __DIR__.'/../header.php'; ?>
<?=$message?>
<?php include_once __DIR__.'/../footer.php'; ?>

Идентичный как для удачной, так и для неудачной активации.


Для выброса PDO исключений использую штатный механизм. DbExceptions выбрасываю на уровне запросов в БД в классе Db, а ловлю в index.php

Проверка на корректность активационных данных происходит при проверке кода и идентификатора юзера. Можно их конечно отдельно делать и выбрасывать исключения поотдельности, но смысла наверное в этом особого нет, пользователю вообще не нужно показывать какие конкретно данные активации не прошли. Пользователь же должен оперировать только на уровне готовой ссылки?

ivashkevich 19.05.2020 в 17:55
  1. Потому что это письмо, а не веб-страничка. Но если хотите, можете и там положить, или вообще отдельную папку mail_templates завести, ничего от этого не изменится особо.
  2. В ActiveRecord модель - это одна таблица. Чтобы в таблице юзеров не хранить токены был сделан отдельный сервис для работы с ними.
  3. По ДЗ - отлично

Пользователь же должен оперировать только на уровне готовой ссылки?

Да

OneMoreTime 19.05.2020 в 21:02

Потому что это письмо, а не веб-страничка. Но если хотите, можете и там положить, или вообще отдельную папку mail_templates завести, ничего от этого не изменится особо.

Но шаблон для письма формируется же в формате странички? Я в своем третьем вопросе как раз про формат передачи пользователю кода активации спрашивал. Можно же не создавать отдельный класс EmailSender, не создавать шаблон в письмо, а просто в юзер-контролере, если регистрация успешна, отправить ссылку активации на указанную почту? Отредактировал ваш пример класса регистрации из урока:

public function signUp()
{
    if (!empty($_POST)) {
        try {
            $user = User::signUp($_POST);
        } catch (InvalidArgumentException $e) {
            $this->view->renderHtml('users/signUp.php', ['error' => $e->getMessage()]);
            return;
        }

        if ($user instanceof User) {
            $code = UserActivationService::createActivationCode($user);

            $receiver = $user->getEmail();
            $subject = 'Activation on Blog';
            $link = 'http://myproject.loc/users/'.$user->getId().'/activate/'.$code;
            $body = 'To finish registration on Blog follow this link: '.$link;

            mail($receiver, $subject, $body);

            $this->view->renderHtml('users/signUpSuccessful.php');
            return;
        }
    }

    $this->view->renderHtml('users/signUp.php');
}

В чем может быть недостаток такого способа на данный момент, в будущем?

ivashkevich 20.05.2020 в 08:03

Потому что у более-менее серьезного письма всегда будет шаблон, в нем будет шапка и футер, которые тоже будут где-то храниться.
Когда логика отправки писем инкапсулирована внутри отдельного класса это даёт возможность изменять реализацию отправки. К примеру, захотите использовать PhpMailer, возьмете и поменяете вызов mail() в одном месте. А так придется по всему проекту менять. Более того, реализация отправки через phpMailer займёт далеко не одну строку, поэтому эту логику в дальнейшем всё равно придется выносить в отдельный класс, чтобы не было дублирования кода в 20 строк везде, где нужно отправить письмо.

titelivus 20.05.2020 в 18:09

UsersController.php

...
    public function activate(int $userId, string $activationCode)
    {
        try {
            $user = User::getById($userId);

            if ($user === null) {
                throw new UserActivationException('Пользователь не найден.');
            }

            if ($user->isConfirmed()) {
                throw new UserActivationException('Пользователь с таким email уже был подтвержден.');
            }

            $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);

            if (!$isCodeValid) {
                throw new UserActivationException('Неверный код активации!');
            }

            UserActivationService::deleteActivationCode($user);
            $user->activate();
            $this->view->renderHtml('users/userActivated.php');
            return;

        } catch (UserActivationException $e) {
            $this->view->renderHtml('users/userActivationFail.php', ['error' => $e->getMessage()]);
        }
    }
...
ivashkevich 20.05.2020 в 18:46
            if (!$user->getActivationStatus()) {
                throw new UserActivationException('Пользователь уже зарегистрирован.');
            }

что возвращает этот метод? какой тип данных?

текст ошибки не про то

titelivus 20.05.2020 в 19:23

Точно) исправил. Поменял название метода с getActivationStatus на isConfirmed так более понятно.
Просто я сомневался стоит ли его так называть это же геттер. Название isConfirmed здесь очень уместно подходит, сразу становится понятно что он возвращает булево значение, чего не скажешь про getActivationStatus не поймешь пока не увидишь)

ivashkevich 20.05.2020 в 19:50

Отличное название)

ivashkevich 20.05.2020 в 19:50

И в целом дз тоже)

studentDev Patron 21.05.2020 в 14:44
//Задание #1
public static function deleteActivationCode(User $user)
        {
            $db = Db::getInstance();
            $delete = $db->query(
                'DELETE FROM ' . static::TABLE_NAME . ' WHERE user_id = :user_id',
                [
                    'user_id' => $user->getId()
                ]
            );
        }
//Задание #2

 public function activate(int $userId, string $activationCode) //If click on URN
        {
                $user = User::getById($userId);

                //UserId not found
                if($user == null) {
                    try {
                        throw new IdNotFoundException('ID был не найден!', 404);
                    } catch (IdNotFoundException $e) {
                        $this->view->renderHtml('errors/idNotFound.php', ['error' => $e->getCode(), 'eMessage' => $e->getMessage()], 404);
                    }
                    return;
                }

                $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode); //VALUE: !empty($result) | $result this array;
                if ($isCodeValid) {
                    $user->activate();
                    echo 'OK';
                }

                //Code not found
                if(!$isCodeValid) {
                    try {
                        throw new NotFoundException('Неверный код активации или он был использован :(', 404);
                     } catch (NotFoundException $e) {
                        $this->view->renderHtml('errors/codeNotFound.php', ['error' => $e->getCode(), 'eMessage' => $e->getMessage()], 404);
                    }
                    return;
                }

            UserActivationService::deleteActivationCode($user);
        }
    }

В принципе работает :/
?

ivashkevich 22.05.2020 в 07:59
                if($user == false) {

Используйте строгое сравнение с null. Именно это значение лежит в переменной, когда пользователь неавторизован.

                    try {
                        throw new IdNotFoundException('ID был не найден!', 404);
                    } catch (IdNotFoundException $e) {
                        $this->view->renderHtml('errors/idNotFound.php', ['error' => $e->getCode(), 'eMessage' => $e->getMessage()], 404);
                    }

Для чего всё это?

if($isCodeValid == false)

Во-первых, вместо этого стоит писать

if(!$isCodeValid)

Во-вторых, вы уже выше проверили кейс когда код валидный. Там можно сразу удалить его и сделать return, а здесь уже не делать никаких условий.

С темой исключений у вас проблема. Рекомендую открывать урок по ним и перечитать еще пару раз. И начиная с того урока смотреть все комментарии, изучать другие решения учеников. С текущим уровнем понимания исключений дальше учиться нет смысла.

studentDev Patron 22.05.2020 в 11:06

Хорошо, понял. Согласен с исключением у меня проблемы(
Буду перечитывать исключение...

ivashkevich 22.05.2020 в 18:56

Ничего страшного, шаг назад, чтобы потом сделать два вперед

studentDev Patron 30.05.2020 в 11:08
public function activate(int $userId, string $activationCode)
        {
               try {
                   $user = User::getById($userId);

                   if(!$user) {
                      throw new IdNotFoundException('ID не найден!', 400);
                   };

                   $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
                   if ($isCodeValid) {
                       $user->activate();
                       echo 'OK';
                   }

                   UserActivationService::deleteActivationCode($user);
               } catch (IdNotFoundException $e) {
                   $this->view->renderHtml('errors/idNotFound.php', ['eMessage' => $e->getMessage(), 'error' => $e->getCode()], $e->getCode());
               }
        }
    }
  • Обновил
ivashkevich 01.06.2020 в 07:28
                   if($user == null) throw new IdNotFoundException('ID не найден!', 404);

Всегда используйте фигурные скобки для блоков if-else.

                   if($isCodeValid == null) throw new CodeNotFoundException('Код не действителен!', 404);

аналогично

Почему 404 код?

И почему сравниваете с null? Там ведь булево значение возвращается.

И опять бросаете исключение и ловите в том же слое. Я уже несколько раз писал вам об этом.

e.p.afonchenko@gmail.com 30.05.2020 в 21:42

UsersController.php

public function activate(int $userId, string $activationCode)
    {
        try {
            $user = User::getById($userId);

            if(empty($user)) {
                throw new DbException('Пользователь не найден.');
            }

            if($user->getIsConfirmed()) {
                throw new InvalidArgumentException();
            }

                if (UserActivationService::checkActivationCode($user, $activationCode)) {
                    $user->activate();
                    UserActivationService::removeActivationCode($user, $activationCode);
                    $this->view->renderHtml('/Users/ActivationSuccess.php');
                }
        }
        catch (DbException $e) {
            $this->view->renderHtml('/Users/ActivationError.php', ['error' => $e->getMessage()]);
        }
        catch (InvalidArgumentException $e) {
            $this->view->renderHtml('/Users/ActivationAlready.php');
        }
    }

UserActivationService.php

public static function checkActivationCode(User $user, string $code): bool
    {
        $db = Db::getInstance();
        $result = $db->query(
            'select * from ' . self::TABLE_NAME . ' where user_id = :user_id and code = :code;',
            [':user_id' => $user->getId(), ':code' => $code]
        );

        if (empty($result)) {
            throw new DbException('Неверный код активации.');
        }
        return true;
    }

Шаблоны: ActivationSuccess.php

<?php include __DIR__.'/../header.php' ?>
Поздравляем! Все прошло успешно, ваш email подтвержден.
<br>
<a href="/">На главную.</a>.
<?php include __DIR__.'/../footer.php' ?>

ActivationError.php

<?php include __DIR__.'/../header.php' ?>
<?= $error ?> Проверьте правильность ссылки для активации.
<?php include __DIR__.'/../footer.php' ?>

ActivationAlready.php

<?php include __DIR__.'/../header.php' ?>
Пользователь уже активирован.
<?php include __DIR__.'/../footer.php' ?>
ivashkevich 01.06.2020 в 07:36

С исключениями полная ерунда получилась. Смысл бросать их и ловить в одном и том же слое?

                throw new DbException('Пользователь не найден.');

При чем тут Db? Это ошибка базы?

                throw new InvalidArgumentException();

Где контекст? Как понять, что произошло?

HardBass Patron 02.07.2020 в 09:39

UserActivationService.php

...
    public static function deleteActivationCode(User $user, string $code)
    {
        $db = Db::getInstance();
        $db->query(
            'DELETE FROM ' . self::TABLE_NAME . ' WHERE `user_id` = :user_id AND `code` = :code;',
            [
                ':user_id' => $user->getId(),
                ':code' => $code
            ]
        );
    }
...

UsersController

...
public function activate(int $userId, string $activationCode)
    {
        $user = User::getById($userId);
        if ($user === null){ //если пользователя нет - исключение
            throw new \MyProject\Exceptions\UserNotFoundException('Такого пользователя не существет!');
        }
        $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);
        $isConfirmed = $user->getIsConfirmed();
        if ($isCodeValid) {
            $user->activate();
            echo 'OK!';
        }
        elseif ($isConfirmed == 1){
            throw new \Myproject\Exceptions\CodeNotFoundException('Пользователь уже активирован!');
        }
        else{
            throw new \Myproject\Exceptions\CodeNotFoundException('Код активации не найден!');
        }

        UserActivationService::deleteActivationCode($user, $activationCode);
    }
...

index.php

...
catch (\MyProject\Exceptions\UserNotFoundException $e) {
    $view = new \MyProject\View\View(__DIR__ . '/../templates/errors');
    $view->renderHtml('ActivationError.php', ['error' => $e->getMessage()], 404);
}
catch (\MyProject\Exceptions\CodeNotFoundException $e) {
    $view = new \MyProject\View\View(__DIR__ . '/../templates/errors');
    $view->renderHtml('ActivationError.php', ['error' => $e->getMessage()], 404);
}
...

ActivationError.php

...
<h1>Хьюстон, у нас очень большая проблема!</h1>
<?= $error ?>
...

P.S.
Возникла проблемы с отправкой письма на e-mail. В чем именно проблема разобраться не могу - настройки smtp менял, настройки безопасности аккаунта менял, но письмо как не приходило, так и не приходит. Пробовал ставить разные порты, создавать разные почты, гуглить разумеется, ответа пока не нашел.

ivashkevich 03.07.2020 в 09:07
        if ($user === null){ //если пользователя нет - исключение
            throw new \MyProject\Exceptions\UserNotFoundException('Такого пользователя не существет!');
        }

От подобных комментариев пользы нет. Это и так понятно.

Проблема с форматированием. Делайте отступы и переносы как в уроках. Для этого в шторме можно нажать Ctrl+Alt+L

tsaruk4356@gmail.com Patron 06.07.2020 в 13:55

UserActivationService

public static function deleteActivationCode(User $user, string $code)
    {
        $db = Db::getInstance();
        $db->query(
            'DELETE FROM ' . self::TABLE_NAME . ' WHERE user_id = :user_id AND code = :code',
            [
                'user_id' => $user->getId(),
                'code' => $code
            ]
        );
    }

UsersController

public function activate(int $userId, string $activationCode)
    {
        try {
            $user = User::getById($userId);

            if ($user === null) {
                throw new ActivationException('Такого пользователя не существует');
            }

            $isCodeValid = UserActivationService::checkActivationCode($user, $activationCode);

            if (!$isCodeValid) {
                throw new ActivationException('Неверный код активации');
            }

            if ($isCodeValid) {
                $user->activate();
                $this->view->renderHtml('users/successfulActivation.php');
                UserActivationService::deleteActivationCode($user, $activationCode);
                return;
            }
        } catch (ActivationException $e) {
            $this->view->renderHtml('users/activationFailed.php', ['error' => $e->getMessage()]);
        }
    }

successfulActivation

<?php include __DIR__ . '/../header.php'; ?>
<div style="text-align: center;">
    <h1>Активация прошла успешно!</h1>
</div>
<?php include __DIR__ . '/../footer.php'; ?>

activationFailed

<?php include __DIR__ . '/../header.php'; ?>
<div style="text-align: center;">
    <h1>Не удалось успешно закончить активацию</h1>
    <br>
    <?= $error ?>
</div>
<?php include __DIR__ . '/../footer.php'; ?>
ivashkevich 06.07.2020 в 14:58

Отлично

Логические задачи с собеседований