-
Notifications
You must be signed in to change notification settings - Fork 27
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
Merge Master
into feat-sonarcloud
#2269
base: feat-sonarcloud
Are you sure you want to change the base?
Conversation
Merge `Master(v2.7.2)` into `release-2.8.0`
Merge `Master(v2.7.3)` into `release-2.8.0`
Merge 'Master(2.7.6) into 'release-2.8.0'
…to the ONDC Pre Prod Flow (#2251)
Merge `Release 2.7.7` into `Master`
Merge `Master(2.7.7)` into `release-2.8.0`
Merge `Release-2.8.0` into `Master`
* removed appliction form * removed appliction form
Merge `Release 2.7.8` into `Master`
* comment sentry init and breadcrumb * comment sentry init and breadcrumb * comment sentry init and breadcrumb
Merge `release(2.7.9)` into `Master`
Warning Rate limit exceeded@sagarkoshti1990 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 29 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe pull request introduces several significant changes across multiple components in the front-end application. Key modifications include restructuring components, enhancing prop type definitions, and improving rendering logic. Notably, the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LandingPage
participant FeatureCard
participant List
participant VolunteerLandingPage
User->>LandingPage: Request Landing Page
LandingPage->>FeatureCard: Render Feature Cards
FeatureCard-->>LandingPage: Return Rendered Cards
LandingPage-->>User: Display Landing Page with Cards
User->>List: Navigate to List
List-->>User: Display List with Paginated Data
User->>VolunteerLandingPage: Navigate to Volunteer Page
VolunteerLandingPage-->>User: Display Volunteer Page
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 16
🧹 Outside diff range and nitpick comments (28)
apps/front-end/src/pages/front-end/Camp/CampDashboard.js (1)
Line range hint
48-54
: Consider simplifying the nested conditional logicThe current nested conditions can be simplified for better readability and maintenance.
Consider this alternative structure:
- {(stateName === "RAJASTHAN" || stateName === "BIHAR") && ( - <> - {stateName === "RAJASTHAN" && <EpcpCard />} - {/* Temp Comment */} - <ExamPreparationCard /> - {/* Temp Comment End*/} - </> - )} + {stateName === "RAJASTHAN" && <EpcpCard />} + {(stateName === "RAJASTHAN" || stateName === "BIHAR") && <ExamPreparationCard />}apps/front-end/src/onest/VolunteerLandingPage.js (2)
1-9
: Consider organizing imports by categoryThe imports could be better organized by grouping them into:
- External dependencies (react, prop-types)
- UI components (native-base)
- Internal components/utilities
+// External dependencies import { chunk } from "@shiksha/common-lib"; -import moment from "moment"; -import { HStack, VStack } from "native-base"; -import PropTypes from "prop-types"; import { useEffect, useState } from "react"; +import PropTypes from "prop-types"; +import moment from "moment"; + +// UI components +import { HStack, VStack } from "native-base"; + +// Internal components import { useNavigate } from "react-router-dom"; import { dataConfig } from "./card"; import Layout from "./Layout"; import { FeatureCard } from "./LandingPage";
Line range hint
21-52
: Security and implementation concerns in user data handlingSeveral issues need attention:
- Storing sensitive user information in localStorage poses a security risk
- Hardcoded email domain in the fallback
- Missing cleanup in useEffect
- No error handling for localStorage operations
Consider these improvements:
- Store only necessary non-sensitive data
- Use sessionStorage for temporary data
- Implement proper cleanup
- Add error handling
useEffect((e) => { const fetchData = () => { const Address = [ authUser?.state, authUser?.district, authUser?.block, authUser?.village, authUser?.grampanchayat, ] .filter((e) => e) .join(", "); const userDetails = { - "Education Qualification": - authUser?.qualifications?.qualification_master?.name || "", - gender: authUser?.gender || "", - "Student Name": authUser?.first_name + " " + authUser?.last_name, - name: authUser?.first_name + " " + authUser?.last_name, - email: authUser?.email_id || `${authUser?.first_name}@gmail.com`, - "Date Of Birth": authUser?.dob, - birth_date: authUser?.dob, - "mobile number": authUser?.mobile, - phone: authUser?.mobile, - contact: authUser?.mobile, - Address, + id: authUser?.id, + name: `${authUser?.first_name} ${authUser?.last_name}`, createdAt: moment().format("YYYY-MM-DD HH:mm"), - user_id: authUser?.id, }; - localStorage.setItem("userData", JSON.stringify(userDetails)); + try { + sessionStorage.setItem("userData", JSON.stringify(userDetails)); + } catch (error) { + console.error("Error storing user data:", error); + } }; fetchData(); + + return () => { + try { + sessionStorage.removeItem("userData"); + } catch (error) { + console.error("Error cleaning up user data:", error); + } + }; }, []);apps/front-end/src/onest/LandingPage.js (3)
15-15
: Consider translating carousel itemsWhile translation is correctly implemented for feature cards using
t(title)
, the carousel items' titles are not translated. Consider applying translations toCAROUSEL_LIST
items as well for consistency.const CAROUSEL_LIST = [ - { bgImage: slide2, title: "What would you like to explore today?" }, - { bgImage: slide3, title: "Jobs & Internships" }, - { bgImage: slide4, title: "Scholarships" }, - { bgImage: slide5, title: "Skill Development & Learning" }, + { bgImage: slide2, title: "landing_page.explore_today" }, + { bgImage: slide3, title: "landing_page.jobs_internships" }, + { bgImage: slide4, title: "landing_page.scholarships" }, + { bgImage: slide5, title: "landing_page.skill_development" }, ]; // Then in the render: <Text>{t(item.title)}</Text>Also applies to: 169-169
Line range hint
53-73
: Improve error handling and data managementA few suggestions to enhance the component:
- The error in
handleCardClick
is only logged, consider showing a user-friendly error message.- The data array transformation could be simplified by moving it out of the effect.
- useEffect(() => { - const chuckArr = Object.values(dataConfig); - const newArr = chunk(chuckArr, 2); - setDataArray(newArr); - }, []); + const dataArray = React.useMemo(() => { + return chunk(Object.values(dataConfig), 2); + }, []); const handleCardClick = async (title) => { try { navigate(`/${title}`); } catch (error) { + // Consider using a toast or alert component + console.error(`Error navigating to ${title}:`, error); + // Show user-friendly error message } };
175-183
: Strengthen prop type definitionsThe current prop types use
PropTypes.any
which reduces type safety. Consider using more specific prop types.FeatureCard.propTypes = { title: PropTypes.string, onClick: PropTypes.func, - imageUrl: PropTypes.any, + imageUrl: PropTypes.string, }; LandingPage.propTypes = { - footerLinks: PropTypes.any, + footerLinks: PropTypes.arrayOf( + PropTypes.shape({ + title: PropTypes.string, + link: PropTypes.string, + }) + ), };apps/front-end/src/pages/admin/beneficiaries/enrollment/schema.js (4)
Line range hint
1-3
: Add error handling for state managementThe current implementation directly parses localStorage without error handling, which could lead to runtime errors if the stored data is invalid or missing.
Consider implementing a safer state retrieval:
-let state = jsonParse(localStorage.getItem("program")); +let state; +try { + state = jsonParse(localStorage.getItem("program")) || {}; +} catch (error) { + console.error("Failed to parse program state:", error); + state = {}; +}
71-74
: Simplify nested conditional logicThe nested ternary operator makes the code harder to read and maintain.
Consider extracting the logic to a more readable format:
-label: getLabelBasedOnState( - "APPLICATION_ID", - state?.state_name == "MADHYA PRADESH" - ? "ROLL_NUMBER" - : "ENROLLMENT_NO", -), +const getEnrollmentLabel = () => { + if (state?.state_name === "RAJASTHAN") return "APPLICATION_ID"; + if (state?.state_name === "MADHYA PRADESH") return "ROLL_NUMBER"; + return "ENROLLMENT_NO"; +}; +label: getEnrollmentLabel(),
125-139
: Remove commented codeMaintaining commented-out code creates confusion and technical debt. If this code is no longer needed, it should be removed. If it might be needed later, it should be tracked in version control history.
Consider either removing these commented sections entirely or adding a TODO comment explaining why they're kept if they're needed for future reference.
The search results show that Devanagari characters ([\u0900-\u097F]) are only used in the UI components for language selection (हिं for Hindi), not for name validation. This indicates that blocking Devanagari script in name validation is likely unintentional and could prevent users from entering valid Indian names.
Name validation regex needs to be more inclusive
The current regex
/^(?!.*[\u0900-\u097F])[A-Za-z\s\p{P}]+$/
explicitly blocks Devanagari characters while the application supports Hindi language, creating an inconsistent user experience.Consider updating the regex to:
- Remove the negative lookahead
(?!.*[\u0900-\u097F])
to allow Devanagari characters- Add Devanagari range to allowed characters:
/^[A-Za-z\u0900-\u097F\s\p{P}]+$/
🔗 Analysis chain
Line range hint
4-9
: Review name validation regexThe current regex
/^(?!.*[\u0900-\u097F])[A-Za-z\s\p{P}]+$/
blocks all Devanagari script characters, which might prevent users from entering valid Indian names.Let's verify if this is intentional:
Consider updating the regex to be more inclusive of different name formats while maintaining necessary validation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any other name validation patterns in the codebase rg -l "[\u0900-\u097F]" --type js # Look for related validation logic ast-grep --pattern 'regex = /^(?!.*[\u0900-\u097F])[A-Za-z\s\p{P}]+$/'Length of output: 367
Script:
#!/bin/bash # Let's check the content of these files to understand the context of Devanagari usage rg -A 5 -B 5 "[\u0900-\u097F]" --type js # Also search for any name validation related patterns or schemas rg -i "name.*validation|validation.*name" --type js # Look for similar schema patterns ast-grep --pattern 'nameProperties = $_'Length of output: 123936
lib/common-lib/src/components/admin_layout/Layout.js (3)
Line range hint
134-141
: Improve role-based access control implementationThe current implementation has several concerns:
- Using
window.location.reload()
is a harsh way to handle unauthorized access- No feedback is provided to the user
- Could potentially cause infinite reloads if the role check consistently fails
Consider this alternative approach:
useEffect(() => { const { resource_access } = getTokernUserInfo() if ( resource_access?.hasura?.roles && !resource_access?.hasura?.roles?.includes('staff') ) { - window.location.reload() + // Redirect to unauthorized page or show error message + navigate('/unauthorized', { + state: { message: 'Staff access required' } + }) } }, [location])
209-218
: Enhance PropTypes definitions for better type safetyWhile the addition of PropTypes is good, consider these improvements:
- Mark required props with
isRequired
- Define specific shapes for object props
Layout.propTypes = { isDisabledAppBar: PropTypes.bool, - children: PropTypes.node, + children: PropTypes.node.isRequired, imageUrl: PropTypes.string, getRefAppBar: PropTypes.func, - _appBar: PropTypes.object, + _appBar: PropTypes.shape({ + isEnableSearchBtn: PropTypes.bool, + isShowNotificationButton: PropTypes.bool, + _box: PropTypes.object + }), _header: PropTypes.object, _sidebar: PropTypes.object, loading: PropTypes.bool }
Line range hint
13-108
: Consider memoizing the menu configurationThe
menus
array is recreated on every render. Since it's static, consider moving it outside the component or memoizing it.+ const ADMIN_MENUS = React.memo(() => ([ { title: 'HOME', icon: 'Home4LineIcon', // ... rest of the menu configuration } + ])); export default function Layout({...}) { - const menus = [...] + const menus = ADMIN_MENUS(); // ... rest of the component }apps/front-end/src/onest/List.js (2)
115-118
: Consider user preferences for smooth scrollingWhile smooth scrolling improves UX, some users may prefer reduced motion. Consider respecting the user's motion preferences:
window.scrollTo({ top: 0, - behavior: "smooth", + behavior: window.matchMedia('(prefers-reduced-motion: reduce)').matches ? 'auto' : 'smooth' });
441-454
: Optimize filter performanceThe current implementation may be inefficient for large datasets. Consider these optimizations:
- Cache the lowercase filter values outside the loop
- Use early returns for empty filters
const filKeys = Object.keys(filter || {}); +const lowerCaseFilters = {}; +filKeys.forEach(key => { + lowerCaseFilters[key] = filter[key]?.toLowerCase() || ''; +}); return data.filter((item) => { return filKeys.every((key) => { if (filter[key] === "") return true; + const itemValue = item[key]?.toString()?.toLowerCase(); + if (!itemValue) return false; - return item[key]?.toString()?.toLowerCase().includes(filter[key]?.toLowerCase()); + return itemValue.includes(lowerCaseFilters[key]); }); });apps/front-end/src/onest/scholarship/View.jsx (2)
Line range hint
237-255
: Enhance API Call Implementation with Better Error HandlingThe current fetch implementation lacks robust error handling and could be improved for better reliability.
Consider implementing these improvements:
useEffect(() => { + const controller = new AbortController(); const requestOptions = { method: "POST", headers: { "Content-Type": "application/json", }, body: JSON.stringify({ item_id: jobId }), + signal: controller.signal, + timeout: 5000, }; fetch(`${baseUrl}/content/search`, requestOptions) .then((response) => { + if (!response.ok) { + throw new Error(`HTTP error! status: ${response.status}`); + } return response.text(); }) .then((result) => { result = JSON.parse(result); setJobInfo(result?.data[db_cache][0]); if (transactionId !== undefined) { fetchJobDetails(result?.data[db_cache][0]); } }) - .catch((error) => console.log("error", error)); + .catch((error) => { + if (error.name === 'AbortError') { + console.log('Fetch aborted'); + return; + } + console.error("API Error:", error); + errorMessage(t("Failed to fetch scholarship details")); + }); + + return () => controller.abort(); }, [transactionId]);
184-186
: Improve Error Message Handling and LoggingThe current error message includes technical details (transactionId) that may not be helpful to end-users.
Consider this improvement:
- errorMessage( - t("Delay_in_fetching_the_details") + "(" + transactionId + ")", - ); + // Log technical details for debugging + console.error("Failed to fetch details:", { transactionId, jobId }); + // Show user-friendly message + errorMessage(t("Unable to load scholarship details. Please try again later."));apps/front-end/src/pages/front-end/Camp/CampList/CampList.js (3)
100-104
: Consider making the observation type configurableThe observation type "EPCP" is hardcoded. Consider making this configurable through props or environment variables for better maintainability and flexibility.
- let observation = "EPCP"; + const observation = props.observationType || "EPCP";
Line range hint
223-239
: Consider extracting status checks into constantsThe status checks are repeated in multiple places. Consider extracting these into named constants to improve maintainability and reduce potential errors.
+ const ALLOWED_STATUSES = ["selected_for_onboarding", "selected_prerak"]; - ["selected_for_onboarding", "selected_prerak"].includes(ipStatus) + ALLOWED_STATUSES.includes(ipStatus)
371-373
: Consider making supported states configurableThe state-based chart rendering is hardcoded for "RAJASTHAN" and "BIHAR". Consider making this list of supported states configurable to make the component more maintainable and scalable.
+ const SUPPORTED_STATES = ["RAJASTHAN", "BIHAR"]; - {(stateName === "RAJASTHAN" || stateName === "BIHAR") && ( + {SUPPORTED_STATES.includes(stateName) && (apps/front-end/src/pages/admin/beneficiaries/EnrollmentReceiptView.js (4)
Line range hint
453-473
: Remove commented code instead of keeping it.Rather than commenting out the
ActiveButton
components, consider removing them entirely if they're no longer needed. If this functionality might be needed in the future, document the requirements in the codebase or issue tracker.- {/* <ActiveButton - isActive={ - receiptUrl?.doc_id === paymentDocId?.application_form - } - onPress={() => { - handleButtonClick(paymentDocId?.application_form); - }} - > - {t("APPLICATION_FORM")} - </ActiveButton> */} - {/* <ActiveButton - isActive={ - receiptUrl?.doc_id === - paymentDocId?.application_login_id - } - onPress={() => { - handleButtonClick(paymentDocId?.application_login_id); - }} - > - {t("APPLICATION_LOGIN_ID_SS")} - </ActiveButton> */}
Line range hint
192-234
: Separate education standard validation from submission logic.The submission logic mixes multiple concerns: education standard validation, warning modal handling, and API calls. Consider extracting the validation logic into a separate function for better maintainability.
+const validateEducationStandard = (lastStandard) => { + const MINIMUM_STANDARD = 5; + return { + isValid: !isNaN(lastStandard) && lastStandard >= MINIMUM_STANDARD, + lastStandard + }; +}; const submit = useCallback( async (status) => { if (!checkValidation()) return; - const lastStandard = parseInt( - data?.core_beneficiaries?.last_standard_of_education ?? "", - 10, - ); - const hasWarning = isNaN(lastStandard) || lastStandard < 5; + const { isValid } = validateEducationStandard( + parseInt(data?.core_beneficiaries?.last_standard_of_education ?? "", 10) + ); - if (hasWarning && !openWarningModal) { + if (!isValid && !openWarningModal) { setOpenWarningModal(true); return; }
Line range hint
8-24
: Add PropTypes validation for all components.Several components (
Body
,TextInfo
,ValidationBox
,Message
,LearnerInfo
,ActiveButton
) lack prop type validation. Add PropTypes to improve code reliability and documentation.+Body.propTypes = { + data: PropTypes.shape({ + program_beneficiaries: PropTypes.object, + is_duplicate: PropTypes.string, + is_deactivated: PropTypes.string + }).isRequired, + children: PropTypes.node.isRequired +}; +TextInfo.propTypes = { + data: PropTypes.object.isRequired, + _box: PropTypes.object, + arr: PropTypes.arrayOf( + PropTypes.shape({ + label: PropTypes.string.isRequired, + keyArr: PropTypes.string, + value: PropTypes.node + }) + ).isRequired, + board: PropTypes.string +}; +ActiveButton.propTypes = { + isActive: PropTypes.bool.isRequired, + children: PropTypes.node.isRequired, + onPress: PropTypes.func.isRequired +};Also applies to: 799-801
Line range hint
52-65
: Improve error handling for API calls.The
handleSetReceiptUrl
andprofileDetails
functions lack proper error handling. Consider adding try-catch blocks and user-friendly error messages.const handleSetReceiptUrl = async (doc_id) => { setIsButtonLoading(true); - const newResult = await uploadRegistryService.getOne({ - document_id: doc_id, - }); - setReceiptUrl({ url: newResult, doc_id }); - setFileType(newResult?.key?.split(".").pop()); - setIsButtonLoading(false); + try { + const newResult = await uploadRegistryService.getOne({ + document_id: doc_id, + }); + if (!newResult) { + throw new Error('Failed to fetch receipt URL'); + } + setReceiptUrl({ url: newResult, doc_id }); + setFileType(newResult?.key?.split(".").pop()); + } catch (error) { + setError(prev => ({ + ...prev, + receipt: t('FAILED_TO_LOAD_RECEIPT') + })); + } finally { + setIsButtonLoading(false); + } };Also applies to: 89-91
apps/front-end/src/pages/front-end/Dashboard.js (1)
Line range hint
950-966
: Consider using constants for state names and improving error handling.The conditional rendering logic for the learner examination section can be improved:
- State names should be defined as constants to prevent typos and improve maintainability
- The navigation to exam schedule/attendance should include error handling for edge cases
+ const SUPPORTED_STATES = { + RAJASTHAN: "RAJASTHAN", + BIHAR: "BIHAR" + }; - {(state_name === "RAJASTHAN" || state_name === "BIHAR") && ( + {(state_name === SUPPORTED_STATES.RAJASTHAN || state_name === SUPPORTED_STATES.BIHAR) && (apps/front-end/src/onest/scholarship/AutomatedForm.jsx (2)
42-42
: Remove unused state variableopenModal
The state variable
openModal
is declared without its setter function and does not appear to be used in the code. Consider removing it to clean up the code.Apply this diff to remove the unused variable:
- const [openModal] = useState(false);
563-613
: Avoid direct DOM manipulation in React componentsDirectly manipulating the DOM using methods like
document.querySelector
andelement.classList.add
within React components is generally discouraged as it can lead to unexpected behavior and side effects. Instead, consider using React state and props to manage the form and its fields.Refactor the code to use React's state management and component lifecycle methods. For example, use React refs or controlled components to manage form inputs and apply classes conditionally.
apps/front-end/src/pages/admin/beneficiaries/enrollment/EnrollmentForm.js (1)
800-801
: Remove commented-out code for cleaner codebaseThe commented-out lines within the
payment_receipt_document_id
array can be removed to maintain a clean and readable codebase. If these lines are no longer needed, it's best practice to delete them.Apply the following diff to remove the commented-out code:
payment_receipt_document_id: [ { id: newdata.payment_receipt_document_id, key: "payment_receipt_document_id", }, - // { id: newdata.application_form, key: "application_form" }, - // { id: newdata.application_login_id, key: "application_login_id" }, ],
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (2)
package-lock.json
is excluded by!**/package-lock.json
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (18)
apps/front-end/src/onest/LandingPage.js
(5 hunks)apps/front-end/src/onest/List.js
(9 hunks)apps/front-end/src/onest/VolunteerLandingPage.js
(3 hunks)apps/front-end/src/onest/scholarship/AutomatedForm.jsx
(12 hunks)apps/front-end/src/onest/scholarship/View.jsx
(9 hunks)apps/front-end/src/onest/services/ApiUrl.json
(0 hunks)apps/front-end/src/onest/services/Apicall.jsx
(0 hunks)apps/front-end/src/onest/services/config.json
(0 hunks)apps/front-end/src/onest/services/index.ts
(0 hunks)apps/front-end/src/pages/admin/beneficiaries/EnrollmentReceiptView.js
(3 hunks)apps/front-end/src/pages/admin/beneficiaries/enrollment/EnrollmentForm.js
(2 hunks)apps/front-end/src/pages/admin/beneficiaries/enrollment/schema.js
(6 hunks)apps/front-end/src/pages/front-end/Camp/CampDashboard.js
(1 hunks)apps/front-end/src/pages/front-end/Camp/CampList/CampList.js
(9 hunks)apps/front-end/src/pages/front-end/Dashboard.js
(2 hunks)lib/common-lib/src/components/AppShell.tsx
(1 hunks)lib/common-lib/src/components/admin_layout/Layout.js
(5 hunks)lib/common-lib/src/components/helper.js
(1 hunks)
💤 Files with no reviewable changes (4)
- apps/front-end/src/onest/services/ApiUrl.json
- apps/front-end/src/onest/services/Apicall.jsx
- apps/front-end/src/onest/services/config.json
- apps/front-end/src/onest/services/index.ts
✅ Files skipped from review due to trivial changes (2)
- lib/common-lib/src/components/AppShell.tsx
- lib/common-lib/src/components/helper.js
🧰 Additional context used
🪛 Biome
apps/front-end/src/onest/scholarship/AutomatedForm.jsx
[error] 209-209: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 388-388: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 501-501: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 502-502: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
🔇 Additional comments (8)
apps/front-end/src/pages/front-end/Camp/CampDashboard.js (1)
51-53
: Clarify the purpose of temporary comments
The presence of temporary comments around ExamPreparationCard
suggests incomplete work or pending changes.
Please clarify:
- Are these comments still needed?
- Is there pending work related to
ExamPreparationCard
?
Would you like me to help track this by creating a GitHub issue?
apps/front-end/src/onest/LandingPage.js (1)
99-99
: LGTM! Unique key implementation
The key generation using both title and index ensures uniqueness and helps React's reconciliation process.
apps/front-end/src/onest/List.js (1)
173-174
: Validate analytics page title
Ensure the type
parameter used for analytics is properly sanitized and validated before use.
✅ Verification successful
The type parameter is properly validated through dataConfig
The type
parameter used for analytics is inherently validated as it:
- Comes from URL params and is used to access a predefined configuration object (
dataConfig
) - Only supports three specific values: "scholarship", "jobs", and "learning"
- The values are used in a controlled manner through the
dataConfig
object which defines all valid configurations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if type parameter is properly validated
rg -A 2 "useParams.*type" .
Length of output: 29
Script:
#!/bin/bash
# Let's check how 'type' is used in List.js and any validation
rg -B 5 -A 5 "type[\"'\s].*analyticsPageTitle" .
# Also check for any parameter validation or sanitization
ast-grep --pattern 'const { type } = $_'
Length of output: 991
Script:
#!/bin/bash
# Let's check how type is validated in List.js and what dataConfig contains
rg -B 10 -A 10 "const { type } = useParams" apps/front-end/src/onest/List.js
# Also check for dataConfig definition
rg "dataConfig\s*=|const\s+dataConfig" apps/front-end/src/onest/List.js
Length of output: 269
Script:
#!/bin/bash
# Let's check the content of List.js to understand the context better
cat apps/front-end/src/onest/List.js
Length of output: 13011
Script:
#!/bin/bash
# Let's check the dataConfig definition in the card file
cat apps/front-end/src/onest/card.js
Length of output: 9892
apps/front-end/src/onest/scholarship/View.jsx (1)
401-401
: Verify uniqueness of mapped items to prevent rendering issues
While using index in key props is acceptable here since it's combined with item data, we should ensure item uniqueness.
Let's verify the structure and uniqueness of the tags data:
Also applies to: 406-406
✅ Verification successful
Let me gather more specific information about the tag structure in the scholarship view.
Let me check one more aspect to understand how the tag objects are structured.
Using index with tag object as key is safe in this implementation
The key prop combines the unique tag object with the index (key={tag + index}
), and the data comes from a server response where each tag has a unique descriptor and list. The tags are rendered in a stable order from the API response, making the current implementation safe from rendering issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for the structure of jobsData.tags to verify item uniqueness
# Look for tag definitions or interfaces
ast-grep --pattern 'interface $_ {
tags: $_[];
}'
# Search for tag assignments or transformations
rg -A 5 'tags.*='
Length of output: 6252
Script:
#!/bin/bash
# Search for the structure of tags in scholarship view
ast-grep --pattern 'const jobsData = $_'
# Look for the data transformation or API response structure
rg -B 2 -A 5 'jobsData.*tags' apps/front-end/src/onest/scholarship/View.jsx
Length of output: 444
Script:
#!/bin/bash
# Look for the API response or data structure that populates jobsData
rg -B 5 -A 2 'setJobsData\(' apps/front-end/src/onest/scholarship/View.jsx
# Also check for any transformations before rendering
rg -B 2 -A 2 'const.*tag.*=' apps/front-end/src/onest/scholarship/View.jsx
Length of output: 452
apps/front-end/src/pages/front-end/Camp/CampList/CampList.js (2)
69-69
: LGTM: Camp count calculation is correct
The total camp count calculation properly sums both PCR camps and main camps.
120-120
: LGTM: Context ID comparison is accurate
The response filtering logic correctly matches context IDs with user IDs.
apps/front-end/src/pages/front-end/Dashboard.js (1)
967-973
: Verify the Rajasthan-specific exam results feature.
The exam results section is only shown for Rajasthan state. This could lead to confusion for users from other states who might expect similar functionality.
✅ Verification successful
Let me search for any state-specific conditions around exam results.
Let me search for any state-specific conditions around the exam results card.
The exam results feature is indeed Rajasthan-specific
The code shows a clear state-specific condition state_name === "RAJASTHAN"
that controls the visibility of the exam results card. This confirms that the review comment is correct in identifying this as a Rajasthan-specific feature.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any other state-specific exam result implementations
# that might need to be considered for Bihar state as well
# Search for exam result related components and their state-specific logic
rg -A 5 "exam.*result" --glob '*.{js,jsx}'
Length of output: 14135
Script:
#!/bin/bash
# Search for state-specific conditions around exam results
rg -B 3 -A 3 "rajasthan|bihar|state.*exam" --glob '*.{js,jsx}'
# Also check the Dashboard component for any state-specific rendering conditions
ast-grep --pattern 'const $_ = () => {
$$$
return (
$$$
<DashboardCard
title={"LEARNER_EXAM_RESULTS"}
$$$
/>
$$$
)
}'
Length of output: 137068
Script:
#!/bin/bash
# Search for state-specific conditions around the exam results card in Dashboard.js
rg -B 5 -A 5 "LEARNER_EXAM_RESULTS" apps/front-end/src/pages/front-end/Dashboard.js
Length of output: 562
apps/front-end/src/pages/admin/beneficiaries/enrollment/EnrollmentForm.js (1)
729-729
: Efficiently removing dynamic keys from the errors object
Great use of computed property names in destructuring to exclude the dynamic key [fieldId]
from the errors
object. This enhances code clarity and ensures that only relevant errors are maintained.
VolunteerLandingPage.propTypes = { | ||
userTokenInfo: PropTypes.any, | ||
footerLinks: PropTypes.any, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Define specific PropTypes instead of using 'any'
Using PropTypes.any
reduces type safety. Based on the component usage, we should define more specific prop types.
VolunteerLandingPage.propTypes = {
- userTokenInfo: PropTypes.any,
- footerLinks: PropTypes.any,
+ userTokenInfo: PropTypes.shape({
+ authUser: PropTypes.shape({
+ id: PropTypes.string.isRequired,
+ first_name: PropTypes.string.isRequired,
+ last_name: PropTypes.string,
+ email_id: PropTypes.string,
+ mobile: PropTypes.string,
+ dob: PropTypes.string,
+ state: PropTypes.string,
+ district: PropTypes.string,
+ block: PropTypes.string,
+ village: PropTypes.string,
+ grampanchayat: PropTypes.string,
+ gender: PropTypes.string,
+ qualifications: PropTypes.shape({
+ qualification_master: PropTypes.shape({
+ name: PropTypes.string
+ })
+ }),
+ user_roles: PropTypes.arrayOf(PropTypes.object)
+ }).isRequired
+ }).isRequired,
+ footerLinks: PropTypes.arrayOf(PropTypes.object).isRequired
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
VolunteerLandingPage.propTypes = { | |
userTokenInfo: PropTypes.any, | |
footerLinks: PropTypes.any, | |
VolunteerLandingPage.propTypes = { | |
userTokenInfo: PropTypes.shape({ | |
authUser: PropTypes.shape({ | |
id: PropTypes.string.isRequired, | |
first_name: PropTypes.string.isRequired, | |
last_name: PropTypes.string, | |
email_id: PropTypes.string, | |
mobile: PropTypes.string, | |
dob: PropTypes.string, | |
state: PropTypes.string, | |
district: PropTypes.string, | |
block: PropTypes.string, | |
village: PropTypes.string, | |
grampanchayat: PropTypes.string, | |
gender: PropTypes.string, | |
qualifications: PropTypes.shape({ | |
qualification_master: PropTypes.shape({ | |
name: PropTypes.string | |
}) | |
}), | |
user_roles: PropTypes.arrayOf(PropTypes.object) | |
}).isRequired | |
}).isRequired, | |
footerLinks: PropTypes.arrayOf(PropTypes.object).isRequired | |
}; |
{dataArray?.map((pItem) => ( | ||
<HStack space={"6%"} key={pItem}> | ||
{pItem.map((item) => { | ||
return ( | ||
<FeatureCard | ||
imageUrl={item?.imageUrl} | ||
onClick={() => handleCardClick(item?.listLink)} | ||
height={"180px"} | ||
width={"47%"} | ||
title={item?.title} | ||
key={item} | ||
/> | ||
); | ||
})} | ||
</HStack> | ||
))} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Multiple improvements needed in the feature card rendering logic
Several issues need to be addressed in the card rendering implementation:
- Using an object as a key prop is an anti-pattern
- Hardcoded width values might cause layout issues
- Missing error boundary and loading states
- No handling for empty data
-{dataArray?.map((pItem) => (
- <HStack space={"6%"} key={pItem}>
- {pItem.map((item) => {
- return (
- <FeatureCard
- imageUrl={item?.imageUrl}
- onClick={() => handleCardClick(item?.listLink)}
- height={"180px"}
- width={"47%"}
- title={item?.title}
- key={item}
- />
- );
- })}
- </HStack>
-))}
+{dataArray?.length > 0 ? (
+ dataArray.map((row, rowIndex) => (
+ <HStack
+ space={"6%"}
+ key={`row-${rowIndex}`}
+ justifyContent="center"
+ >
+ {row.map((item, itemIndex) => (
+ <FeatureCard
+ imageUrl={item?.imageUrl}
+ onClick={() => handleCardClick(item?.listLink)}
+ height={"180px"}
+ width={{ base: "100%", sm: "47%" }}
+ title={item?.title}
+ key={`card-${rowIndex}-${itemIndex}`}
+ />
+ ))}
+ </HStack>
+ ))
+) : (
+ <VStack alignItems="center" py={4}>
+ <Text>No features available</Text>
+ </VStack>
+)}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
{dataArray?.map((pItem) => ( | |
<HStack space={"6%"} key={pItem}> | |
{pItem.map((item) => { | |
return ( | |
<FeatureCard | |
imageUrl={item?.imageUrl} | |
onClick={() => handleCardClick(item?.listLink)} | |
height={"180px"} | |
width={"47%"} | |
title={item?.title} | |
key={item} | |
/> | |
); | |
})} | |
</HStack> | |
))} | |
{dataArray?.length > 0 ? ( | |
dataArray.map((row, rowIndex) => ( | |
<HStack | |
space={"6%"} | |
key={`row-${rowIndex}`} | |
justifyContent="center" | |
> | |
{row.map((item, itemIndex) => ( | |
<FeatureCard | |
imageUrl={item?.imageUrl} | |
onClick={() => handleCardClick(item?.listLink)} | |
height={"180px"} | |
width={{ base: "100%", sm: "47%" }} | |
title={item?.title} | |
key={`card-${rowIndex}-${itemIndex}`} | |
/> | |
))} | |
</HStack> | |
)) | |
) : ( | |
<VStack alignItems="center" py={4}> | |
<Text>No features available</Text> | |
</VStack> | |
)} |
export const FeatureCard = ({ title, onClick, imageUrl, ...props }) => { | ||
const { t } = useTranslation(); | ||
|
||
return ( | ||
<VStack | ||
p="6" | ||
borderWidth="1px" | ||
borderColor="gray.300" | ||
borderRadius="10px" | ||
alignItems="center" | ||
textAlign="center" | ||
shadow="4" | ||
onClick={onClick} | ||
cursor="pointer" | ||
{...props} | ||
> | ||
{imageUrl && ( | ||
<Image | ||
src={imageUrl} | ||
source={{ uri: imageUrl }} | ||
alt={title} | ||
mb="4" | ||
size={"lg"} | ||
height={"74px"} | ||
width={"74px"} | ||
color={"black"} | ||
/> | ||
)} | ||
<Heading as="h2" size="md" mb="2" fontSize={"16px"} fontWeight={"500"}> | ||
{t(title) || "Untitled"} | ||
</Heading> | ||
</VStack> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix duplicate image source and improve translation fallback
The Image component has both src
and source
props, which is redundant. Also, the "Untitled" fallback should be translated.
<Image
- src={imageUrl}
source={{ uri: imageUrl }}
alt={title}
mb="4"
size={"lg"}
height={"74px"}
width={"74px"}
color={"black"}
/>
<Heading as="h2" size="md" mb="2" fontSize={"16px"} fontWeight={"500"}>
- {t(title) || "Untitled"}
+ {t(title) || t("common.untitled")}
</Heading>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export const FeatureCard = ({ title, onClick, imageUrl, ...props }) => { | |
const { t } = useTranslation(); | |
return ( | |
<VStack | |
p="6" | |
borderWidth="1px" | |
borderColor="gray.300" | |
borderRadius="10px" | |
alignItems="center" | |
textAlign="center" | |
shadow="4" | |
onClick={onClick} | |
cursor="pointer" | |
{...props} | |
> | |
{imageUrl && ( | |
<Image | |
src={imageUrl} | |
source={{ uri: imageUrl }} | |
alt={title} | |
mb="4" | |
size={"lg"} | |
height={"74px"} | |
width={"74px"} | |
color={"black"} | |
/> | |
)} | |
<Heading as="h2" size="md" mb="2" fontSize={"16px"} fontWeight={"500"}> | |
{t(title) || "Untitled"} | |
</Heading> | |
</VStack> | |
); | |
}; | |
export const FeatureCard = ({ title, onClick, imageUrl, ...props }) => { | |
const { t } = useTranslation(); | |
return ( | |
<VStack | |
p="6" | |
borderWidth="1px" | |
borderColor="gray.300" | |
borderRadius="10px" | |
alignItems="center" | |
textAlign="center" | |
shadow="4" | |
onClick={onClick} | |
cursor="pointer" | |
{...props} | |
> | |
{imageUrl && ( | |
<Image | |
source={{ uri: imageUrl }} | |
alt={title} | |
mb="4" | |
size={"lg"} | |
height={"74px"} | |
width={"74px"} | |
color={"black"} | |
/> | |
)} | |
<Heading as="h2" size="md" mb="2" fontSize={"16px"} fontWeight={"500"}> | |
{t(title) || t("common.untitled")} | |
</Heading> | |
</VStack> | |
); | |
}; |
// { | ||
// title: 'REPORTS', | ||
// icon: 'FileChartLineIcon', | ||
// route: '/admin/reports', | ||
// subMenu: [ | ||
// { | ||
// title: 'PRERAKS', | ||
// icon: 'GroupLineIcon', | ||
// route: '/admin/reports/prerak', | ||
// select: 4 | ||
// }, | ||
// { | ||
// title: 'LEARNERS', | ||
// icon: 'GraduationCap', | ||
// route: '/admin/reports/learner', | ||
// select: 4 | ||
// }, | ||
// { | ||
// title: 'CAMPS', | ||
// icon: 'CommunityLineIcon', | ||
// route: '/admin/reports/camp', | ||
// select: 4 | ||
// }, | ||
// { | ||
// title: 'EPCP_REPORT', | ||
// icon: 'MacLineIcon', | ||
// route: '/admin/reports/epcp', | ||
// select: 4 | ||
// }, | ||
// { | ||
// title: 'TRAINING', | ||
// icon: 'DraftLineIcon ', | ||
// route: '/admin/reports/training', | ||
// select: 4 | ||
// }, | ||
// { | ||
// title: 'CAMP_EXECUTION', | ||
// icon: 'CommunityLineIcon', | ||
// route: '/admin/reports/camp_execution', | ||
// select: 4 | ||
// }, | ||
// { | ||
// title: 'SUBJECTS', | ||
// icon: 'Book2LineIcon', | ||
// route: '/admin/reports/subjects', | ||
// select: 4 | ||
// } | ||
// ] | ||
// }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Do not remove the REPORTS menu section as it's actively used
The commented-out REPORTS menu section should not be removed as these routes are actively being used in the application:
/admin/reports/learner
is used in AdminBeneficiariesList.js for learners list navigation/admin/reports/prerak
is used in List.js for prerak reports navigation
These routes need to remain accessible through the menu structure to maintain the application's functionality. Please uncomment the REPORTS section in the Layout.js file.
🔗 Analysis chain
Consider cleaning up commented code instead of preserving it
Large blocks of commented code can make maintenance more difficult. If the REPORTS section is intentionally removed, consider:
- Removing the code entirely if it's no longer needed
- Moving it to a separate configuration file if it's temporary
Let's verify if these routes are still referenced elsewhere:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any references to the removed report routes
rg -l "/admin/reports/(prerak|learner|camp|epcp|training|camp_execution|subjects)"
Length of output: 259
Script:
#!/bin/bash
# Check the actual usage of these routes in the files
rg -A 5 -B 5 "/admin/reports/(prerak|learner|camp|epcp|training|camp_execution|subjects)" apps/front-end/src/pages/admin/facilitator/List.js apps/front-end/src/pages/admin/beneficiaries/AdminBeneficiariesList.js
Length of output: 2292
let description = ""; | ||
|
||
if (obj.shortDescription) { | ||
description = obj.shortDescription; | ||
} else if (obj.description) { | ||
description = obj.description.substring(0, 100) + "..."; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Address potential XSS vulnerability
Using dangerouslySetInnerHTML
with unvalidated content poses a security risk. Consider sanitizing the HTML content:
+import DOMPurify from 'dompurify';
let description = "";
if (obj.shortDescription) {
- description = obj.shortDescription;
+ description = DOMPurify.sanitize(obj.shortDescription);
} else if (obj.description) {
- description = obj.description.substring(0, 100) + "...";
+ description = DOMPurify.sanitize(obj.description.substring(0, 100) + "...");
}
Also applies to: 377-377
const userData = JSON.parse(localStorage.getItem("userData")); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Handle parsing errors when retrieving userData
from localStorage
When parsing userData
from localStorage
, there is a risk of encountering SyntaxError
if the data is not valid JSON. Wrap the parsing logic in a try-catch block to handle potential errors gracefully.
Apply this diff:
- const userData = JSON.parse(localStorage.getItem("userData"));
+ let userData = null;
+ try {
+ userData = JSON.parse(localStorage.getItem("userData"));
+ } catch (error) {
+ console.error("Error parsing userData from localStorage:", error);
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const userData = JSON.parse(localStorage.getItem("userData")); | |
let userData = null; | |
try { | |
userData = JSON.parse(localStorage.getItem("userData")); | |
} catch (error) { | |
console.error("Error parsing userData from localStorage:", error); | |
} |
// confirmDetails(formData); | ||
} | ||
} else if ( | ||
data?.responses[0]?.message?.order?.items[0].hasOwnProperty("xinput") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use Object.hasOwn()
instead of hasOwnProperty
Avoid accessing hasOwnProperty
directly from the object. Replace hasOwnProperty("xinput")
with Object.hasOwn(object, "xinput")
for safer property checks.
Apply this diff:
- data?.responses[0]?.message?.order?.items[0].hasOwnProperty("xinput")
+ Object.hasOwn(data?.responses[0]?.message?.order?.items[0], "xinput")
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
data?.responses[0]?.message?.order?.items[0].hasOwnProperty("xinput") | |
Object.hasOwn(data?.responses[0]?.message?.order?.items[0], "xinput") |
🧰 Tools
🪛 Biome
[error] 388-388: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
error.hasOwnProperty("response") && | ||
error.response.hasOwnProperty("data") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use Object.hasOwn()
instead of hasOwnProperty
Avoid accessing hasOwnProperty
directly from the object. Replace the checks with Object.hasOwn()
for safer property checks.
Apply this diff:
- if (
- error.hasOwnProperty("response") &&
- error.response.hasOwnProperty("data")
- ) {
+ if (
+ Object.hasOwn(error, "response") &&
+ Object.hasOwn(error.response, "data")
+ ) {
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome
[error] 501-501: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 502-502: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
const submitFormDetail = async (action, formData) => { | ||
setLoading(t("SUBMITTING")); | ||
trackReactGA(); | ||
|
||
try { | ||
const axiosResponse = await axios.create().post(action, urlencoded, { | ||
// Post formData (which includes file inputs) without manually setting the Content-Type | ||
const axiosResponse = await axios.create().post(action, formData, { | ||
headers: { | ||
"Content-Type": `application/x-www-form-urlencoded`, | ||
"Content-Type": "multipart/form-data", // Axios automatically manages the boundary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not manually set Content-Type
header when using FormData
with Axios
When uploading FormData
using Axios, you should not manually set the Content-Type
header to multipart/form-data
. Axios automatically sets the correct headers, including the boundary, which is crucial for multipart/form-data
requests. Manually setting it might cause issues with the request.
Apply this diff to let Axios handle the Content-Type
header:
const axiosResponse = await axios.create().post(action, formData, {
headers: {
- "Content-Type": "multipart/form-data", // Axios automatically manages the boundary
},
});
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const submitFormDetail = async (action, formData) => { | |
setLoading(t("SUBMITTING")); | |
trackReactGA(); | |
try { | |
const axiosResponse = await axios.create().post(action, urlencoded, { | |
// Post formData (which includes file inputs) without manually setting the Content-Type | |
const axiosResponse = await axios.create().post(action, formData, { | |
headers: { | |
"Content-Type": `application/x-www-form-urlencoded`, | |
"Content-Type": "multipart/form-data", // Axios automatically manages the boundary | |
const submitFormDetail = async (action, formData) => { | |
setLoading(t("SUBMITTING")); | |
trackReactGA(); | |
try { | |
// Post formData (which includes file inputs) without manually setting the Content-Type | |
const axiosResponse = await axios.create().post(action, formData, { | |
headers: { |
// Auto-fill input fields based on the mapping | ||
inputFields.forEach((input) => { | ||
const userDataKey = mapping[input.name]; // Match input name to userData key | ||
if (userDataKey && userData[userDataKey] !== undefined) { | ||
if (input.type === "checkbox") { | ||
input.checked = userData[userDataKey]; | ||
} else { | ||
input.value = userData[userDataKey]; | ||
} | ||
} | ||
} | ||
}); | ||
|
||
// Auto-fill select fields (e.g., gender) | ||
selectFields.forEach((select) => { | ||
const userDataKey = mapping[select.name]; // Match select name to userData key | ||
if (userDataKey && userData[userDataKey] !== undefined) { | ||
select.value = userData[userDataKey]; | ||
} | ||
}); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sanitize user data before populating form fields to prevent XSS attacks
Auto-filling form fields with data from userData
retrieved from localStorage
without sanitization can lead to Cross-Site Scripting (XSS) vulnerabilities if the data is compromised or manipulated. Ensure that the data is properly sanitized before injecting it into the DOM.
Consider sanitizing user inputs before using them. For example:
- input.value = userData[userDataKey];
+ input.value = sanitize(userData[userDataKey]);
Implement a sanitize
function or use a library to sanitize the inputs.
Committable suggestion skipped: line range outside the PR's diff.
Merge release-2.8.2 into Master
Quality Gate failedFailed conditions See analysis details on SonarQube Cloud Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE |
#Merge
Master
intofeat-sonarcloud
I have ensured that following
Pull Request Checklist
is taken care of before sending this PRSummary by CodeRabbit
New Features
LandingPage
andFeatureCard
components for improved structure and reusability.AutomatedForm
component for better user data handling.Bug Fixes
AutomatedForm
component.List
component.Documentation
Chores