rolling-scopes-school / support

15 stars 3 forks source link

Sprint 2: Login, Registration, and Main Pages Implementation - Volasau #921

Closed Volasau closed 1 year ago

Volasau commented 1 year ago
  1. A link to your deployed project
  2. A link to the project repository on Github.
  3. A link to the task
  4. A Link to the checklist for evaluating the task (if it exists)
  5. A screenshot of cross-checking marks The work was not evaluated due to late submission
  6. A final score after self-assessment, with comments Score: 300 / 315 Не выполнено: ❌ (15 points) Implement error handling for failed registration attempts, and display user-friendly error messages. RSS-ECOMM-2_12
  7. A cross-check score of your Score 0
CoracaoDoMundo commented 1 year ago

Review started.

CoracaoDoMundo commented 1 year ago

Общий балл по работе - 255 баллов. Подробнее комментарии по проверке ниже. Все скрины по проверке выложены по ссылке - https://drive.google.com/drive/folders/1MeNR7yx8QDPei6iuo9dODoZCwn3euhU0?usp=sharing

Login Page Implementation

  1. Input Validation 30/40 Поле email валидируется не корректно - есть возможность ввести email с отсутствующей локальной частью, см.скрин по ссылке -5 баллов. В остальном по правилам ТЗ валидация электронной почты корректная, но нужно учитывать, что домены верхнего уровня не могут содержать спец.символы, а у вас в работе они проходят. Вариант пароля, удовлетворяющего всем требованиям валидации не проходит валидацию (скрин также по ссылке) -5 баллов. Тут стоит отметить, что вы, видимо, приняли за специальные символы только то, что указано в ТЗ в скобках, но там указано e.g., что значит например. И почему в текущей реализации "!" является специальным символом, а "?" им не является - не совсем понятно. Если у вас есть какие-то конкретные специальные символы, которые должны использоваться в пароле, а не все возможные специальные символы, то это стоит указывать в сообщении-подсказке или в сообщении, которое предупреждает о некорректно введенном значении поля. У вас этого нет, поэтому правило по специальным символам по сути не выполнено (хотя оно, к слову, было не обязательным). Также нюанс - когда после некорректных данных, отправленных на сервер, вбиваю корректный email, поле продолжает быть подсвечено красным - с т.з. UI это плохая практика. Скрин выложила там же по ссылке.

  2. Integration with Authentication Service 45/45 Сообщения, честно говоря, сложно назвать user-friendly, но за это ничего не снимаю.

  3. Redirection 30/30.

  4. Handle Authentication Token 0/10. У меня были данные для логина (скрин по ссылке выше - registration data), я под ними зашла. Потом вышла и попробовала зайти с другим email (см.также скрин во вложении, repeat login), но тем же паролем, что использовала при регистрации и система меня впустила... это не корректное поведение. Судя по всему токен прошлого входа от CommerceTools сохраняется и/или обрабатывается не корректно -10 баллов.

  5. Navigation to Registration Page 5/5.

Registration Page Implementation

Это не из чек-листа, но галочка в форме регистрации должна быть подписана Use as shipping address, т.к. billing вы добавляете по умолчанию и есть возможность использовать его и как адрес доставки. А в текущей формулировке получается, что адрес для счетов предлагается собственно и использовать как адрес для счетов.

  1. Input Validation 20/45 У валидации email и пароля те же замечания, что в форме логина, но поскольку по специальным символам вопрос спорный, то повторно за это не снимаю, тут снятие только за валидацию email, т.к. она однозначно не корректная -5 баллов. В инпуты почти всех полей можно проставить пробелы или спец.символы и пройти регистрацию. По полям имя, фамилия, улица, город это не корректная валидация (см.скрины по ссылке выше) -20 баллов.

  2. Integration with Authentication Service 10/25. Не реализована ошибка входа при прохождении валидации на стороне клиента. Например, есть возможность зарегистрироваться с одним и тем же email повторно - 15 баллов. При регистрации происходит 3 или 4 запроса к серверу и получается 3 или 4 ответа, в результате получаем 3-4 уведомления о логине и т.п. По ссылке скрин с логами из network и отображением всех уведомлений о входе в систему, где виден обмен данными по одному запросу о регистрации. Баллы за это не снимаю, т.к. интеграция с CommetceTools настроена, но отрабатывает она не корректно.

  3. State Management, Automatic Login, and Redirection 15/15.

  4. Integration with commercetools for User Profiles and Addresses 30/30.

  5. Navigation to Login Page 5/5.

Main Page Enhancements

  1. Centralized Navigation 10/10.

  2. Routing Implementation 30/30.

  3. Evaluation Criteria for Header 25/25.

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

CoracaoDoMundo commented 1 year ago

@valerydluski done! 🖖🏻

RukalaRukala commented 1 year ago

Спасибо большое за проверку и подробные комментарии и рекомендации!

Azimkhan93 commented 1 year ago

Благодарю за проверку!

Volasau commented 1 year ago

Спасибо за проверку нашего пропущеного спринта. Спасибо за подробный комментарий и рекомендации.