Skip to content

Latest commit

 

History

History
524 lines (345 loc) · 14 KB

File metadata and controls

524 lines (345 loc) · 14 KB

Pull Request Guide

How to fill out the PR template with examples for common scenarios.


Example 1 — New Feature

## Description

Add a property listing page at `/user/dashboard` that fetches properties from the API and renders them in a responsive card grid with search and filter controls.

---

## Type of Change

- [ ] Bug fix
- [x] New feature
- [ ] Enhancement / improvement to existing feature
- [ ] Refactor
- [ ] UI / styling update
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Chore / maintenance

---

## Jira Ticket

- [NEX-42](https://your-project.atlassian.net/browse/NEX-42)

---

## What Changed

- Added `src/components/pages/PropertyListPage.tsx` — card grid with search bar and price/status filters
- Added `src/components/PropertyCard.tsx` — individual listing card with image, price badge, and address
- Added `src/hooks/useProperties.ts` — data-fetching hook that calls `GET /api/v1/properties`
- Added `src/app/user/properties/page.tsx` — route entry point
- Updated `src/components/layouts/UserLayout.tsx` — added "Properties" link to the sidebar nav

---

## Components / Pages Affected

| Area | Path | Added / Modified |
|------|------|-----------------|
| Page | `src/app/user/properties/page.tsx` | Added |
| Component | `src/components/pages/PropertyListPage.tsx` | Added |
| Component | `src/components/PropertyCard.tsx` | Added |
| Hook | `src/hooks/useProperties.ts` | Added |
| Layout | `src/components/layouts/UserLayout.tsx` | Modified |

---

## Breaking Changes

None

---

## How Has This Been Tested?

- [x] Manual testing in browser
- [x] Tested on mobile / responsive viewpoints
- [x] Ran `npm run build` (build passes)
- [x] Ran `npm run lint` (no lint errors)
- [x] Verified no TypeScript errors

**Test details:**
Tested with 0, 1, and 50+ listings. Verified empty-state message renders when no properties match filters. Confirmed cards reflow correctly at `sm`, `md`, and `lg` breakpoints.

---

## Screenshots / Screen Recordings (if applicable)

| Before | After |
|--------|-------|
| N/A (new page) | ![property-list](https://example.com/screenshot.png) |

---

## Checklist

- [x] Code follows the project conventions and folder structure
- [x] No hardcoded strings that should be extracted
- [x] Components are properly typed with TypeScript
- [x] No unnecessary `console.log` or debug code left in
- [x] Ran `npm run build` successfully
- [x] Ran `npm run lint` with no new warnings or errors
- [x] Responsive design considered (if UI change)
- [x] Accessibility basics checked (if UI change)

---

## Notes for Reviewers

Focus on `PropertyCard.tsx` — the price formatting logic handles multiple currencies. Also check the filter debounce timing in `useProperties.ts`.

---

## Deployment Notes

- [x] Requires environment variable update (`.env`) — added `NEXT_PUBLIC_API_URL`
- [ ] Requires dependency install (`npm install`)
- [ ] No special deployment steps

Example 2 — Bug Fix

## Description

Fix the login page redirecting to `/unauthorized` instead of `/user/dashboard` after a successful login when the user has the `user` role.

---

## Type of Change

- [x] Bug fix
- [ ] New feature
- [ ] Enhancement / improvement to existing feature
- [ ] Refactor
- [ ] UI / styling update
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Chore / maintenance

---

## Jira Ticket

- [NEX-58](https://your-project.atlassian.net/browse/NEX-58)

---

## What Changed

- Fixed `src/contexts/AuthContext.tsx` — role check was comparing against `"User"` (capital U) instead of `"user"`
- Fixed `src/components/ProtectedRoute.tsx` — added early return when auth state is still loading to prevent premature redirect

---

## Components / Pages Affected

| Area | Path | Added / Modified |
|------|------|-----------------|
| Context | `src/contexts/AuthContext.tsx` | Modified |
| Component | `src/components/ProtectedRoute.tsx` | Modified |

---

## Breaking Changes

None

---

## How Has This Been Tested?

- [x] Manual testing in browser
- [ ] Tested on mobile / responsive viewpoints
- [x] Ran `npm run build` (build passes)
- [x] Ran `npm run lint` (no lint errors)
- [x] Verified no TypeScript errors

**Test details:**
Logged in as `user`, `admin`, and unauthenticated. Verified each role lands on the correct dashboard. Confirmed hard-refreshing a protected page no longer flashes the unauthorized page.

---

## Screenshots / Screen Recordings (if applicable)

| Before | After |
|--------|-------|
| Redirected to `/unauthorized` | Correctly lands on `/user/dashboard` |

---

## Checklist

- [x] Code follows the project conventions and folder structure
- [x] No hardcoded strings that should be extracted
- [x] Components are properly typed with TypeScript
- [x] No unnecessary `console.log` or debug code left in
- [x] Ran `npm run build` successfully
- [x] Ran `npm run lint` with no new warnings or errors
- [ ] Responsive design considered (if UI change)
- [ ] Accessibility basics checked (if UI change)

---

## Notes for Reviewers

Two-line fix in `AuthContext.tsx`. The loading guard in `ProtectedRoute.tsx` is the more important change — without it the redirect fires before the auth state resolves on page reload.

---

## Deployment Notes

- [ ] Requires environment variable update (`.env`)
- [ ] Requires dependency install (`npm install`)
- [x] No special deployment steps

Example 3 — UI / Styling Update

## Description

Redesign the admin dashboard page with a new summary card row, updated typography, and dark-mode support using the existing `ThemeContext`.

---

## Type of Change

- [ ] Bug fix
- [ ] New feature
- [ ] Enhancement / improvement to existing feature
- [ ] Refactor
- [x] UI / styling update
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Chore / maintenance

---

## Jira Ticket

- [NEX-71](https://your-project.atlassian.net/browse/NEX-71)

---

## What Changed

- Rewrote `src/components/pages/AdminDashboardPage.tsx` — replaced placeholder content with stat cards and recent-activity table
- Added `src/components/StatCard.tsx` — reusable card that accepts icon, label, and value
- Updated `src/app/globals.css` — added dark-mode color variables and card utility classes

---

## Components / Pages Affected

| Area | Path | Added / Modified |
|------|------|-----------------|
| Page component | `src/components/pages/AdminDashboardPage.tsx` | Modified |
| Component | `src/components/StatCard.tsx` | Added |
| Styles | `src/app/globals.css` | Modified |

---

## Breaking Changes

None

---

## How Has This Been Tested?

- [x] Manual testing in browser
- [x] Tested on mobile / responsive viewpoints
- [x] Ran `npm run build` (build passes)
- [x] Ran `npm run lint` (no lint errors)
- [x] Verified no TypeScript errors

**Test details:**
Verified light and dark modes via the `ThemeToggle` component. Checked card layout at 320px, 768px, and 1440px widths. Confirmed no overflow or text truncation issues.

---

## Screenshots / Screen Recordings (if applicable)

| Before | After |
|--------|-------|
| ![old-dashboard](https://example.com/before.png) | ![new-dashboard](https://example.com/after.png) |

---

## Checklist

- [x] Code follows the project conventions and folder structure
- [x] No hardcoded strings that should be extracted
- [x] Components are properly typed with TypeScript
- [x] No unnecessary `console.log` or debug code left in
- [x] Ran `npm run build` successfully
- [x] Ran `npm run lint` with no new warnings or errors
- [x] Responsive design considered (if UI change)
- [x] Accessibility basics checked (if UI change)

---

## Notes for Reviewers

The `StatCard` component is intentionally generic so other pages can reuse it. Let me know if the prop interface needs adjustment.

---

## Deployment Notes

- [ ] Requires environment variable update (`.env`)
- [ ] Requires dependency install (`npm install`)
- [x] No special deployment steps

Example 4 — Refactor / Chore

## Description

Upgrade Next.js from 15 to 16 and migrate from the Pages Router leftovers to the App Router pattern. Replace deprecated `next/image` `layout` prop with the `fill` prop.

---

## Type of Change

- [ ] Bug fix
- [ ] New feature
- [ ] Enhancement / improvement to existing feature
- [x] Refactor
- [ ] UI / styling update
- [ ] Documentation update
- [ ] Performance improvement
- [x] Chore / maintenance

---

## Jira Ticket

- [NEX-93](https://your-project.atlassian.net/browse/NEX-93)

---

## What Changed

- Bumped `next` to `16.2.4` and `react` / `react-dom` to `19.2.4` in `package.json`
- Updated `next.config.ts` — enabled `reactCompiler` and `standalone` output
- Refactored `src/components/Providers.tsx` — simplified context nesting order
- Updated all `next/image` usages to use the `fill` prop instead of `layout="fill"`

---

## Components / Pages Affected

| Area | Path | Added / Modified |
|------|------|-----------------|
| Config | `next.config.ts` | Modified |
| Component | `src/components/Providers.tsx` | Modified |
| Multiple | `src/components/**/*` (image prop migration) | Modified |

---

## Breaking Changes

The `next.config.ts` now uses `reactCompiler: true`. All team members need Node >= 22 to build locally.

---

## How Has This Been Tested?

- [x] Manual testing in browser
- [x] Tested on mobile / responsive viewpoints
- [x] Ran `npm run build` (build passes)
- [x] Ran `npm run lint` (no lint errors)
- [x] Verified no TypeScript errors

**Test details:**
Full smoke test of every route. Verified images render correctly after the `fill` prop migration. Confirmed `pnpm build` produces a working standalone output in Docker.

---

## Screenshots / Screen Recordings (if applicable)

| Before | After |
|--------|-------|
| N/A | N/A — no visual changes |

---

## Checklist

- [x] Code follows the project conventions and folder structure
- [x] No hardcoded strings that should be extracted
- [x] Components are properly typed with TypeScript
- [x] No unnecessary `console.log` or debug code left in
- [x] Ran `npm run build` successfully
- [x] Ran `npm run lint` with no new warnings or errors
- [ ] Responsive design considered (if UI change)
- [ ] Accessibility basics checked (if UI change)

---

## Notes for Reviewers

The React Compiler is now enabled — if you see unexpected behavior in any component, check whether it relies on referential identity tricks that the compiler optimizes away.

---

## Deployment Notes

- [ ] Requires environment variable update (`.env`)
- [x] Requires dependency install (`npm install`)
- [ ] No special deployment steps

Example 5 — Enhancement to Existing Feature

## Description

Enhance the `ProtectedRoute` component to support an array of allowed roles instead of a single role, enabling routes that multiple roles can access (e.g., both `admin` and `manager`).

---

## Type of Change

- [ ] Bug fix
- [ ] New feature
- [x] Enhancement / improvement to existing feature
- [ ] Refactor
- [ ] UI / styling update
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Chore / maintenance

---

## Jira Ticket

- [NEX-104](https://your-project.atlassian.net/browse/NEX-104)

---

## What Changed

- Updated `src/components/ProtectedRoute.tsx``allowedRole` prop changed from `string` to `string | string[]`
- Updated all usages in `src/app/admin/**` and `src/app/user/**` to pass role as an array
- Updated `src/hooks/useRole.ts` — added `hasAnyRole()` helper

---

## Components / Pages Affected

| Area | Path | Added / Modified |
|------|------|-----------------|
| Component | `src/components/ProtectedRoute.tsx` | Modified |
| Hook | `src/hooks/useRole.ts` | Modified |
| Pages | `src/app/admin/dashboard/page.tsx` | Modified |
| Pages | `src/app/user/dashboard/page.tsx` | Modified |

---

## Breaking Changes

`ProtectedRoute` prop `allowedRole: string` is now `allowedRole: string | string[]`. Existing single-string usages still work — no action needed by consumers.

---

## How Has This Been Tested?

- [x] Manual testing in browser
- [ ] Tested on mobile / responsive viewpoints
- [x] Ran `npm run build` (build passes)
- [x] Ran `npm run lint` (no lint errors)
- [x] Verified no TypeScript errors

**Test details:**
Tested with `["admin", "manager"]` — both roles can access the page. Tested with a single string `"user"` to confirm backwards compatibility. Tested with an unauthorized role to confirm redirect still works.

---

## Screenshots / Screen Recordings (if applicable)

| Before | After |
|--------|-------|
| N/A | N/A — no visual changes |

---

## Checklist

- [x] Code follows the project conventions and folder structure
- [x] No hardcoded strings that should be extracted
- [x] Components are properly typed with TypeScript
- [x] No unnecessary `console.log` or debug code left in
- [x] Ran `npm run build` successfully
- [x] Ran `npm run lint` with no new warnings or errors
- [ ] Responsive design considered (if UI change)
- [ ] Accessibility basics checked (if UI change)

---

## Notes for Reviewers

The `hasAnyRole()` helper in `useRole.ts` is the key logic — verify it handles edge cases like empty arrays.

---

## Deployment Notes

- [ ] Requires environment variable update (`.env`)
- [ ] Requires dependency install (`npm install`)
- [x] No special deployment steps

Tips

  • Be specific in "What Changed" — list files and what happened in each; don't just say "updated code."
  • Fill out the Components / Pages table — it gives reviewers a fast map of where to look without scanning the diff.
  • Breaking Changes matter — even prop signature changes should be noted so the team knows what to watch for after merge.
  • Test details save time — a one-liner about edge cases covered prevents reviewers from asking "did you test X?"
  • Screenshots are cheap proof — paste a before/after for any UI change; it takes seconds and speeds up review.
  • Deployment Notes prevent surprises — if your PR needs a new env var or pnpm install, check the box so the person deploying isn't caught off guard.
  • One PR, one concern — keep PRs focused. A feature PR shouldn't also sneak in an unrelated refactor.