Skip to content
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

#321 search page bug #322

Merged
merged 7 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from 6 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
Binary file not shown.
4 changes: 1 addition & 3 deletions FrontEnd/src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import AdminPage from './components/adminPage/AdminPage';
import BasicPage from './components/basicPage/BasicPage';
import { AuthContext } from './context';
import { useProvideAuth } from './hooks';
import { Search } from './components/SearchPage/Search';

function App() {
const auth = useProvideAuth();
Expand All @@ -16,8 +15,7 @@ function App() {
<div className="App">
<Routes>
<Route path="/*" element={<BasicPage />} />
<Route path="/admin/*" element={<AdminPage />} />
<Route path="/search" element={<Search isAuthorized={auth} />} />
<Route path="/customadmin/*" element={<AdminPage />} />
</Routes>
</div>
</BrowserRouter>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ const CompanyCard = ({ companyData, isAuthorized }) => {
}

setUsersSavedList(NewList);
if (companyData.id == isAuthorized.UserId) {
setStar(false);
setIsSaved(false);
setSearchPerformed(true);
} else {
if (usersSavedList.includes(companyData.id)) {
setStar(filledStar);
setIsSaved(true);
Expand All @@ -64,7 +69,7 @@ const CompanyCard = ({ companyData, isAuthorized }) => {
setStar(outlinedStar);
}
setSearchPerformed(true);
}
}}

const { trigger } = useSWRMutation(
`${process.env.REACT_APP_BASE_API_URL}/api/saved-list/`,
Expand Down Expand Up @@ -120,11 +125,15 @@ const CompanyCard = ({ companyData, isAuthorized }) => {
<div className={styles['company-card']}>
<div className={styles['company-card__block']}>
<div className={styles['company-card__image-frame']}>
<img
{companyData.banner_image ? (
<img src={companyData.banner_image} alt="Company Banner" />
) : (
<img
className={styles['company-card__image']}
src={`${process.env.REACT_APP_PUBLIC_URL}/companies-logos/defaultcompanybanner.png`}
src={`${process.env.REACT_APP_PUBLIC_URL}/svg/profile-view-image-empty.svg`}
alt={companyData.name}
/>
)}
</div>
<div className={styles['company-card__text-block']}>
<div className={styles['company-card__text-block__header']}>
Expand All @@ -137,7 +146,7 @@ const CompanyCard = ({ companyData, isAuthorized }) => {
<div className={styles['company-card__name-text']}>
<Link
className={styles['company-card__name-text_link']}
to={`/profile/${companyData.id}`}
to={`/profile-detail/${companyData.id}`}
>
{companyData.name}
</Link>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@
height: 220px;
}
.company-card__image {
width: 360px;
height: 290px;
width: 103px;
height: 80px;
flex-shrink: 0;
border-radius: 11px;
margin-top: 80px;
margin-left: 125px;
object-fit: cover;
opacity: 0.7;
}
Expand Down
2 changes: 1 addition & 1 deletion FrontEnd/src/components/adminPage/header/Header.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ function Header () {
return (
<header>
<div className={css['header-content']}>
<Link className={css['header-logo__text']} to="/admin/">admin panel</Link>
<Link className={css['header-logo__text']} to="/customadmin/">admin panel</Link>
<Link className={css['header-view__button']} to="/">Переглянути сайт</Link>
</div>
<div className={css['header-divider']}></div>
Expand Down
4 changes: 2 additions & 2 deletions FrontEnd/src/components/adminPage/menu/Menu.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ const MENU = [
{
id: 'am1',
title: 'Користувачі',
link: '/admin/users/'
link: '/customadmin/users/'
},
{
id: 'am2',
title: 'Компанії',
link: '/admin/companies/'
link: '/customadmin/companies/'
},
];

Expand Down
54 changes: 42 additions & 12 deletions FrontEnd/src/components/basicPage/BasicPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,32 +63,62 @@ function BasicPage() {
>
<Header isAuthorized={auth.isAuth} />
<Routes>
<Route path="/" element={<MainPage isAuthorized={auth.isAuth}/>} />
<Route path="/" element={<MainPage isAuthorized={auth.isAuth} />} />
<Route path="/profile/*" element={<ProfilePage />} />
<Route path="/profile-detail/:id" element={<ProfileDetailPage isAuthorized={auth.isAuth}/>}/>
<Route path="/profiles/:filter" element={<ProfileListPage isAuthorized={auth.isAuth} />}/>
<Route
path="/profile-detail/:id"
element={<ProfileDetailPage isAuthorized={auth.isAuth} />}
/>
<Route
path="/profiles/:filter"
element={<ProfileListPage isAuthorized={auth.isAuth} />}
/>
{auth.isAuth ? (
<Route path="/login" element={<Navigate to="/profile/user-info" />} />
) : (
<Route path="/login" element={<AuthorizationPage />} />
)}
{auth.isAuth ? (
<Route path="/sign-up" element={<Navigate to="/profile/user-info" />} />
<Route
path="/sign-up"
element={<Navigate to="/profile/user-info" />}
/>
) : (
<Route path="/sign-up" element={<SignUpPage />} />
)}
<Route path="/sign-up/modal" element={<SignUpModalPage />} />
<Route path="/sign-up/resend-activation" element={<ResendActivationPage />} />
<Route path="/activate/:uid/:token" element={<ActivationProfilePage />} />
<Route path="/reset-password" element={<SendEmailRestorePasswordPage />} />
<Route path="/reset-password/modal" element={<RestorePasswordModalPage />} />
<Route path="/password/reset/confirm/:uid/:token" element={<RestorePasswordPage />} />
<Route path="/reset-password/successfully" element={<RestorePasswordSuccessPage />} />
<Route path="/reset-password/failed" element={<RestorePasswordFailedPage />} />
<Route
path="/sign-up/resend-activation"
element={<ResendActivationPage />}
/>
<Route
path="/activate/:uid/:token"
element={<ActivationProfilePage />}
/>
<Route
path="/reset-password"
element={<SendEmailRestorePasswordPage />}
/>
<Route
path="/reset-password/modal"
element={<RestorePasswordModalPage />}
/>
<Route
path="/password/reset/confirm/:uid/:token"
element={<RestorePasswordPage />}
/>
<Route
path="/reset-password/successfully"
element={<RestorePasswordSuccessPage />}
/>
<Route
path="/reset-password/failed"
element={<RestorePasswordFailedPage />}
/>
<Route path="/privacy-policy" element={<PrivacyPolicy />} />
<Route path="/terms-and-conditions" element={<TermsAndConditions />} />
<Route path="/cookies-policy" element={<CookiesPolicyComponent />} />
<Route path="/search" element={<Search isAuthorized={auth.isAuth} />} />
<Route path="/search" element={<Search isAuthorized={auth} />} />
</Routes>
<Footer />
<ScrollToTopButton />
Expand Down
10 changes: 6 additions & 4 deletions FrontEnd/src/hooks/useProvideAuth.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,19 @@ import axios from 'axios';
export default function useProvideAuth() {
const [isAuth, setIsAuth] = useState(false);
const [isLoading, setLoading] = useState(true);
const [UserId, setUserId] = useState(null);
const validateToken = async (authToken) => {
try {
await axios.get(
const userData = await axios.get(
`${process.env.REACT_APP_BASE_API_URL}/api/auth/users/me/`,
{
headers: {
Authorization: `Token ${authToken}`,
},
}
);
)
.then((res) => res.data);
setUserId(userData.id);
return true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use existing useUser hook instead of adding userData here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think, we need additional request to api/users/me?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useUser hook provides all the information about authorized user. And you can access this data when you need it.
It is common practice to use this hook. And it's no need to duplicate the logic of getting user id.
And we get that information via request to api/users/me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I as authorized user put smth to the searchpage and press enter iuseProvideAuth sends the request to api/users/me. So I use it. If I'll use useUser it will send this request again.

Copy link
Collaborator

@Lvyshnevska Lvyshnevska Nov 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we have duplicated logic in our code)
I'd recommend to wait for Oleg review

Copy link
Collaborator Author

@BelousSofiya BelousSofiya Nov 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can change code, but I'm not sure in this solution. We are waiting for Oleg))

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have useUser hook. My proposition is to use it instead of adding userData in useProvideAuth.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @Lvyshnevska , inside a useUser hook we utilize a useSWR which should cache fetched data and prevent second API call. There is no need to call /api/auth/users/me/ the second time if it was already called and cached in the useUser

Copy link
Collaborator Author

@BelousSofiya BelousSofiya Nov 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Done. But both hooks useUser and useProvideAuth use the same request /api/auth/users/me/ to the backend. Is it ok? I mean if I can use only useUser to check authenticated user or not and to get his data (id for my case), why do we need useProvideAuth?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as what I see we return different values from these hooks. But since they use the same API I assume that we can combine them both into one hook. Also, we do not use SWR in useProvideAuth that's something that could be fixed

} catch (error) {
if (error.response && error.response.status === 401) {
Expand Down Expand Up @@ -62,6 +65,5 @@ export default function useProvideAuth() {
}
});
});

return { login, logout, isAuth, isLoading };
return { login, logout, isAuth, isLoading, UserId };
}
Loading