Sprint 6/step 1 by dlart · Pull Request #8 · dlart/react-burger

dlart · 2023-06-23T11:52:21Z

No description provided.

gennady-bars

Здравствуйте. (Нужно развернуть общий комментарий ↓)

Посмотрите на гитхабе все комментарии к коду (нужно прокрутить вниз страницу там)

Работа проделана огромная:

Проект задеплоен
Все юнит-тесты (48 шт) проходят успешно
Все cypress-тесты проходят успешно
Код тестов аккуратный и понятный

но есть некоторые недочеты:

constants.js, user.js, order.js, ingredients.js` нужно типизировать
Заказ собран, но кнопка неактивная https://disk.yandex.ru/i/11bycBRldzyJVQ . Неавторизованный пользователь не должен отправлять заказ на сервер. При клике на кнопку его нужно переводить на логин и после удачного логина обратно к заказу.
Скрин https://disk.yandex.ru/i/Ohhav3hkFoM1Fg По заданию нужно сделать логику, при которой бы показывался попап с деталями заказа, если кликнуть по заказу, но если скопировать ссылку на заказ при открытом попапе и вставить в окно браузера, то должна уже показываться страница с этими деталями.
Если перезагрузить страницу с открытым попапом заказа истории, то этот открытый попап опять должен появляться (или страница с деталями). А сейчас ничего не отображается после перезагрузки.
Если пользователь не авторизован и пытается попасть на защищённый маршрут — его переадресовывает на маршрут /login. После авторизации пользователь автоматически переадресовывается на тот маршрут, к которому у него не получилось получить доступ ранее.
initialState не нужно создавать в тестах опять. Нужно импортировать initialState из файла редьюсера. Там уже создан начальный стейт для каждого редьюсера
ProtectedRoute должен использовать Navigate с параметрами для универсального возращения пользователя обратно туда, куда он хотел попасть.
http://localhost:3000 будет повторяться несколько раз в коде тестов. Нужно вынести в константу testUrl, чтобы можно было одним движением изменить урл для тестов
А можно в документации посмотреть, как настроить быстро baseUrl для cypress https://docs.cypress.io/api/commands/visit#Visit-is-automatically-prefixed-with-baseUrl
[data-testid="643d69a5c3f7b9001cfa093c"] дублируется 3 раза
Если строка-селектор повторяется более 2х раз в коде, значит, этот селектор нужно выносить в константу, чтобы можно было одним движением изменить его и не искать по всему коду дублирования этого селектора.

Можно лучше

Желательно очищать конструктор после успешного получения номера заказа с сервера в блоке then, чтобы пользователь мог следующий заказ сделать, не удаляя старые ингредиенты
лучше отображать лоадер (загрузку) при ожидании ответа от сервера с номером заказа
при клике на логотип обычно идет переход на главную страницу

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

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

Удачного рефакторинга кода.

gennady-bars · 2023-06-23T12:14:20Z

+import reducer, { decreaseCount, increaseCount, resetCount } from './ingredients'
+
+describe('ingredients test', () => {
+  let initialState = {


initialState не нужно создавать в тестах опять. Нужно импортировать initialState из файла редьюсера. Там уже создан начальный стейт для каждого редьюсера

gennady-bars · 2023-06-23T12:15:03Z

+      modalOpen: true,
+    });
+  });
+});


Можно лучше

Можно сделать универсальный кастомный хук для контроля любого количества инпутов в любых формах:

export function useForm(inputValues={}) { const [values, setValues] = useState(inputValues); const handleChange = (event) => { const {value, name} = event.target; setValues({...values, [name]: value}); }; return {values, handleChange, setValues}; }

Этот код помещают в отдельный файл useForm.js в папке hooks и импортируют функцию туда, где нужно контролировать инпуты

И Вам не нужно будет теперь вручную создавать функции обработки инпутов и т д. Все будет в одной строчке кода:

const {values, handleChange, setValues} = useForm({});

gennady-bars · 2023-06-23T12:16:11Z

+      modalOpen: true,
+    });
+  });
+});


if (!isLoggedIn) { return <Navigate to="/login" replace />; }

ProtectedRoute должен использовать Navigate с параметрами для универсального возращения пользователя обратно туда, куда он хотел попасть.

return <Navigate to="/login" state={{ from: location}}/>;

gennady-bars · 2023-06-23T12:16:29Z

+      modalOpen: true,
+    });
+  });
+});


Можно лучше

Если интересно, то можно сделать вот так универсальный компонент ProtectedRoute с переадресацией

export default function ProtectedRoute({ children, anonymous = false }) { const isLoggedIn = useSelector((store) => store.user.isLoggedIn); const location = useLocation(); const from = location.state?.from || '/'; // Если разрешен неавторизованный доступ, а пользователь авторизован... if (anonymous && isLoggedIn) { // ...то отправляем его на предыдущую страницу return <Navigate to={ from } />; } // Если требуется авторизация, а пользователь не авторизован... if (!anonymous && !isLoggedIn) { // ...то отправляем его на страницу логин return <Navigate to="/login" state={{ from: location}}/>; } // Если все ок, то рендерим внутреннее содержимое return children; }

gennady-bars · 2023-06-23T12:17:52Z

+      JSON.stringify('refreshToken')
+    );
+
+    cy.visit('http://localhost:3000/');


http://localhost:3000 будет повторяться несколько раз в коде тестов. Нужно вынести в константу testUrl, чтобы можно было одним движением изменить урл для тестов
А можно в документации посмотреть, как настроить быстро baseUrl для cypress https://docs.cypress.io/api/commands/visit#Visit-is-automatically-prefixed-with-baseUrl

Ок. Настроил в конфигурации Cypress

gennady-bars · 2023-06-23T12:18:16Z

+  });
+
+  it('should handle ingredient modal', () => {
+    cy.get('[data-testid="643d69a5c3f7b9001cfa093c"]').click();


[data-testid="643d69a5c3f7b9001cfa093c"] дублируется 3 раза

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

Вроде все повторяющиеся вынес в константы

gennady-bars · 2023-06-23T12:18:55Z

+
+    cy.get('[data-testid="643d69a5c3f7b9001cfa0941"]').trigger('dragstart');
+
+    cy.get('[data-testid="ingredient-drop-target"]').trigger('drop');


[data-testid="ingredient-drop-target"] тоже 3 раза

и так далее

dlart · 2023-06-23T13:17:47Z

"constants.js, user.js, order.js, ingredients.js` нужно типизировать" - ок
"Заказ собран, но кнопка неактивная https://disk.yandex.ru/i/11bycBRldzyJVQ . Неавторизованный пользователь не должен отправлять заказ на сервер. При клике на кнопку его нужно переводить на логин и после удачного логина обратно к заказу." - проверил, кнопка неактивна если пользователь неавторизован и активна если авторизован, тут непонятно..
"initialState не нужно создавать в тестах опять. Нужно импортировать initialState из файла редьюсера. Там уже создан начальный стейт для каждого редьюсера" -Ок
ProtectedRoute должен использовать Navigate с параметрами для универсального возращения пользователя обратно туда, куда он хотел попасть. - Ок
"http://localhost:3000 будет повторяться несколько раз в коде тестов. Нужно вынести в константу testUrl, чтобы можно было одним движением изменить урл для тестов" - Ок
"[data-testid="643d69a5c3f7b9001cfa093c"] дублируется 3 раза
Если строка-селектор повторяется более 2х раз в коде, значит, этот селектор нужно выносить в константу, чтобы можно было одним движением изменить его и не искать по всему коду дублирования этого селектора." - Ок

gennady-bars

Поздравляю! Ваша работа принята.

Вы отлично потрудились.

Удачи в Вашей новой профессии.

Несколько советов для дальнейшего развития:

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

https://www.youtube.com/channel/UCZeU17nbVfzczAkJVTay9vw
(ссылки на каналы могут быть недоступными сейчас. Попробуйте через vpn)

https://soundcloud.com/everyonecan

https://soundcloud.com/podlodka

https://soundcloud.com/devschacht

https://soundcloud.com/begebot

https://soundcloud.com/web-standards

dlart added 2 commits June 23, 2023 14:26

sprint-6/step-1

fd4675f

sprint-6/step-1

bfca0aa

gennady-bars reviewed Jun 23, 2023

View reviewed changes

sprint-6/step-1

d8d1e7e

gennady-bars approved these changes Jun 23, 2023

View reviewed changes


		cy.get('[data-testid="643d69a5c3f7b9001cfa0941"]').trigger('dragstart');

		cy.get('[data-testid="ingredient-drop-target"]').trigger('drop');

Conversation

dlart commented Jun 23, 2023

Uh oh!

gennady-bars left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dlart commented Jun 23, 2023

Uh oh!

gennady-bars left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants