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

Jump to bottom

project type config simplified #1539

Merged
merged 3 commits into from
Oct 17, 2024
Merged

project type config simplified #1539

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
52bd9d6
project type config simplified
Bhavya-egov Oct 17, 2024
d39e5d9
enchanced code readability
Bhavya-egov Oct 17, 2024
1999a4f
Merge branch 'console' into HCMPRE-866
jagankumar-egov Oct 17, 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
120 changes: 84 additions & 36 deletions ...eb/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignSummary.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,44 +85,92 @@ function reverseDeliveryRemap(data, t) {
let currentCycleIndex = null;
let currentCycle = null;

// data.forEach((item, index) => {
// if (currentCycleIndex !== item.cycleNumber) {
// currentCycleIndex = item.cycleNumber;
// currentCycle = {
// cycleIndex: currentCycleIndex.toString(),
// startDate: item?.startDate ? Digit.Utils.date.convertEpochToDate(item?.startDate) : null,
// endDate: item?.endDate ? Digit.Utils.date.convertEpochToDate(item?.endDate) : null,
// active: index === 0, // Initialize active to false
// deliveries: [],
// };
// reversedData.push(currentCycle);
// }
const operatorMapping = {
"<=": "LESS_THAN_EQUAL_TO",
">=": "GREATER_THAN_EQUAL_TO",
"<": "LESS_THAN",
">": "GREATER_THAN",
"==": "EQUAL_TO",
"!=": "NOT_EQUAL_TO",
"IN_BETWEEN": "IN_BETWEEN"
};

const cycles = data?.[0]?.cycles || [];
const transformedCycles = cycles.map((cycle) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

make them into multiple readble functions

const deliveries = cycle.deliveries?.map((delivery, deliveryIndex) => {
const doseCriteria = delivery.doseCriteria?.flatMap((criteria, ruleKey) => {
const products = criteria.ProductVariants.map((variant, key) => ({
key: key + 1,
count: 1,
value: variant.productVariantId,
name: variant.name
}));

const condition = criteria.condition;
let conditionParts = condition.split("and").map(part => part.trim());
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

Use const instead of let for conditionParts

The variable conditionParts is declared with let but is never reassigned. Using const for variables that are not reassigned can prevent unintended mutations and improve code readability.

Apply this fix:

-                let conditionParts = condition.split("and").map(part => part.trim()); 
+                const conditionParts = condition.split("and").map(part => part.trim());
📝 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
let conditionParts = condition.split("and").map(part => part.trim());
const conditionParts = condition.split("and").map(part => part.trim());
🧰 Tools
🪛 Biome

[error] 110-110: This let declares a variable that is only assigned once.

'conditionParts' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

let rules = [];
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

Use const instead of let for rules

The variable rules is declared with let but is never reassigned. Consider using const for variables that are not reassigned.

Apply this fix:

-                let rules = [];
+                const rules = [];
📝 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
let rules = [];
const rules = [];
🧰 Tools
🪛 Biome

[error] 111-111: This let declares a variable that is only assigned once.

'rules' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

conditionParts.forEach((part) => {
const parts = part.split(' ').filter(Boolean);

let attributes = [];
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

Use const instead of let for attributes

The variable attributes is declared with let but is never reassigned. It's best practice to use const for variables that do not change.

Apply this fix:

-                    let attributes = [];
+                    const attributes = [];
📝 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
let attributes = [];
const attributes = [];
🧰 Tools
🪛 Biome

[error] 115-115: This let declares a variable that is only assigned once.

'attributes' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

if (parts.length === 5 && (parts[1] === "<=" || parts[1] === "<") && (parts[3] === "<" || parts[3] === "<=")) {
const toValue = parts[0];
const fromValue = parts[4];
attributes.push({
key: 1, // Incrementing key for each attribute
operator: { code: operatorMapping["IN_BETWEEN"] },
attribute: { code: parts[2] },
fromValue,
toValue
});
} else {
// Parse single conditions using regex
const match = part.match(/(.*?)\s*(<=|>=|<|>|==|!=)\s*(.*)/);
if (match) {
const attributeCode = match[1].trim();
const operatorSymbol = match[2].trim();
const value = match[3].trim();
attributes.push({
key: attributes.length + 1, // Incrementing key for each attribute
value,
operator: { code: operatorMapping[operatorSymbol] },
attribute: { code: attributeCode }
});
}
}

// const deliveryIndex = item.deliveryNumber.toString();
// Add each part as a new delivery rule
rules.push({
ruleKey: ruleKey + 1,
delivery: {},
products,
attributes
});
});

// let delivery = currentCycle.deliveries.find((delivery) => delivery.deliveryIndex === deliveryIndex);
return rules;
});

// if (!delivery) {
// delivery = {
// deliveryIndex: deliveryIndex,
// active: item.deliveryNumber === 1, // Set active to true only for the first delivery
// deliveryRules: [],
// };
// currentCycle.deliveries.push(delivery);
// }
return {
active: true,
deliveryIndex: String(deliveryIndex + 1),
deliveryRules: doseCriteria
};
});

return {
active: true,
cycleIndex: String(cycle.id),
deliveries: deliveries
};
});

return transformedCycles;

// delivery.deliveryRules.push({
// ruleKey: item.deliveryRuleNumber,
// delivery: {},
// attributes: loopAndReturn(item.conditions, t),
// products: [...item.products],
// });
// });
return data;

return reversedData;
}


