Skip to content

DONT MERGE! Code review #186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .env.development
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,8 @@ VITE_LOGIN_URI=/login
VITE_LOGOUT_URI=/logout
VITE_AVATAR_UPLOAD_URI=/api/v1/upload/avatar
VITE_AVATAR_DELETE_URI=/api/v1/avatar
# уберите это, это оч вряд ли поменяется, а если поменяется - то нужно переделать больше чем переменные
# лишние переменные, захардкодить


VITE_APP_ENDPOINT="http://app-stage.qa.guru:8080"
1 change: 1 addition & 0 deletions src/config.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// про избыточные переменные окружения - слишком много кода это рождает
export const GRAPHQL_URI = import.meta.env.VITE_GRAPHQL_URI;
export const LOGIN_URI = import.meta.env.VITE_LOGIN_URI;
export const LOGOUT_URI = import.meta.env.VITE_LOGOUT_URI;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
import { FC } from "react";

import { IAnswerCommentContainer } from "./answer-comment-container.types";
// уберите релатив импорты начинающиеся с ../ - это плоххая практика
import AnswerComment from "../../views/answer-comment";

type AnswerCommentItem = {
Expand All @@ -25,6 +26,17 @@ const AnswerCommentContainer: FC<IAnswerCommentContainer> = (props) => {
fragment: SubCommentHomeWorkDtoFragmentDoc,
});

// можно, но пожалуйста, переделайте на state
// так бизнеслогику в слой кэширования запихали, еще
// и сравнение по рефам внутри этого. сделайте плз класс типа
// class CommentsService {
// comments: any[],
// loading: boolean
//
// async fetch() {
// ...
// }
// }
cache.modify({
fields: {
commentsHomeWorkByHomeWork(existingComments = { items: [] }) {
Expand Down
2 changes: 2 additions & 0 deletions src/features/training-lectures/constants/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
// вам норм такие конструкции писать?) убираем папку и файл index.ts -
// ничего не меняется. Тогда ради чего модуль?
export { INDEX_OFFSET, ROUTES } from "./constants";
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { INDEX_OFFSET } from "../../constants";
const TrainingLectures: FC<ITrainingLectures> = (props) => {
const { dataTrainingLectures, trainingId, dataTraining } = props;
const { trainingLectures } = dataTrainingLectures;
// передавайте во вьюххи дату уже выковыряную из объекта квери
const name = dataTraining?.training?.name;

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const TrainingCalendarContainer: React.FC = () => {

useEffect(() => {
setData({
// эээ, что это за захардкоженые даты?
classes: [
"2023-12-07",
"2023-12-09",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ const TrainingPurchases: FC<ITrainings> = ({ data }) => {
}
};

// очень очень странное место. не useEffect, так еще и динамический массив Ref
// ... что? обсудить отдельно. очень сложная конструкция
trainingPurchases?.forEach((item) => {
const id = item?.trainingTariff.training?.id;
if (id !== undefined) {
Expand All @@ -56,6 +58,10 @@ const TrainingPurchases: FC<ITrainings> = ({ data }) => {
<Container>
<Typography variant="h2">Мои курсы</Typography>
<StyledGrid container spacing="30px">
{/*
дело вкуса, но я бы этот мап написал так
trainingPurchases?.map(({trainingTariff: {training: { id, name }}}) => <>other</>
*/}
{trainingPurchases?.map((item) => {
const { id, name } = item?.trainingTariff.training || {};

Expand Down
12 changes: 6 additions & 6 deletions src/features/user-detail/views/user-detail/user-detail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import { FC } from "react";
import { Box, Container, Stack, Typography } from "@mui/material";
import useRatingColor from "shared/hooks/use-rating-color";
import { formatDate } from "shared/helpers";
import { ReactComponent as WorkIcon } from "assets/icons/work-field.svg";
import { ReactComponent as StackOverflowIcon } from "assets/icons/stack-overflow.svg";
import { ReactComponent as GitIcon } from "assets/icons/git-hub.svg";
import { ReactComponent as LinkedIn } from "assets/icons/linked-in.svg";
import { ReactComponent as Telegram } from "assets/icons/telegram.svg";
import { ReactComponent as WebSiteIcon } from "assets/icons/website.svg";
import WorkIcon from "assets/icons/work-field.svg";
import StackOverflowIcon from "assets/icons/stack-overflow.svg";
import GitIcon from "assets/icons/git-hub.svg";
import LinkedIn from "assets/icons/linked-in.svg";
import Telegram from "assets/icons/telegram.svg";
import WebSiteIcon from "assets/icons/website.svg";
import AvatarCustom from "shared/components/avatar-custom";

import {
Expand Down
2 changes: 2 additions & 0 deletions src/i18n/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ const DETECTION_OPTIONS = {
checkWhitelist: true,
};

// Q: зачем вам интернационализация?
// A: выпилить и писать пока все по русски, запуститесь сначала полностью как продукт
use(initReactI18next)
.use(LanguageDetector)
.init({
Expand Down
6 changes: 6 additions & 0 deletions src/pages/auth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,9 @@ export { default as SignUpPage } from "./sign-up";
export { default as ResetPage } from "./reset";
export { default as SetPasswordPage } from "./set-password";
export { default as ConfirmTokenPage } from "./confirm-token";
// причина по которой export default плохо


// а еще, вся эта папка pages - бойлерплейтный однотипный код. не нужно
// нужно то что у вас называется container (в основном это все таки page)
// класть прям сюда и импортировать хуки и view из features
2 changes: 2 additions & 0 deletions src/pages/auth/set-password.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,6 @@ const SetPasswordPage: FC = () => {
return <SetNewPassword />;
};

// export default запрещаем на уровне линтера, оставляем только экспорты вида
// export {}, export const и etc.
export default SetPasswordPage;
2 changes: 1 addition & 1 deletion src/routes/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ interface IRoutnig {
roles?: Maybe<Maybe<UserRole>[]>;
}

const ProtectedRoute: FC<IProtectedRoute> = ({ children }) => {
const ProtectedRoute: FC<IProtectedRoute> = ({ children }) => {
const { isAuth } = useAuth();

if (isAuth) {
Expand Down