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.

Sign up for GitHub

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

Hcmpre 1290 #1834

Merged
merged 7 commits into from
Nov 15, 2024
Merged

Hcmpre 1290 #1834

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3f88737
fixed app delivery condition
Bhavya-egov Nov 13, 2024
34c2409
Merge branch 'console' of https://github.com/egovernments/DIGIT-Front…
Bhavya-egov Nov 14, 2024
c075921
added my microplan screen
Bhavya-egov Nov 14, 2024
a86b7f7
Merge branch 'console' into HCMPRE-1290
jagankumar-egov Nov 14, 2024
c79c4fd
Merge branch 'console' into HCMPRE-1290
jagankumar-egov Nov 15, 2024
f076ed0
changes in the url
Bhavya-egov Nov 15, 2024
978884b
changes
Bhavya-egov Nov 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions ...i/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignCard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const CampaignCard = () => {
}

const { t } = useTranslation();
const userId = Digit.UserService.getUser().info.uuid;
const microplanStatus = "RESOURCE_ESTIMATIONS_APPROVED"
Comment on lines +25 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Consider improving variable management and error handling.

  1. The hardcoded microplanStatus should be moved to a constants file for better maintainability.
  2. Add error handling for the user service call.
+ import { MICROPLAN_STATUSES } from "../constants";
+
  const CampaignCard = () => {
-   const userId = Digit.UserService.getUser().info.uuid;
-   const microplanStatus = "RESOURCE_ESTIMATIONS_APPROVED"
+   const userId = Digit.UserService.getUser()?.info?.uuid;
+   if (!userId) {
+     throw new Error("User information not available");
+   }
+   const microplanStatus = MICROPLAN_STATUSES.RESOURCE_ESTIMATIONS_APPROVED;

Committable suggestion skipped: line range outside the PR's diff.


let links = [

Expand All @@ -36,9 +38,9 @@ const CampaignCard = () => {
roles: ROLES.CAMPAIGN_MANAGER,
// count: isLoading?"-":data
},
{ //@Bhavya put the new url and remove the comment
{
label: t("ACTION_TEST_SETUP_CAMPAIGN_FROM_MICROPLAN"),
link: `/${window?.contextPath}/employee/campaign/setup-campaign`,
link: `/${window?.contextPath}/employee/campaign/setup-microplan&userId=${userId}&status=${microplanStatus}`,
Bhavya-egov marked this conversation as resolved.
Show resolved Hide resolved
roles: ROLES.CAMPAIGN_MANAGER
},
{
Expand All @@ -58,7 +60,7 @@ const CampaignCard = () => {
link: `/${window?.contextPath}/employee/campaign/boundary/home`,
roles: ROLES.BOUNDARY_MANAGER,
// count: isLoading?"-":data
},
}
];

links = links.filter((link) => (link?.roles && link?.roles?.length > 0 ? Digit.Utils.didEmployeeHasAtleastOneRole(link?.roles) : true));
Expand Down
16 changes: 13 additions & 3 deletions ...i/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignName.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,24 @@ import { ErrorMessage, FieldV1 , Stepper , TextBlock ,Card } from "@egovernments

const CampaignName = ({ onSelect, formData, control, formState, ...props }) => {
const { t } = useTranslation();
const [name, setName] = useState(props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName || "");

const [executionCount, setExecutionCount] = useState(0);
const [startValidation, setStartValidation] = useState(null);
const [error, setError] = useState(null);
const searchParams = new URLSearchParams(location.search);
const microplanName = searchParams.get("microName");
const source = searchParams.get("source");
Comment on lines +13 to +15
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add location import and consider browser compatibility

The location object is being used without being imported. Additionally, consider adding a fallback for browsers that don't support URLSearchParams.

Apply this diff to fix the issues:

- const searchParams = new URLSearchParams(location.search);
+ import { useLocation } from 'react-router-dom';
+ 
+ // In component:
+ const location = useLocation();
+ const searchParams = window.URLSearchParams ? new URLSearchParams(location.search) : new CustomURLSearchParams(location.search);

Committable suggestion skipped: line range outside the PR's diff.

const [name, setName] = useState(props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName || "");
useEffect(() => {
setName(props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName);
if(source === "microplan"){
const sessionName = props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName.replace(/&/g, "and");
if(sessionName === microplanName){
setName("");
}
}
else setName(props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName);
Comment on lines +16 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Simplify state management logic

The current implementation has complex nested conditions and string manipulation that could be simplified.

Consider this cleaner approach:

- const [name, setName] = useState(props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName || "");
+ const sessionName = props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName;
+ const [name, setName] = useState(sessionName || "");

  useEffect(() => {
-   if(source === "microplan"){
-         const sessionName = props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName.replace(/&/g, "and");
-         if(sessionName === microplanName){
-           setName("");
-         }
-       }
-   else setName(props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName);
+   const campaignName = props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName;
+   const shouldResetName = source === "microplan" && 
+     campaignName?.replace(/&/g, "and") === microplanName;
+   setName(shouldResetName ? "" : campaignName);
  }, [props?.props?.sessionData?.HCM_CAMPAIGN_NAME]);
📝 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.

Suggested change
const [name, setName] = useState(props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName || "");
useEffect(() => {
setName(props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName);
if(source === "microplan"){
const sessionName = props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName.replace(/&/g, "and");
if(sessionName === microplanName){
setName("");
}
}
else setName(props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName);
const sessionName = props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName;
const [name, setName] = useState(sessionName || "");
useEffect(() => {
const campaignName = props?.props?.sessionData?.HCM_CAMPAIGN_NAME?.campaignName;
const shouldResetName = source === "microplan" &&
campaignName?.replace(/&/g, "and") === microplanName;
setName(shouldResetName ? "" : campaignName);
}, [props?.props?.sessionData?.HCM_CAMPAIGN_NAME]);

}, [props?.props?.sessionData?.HCM_CAMPAIGN_NAME]);
const searchParams = new URLSearchParams(location.search);

const [currentStep , setCurrentStep] = useState(1);
const currentKey = searchParams.get("key");
const [key, setKey] = useState(() => {
Expand Down
5 changes: 4 additions & 1 deletion ...i/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignType.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const CampaignSelection = ({ onSelect, formData, formState, ...props }) => {
const searchParams = new URLSearchParams(location.search);
const [currentStep , setCurrentStep] = useState(1);
const currentKey = searchParams.get("key");
const source = searchParams.get("source");
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consolidate URL parameter handling into a custom hook

Following up on previous review feedback, the URL parameter handling should be extracted into a custom hook for better maintainability and reusability.

Create a new hook useCampaignParams:

const useCampaignParams = () => {
  const validSources = ["microplan"];
  const searchParams = new URLSearchParams(location.search);
  const rawSource = searchParams.get("source");
  
  return {
    source: validSources.includes(rawSource) ? rawSource : null,
    key: searchParams.get("key") ? parseInt(searchParams.get("key")) : 1,
    isMicroplan: () => rawSource === "microplan",
    updateParams: (params) => {
      const url = new URL(window.location.href);
      Object.entries(params).forEach(([key, value]) => {
        url.searchParams.set(key, value);
      });
      window.history.replaceState({}, "", url);
    }
  };
};

const [key, setKey] = useState(() => {
const keyParam = searchParams.get("key");
return keyParam ? parseInt(keyParam) : 1;
Expand Down Expand Up @@ -118,7 +119,7 @@ const CampaignSelection = ({ onSelect, formData, formState, ...props }) => {
<div
className="campaign-type-wrapper"
onClick={(e) => {
if (props?.props?.sessionData?.HCM_CAMPAIGN_TYPE?.projectType && !canUpdate) {
if (props?.props?.sessionData?.HCM_CAMPAIGN_TYPE?.projectType && !canUpdate && productType!=="microplan") {
Bhavya-egov marked this conversation as resolved.
Show resolved Hide resolved
setShowPopUp(true);
return;
}
Expand All @@ -143,6 +144,8 @@ const CampaignSelection = ({ onSelect, formData, formState, ...props }) => {
setStartValidation(true);
handleChange(value);
}}
disabled = {source === "microplan"}

Comment on lines +147 to +148
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove whitespace and extract condition into a function

  1. Remove the unnecessary whitespace after the disabled prop
  2. Extract the condition into a descriptive function as suggested in previous review
-  disabled = {source === "microplan"}
-  
+  disabled={checkIfMicroplan()}

Add this function at the component level:

const checkIfMicroplan = () => {
  return source === "microplan";
};

/>
{error?.message && <ErrorMessage message={t(error?.message)} showIcon={true} />}
</div>
Expand Down
78 changes: 66 additions & 12 deletions .../web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ export const UICustomizations = {
history.push(`/${window.contextPath}/employee/campaign/checklist/create?campaignName=${campaignName}&role=${role_code}&checklistType=${cl_code}&projectType=${projectType}&campaignId=${campaignId}`)
}}
/>
)
}
);
}
default:
return value;
}
Expand Down Expand Up @@ -299,11 +299,63 @@ export const UICustomizations = {
}}
/>
</>
)
);
}

},
},
MicroplanCampaignSearchConfig: {
preProcess: (data, additionalDetails) => {
const url = window.location.pathname;
const queryString = url.includes("?") ? url.split("?")[1] : url.split("&").slice(1).join("&");
const searchParams = new URLSearchParams(queryString);
const userId = searchParams.get("userId");
const status = searchParams.get("status");
data.body.PlanConfigurationSearchCriteria.userUuid = userId;
data.body.PlanConfigurationSearchCriteria.status = [status];
data.body.PlanConfigurationSearchCriteria.name = data?.state?.searchForm?.microplanName;
// data.body.PlanConfigurationSearchCriteria.campaignType = data?.state?.searchForm?.campaignType?.[0]?.code;
return data;
},
additionalCustomizations: (row, key, column, value, t, searchResult) => {
switch (key) {
case "NAME_OF_MICROPLAN":
if (value && value !== "NA") {
return (
<div
style={{
maxWidth: "15rem",
wordWrap: "break-word",
whiteSpace: "normal",
overflowWrap: "break-word",
}}
>
<p>{t(value)}</p>
</div>
);
} else {
return (
<div>
<p>{t("NA")}</p>
</div>
);
}

case "CAMPAIGN_TYPE":
if (value && value != "NA") {
return <p>{t(Digit.Utils.locale.getTransformedLocale("CAMPAIGN_TYPE_" + value))}</p>;
} else {
return (
<div>
<p>{t("NA")}</p>
</div>
);
}
case "LAST_MODIFIED_TIME":
return Digit.DateUtils.ConvertEpochToDate(value);
default:
return null; // Handle any unexpected keys here if needed
jagankumar-egov marked this conversation as resolved.
Show resolved Hide resolved
}
},
},
MyCampaignConfigOngoing: {
preProcess: (data, additionalDetails) => {
Expand Down Expand Up @@ -397,7 +449,6 @@ export const UICustomizations = {
setTimeline(true);
break;
case "ACTION_LABEL_CONFIGURE_APP":

window.history.pushState(
{
name: row?.campaignName,
Expand Down Expand Up @@ -715,7 +766,6 @@ export const UICustomizations = {
setTimeline(true);
break;


case "ACTION_LABEL_UPDATE_BOUNDARY_DETAILS":
window.history.pushState(
{
Expand All @@ -735,7 +785,7 @@ export const UICustomizations = {
name: row?.campaignName,
data: row,
projectId: row?.projectId,
campaignType: row?.projectType
campaignType: row?.projectType,
},
"",
`/${window.contextPath}/employee/campaign/checklist/search?name=${row?.campaignName}&campaignId=${row?.id}&projectType=${row?.projectType}`
Expand Down Expand Up @@ -886,7 +936,7 @@ export const UICustomizations = {
return value ? t("CM_UPDATE_REQUEST") : t("CM_CREATE_REQUEST");
case "CAMPAIGN_START_DATE":
return Digit.DateUtils.ConvertEpochToDate(value);
case "CAMPAIGN_END_DATE":
case "LAST_MODIFIED_TIME":
return Digit.DateUtils.ConvertEpochToDate(value);
default:
return "case_not_found";
Expand Down Expand Up @@ -977,8 +1027,8 @@ export const UICustomizations = {
setTimeline(true);
break;
case "ACTION_LABEL_RETRY":
retryCampaign(row,searchResult);
break;
retryCampaign(row, searchResult);
break;
Comment on lines +1030 to +1031
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

'searchResult' May Be Undefined in 'onActionSelect'

At lines 1030-1031, retryCampaign(row, searchResult); is called within the onActionSelect function, but searchResult is not defined in this scope. This could lead to a ReferenceError when onActionSelect is executed.

Consider passing searchResult as a parameter to onActionSelect or retrieving it within the function. Here's a possible fix:

- const onActionSelect = (value, row) => {
+ const onActionSelect = (value, row, searchResult) => {
    switch (value?.code) {
      case "ACTION_LABEL_VIEW_TIMELINE":
        setTimeline(true);
        break;
      case "ACTION_LABEL_RETRY":
        retryCampaign(row, searchResult);
        break;

Ensure that when onActionSelect is called, searchResult is passed appropriately.

Committable suggestion skipped: line range outside the PR's diff.

default:
console.log(value);
break;
Expand All @@ -993,7 +1043,8 @@ export const UICustomizations = {
</Link>
</span>
);

case "CM_DRAFT_TYPE":
return value ? t("CM_UPDATE_REQUEST") : t("CM_CREATE_REQUEST");
case "CAMPAIGN_START_DATE":
return Digit.DateUtils.ConvertEpochToDate(value);
case "CAMPAIGN_END_DATE":
Expand All @@ -1006,7 +1057,10 @@ export const UICustomizations = {
type="actionButton"
variation="secondary"
label={"Action"}
options={[{ key: 1, code: "ACTION_LABEL_VIEW_TIMELINE", i18nKey: t("ACTION_LABEL_VIEW_TIMELINE") },{ key: 2, code: "ACTION_LABEL_RETRY", i18nKey: t("ACTION_LABEL_RETRY") }].filter(obj=>Digit.Utils.didEmployeeHasAtleastOneRole(["SYSTEM_ADMINISTRATOR"]||obj?.key!=2))} //added retry for system adminstrator for failed campaign
options={[
{ key: 1, code: "ACTION_LABEL_VIEW_TIMELINE", i18nKey: t("ACTION_LABEL_VIEW_TIMELINE") },
{ key: 2, code: "ACTION_LABEL_RETRY", i18nKey: t("ACTION_LABEL_RETRY") },
].filter((obj) => Digit.Utils.didEmployeeHasAtleastOneRole(["SYSTEM_ADMINISTRATOR"] || obj?.key != 2))} //added retry for system adminstrator for failed campaign
Comment on lines +1060 to +1063
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct Logical Error in Filtering Action Options

The filter function in lines 1060-1063 may not work as intended due to incorrect use of the logical || operator. The condition should ensure that the 'Retry' option is only available to users with the SYSTEM_ADMINISTRATOR role.

Adjust the filter logic as follows:

- ].filter((obj) => Digit.Utils.didEmployeeHasAtleastOneRole(["SYSTEM_ADMINISTRATOR"] || obj?.key != 2))
+ ].filter((obj) => obj?.key !== 2 || Digit.Utils.didEmployeeHasAtleastOneRole(["SYSTEM_ADMINISTRATOR"]))

This change ensures that:

  • If the option's key is not 2 (not 'Retry'), it is included.
  • If the option's key is 2 (the 'Retry' action), it is included only if the user has the SYSTEM_ADMINISTRATOR role.
📝 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.

Suggested change
options={[
{ key: 1, code: "ACTION_LABEL_VIEW_TIMELINE", i18nKey: t("ACTION_LABEL_VIEW_TIMELINE") },
{ key: 2, code: "ACTION_LABEL_RETRY", i18nKey: t("ACTION_LABEL_RETRY") },
].filter((obj) => Digit.Utils.didEmployeeHasAtleastOneRole(["SYSTEM_ADMINISTRATOR"] || obj?.key != 2))} //added retry for system adminstrator for failed campaign
options={[
{ key: 1, code: "ACTION_LABEL_VIEW_TIMELINE", i18nKey: t("ACTION_LABEL_VIEW_TIMELINE") },
{ key: 2, code: "ACTION_LABEL_RETRY", i18nKey: t("ACTION_LABEL_RETRY") },
].filter((obj) => obj?.key !== 2 || Digit.Utils.didEmployeeHasAtleastOneRole(["SYSTEM_ADMINISTRATOR"]))} //added retry for system adminstrator for failed campaign

optionsKey="i18nKey"
showBottom={true}
isSearchable={false}
Expand Down
18 changes: 14 additions & 4 deletions .../web/micro-ui-internals/packages/modules/campaign-manager/src/configs/myCampaignConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,11 +524,16 @@ export const myCampaignConfig = {
additionalCustomization: true,
// disableSortBy: true,
},
// {
// label: "CAMPAIGN_END_DATE",
// jsonPath: "endDate",
// additionalCustomization: true,
// // disableSortBy: true,
// },
{
label: "CAMPAIGN_END_DATE",
jsonPath: "endDate",
additionalCustomization: true,
// disableSortBy: true,
label:"LAST_MODIFIED_TIME",
jsonPath:"auditDetails.lastModifiedTime",
additionalCustomization:true
jagankumar-egov marked this conversation as resolved.
Show resolved Hide resolved
},
],
enableGlobalSearch: false,
Expand Down Expand Up @@ -637,6 +642,11 @@ export const myCampaignConfig = {
jsonPath: "campaignName",
// additionalCustomization: true,
},
{
label: "CM_DRAFT_TYPE",
jsonPath: "parentId",
additionalCustomization: true,
},
Comment on lines +645 to +649
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Clarify CM_DRAFT_TYPE implementation and add documentation

The new CM_DRAFT_TYPE column uses parentId without clear documentation. Consider:

  1. Adding comments explaining the mapping between parentId and draft types
  2. Adding translation support if draft types need localization
 {
   label: "CM_DRAFT_TYPE",
   jsonPath: "parentId",
-  additionalCustomization: true,
+  additionalCustomization: true,
+  translate: true,
+  prefix: "CM_DRAFT_TYPE_",
+  // Maps parentId to specific draft types:
+  // null -> New Draft
+  // campaign-id -> Revision Draft
 },
📝 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.

Suggested change
{
label: "CM_DRAFT_TYPE",
jsonPath: "parentId",
additionalCustomization: true,
},
{
label: "CM_DRAFT_TYPE",
jsonPath: "parentId",
additionalCustomization: true,
translate: true,
prefix: "CM_DRAFT_TYPE_",
// Maps parentId to specific draft types:
// null -> New Draft
// campaign-id -> Revision Draft
},

{
label: "CAMPAIGN_TYPE",
jsonPath: "projectType",
Expand Down
100 changes: 100 additions & 0 deletions ...web/micro-ui-internals/packages/modules/campaign-manager/src/configs/myMicroplanConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@


const tenantId = Digit.ULBService.getCurrentTenantId();
const mdms_context_path = window?.globalConfigs?.getConfig("MDMS_V2_CONTEXT_PATH") || "mdms-v2";
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove unused variable declaration

The mdms_context_path variable is declared but never used in the configuration.

-  const mdms_context_path = window?.globalConfigs?.getConfig("MDMS_V2_CONTEXT_PATH") || "mdms-v2";
📝 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.

Suggested change
const mdms_context_path = window?.globalConfigs?.getConfig("MDMS_V2_CONTEXT_PATH") || "mdms-v2";

export const MicroplanCampaignSearchConfig = [
{
label: "MICROPLAN_SEARCH",
type: "search",
apiDetails: {
serviceName: "/plan-service/config/_search",
requestParam: {
},
requestBody: {
"PlanConfigurationSearchCriteria": {
"limit": 10,
"offset": 0,
"tenantId": tenantId,
},
},
masterName: "commonUiConfig",
moduleName: "MicroplanCampaignSearchConfig",
Copy link
Collaborator

Choose a reason for hiding this comment

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

just config, moduleName is differnt for microplan UI and here ?

minParametersForSearchForm: 0,
tableFormJsonPath: "requestBody.PlanConfigurationSearchCriteria.pagination",
// filterFormJsonPath: "requestBody.MdmsCriteria.customs",
searchFormJsonPath: "requestBody.PlanConfigurationSearchCriteria",
},
Comment on lines +9 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Enhance API configuration robustness

The API configuration could be improved in several ways:

  1. Consider adding error handling parameters
  2. Add request timeout configuration
  3. Include API version in the service name
 apiDetails: {
   serviceName: "/plan-service/config/_search", 
+  version: "v1",
+  timeout: 5000,
+  handleError: true,
   requestParam: {
+    timeout: 5000,
+    validateResponse: true
   },
   requestBody: {
     "PlanConfigurationSearchCriteria": {
       "limit": 10,
       "offset": 0,
-      "tenantId": tenantId,
+      "tenantId": Digit.ULBService.getCurrentTenantId(),
     },
   },

Committable suggestion skipped: line range outside the PR's diff.

sections: {
search: {
uiConfig: {
formClassName: "custom-both-clear-search",
primaryLabel: "ES_COMMON_SEARCH",
secondaryLabel: "ES_COMMON_CLEAR_SEARCH",
Comment on lines +30 to +32
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Use proper i18n label keys

The form labels should follow proper i18n naming conventions for better internationalization support.

-  primaryLabel: "ES_COMMON_SEARCH",
-  secondaryLabel: "ES_COMMON_CLEAR_SEARCH",
+  primaryLabel: "ES_CAMPAIGN_MICROPLAN_SEARCH_BTN",
+  secondaryLabel: "ES_CAMPAIGN_MICROPLAN_CLEAR_SEARCH_BTN",
📝 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.

Suggested change
formClassName: "custom-both-clear-search",
primaryLabel: "ES_COMMON_SEARCH",
secondaryLabel: "ES_COMMON_CLEAR_SEARCH",
formClassName: "custom-both-clear-search",
primaryLabel: "ES_CAMPAIGN_MICROPLAN_SEARCH_BTN",
secondaryLabel: "ES_CAMPAIGN_MICROPLAN_CLEAR_SEARCH_BTN",

minReqFields: 0,
defaultValues:{
microplanName: "",
campaignType: "",
},
fields: [
{
label: "NAME_OF_MICROPLAN",
isMandatory: false,
key: "microplanName",
type: "text",
populators: {
name: "microplanName"
},
},
Comment on lines +38 to +47
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Add input validation rules

Consider adding validation rules for the microplan name input to prevent invalid searches.

 {
   label: "NAME_OF_MICROPLAN",
   isMandatory: false,
   key: "microplanName",
   type: "text",
   populators: {
-    name: "microplanName"
+    name: "microplanName",
+    validation: {
+      pattern: "^[a-zA-Z0-9\\s-]{2,50}$",
+      minLength: 2,
+      maxLength: 50
+    }
   },
 },
📝 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.

Suggested change
fields: [
{
label: "NAME_OF_MICROPLAN",
isMandatory: false,
key: "microplanName",
type: "text",
populators: {
name: "microplanName"
},
},
fields: [
{
label: "NAME_OF_MICROPLAN",
isMandatory: false,
key: "microplanName",
type: "text",
populators: {
name: "microplanName",
validation: {
pattern: "^[a-zA-Z0-9\\s-]{2,50}$",
minLength: 2,
maxLength: 50
}
},
},

// {
// label: "CAMPAIGN_SEARCH_TYPE",
// type: "apidropdown",
// isMandatory: false,
// disable: false,
// populators: {
// optionsCustomStyle: {
// top: "2.3rem",
// },
// name: "campaignType",
// optionsKey: "code",
// allowMultiSelect: false,
// masterName: "commonUiConfig",
// moduleName: "MyCampaignConfigDrafts",
// customfn: "populateCampaignTypeReqCriteria",
// },
// },
],
},

show: true,
},
searchResult: {
uiConfig: {
columns: [
{
label: "NAME_OF_MICROPLAN",
jsonPath: "name",
additionalCustomization:true
},
{
label:"CAMPAIGN_TYPE",
jsonPath:"additionalDetails.campaignType",
additionalCustomization:true
},
{
label:"LAST_MODIFIED_TIME",
jsonPath:"auditDetails.lastModifiedTime",
additionalCustomization:true
},
],
resultsJsonPath: "PlanConfiguration",

enableColumnSort: true,
},
show: true,
},

},
},
];


32 changes: 32 additions & 0 deletions ...b/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/MyMicroplans.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { Header, InboxSearchComposer } from "@egovernments/digit-ui-react-components";
import React, { useState, useEffect } from "react";
import { useTranslation } from "react-i18next";
import { useHistory } from "react-router-dom";
import { MicroplanCampaignSearchConfig } from "../../configs/myMicroplanConfig";

const MyMicroplans = () => {
const { t } = useTranslation();
const history = useHistory();
const onClickRow = ({ original: row }) => {
console.log("row" , row);
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Remove console.log statement.

Remove debugging statements before deploying to production.

-    console.log("row" , row);
📝 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.

Suggested change
console.log("row" , row);

Bhavya-egov marked this conversation as resolved.
Show resolved Hide resolved
const updatedName = row.name.replace(/&/g, "and");
history.push(`/${window.contextPath}/employee/campaign/setup-campaign?id=${row.campaignId}&draft=true&fetchBoundary=true&draftBoundary=true&source=microplan&microName=${updatedName}`);
Comment on lines +25 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Consider enhancing string handling and URL construction.

The current implementation has potential areas for improvement:

  1. The string replacement might need more robust handling for special characters
  2. URL parameters should be properly encoded
-    const updatedName = row.name.replace(/&/g, "and");
-    history.push(`/${window.contextPath}/employee/campaign/setup-campaign?id=${row.campaignId}&draft=true&fetchBoundary=true&draftBoundary=true&source=microplan&microName=${updatedName}`);
+    const updatedName = encodeURIComponent(row.name.replace(/&/g, "and"));
+    const params = new URLSearchParams({
+      id: row.campaignId,
+      draft: "true",
+      fetchBoundary: "true",
+      draftBoundary: "true",
+      source: "microplan",
+      microName: updatedName
+    });
+    history.push(`/${window.contextPath}/employee/campaign/setup-campaign?${params}`);
📝 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.

Suggested change
const updatedName = row.name.replace(/&/g, "and");
history.push(`/${window.contextPath}/employee/campaign/setup-campaign?id=${row.campaignId}&draft=true&fetchBoundary=true&draftBoundary=true&source=microplan&microName=${updatedName}`);
const updatedName = encodeURIComponent(row.name.replace(/&/g, "and"));
const params = new URLSearchParams({
id: row.campaignId,
draft: "true",
fetchBoundary: "true",
draftBoundary: "true",
source: "microplan",
microName: updatedName
});
history.push(`/${window.contextPath}/employee/campaign/setup-campaign?${params}`);

};
return (
<React.Fragment>

<Header styles={{ fontSize: "32px" }}>{t("MY_MICROPLANS_HEADING")}</Header>
jagankumar-egov marked this conversation as resolved.
Show resolved Hide resolved
<div className="inbox-search-wrapper">
<InboxSearchComposer
configs={MicroplanCampaignSearchConfig?.[0]}
additionalConfig={{
resultsTable: {
onClickRow,
},
}}
></InboxSearchComposer>
</div>
Comment on lines +33 to +41
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Use self-closing tag for InboxSearchComposer.

Since the InboxSearchComposer component doesn't have any children, it should use a self-closing tag.

-        <InboxSearchComposer
-        configs={MicroplanCampaignSearchConfig?.[0]}
-        additionalConfig={{
-          resultsTable: {
-            onClickRow,
-          },
-        }}
-        ></InboxSearchComposer>
+        <InboxSearchComposer
+          configs={MicroplanCampaignSearchConfig?.[0]}
+          additionalConfig={{
+            resultsTable: {
+              onClickRow,
+            },
+          }}
+        />
📝 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.

Suggested change
<InboxSearchComposer
configs={MicroplanCampaignSearchConfig?.[0]}
additionalConfig={{
resultsTable: {
onClickRow,
},
}}
></InboxSearchComposer>
</div>
<InboxSearchComposer
configs={MicroplanCampaignSearchConfig?.[0]}
additionalConfig={{
resultsTable: {
onClickRow,
},
}}
/>
</div>
🧰 Tools
🪛 Biome

[error] 20-27: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.

Unsafe fix: Use a SelfClosingElement instead

(lint/style/useSelfClosingElements)

</React.Fragment>
);
};
export default MyMicroplans;
Loading