function boundaryDataGrp(boundaryData) {
// Create an empty object to hold grouped data by type
const groupedData = {};
Expand Down Expand Up @@ -406,16 +454,16 @@ const CampaignSummary = (props) => {
{
key: "CAMPAIGN_NO_OF_CYCLES",
value:
data?.[0]?.deliveryRules && data?.[0]?.deliveryRules.map((item) => item.cycleIndex)?.length > 0
? Math.max(...data?.[0]?.deliveryRules.map((item) => item.cycleIndex))
data?.[0]?.additionalDetails?.cycleData?.cycleConfgureDate?.cycle ?
data?.[0]?.additionalDetails?.cycleData?.cycleConfgureDate?.cycle
: t("CAMPAIGN_SUMMARY_NA"),
},
{
key: "CAMPAIGN_NO_OF_DELIVERIES",
value:
data?.[0]?.deliveryRules && data?.[0]?.deliveryRules?.flatMap((rule) => rule?.deliveries?.map((delivery) => delivery?.deliveryIndex))?.length > 0
? Math.max(...data?.[0]?.deliveryRules?.flatMap((rule) => rule?.deliveries?.map((delivery) => delivery?.deliveryIndex)))
: t("CAMPAIGN_SUMMARY_NA"),
data?.[0]?.additionalDetails?.cycleData?.cycleConfgureDate?.deliveries ?
data?.[0]?.additionalDetails?.cycleData?.cycleConfgureDate?.deliveries
: t("CAMPAIGN_SUMMARY_NA"),
},
],
},
Expand Down
6 changes: 3 additions & 3 deletions ...ro-ui-internals/packages/modules/campaign-manager/src/components/CampaignUpdateSummary.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ const CampaignUpdateSummary = (props) => {
},
cardHeader: { value: t("TARGET_DETAILS"), inlineStyles: { marginTop: 0, fontSize: "1.5rem" } },
cardSecondaryAction: noAction !== "false" && (
<div className="campaign-preview-edit-container" onClick={() => handleRedirect(2)}>
<div className="campaign-preview-edit-container" onClick={() => handleRedirect(4)}>
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)

Approve changes to handleRedirect function with a minor suggestion.

The updates to the handleRedirect function improve URL parameter handling using URLSearchParams. This approach is more robust and maintainable. However, consider extracting the URL parameter update logic into a separate function for better readability and reusability.

Consider refactoring the URL parameter update logic into a separate function:

const updateUrlParams = (params) => {
  const urlParams = new URLSearchParams(window.location.search);
  Object.entries(params).forEach(([key, value]) => urlParams.set(key, value));
  return `${window.location.pathname}?${urlParams.toString()}`;
};

const handleRedirect = (step, activeCycle) => {
  const params = { key: step, preview: false };
  if (activeCycle) params.activeCycle = activeCycle;
  const newUrl = updateUrlParams(params);
  history.push(newUrl);
};

This refactoring improves code organization and makes it easier to update URL parameters consistently across the component.

Also applies to: 237-237, 257-257

🧰 Tools
🪛 Biome

[error] 217-217: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.

(lint/a11y/useKeyWithClickEvents)

⚠️ Potential issue

Update redirection steps and improve accessibility for secondary actions.

The changes to the redirection steps in the onClick handlers for "target", "facility", and "user" sections appear to be intentional, likely reflecting a reorganization of the campaign setup process. However, there are accessibility concerns that need to be addressed.

To improve accessibility, add keyboard event handlers to these clickable div elements. Here's an example of how to modify the "target" section (apply similar changes to "facility" and "user" sections):

<div 
  className="campaign-preview-edit-container" 
  onClick={() => handleRedirect(4)}
  onKeyDown={(e) => e.key === 'Enter' && handleRedirect(4)}
  tabIndex={0}
  role="button"
  aria-label={t("CAMPAIGN_EDIT_TARGET")}
>
  <span>{t(`CAMPAIGN_EDIT`)}</span>
  <EditIcon />
</div>

These changes ensure that the edit actions are accessible to keyboard users, improving the overall accessibility of the component.

Also applies to: 237-237, 257-257

🧰 Tools
🪛 Biome

[error] 217-217: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.

(lint/a11y/useKeyWithClickEvents)

<span>{t(`CAMPAIGN_EDIT`)}</span>
<EditIcon />
</div>
Expand All @@ -234,7 +234,7 @@ const CampaignUpdateSummary = (props) => {
},
cardHeader: { value: t("FACILITY_DETAILS"), inlineStyles: { marginTop: 0, fontSize: "1.5rem" } },
cardSecondaryAction: noAction !== "false" && (
<div className="campaign-preview-edit-container" onClick={() => handleRedirect(3)}>
<div className="campaign-preview-edit-container" onClick={() => handleRedirect(2)}>
<span>{t(`CAMPAIGN_EDIT`)}</span>
<EditIcon />
</div>
Expand All @@ -254,7 +254,7 @@ const CampaignUpdateSummary = (props) => {
},
cardHeader: { value: t("USER_DETAILS"), inlineStyles: { marginTop: 0, fontSize: "1.5rem" } },
cardSecondaryAction: noAction !== "false" && (
<div className="campaign-preview-edit-container" onClick={() => handleRedirect(4)}>
<div className="campaign-preview-edit-container" onClick={() => handleRedirect(3)}>
<span>{t(`CAMPAIGN_EDIT`)}</span>
<EditIcon />
</div>
Expand Down
Loading