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

Task #211821 Redux Tool-kit in project #1010

Draft
wants to merge 11 commits into
base: release-2.8.0
Choose a base branch
from
2 changes: 1 addition & 1 deletion apps/front-end/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"react-native-safe-area-context": "^3.4.1",
"react-native-web": "^0.17.7",
"react-qr-reader": "3.0.0-beta-1",
"react-redux": "^9.1.0",
"react-redux": "8.0.0",
"react-router-dom": "^6.15.0",
"react-scripts": "5.0.0",
"react-zoom-pan-pinch": "^3.1.0",
Expand Down
6 changes: 4 additions & 2 deletions apps/front-end/src/bootstrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import "./index.css";
import App from "./App";
import * as serviceWorkerRegistration from "./serviceWorkerRegistration";
import reportWebVitals from "./reportWebVitals";
import { Provider } from "react-redux";
import store from "store/store";

ReactDOM.render(
<React.StrictMode>
<Provider store={store}>
<App />
</React.StrictMode>,
</Provider>,
document.getElementById("root")
);

Expand Down
10 changes: 9 additions & 1 deletion apps/front-end/src/pages/front-end/BenificiaryListView.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ import {
CardComponent,
} from "@shiksha/common-lib";
import { HStack, VStack, Box, Select, Pressable } from "native-base";
import React from "react";
import React, { useEffect } from "react";
import { useNavigate } from "react-router-dom";
import { ChipStatus } from "component/BeneficiaryStatus";
import InfiniteScroll from "react-infinite-scroll-component";
import Clipboard from "component/Clipboard";
import Chip from "component/Chip";
import { useDispatch } from "react-redux";
import { fetchLearnerData } from "store/Slices/LearnerSlice";

const LearnerMessage = ({ program_beneficiaries }) => {
const [reason, setReason] = React.useState({});
Expand Down Expand Up @@ -242,6 +244,12 @@ export default function PrerakListView({ userTokenInfo, footerLinks }) {
const ref = React.useRef(null);
const fa_id = localStorage.getItem("id");

const dispatch = useDispatch();

useEffect(() => {
dispatch(fetchLearnerData());
}, []);

React.useEffect(async () => {
const data = await benificiaryRegistoryService.getStatusList();
if (data.length > 0) {
Expand Down
64 changes: 45 additions & 19 deletions apps/front-end/src/pages/front-end/Dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,23 @@ import {
Alert,
Modal,
CloseIcon,
Menu,
Pressable,
Select,
BodyLarge,
CheckIcon,
} from "native-base";
import React, { useState, useEffect } from "react";
import { useTranslation } from "react-i18next";
import { useNavigate } from "react-router-dom";
import moment from "moment";
import {
fetchEnumListData,
selectenumData,
} from "store/Slices/commonSlices/enumListSlice";
import { useDispatch, useSelector } from "react-redux";
import {
fetchLearnerData,
selectedLearnerData,
} from "store/Slices/LearnerSlice";
// import { useLanguage } from "component/common_components/i18n-new";

const styles = {
inforBox: {
Expand All @@ -54,7 +61,6 @@ const styles = {
};

export default function Dashboard({ userTokenInfo, footerLinks }) {
const { t } = useTranslation();
const [facilitator, setFacilitator] = React.useState({ notLoaded: true });
const [certificateData, setCertificateData] = React.useState({});
const [loading, setLoading] = React.useState(true);
Expand All @@ -63,7 +69,7 @@ export default function Dashboard({ userTokenInfo, footerLinks }) {
const [modalVisible, setModalVisible] = React.useState(false);
const fa_id = localStorage.getItem("id");
const [isEventActive, setIsEventActive] = React.useState(false);
const [lmsDEtails, setLmsDetails] = React.useState();
const [lmsDetails, setLmsDetails] = React.useState();
const { id } = userTokenInfo?.authUser || [];
const [random, setRandom] = React.useState();
const [events, setEvents] = React.useState("");
Expand All @@ -81,6 +87,19 @@ export default function Dashboard({ userTokenInfo, footerLinks }) {
const [selectCohortForm, setSelectCohortForm] = useState(false);
const [academicYear, setAcademicYear] = useState(null);
const [academicData, setAcademicData] = useState([]);
const dispatch = useDispatch();
const data = useSelector(selectenumData);
// const { selectedLanguage, changeLanguage } = useLanguage();
const { t } = useTranslation();

const userInfoLearner = useSelector(selectedLearnerData);

useEffect(() => {
if (!data?.data) {
dispatch(fetchEnumListData());
}
dispatch(fetchLearnerData());
}, []);

useEffect(() => {
async function fetchData() {
Expand Down Expand Up @@ -369,6 +388,13 @@ export default function Dashboard({ userTokenInfo, footerLinks }) {
>
<VStack bg="primary.50" pb="5" style={{ zIndex: -1 }}>
<VStack space="5">
{/* <select
value={selectedLanguage}
onChange={(e) => changeLanguage(e.target.value)}
>
<option value="en">English</option>
<option value="hi">हिन्दी</option>
</select> */}
Comment on lines +391 to +397
Copy link

Choose a reason for hiding this comment

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

The commented-out code for language selection should be removed if it's not intended for use.

Remove the commented-out code to keep the codebase clean and maintainable.

{facilitator?.status === "applied" && (
<InfoBox status={facilitator?.status} progress={progress} />
)}
Expand Down Expand Up @@ -416,7 +442,7 @@ export default function Dashboard({ userTokenInfo, footerLinks }) {
</FrontEndTypo.Primarybutton>
</HStack>
)
: lmsDEtails?.id && (
: lmsDetails?.id && (
<HStack py="2" flex="1" px="4">
<FrontEndTypo.Primarybutton
fontSize
Expand Down Expand Up @@ -444,27 +470,27 @@ export default function Dashboard({ userTokenInfo, footerLinks }) {
</Modal.Header>
<Modal.Body alignItems="center">
<VStack>
{lmsDEtails === undefined && (
{lmsDetails === undefined && (
<AdminTypo.H3 color="textGreyColor.500">
{t(events)}
</AdminTypo.H3>
)}
{lmsDEtails?.certificate_status === null ? (
{lmsDetails?.certificate_status === null ? (
<AdminTypo.H3 color="textGreyColor.500">
{t("CERTIFICATION_IS_PENDING")}
</AdminTypo.H3>
) : lmsDEtails?.certificate_status === false &&
lmsDEtails?.score >= floatValue ? (
) : lmsDetails?.certificate_status === false &&
lmsDetails?.score >= floatValue ? (
<AdminTypo.H3 color="textGreyColor.500">
{t(`TRAINING_INCOMPLETE`)}
{lmsDEtails.score + "%"}
{lmsDetails.score + "%"}
</AdminTypo.H3>
) : lmsDEtails?.certificate_status === true ? (
) : lmsDetails?.certificate_status === true ? (
<AdminTypo.H3 color="textGreyColor.500">
{t(`TRAINING_TEST_DOWNLOAD_CERTIFICATE`)}
{lmsDEtails.score + "%"}
{lmsDetails.score + "%"}
</AdminTypo.H3>
) : lmsDEtails?.certificate_status === false ? (
) : lmsDetails?.certificate_status === false ? (
Comment on lines +473 to +493
Copy link

Choose a reason for hiding this comment

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

There are multiple conditional renderings based on lmsDetails properties. This can be simplified for better readability.

Refactor the conditional rendering logic to reduce complexity and improve readability.

<AdminTypo.H3 color="textGreyColor.500">
{t("TRAINING_NOT_PASSED")}
</AdminTypo.H3>
Expand All @@ -475,8 +501,8 @@ export default function Dashboard({ userTokenInfo, footerLinks }) {
</Modal.Body>
<Modal.Footer alignSelf={"center"}>
<HStack space={"6"}>
{lmsDEtails === undefined ||
(lmsDEtails?.certificate_status === true && (
{lmsDetails === undefined ||
(lmsDetails?.certificate_status === true && (
<FrontEndTypo.DefaultButton
textColor={"black"}
onPress={() => {
Expand All @@ -486,7 +512,7 @@ export default function Dashboard({ userTokenInfo, footerLinks }) {
{t("GO_BACK")}
</FrontEndTypo.DefaultButton>
))}
{lmsDEtails?.certificate_status === false && (
{lmsDetails?.certificate_status === false && (
<FrontEndTypo.DefaultButton
background={"textRed.400"}
onPress={() => {
Comment on lines 512 to 518
Copy link

Choose a reason for hiding this comment

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

Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [497-531]

The conditional rendering inside Modal.Footer is complex and hard to read.

Consider refactoring this section to simplify the logic and improve readability, possibly by breaking it down into smaller components or using a more declarative approach.

Expand All @@ -496,7 +522,7 @@ export default function Dashboard({ userTokenInfo, footerLinks }) {
{t("OK")}
</FrontEndTypo.DefaultButton>
)}
{lmsDEtails === undefined &&
{lmsDetails === undefined &&
!(
certificateData?.params?.do_id == null ||
(Array.isArray(certificateData?.params?.do_id) &&
Expand All @@ -509,7 +535,7 @@ export default function Dashboard({ userTokenInfo, footerLinks }) {
{t("START_TEST")}
</FrontEndTypo.DefaultButton>
)}
{lmsDEtails?.certificate_status === true && (
{lmsDetails?.certificate_status === true && (
<FrontEndTypo.DefaultButton
background={"textRed.400"}
onPress={() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import React, { useState, useCallback, useEffect } from "react";
import validator from "@rjsf/validator-ajv8";
import { get, set } from "idb-keyval";
import moment from "moment";
import { useDispatch, useSelector } from "react-redux";
import {
widgets,
templates,
Expand All @@ -19,7 +20,10 @@ import { useNavigate } from "react-router-dom";

import * as formSchemas from "./onboarding.schema";

import { debounce } from "lodash";
import {
fetchIpUserData,
selectedIpData,
} from "../../../../store/Slices/ipUserInfoSlice";

const {
dateOfBirthSchema,
Expand Down Expand Up @@ -53,23 +57,25 @@ const FacilitatorOnboarding = () => {

const [countLoad, setCountLoad] = useState(0);

const dispatch = useDispatch();
const ipData = useSelector(selectedIpData);

useEffect(() => {
dispatch(fetchIpUserData());
}, []);

useEffect(() => {
const fetchData = async () => {
try {
console.log("Fetching data from IndexedDB...");
let userData = await get("user_data");
console.log("Fetched data:", userData);

// Update form data states here
setUserData(userData);

// Example of updating date of birth state
if (userData?.users?.dob) {
set_date_of_birth({
dob: userData.users.dob,
mobile: userData.users.mobile,
alternative_mobile_number: userData.users.alternative_mobile_number,
// Add other properties as needed
});
}

Comment on lines 57 to 81
Copy link

Choose a reason for hiding this comment

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

Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [67-98]

The second useEffect hook is fetching user data from IndexedDB. Ensure that error handling is robust and consider the implications of setting state directly from IndexedDB without going through Redux if the state is meant to be globally accessible.

Consider refactoring to use Redux for all state management, including asynchronous operations with IndexedDB.

Expand All @@ -86,15 +92,12 @@ const FacilitatorOnboarding = () => {
});
}
if (userData?.users?.gender) {
// Example of updating basic details state
setFormDataBasicDetails({
gender: userData.users.gender,
marital_status: userData.extended_users?.marital_status || "", // Add other properties as needed
marital_status: userData.extended_users?.marital_status || "",
});
}

// Update other form data states in a similar way

setCountLoad(2);
} catch (error) {
console.error("Error fetching data from IndexedDB:", error);
Expand Down Expand Up @@ -231,17 +234,14 @@ const FacilitatorOnboarding = () => {
const experienceArray = Array.isArray(user_data.experience)
? user_data.experience
: [];
console.log(experienceArray, "Experience");
const updatedUserData = {
...user_data,
experience: [...experienceArray, ...newExperiences],
};
console.log(updatedUserData.experience, "After Update");

await set("user_data", updatedUserData);
setUserData(updatedUserData);
setPage((prevPage) => prevPage + 1);
console.log("hi");
handleNextScreen("jobExperience");
};

Expand Down
39 changes: 39 additions & 0 deletions apps/front-end/src/store/Slices/LearnerSlice.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { benificiaryRegistoryService } from "@shiksha/common-lib";
import { createAsyncThunk, createSlice } from "@reduxjs/toolkit";
import { get, set } from "idb-keyval";

export const fetchLearnerData = createAsyncThunk("learnerData", async () => {
const data = await benificiaryRegistoryService.getBeneficiariesList();
return data;
});

const learnerSlice = createSlice({
name: "learnerData",
initialState: {
data: (await get("learnerData")) || null,
Copy link

Choose a reason for hiding this comment

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

Using await outside of an async function is not valid. The initialState should not perform asynchronous operations.

- data: (await get("learnerData")) || null,
+ data: null, // initialState should be synchronous; consider setting this in an effect or async thunk

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.

Suggested change
data: (await get("learnerData")) || null,
data: null, // initialState should be synchronous; consider setting this in an effect or async thunk

status: "idle",
error: null,
},
reducers: {},
extraReducers: (builder) => {
builder
.addCase(fetchLearnerData.pending, (state) => {
state.status = "loading";
})
.addCase(fetchLearnerData.fulfilled, (state, action) => {
state.status = "succeeded";
state.data = action.payload;
set("learnerData", action.payload)
.then(() => console.log("Data stored successfully in IndexedDB"))
.catch((error) => console.error("Error setting data:", error));
Comment on lines +26 to +28
Copy link

Choose a reason for hiding this comment

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

Logging to the console in production code is not recommended due to potential information leakage and performance impact.

- .then(() => console.log("Data stored successfully in IndexedDB"))
- .catch((error) => console.error("Error setting data:", error));
+ .then(() => {
+   // Handle success scenario, possibly with a state update
+ })
+ .catch((error) => {
+   // Handle error scenario, possibly with a state update or error logging mechanism
+ });

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.

Suggested change
set("learnerData", action.payload)
.then(() => console.log("Data stored successfully in IndexedDB"))
.catch((error) => console.error("Error setting data:", error));
set("learnerData", action.payload)
.then(() => {
// Handle success scenario, possibly with a state update
})
.catch((error) => {
// Handle error scenario, possibly with a state update or error logging mechanism
});

})
.addCase(fetchLearnerData.rejected, (state, action) => {
state.status = "failed";
state.error = action.error.message;
});
},
});
export const getData = learnerSlice.actions;
Copy link

Choose a reason for hiding this comment

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

The getData export is misleading as it suggests a data retrieval operation, but it actually exports the actions of the slice.

- export const getData = learnerSlice.actions;
+ export const learnerActions = learnerSlice.actions;

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.

Suggested change
export const getData = learnerSlice.actions;
export const learnerActions = learnerSlice.actions;

export const selectedLearnerData = (state) => state?.learnerData;

export default learnerSlice.reducer;
45 changes: 45 additions & 0 deletions apps/front-end/src/store/Slices/commonSlices/enumListSlice.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { createAsyncThunk, createSlice } from "@reduxjs/toolkit";
import { enumRegistryService } from "@shiksha/common-lib";
import { get, set } from "idb-keyval";

export const fetchEnumListData = createAsyncThunk(
"enum/fetchEnumListData",
async () => {
const result = await enumRegistryService.listOfEnum();
const data = await result?.data;
return data;
}
);

const enumListSlice = createSlice({
name: "enumData",
initialState: {
data: (await get("enumData")) || null,
Copy link

Choose a reason for hiding this comment

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

Using await outside of an async function is not valid. The initialState should not perform asynchronous operations.

- data: (await get("enumData")) || null,
+ data: null, // initialState should be a static value

Consider initializing data to null or a static value and then populating it with enumData from IndexedDB inside an effect when the component using this slice is mounted.


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.

Suggested change
data: (await get("enumData")) || null,
data: null, // initialState should be a static value

status: "idle",
error: null,
},

reducers: {},
extraReducers: (builder) => {
builder
.addCase(fetchEnumListData.pending, (state) => {
state.status = "loading";
})
.addCase(fetchEnumListData.fulfilled, (state, action) => {
state.status = "succeeded";
state.data = action.payload;
set("enumData", action.payload).catch((error) => {
console.error("Error setting data:", error);
});
Comment on lines +31 to +33
Copy link

Choose a reason for hiding this comment

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

Catching an error and logging it to the console is not a robust error handling strategy.

- set("enumData", action.payload).catch((error) => {
-   console.error("Error setting data:", error);
- });
+ try {
+   await set("enumData", action.payload);
+ } catch (error) {
+   // Handle error appropriately, possibly updating the state to reflect the error
+ }

Replace the .catch with a try...catch block and handle the error more effectively, possibly by updating the state to reflect that an error occurred during the data setting process.


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.

Suggested change
set("enumData", action.payload).catch((error) => {
console.error("Error setting data:", error);
});
try {
await set("enumData", action.payload);
} catch (error) {
// Handle error appropriately, possibly updating the state to reflect the error
}

})
.addCase(fetchEnumListData.rejected, (state, action) => {
state.status = "failed";
state.error = action.error.message;
});
},
});

export const { getData } = enumListSlice.actions;
Copy link

Choose a reason for hiding this comment

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

The getData action is exported but not defined within the reducers object.

Remove the getData export if it's not being used, or define it properly within the reducers object.

export const selectenumData = (state) => state?.enumData;

export default enumListSlice.reducer;
Loading