feat(videos): [255,283,284] add routing, loading optimization and responsive layout fixes#156
Merged
Merged
Conversation
arbisoft-qaisarirfan
force-pushed
the
255-283-284
branch
from
a9cb133 to
53e0787
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves the videos experience and auth routing by hardening login redirects, optimizing RTK Query list caching/loading behavior, and applying responsive UI adjustments across listing/search/detail pages, with expanded Jest coverage and accompanying docs updates.
Changes:
- Add internal
redirect_tovalidation and apply it in both middleware and LoginPage redirect handling. - Improve events list caching/loading behavior (RTK Query query-arg serialization + listing/search infinite-scroll UX tweaks).
- Update responsive layout/styling and expand Jest tests + documentation for the updated flows.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/utils.ts | Adds isValidInternalRedirectPath helper for safe internal redirects. |
| src/utils/utils.test.ts | Adds unit tests for redirect-path validation. |
| src/middleware.ts | Uses validated redirect_to for authenticated /login requests; keeps /videos* protection. |
| src/middleware.test.ts | Adds tests for authenticated /login redirect behavior and external redirect rejection. |
| src/features/LoginPage/loginPage.tsx | Validates redirect_to and adds a fallback read from window.location.search. |
| src/features/LoginPage/loginPage.test.tsx | Adds tests for invalid redirect handling and window-location fallback behavior. |
| src/redux/events/apiSlice.ts | Expands RTK Query cache key serialization to cover more filter params (excluding page). |
| src/features/VideosListingPage/videosListingPage.tsx | Adjusts loading detection and infinite scroll footer/loading behavior. |
| src/features/VideosListingPage/styled.tsx | Updates grid breakpoints for better medium/large screen layouts. |
| src/features/SearchResultsPage/searchResultsPage.tsx | Tweaks infinite scroll viewport + footer loading behavior; removes navigate onClick. |
| src/features/VideoDetail/videoDetail.tsx | Avoids rendering the video player when no video_file is present. |
| src/redux/store/configureStore.tsx | Switches devtools gating to use a constants export. |
| src/constants/constants.ts | Centralizes additional env-derived constants (CLIENT_ID, GTM_ID, NODE_ENV, CI). |
| src/app/layout.tsx | Gates GTM/Hotjar injection to production and uses centralized env constants. |
| src/features/HomePage/homePage.tsx | Removes use client from a purely presentational component. |
| src/components/theme/init-color-scheme-script.tsx | Removes use client from a simple wrapper component. |
| src/components/containers/MainLayoutContainer/styled.tsx | Adjusts large-breakpoint padding when sidebar is not available. |
| playwright.config.ts | Switches CI detection to use a constants export. |
| README.md | Documents npm run test:cov for coverage runs. |
| docs/testing-guide.md | Adds a test coverage section. |
| docs/state-management-and-api-layer.md | Adds an Events API overview section. |
| docs/routing-and-application-flows.md | Updates auth flow docs to reflect validated redirect_to redirect behavior. |
Comments suppressed due to low confidence (1)
docs/testing-guide.md:66
- In this section, the bullet list describing
jest.setup.tsbehavior appears to be split: the items starting with “enablesjest-fetch-mock/ mocks … / resets …” are currently placed under “## Test Coverage”, which makes the markdown structure confusing. Consider moving those bullets back under “## Jest Setup Behavior” (or adding a separate heading) so setup behaviors and coverage scope are clearly separated.
## Jest Setup Behavior
`jest.setup.ts` performs several important actions:
- imports `@testing-library/jest-dom`
- imports `whatwg-fetch`
## Test Coverage
Current test suite includes unit tests for:
- Utility functions (`src/utils/utils.ts`)
- Middleware authentication (`src/middleware.ts`)
- Feature pages: HomePage, LoginPage, VideosListingPage, SearchResultsPage, VideoDetail
- Redux slices and API integrations
- Reusable components (Navbar, Sidebar, etc.)
New tests added for previously untested feature pages to improve coverage on routing, loading states, and error handling.
- enables `jest-fetch-mock`
- mocks `@/hooks/useFeatureFlags`
- resets fetch mocks before each test
- populates a global `preloadedState` for Redux-backed tests
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,8 +1,28 @@ | |||
| import { omit } from "lodash"; | |||
| import { useTheme } from "@mui/material/styles"; | ||
| import Typography from "@mui/material/Typography"; | ||
| import useMediaQuery from "@mui/material/useMediaQuery"; | ||
| import { isEqual, omit } from "lodash"; |
Comment on lines
1
to
11
| import { defineConfig, devices } from "@playwright/test"; | ||
| import { config } from "dotenv"; | ||
|
|
||
| import { CI } from "./src/constants/constants"; | ||
| /** | ||
| * Read environment variables from file. | ||
| * https://github.com/motdotla/dotenv | ||
| */ | ||
| import { config } from "dotenv"; | ||
|
|
||
| config({ path: ".env.local" }); | ||
|
|
Comment on lines
+50
to
+70
| 5. On success, the frontend sends the Google `access_token` as `auth_token` | ||
| 6. Request is sent to `POST /users/login` | ||
| 7. On success, the app navigates to the validated `redirect_to` path or defaults to `/videos` | ||
| 8. On failure, a notification is shown |
arbisoft-qaisarirfan
force-pushed
the
255-283-284
branch
from
53e0787 to
fd073ff
Compare
…ponsive layout fixes
arbisoft-qaisarirfan
force-pushed
the
255-283-284
branch
from
fd073ff to
d4b55e7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚀 Description
This PR merges related frontend improvements (255/283/284) with:
📌 Summary
loginPagenow redirects to a validatedredirect_tovideo path (fallback/videos)./videos*routes.VideosListingPage,SearchResultsPage, andVideoDetailUI paths and state branches improved.🔧 Changes Implemented
src/middleware.ts: token-based control + safe redirect resolution.src/features/LoginPage/loginPage.tsx:redirect_tohandling + path validation.src/features/VideosListingPage/*: filter, empty state, loading, responsive padding/layout.src/features/SearchResultsPage/*: search/no-result/error flows.src/features/VideoDetail/*: loading/recommendations + error redirect.src/utils/utils.ts: utility edge-case correctness.src/features/*/*.test.tsx: Jest tests for all above.🛠️ How It Works
loginPageobtains redirect param from query/window, validates withisValidInternalRedirectPath./login-> redirect to requested video or/videos./videos*uses token validation and login redirect query.getEvents(apiParams)+ state output.✅ Checklist Before Merging
npx jest --runInBandpasses.📸 Screenshots
Not applicable.
🔗 Related Issues
📢 Notes for Reviewers @farhan2742 @arslanather @shaheer-alam-arbisoft
isValidInternalRedirectPath).middlewareroute logic for protected and auth paths.useTransitionlogin path fallback and error notifications.