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.1
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
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,11 @@ import { useNavigate } from "react-router-dom";

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

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

const {
dateOfBirthSchema,
Expand Down Expand Up @@ -53,23 +58,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 +93,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 +235,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
35 changes: 35 additions & 0 deletions apps/front-end/src/store/Slices/dataTableSlice.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { createAsyncThunk, createSlice } from "@reduxjs/toolkit";

export const fetchUsers = createAsyncThunk('users/fetchUsers', async () => {
const response = await fetch('https://dummyjson.com/users');
const data = await response.json();
return data;
});


const dataTableSlice = createSlice({

name :"dataTable",
initialState:{data:null,status: 'idle',
error: null,},
reducers:{},
extraReducers: (builder) => {
builder
.addCase(fetchUsers.pending, (state) => {
state.status = 'loading';
})
.addCase(fetchUsers.fulfilled, (state, action) => {
state.status = 'succeeded';
state.data = action.payload;
})
.addCase(fetchUsers.rejected, (state, action) => {
state.status = 'failed';
state.error = action.error.message;
});
},
})

export const {getData} = dataTableSlice.actions
export const selectdataTable = (state) => state?.dataTable;

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

export const fetchIpUserData = createAsyncThunk(
"ipData/fetchIpUserData",
async () => {
const result = facilitatorRegistryService.getInfo();
const data = await result;
return data;
}
Comment on lines +5 to +11
Copy link

Choose a reason for hiding this comment

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

The fetchIpUserData async thunk should handle potential errors in the data fetching process to avoid uncaught promises.

  async () => {
+   try {
      const result = facilitatorRegistryService.getInfo();
      const data = await result;
      return data;
+   } catch (error) {
+     // Handle or throw the error appropriately
+   }
  }

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 fetchIpUserData = createAsyncThunk(
"ipData/fetchIpUserData",
async () => {
const result = facilitatorRegistryService.getInfo();
const data = await result;
return data;
}
export const fetchIpUserData = createAsyncThunk(
"ipData/fetchIpUserData",
async () => {
try {
const result = facilitatorRegistryService.getInfo();
const data = await result;
return data;
} catch (error) {
// Handle or throw the error appropriately
}
}

</details>
<!-- suggestion_end -->

<!-- This is an auto-generated comment by CodeRabbit -->

);

const ip_ListSlice = createSlice({
name: "ipData",
initialState: {
data: (await get("ipData")) || null,
status: "idle",
error: null,
},
Comment on lines +14 to +20
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.

  initialState: {
-   data: (await get("ipData")) || null,
+   data: null, // Set initial state to null or a synchronous value
    status: "idle",
    error: null,
  },

You will need to handle the asynchronous retrieval of ipData from IndexedDB elsewhere, possibly in a component's useEffect hook or an 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
const ip_ListSlice = createSlice({
name: "ipData",
initialState: {
data: (await get("ipData")) || null,
status: "idle",
error: null,
},
const ip_ListSlice = createSlice({
name: "ipData",
initialState: {
data: null, // Set initial state to null or a synchronous value
status: "idle",
error: null,
},


reducers: {},
extraReducers: (builder) => {
builder
.addCase(fetchIpUserData.pending, (state) => {
state.status = "loading";
})
.addCase(fetchIpUserData.fulfilled, (state, action) => {
state.status = "succeeded";
state.data = action.payload;
set("ipData", 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.

Logging errors to the console is not a robust error handling strategy for production code. Consider dispatching an action to store the error in the state or implementing a more user-friendly error reporting mechanism.

- console.error("Error setting data:", error);
+ // Dispatch an action to store the error in the state or handle it appropriately

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("ipData", action.payload).catch((error) => {
console.error("Error setting data:", error);
});
set("ipData", action.payload).catch((error) => {
// Dispatch an action to store the error in the state or handle it appropriately
});

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

export const getData = ip_ListSlice.actions;
export const selectedIpData = (state) => state?.ipData;

export default ip_ListSlice.reducer;
24 changes: 24 additions & 0 deletions apps/front-end/src/store/Slices/userSlice.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// userSlice.js
import { createSlice } from '@reduxjs/toolkit';
import { get, set } from 'idb-keyval';

const userSlice = createSlice({
name: 'user',
initialState: {data: await get("data") || {} },
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.

- initialState: {data: await get("data") || {} },
+ initialState: {data: {} },

You will need to dispatch an action to populate this state after the store has been created.


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
initialState: {data: await get("data") || {} },
initialState: {data: {} },

reducers: {
setUser: (state, action) => {
state.data = action.payload;
set('data', action.payload)
.then(() => {
console.log('Data set successfully');
})
.catch((error) => {
console.error('Error setting data:', error);
});
Comment on lines +9 to +17
Copy link

Choose a reason for hiding this comment

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

The reducer setUser performs an asynchronous operation with IndexedDB which is not recommended. Reducers should be pure functions without side effects.

- set('data', action.payload)
- .then(() => {
-     console.log('Data set successfully');
- })
- .catch((error) => {
-     console.error('Error setting data:', error);
- });
+ // Move IndexedDB operations to a middleware or an async thunk.

Consider using an async thunk for IndexedDB operations.


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
setUser: (state, action) => {
state.data = action.payload;
set('data', action.payload)
.then(() => {
console.log('Data set successfully');
})
.catch((error) => {
console.error('Error setting data:', error);
});
setUser: (state, action) => {
state.data = action.payload;
// Move IndexedDB operations to a middleware or an async thunk.
}

},
},
});

export const { setUser } = userSlice.actions;
export const selectUser = (state) => state?.user?.data;
export default userSlice.reducer;
14 changes: 14 additions & 0 deletions apps/front-end/src/store/rootReducer.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// mainSlice.js
import { combineReducers } from "@reduxjs/toolkit";
import userReducer from "./Slices/userSlice";
import dataTableReducer from "./Slices/dataTableSlice";
import IpUserInfoSlice from "./Slices/ipUserInfoSlice";
Copy link

Choose a reason for hiding this comment

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

The import IpUserInfoSlice should follow the camelCase naming convention for non-component variables.

- import IpUserInfoSlice from "./Slices/ipUserInfoSlice";
+ import ipUserInfoSlice from "./Slices/ipUserInfoSlice";

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
import IpUserInfoSlice from "./Slices/ipUserInfoSlice";
import ipUserInfoSlice from "./Slices/ipUserInfoSlice";


const rootReducer = combineReducers({
user: userReducer,
dataTable: dataTableReducer,
ipUserInfo: IpUserInfoSlice,
// Add more child slices as needed
});

export default rootReducer;
Copy link

Choose a reason for hiding this comment

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

The import for IpUserInfoSlice should follow the camelCase naming convention for consistency with other imports.

- import IpUserInfoSlice from "./Slices/ipUserInfoSlice";
+ import ipUserInfoReducer from "./Slices/ipUserInfoSlice";

Then, update the combineReducers call to use the corrected import name:

  ipUserInfo: IpUserInfoSlice,
+ ipUserInfo: ipUserInfoReducer,

11 changes: 11 additions & 0 deletions apps/front-end/src/store/store.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// store.js
import { configureStore } from "@reduxjs/toolkit";
// import rootReducer from './rootReducer';
import rootReducer from "./rootReducer";

const store = configureStore({
reducer: rootReducer,
// Add middleware or enhancers as needed
});

export default store;
3 changes: 2 additions & 1 deletion lib/common-lib/src/components/layout/AppBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import {
InputRightAddon,
Button,
Modal,
Image
Image,
Alert
// Divider
} from 'native-base'
import 'react-modern-drawer/dist/index.css'
Expand Down
Loading