Add patent module dashboard UI components#242
Conversation
vikrantwiz02
left a comment
There was a problem hiding this comment.
Code Review
Reviewed the full diff. The PR adds substantial value with complete role-based dashboards, but there are several critical bugs (Attorney role is unreachable), duplicate components, and code quality issues that must be resolved before merging. See inline comments for specifics.
| "student", | ||
| "alumini", | ||
| "Professor", | ||
| "Associate Professor", |
There was a problem hiding this comment.
Typo: "alumini" should be "alumni". This will break role matching if the backend sends "alumni".
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
Attorney role is missing and unreachable. This is the component actually wired into App.jsx via <PatentModulePage />, but there is no branch for role === "Attorney". Attorney users will always hit the return null at the bottom.
The Attorney branch does exist in src/Modules/Patent/pages/PatentModulePage.jsx, but that file is only used inside PatentRoutes.jsx — which is never imported anywhere in the app.
Fix: Either add the Attorney branch here, or wire PatentRoutes.jsx into App.jsx and delete this root-level duplicate.
| function ApplicantMainDashboard() { | ||
| const [activeTab, setActiveTab] = useState("0"); | ||
| const [sortedBy, setSortedBy] = useState("Most Recent"); | ||
| const [loading, setLoading] = useState(false); |
There was a problem hiding this comment.
dispatch imported but never used. useDispatch() is called here and dispatch is even in the useEffect dependency array, but dispatch is never actually called anywhere in this component. Remove useDispatch import and this declaration.
| console.error("Error fetching data:", error); | ||
| } finally { | ||
| setLoading(false); | ||
| } |
There was a problem hiding this comment.
Dead useEffect — placeholder never implemented. fetchData toggles loading on and off but never fetches anything. The // Fetch data logic here if needed comment signals this was left as a stub. Either implement the actual data fetch or remove the whole useEffect block.
| onChange={setSortedBy} | ||
| placeholder="Sort By" | ||
| /> | ||
| </Flex> |
There was a problem hiding this comment.
Sort dropdown has no effect on data. sortedBy state is updated by the <Select> but is never used to sort or filter any list. Either implement the sort logic or remove the dropdown until it is ready.
|
|
||
| function ApplicationForm({ setActiveTab }) { | ||
| ApplicationForm.propTypes = { | ||
| setActiveTab: PropTypes.func.isRequired, |
There was a problem hiding this comment.
Anti-pattern: PropTypes assigned inside the function body. This re-runs on every render and mutates the function object each time. Move it outside:
function ApplicationForm({ setActiveTab }) { ... }
ApplicationForm.propTypes = {
setActiveTab: PropTypes.func.isRequired,
};
export default ApplicationForm;Every other component in this PR correctly places PropTypes outside the function — this is the only exception.
|
|
||
| export const API_BASE_URL = `${host}/patentsystem`; | ||
|
|
||
| const apiClient = axios.create({ |
There was a problem hiding this comment.
apiClient has no default auth interceptor. Every caller must manually invoke authHeaders() and attach headers. One missed call means a silent 401. Add a request interceptor instead so auth is applied automatically:
apiClient.interceptors.request.use((config) => {
const token = getAuthToken();
if (token) config.headers.Authorization = `Token ${token}`;
return config;
});| }, | ||
| }); | ||
|
|
||
| function RequireAuth({ children }) { |
There was a problem hiding this comment.
hasToken is captured once at mount and goes stale. Boolean(localStorage.getItem('authToken')) is evaluated when App first renders. If the token is added or removed later in the session (e.g. after login without a full page reload), hasToken keeps its initial value and <ValidateAuth /> / <InactivityHandler /> will behave incorrectly. Prefer deriving this from Redux auth state so it re-renders reactively.
| import DirectorMainDashboard from "../components/Director/DirectorMainDashboard"; | ||
| import PCCAdminMainDashboard from "../components/PCCAdmin/PCCAdminMainDashboard"; | ||
|
|
||
| function PatentModulePage({ role }) { |
There was a problem hiding this comment.
This file is dead code — it is never reached from App.jsx. App.jsx imports PatentModulePage from src/Modules/Patent/PatentModulePage.jsx (root level). PatentRoutes.jsx imports this pages version, but PatentRoutes.jsx itself is never imported anywhere. Delete one of the two PatentModulePage files and consolidate the logic.
|
Merge conflict on Steps to fix: git fetch origin
git checkout patent-management-v1 # your feature branch
git merge origin/patent-management-v1 # or rebase if preferred
# resolve conflicts in src/App.jsx
git add src/App.jsx
git commit
git pushAfter resolving, verify the |
Patent Module - Frontend Dashboard Components
Changes:
Components Added: