Fix: Cycle graphs refactor #5745
Conversation
WalkthroughThe pull request introduces several modifications across multiple files, primarily focusing on TypeScript type definitions and React components related to cycle management. Key changes include the addition of a Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (4)
web/core/constants/cycle.ts (1)
Line range hint
17-26: Consider updating tab name for consistencyWith the change of "Active" to "In progress" for the cycle status, consider updating the tab name in
CYCLE_TABS_LISTfor consistency:export const CYCLE_TABS_LIST: { key: TCycleTabOptions; name: string; }[] = [ { key: "active", - name: "Active", + name: "In progress", }, { key: "all", name: "All", }, ];This would ensure consistent terminology throughout the application.
web/helpers/cycle.helper.ts (3)
87-118: Ensure consistent handling of extended dates informatV1Data.In
formatV1Data, when extending the progress data withextendedArray, ensure that dates beyond the cycle's end date are handled appropriately. Currently, only theendDateis added, which may not be necessary if it's already included indata.completion_chart.Consider checking if
endDateis already present and avoid duplication:const extendedArray = generateDateArray(endDate, endDate).map((d) => d.date); +const existingDates = Object.keys(data.completion_chart); +const datesToAdd = extendedArray.filter((date) => !existingDates.includes(date)); +const progress = [...existingDates, ...datesToAdd].map((p) => { /* ... */ });
1-6: Organize imports for better readability.Consider grouping and ordering imports logically: external libraries first, then local modules, and helpers last. This improves code readability and maintainability.
Apply this diff to reorder the imports:
+import { isEmpty, orderBy, uniqBy, sortBy } from "lodash"; import { startOfToday, format } from "date-fns"; -import { isEmpty, orderBy, uniqBy } from "lodash"; -import sortBy from "lodash/sortBy"; import { ICycle, TCycleFilters } from "@plane/types"; // helpers import { findTotalDaysInRange, generateDateArray, getDate } from "@/helpers/date-time.helper"; import { satisfiesDateFilter } from "@/helpers/filter.helper";
120-159: Optimize repeated calls toformat(startOfToday(), "yyyy-MM-dd").You're calling
format(startOfToday(), "yyyy-MM-dd")multiple times. Assigning it once to a variable reduces function calls and improves performance.Assign the formatted date once:
let today: Date | string = format(startOfToday(), "yyyy-MM-dd"); const extendedArray = endDate > today ? generateDateArray(today as Date, endDate) : []; if (isEmpty(cycle.progress)) return extendedArray; - today = format(startOfToday(), "yyyy-MM-dd");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
- packages/types/src/cycle/cycle.d.ts (2 hunks)
- web/core/components/cycles/active-cycle/root.tsx (0 hunks)
- web/core/components/cycles/list/root.tsx (1 hunks)
- web/core/constants/cycle.ts (1 hunks)
- web/core/store/cycle.store.ts (5 hunks)
- web/helpers/cycle.helper.ts (2 hunks)
- web/helpers/date-time.helper.ts (1 hunks)
💤 Files with no reviewable changes (1)
- web/core/components/cycles/active-cycle/root.tsx
🔇 Additional comments (9)
web/core/constants/cycle.ts (2)
63-63: LGTM: Status title updated for clarityThe change from "Active" to "In progress" for the "current" cycle status is a good improvement. It provides a clearer description of the cycle's state, which aligns better with the "day left" label.
Line range hint
1-126: Verify alignment with PR objectivesThe PR is titled "Fix: community changes for cycle graphs", but the change here is more about updating terminology. While this is a valuable change, it's not clear how it relates to cycle graphs or community changes. Could you provide more context on how this change fits into the larger scope of the PR?
web/core/components/cycles/list/root.tsx (1)
31-31: Verify the impact of removing props from ActiveCycleRootThe change looks good, assuming it's intentional. However, please verify the following:
- Ensure that
ActiveCycleRootcan function correctly withoutworkspaceSlugandprojectIdprops. It should either fetch its own data or access this information through other means (e.g., context or global state).- Check for any performance implications if
ActiveCycleRootis now responsible for fetching its own data.- Confirm that this change is consistent across the codebase wherever
ActiveCycleRootis used.To verify the usage of
ActiveCycleRootacross the codebase, please run the following script:If
workspaceSlugandprojectIdare no longer needed in theCyclesListcomponent, consider removing them from its props interface to keep the code clean:export interface ICyclesList { completedCycleIds: string[]; upcomingCycleIds?: string[] | undefined; cycleIds: string[]; - workspaceSlug: string; - projectId: string; isArchived?: boolean; }Make sure to update the destructuring in the component accordingly if you make this change.
packages/types/src/cycle/cycle.d.ts (2)
106-106: LGTM. Verify the impact of adding the version property.The addition of the
version: numberproperty to theICycleinterface is a good way to support versioning for cycle entities. This change looks good, but it's important to ensure that it's properly integrated across the codebase.To verify the impact and usage of this new property, we can run the following script:
#!/bin/bash # Description: Check for ICycle interface usage and version property handling # Search for ICycle interface usage echo "Searching for ICycle interface usage:" rg --type typescript "ICycle" -A 5 -B 5 # Search for version property usage within objects that might be of ICycle type echo "\nSearching for version property usage in potential ICycle objects:" rg --type typescript "version\s*:\s*\d+" -A 2 -B 2 # Search for API endpoints or services that might need to handle the new version property echo "\nSearching for potential API endpoints or services that might need updating:" rg --type typescript "(get|post|put|patch|delete).*cycle" -A 5 -B 5Please review the results of this script to ensure that:
- All places using the
ICycleinterface are aware of the newversionproperty.- Any code creating or updating
ICycleobjects includes theversionproperty.- API endpoints or services handling cycle data are updated to include the
versionproperty in requests/responses if necessary.
49-49: LGTM. Can you clarify the removal and re-addition process?The
actual: numberproperty in theTCycleProgresstype looks good and is consistent with other properties in this type. However, the AI summary mentions that this property was removed and then re-added. Could you provide some context on why it was initially removed and then added back? This information might be helpful for future reference and understanding the decision-making process.To verify if this property is used elsewhere in the codebase, we can run the following script:
web/helpers/date-time.helper.ts (2)
Line range hint
1-390: Address removed functions and update documentationThe AI summary indicates that several functions (
getToday,dateFormatter, anddaysLeft) have been removed from this file. However, these removals are not visible in the provided code snippet. It's crucial to ensure that these changes are intentional and that their removal doesn't negatively impact the codebase.To verify the impact of these removals and ensure that they don't break existing functionality, please run the following verification script:
#!/bin/bash # Description: Verify the usage of removed functions in the codebase # Search for usage of removed functions echo "Searching for usage of removed functions:" rg --type typescript "getToday\(|dateFormatter\(|daysLeft\(" -A 2 -B 1 # Check for any imports of these functions in other files echo "Checking for imports of removed functions:" rg --type typescript "import.*\{.*(?:getToday|dateFormatter|daysLeft).*\}.*from.*date-time\.helper" -A 1 -B 1This script will help identify any remaining usage of the removed functions in the codebase, allowing you to update or remove those occurrences as needed.
Additionally, consider updating the file's documentation or comments to reflect these removals and any other significant changes. This will help maintain clear and up-to-date documentation for other developers working on this file in the future.
Line range hint
367-390: Improve date range generation logic and error handlingThe
generateDateArrayfunction has been updated to accept both string and Date types, which improves its flexibility. However, there are a few issues that need to be addressed:
- The start date is not included in the output array due to immediate increment.
- The end date is included twice due to the initial increment and the loop condition.
- There's no error handling for invalid date inputs.
Consider applying the following changes to address these issues:
export const generateDateArray = (startDate: string | Date, endDate: string | Date) => { - // Convert the start and end dates to Date objects if they aren't already - const start = new Date(startDate); - // start.setDate(start.getDate() + 1); - const end = new Date(endDate); - end.setDate(end.getDate() + 1); + // Convert and validate the start and end dates + const start = getDate(startDate); + const end = getDate(endDate); + + if (!start || !end) { + throw new Error("Invalid date input"); + } + + if (start > end) { + throw new Error("Start date must be before or equal to end date"); + } // Create an empty array to store the dates const dateArray = []; // Use a while loop to generate dates between the range - while (start <= end) { + const current = new Date(start); + while (current <= end) { // Push the current date (converted to ISO string for consistency) dateArray.push({ - date: new Date(start).toISOString().split("T")[0], + date: current.toISOString().split("T")[0], }); // Increment the date by 1 day (86400000 milliseconds) - start.setDate(start.getDate() + 1); + current.setDate(current.getDate() + 1); } return dateArray; };These changes will:
- Include the start date in the output array.
- Prevent duplicate inclusion of the end date.
- Add error handling for invalid date inputs using the existing
getDatefunction.- Ensure that the start date is before or equal to the end date.
- Use a separate
currentdate variable for iteration to avoid modifying the originalstartdate.To ensure that these changes don't break existing functionality, please run the following verification script:
This script will help identify any potential issues with the existing usage of
generateDateArrayand highlight any areas where directnew Date()calls might be affected by the changes.web/core/store/cycle.store.ts (2)
401-407: Improved clarity in method comments.The updates to the comments for
getPlotTypeByCycleId,getEstimateTypeByCycleId,setPlotType, andsetEstimateTypeimprove clarity by specifying they are for the cycle store. This enhancement aids in code maintenance and understanding.Also applies to: 419-427
74-74: Verify the impact of changing the return type offetchActiveCycleProgressPro.The return type of
fetchActiveCycleProgressProhas been changed fromPromise<TProgressSnapshot> | Promise<null>toPromise<void>. This change might affect how errors are handled or how data is processed in components that use this method. Please ensure that all callers of this method have been updated accordingly.To check for potential issues, run the following script:
✅ Verification successful
Change to
fetchActiveCycleProgressProreturn type correctly handled across the codebase.The return type of
fetchActiveCycleProgressProhas been updated toPromise<void>, and all usages have been adjusted accordingly. No remaining references expect a return value from this method.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of fetchActiveCycleProgressPro and verify they handle the new return type correctly rg "fetchActiveCycleProgressPro" --type typescript -A 5 -B 5Length of output: 95
Script:
#!/bin/bash # Search for usages of fetchActiveCycleProgressPro and verify they handle the new return type correctly rg "fetchActiveCycleProgressPro" --type ts -A 5 -B 5Length of output: 2721
Script:
#!/bin/bash # Search for usages of fetchActiveCycleProgressPro in .tsx files to ensure all usages are updated correctly rg "fetchActiveCycleProgressPro" --type tsx -A 5 -B 5Length of output: 81
Script:
#!/bin/bash # Search for usages of fetchActiveCycleProgressPro in .tsx files to ensure all usages are updated correctly rg "fetchActiveCycleProgressPro" -g "*.tsx" -A 5 -B 5Length of output: 53
| @@ -545,7 +540,7 @@ export class CycleStore implements ICycleStore { | |||
| * @param cycleId | |||
| * @returns | |||
| */ | |||
| fetchActiveCycleProgressPro = async (workspaceSlug: string, projectId: string, cycleId: string) => null; | |||
| fetchActiveCycleProgressPro = action(async (workspaceSlug: string, projectId: string, cycleId: string) => {}); | |||
There was a problem hiding this comment.
Implement the fetchActiveCycleProgressPro method.
The fetchActiveCycleProgressPro method has been replaced with an empty async function. This implementation doesn't match the method's name or its expected behavior. Please implement the method to fetch the active cycle progress for pro users.
Consider implementing the method similar to the non-pro version, adjusting for any pro-specific features:
fetchActiveCycleProgressPro = action(async (workspaceSlug: string, projectId: string, cycleId: string) => {
this.progressLoader = true;
try {
const progress = await this.cycleService.workspaceActiveCyclesProgressPro(workspaceSlug, projectId, cycleId);
runInAction(() => {
set(this.cycleMap, [cycleId], { ...this.cycleMap[cycleId], ...progress });
});
} catch (error) {
console.error("Failed to fetch active cycle progress for pro users", error);
} finally {
runInAction(() => {
this.progressLoader = false;
});
}
});| const { cycle, isBurnDown, isTypeIssue } = args; | ||
| let today = getToday(); | ||
| const endDate: Date | string = new Date(cycle.end_date!); | ||
| const scope = (p: any, isTypeIssue: boolean) => (isTypeIssue ? p.total_issues : p.total_estimate_points); |
There was a problem hiding this comment.
Avoid using the any type for parameter p.
Using any defeats the purpose of TypeScript's static typing and can lead to runtime errors. Consider defining a specific interface or type for p to ensure type safety.
Apply this diff to specify the parameter type:
-const scope = (p: any, isTypeIssue: boolean) => (isTypeIssue ? p.total_issues : p.total_estimate_points);
+interface ProgressData {
+ total_issues?: number;
+ total_estimate_points?: number;
+}
+
+const scope = (p: ProgressData, isTypeIssue: boolean) => (isTypeIssue ? p.total_issues : p.total_estimate_points);📝 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 scope = (p: any, isTypeIssue: boolean) => (isTypeIssue ? p.total_issues : p.total_estimate_points); | |
| interface ProgressData { | |
| total_issues?: number; | |
| total_estimate_points?: number; | |
| } | |
| const scope = (p: ProgressData, isTypeIssue: boolean) => (isTypeIssue ? p.total_issues : p.total_estimate_points); |
| const ideal = (date: string, scope: number, cycle: ICycle) => | ||
| Math.floor( | ||
| ((findTotalDaysInRange(date, cycle.end_date) || 0) / | ||
| (findTotalDaysInRange(cycle.start_date, cycle.end_date) || 0)) * | ||
| scope |
There was a problem hiding this comment.
Potential division by zero in the ideal function.
In the ideal function, there's a risk of dividing by zero if findTotalDaysInRange(cycle.start_date, cycle.end_date) returns 0 or undefined. This will result in a NaN value.
Apply this diff to handle the division by zero:
const ideal = (date: string, scope: number, cycle: ICycle) =>
Math.floor(
- ((findTotalDaysInRange(date, cycle.end_date) || 0) /
- (findTotalDaysInRange(cycle.start_date, cycle.end_date) || 0)) *
+ ((findTotalDaysInRange(date, cycle.end_date) || 0) /
+ (findTotalDaysInRange(cycle.start_date, cycle.end_date) || 1)) *
scope
);This change ensures that if the denominator is 0 or falsy, it defaults to 1, preventing division by zero.
📝 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 ideal = (date: string, scope: number, cycle: ICycle) => | |
| Math.floor( | |
| ((findTotalDaysInRange(date, cycle.end_date) || 0) / | |
| (findTotalDaysInRange(cycle.start_date, cycle.end_date) || 0)) * | |
| scope | |
| const ideal = (date: string, scope: number, cycle: ICycle) => | |
| Math.floor( | |
| ((findTotalDaysInRange(date, cycle.end_date) || 0) / | |
| (findTotalDaysInRange(cycle.start_date, cycle.end_date) || 1)) * | |
| scope | |
| ); |
| export const formatActiveCycle = (args: { | ||
| cycle: ICycle; | ||
| isBurnDown?: boolean | undefined; | ||
| isTypeIssue?: boolean | undefined; | ||
| }) => { | ||
| const { cycle, isBurnDown, isTypeIssue } = args; | ||
| const endDate: Date | string = new Date(cycle.end_date!); | ||
|
|
||
| return cycle.version === 1 | ||
| ? formatV1Data(isTypeIssue!, cycle, isBurnDown!, endDate) | ||
| : formatV2Data(isTypeIssue!, cycle, isBurnDown!, endDate); |
There was a problem hiding this comment.
Avoid using non-null assertions with potentially undefined values.
In formatActiveCycle, isTypeIssue and isBurnDown are optional and may be undefined, but non-null assertions (!) are used when passing them to formatV1Data and formatV2Data, which could lead to runtime errors.
Provide default values to ensure safety:
export const formatActiveCycle = (args: {
cycle: ICycle;
isBurnDown?: boolean;
isTypeIssue?: boolean;
}) => {
const { cycle, isBurnDown = false, isTypeIssue = false } = args;
const endDate: Date | string = new Date(cycle.end_date!);
return cycle.version === 1
- ? formatV1Data(isTypeIssue!, cycle, isBurnDown!, endDate)
- : formatV2Data(isTypeIssue!, cycle, isBurnDown!, endDate);
+ ? formatV1Data(isTypeIssue, cycle, isBurnDown, endDate)
+ : formatV2Data(isTypeIssue, cycle, isBurnDown, endDate);
};This approach removes the need for non-null assertions and sets default values if they are not provided.
📝 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 formatActiveCycle = (args: { | |
| cycle: ICycle; | |
| isBurnDown?: boolean | undefined; | |
| isTypeIssue?: boolean | undefined; | |
| }) => { | |
| const { cycle, isBurnDown, isTypeIssue } = args; | |
| const endDate: Date | string = new Date(cycle.end_date!); | |
| return cycle.version === 1 | |
| ? formatV1Data(isTypeIssue!, cycle, isBurnDown!, endDate) | |
| : formatV2Data(isTypeIssue!, cycle, isBurnDown!, endDate); | |
| export const formatActiveCycle = (args: { | |
| cycle: ICycle; | |
| isBurnDown?: boolean; | |
| isTypeIssue?: boolean; | |
| }) => { | |
| const { cycle, isBurnDown = false, isTypeIssue = false } = args; | |
| const endDate: Date | string = new Date(cycle.end_date!); | |
| return cycle.version === 1 | |
| ? formatV1Data(isTypeIssue, cycle, isBurnDown, endDate) | |
| : formatV2Data(isTypeIssue, cycle, isBurnDown, endDate); | |
| }; |
| const scope = (p: any, isTypeIssue: boolean) => (isTypeIssue ? p.total_issues : p.total_estimate_points); | ||
| const ideal = (date: string, scope: number, cycle: ICycle) => | ||
| Math.floor( | ||
| ((findTotalDaysInRange(date, cycle.end_date) || 0) / | ||
| (findTotalDaysInRange(cycle.start_date, cycle.end_date) || 0)) * | ||
| scope | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider typing the scope and ideal functions for clarity.
Explicitly typing the return types and parameters enhances code clarity and TypeScript's type checking capabilities.
Specify types for the functions:
-const scope = (p: ProgressData, isTypeIssue: boolean) => (isTypeIssue ? p.total_issues : p.total_estimate_points);
+const scope = (p: ProgressData, isTypeIssue: boolean): number => (isTypeIssue ? p.total_issues || 0 : p.total_estimate_points || 0);
-const ideal = (date: string, scope: number, cycle: ICycle) =>
+const ideal = (date: string, scope: number, cycle: ICycle): number =>Committable suggestion was skipped due to low confidence.
| let progress = [...orderBy(cycle?.progress, "date"), ...extendedArray].map((p) => { | ||
| const pending = isTypeIssue | ||
| ? p.total_issues - p.completed_issues - p.cancelled_issues | ||
| : p.total_estimate_points - p.completed_estimate_points - p.cancelled_estimate_points; | ||
| const completed = isTypeIssue ? p.completed_issues : p.completed_estimate_points; | ||
| const dataDate = p.progress_date ? format(new Date(p.progress_date), "yyyy-MM-dd") : p.date; | ||
|
|
||
| return { | ||
| date: p.date, | ||
| scope: p.date! < today ? scope(p) : p.date! < cycle.end_date! ? scopeToday : null, | ||
| date: dataDate, | ||
| scope: dataDate! < today ? scope(p, isTypeIssue) : dataDate! <= cycle.end_date! ? scopeToday : null, | ||
| completed, | ||
| backlog: isTypeIssue ? p.backlog_issues : p.backlog_estimate_points, | ||
| started: isTypeIssue ? p.started_issues : p.started_estimate_points, | ||
| unstarted: isTypeIssue ? p.unstarted_issues : p.unstarted_estimate_points, | ||
| cancelled: isTypeIssue ? p.cancelled_issues : p.cancelled_estimate_points, | ||
| pending: Math.abs(pending), | ||
| // TODO: This is a temporary logic to show the ideal line in the cycle chart | ||
| ideal: p.date! < today ? ideal(p) : p.date! < cycle.end_date! ? idealToday : null, | ||
| actual: p.date! <= today ? (isBurnDown ? Math.abs(pending) : completed) : undefined, | ||
| ideal: | ||
| dataDate! < today | ||
| ? ideal(dataDate, scope(p, isTypeIssue), cycle) | ||
| : dataDate! < cycle.end_date! | ||
| ? idealToday | ||
| : null, | ||
| actual: dataDate! <= today ? (isBurnDown ? Math.abs(pending) : completed) : undefined, |
There was a problem hiding this comment.
Handle different types of p in the progress mapping.
In formatV2Data, the progress array combines cycle.progress objects and extendedArray dates. When mapping over this array, the code assumes that each p has certain properties, which may not be true for dates from extendedArray.
Separate the handling of progress data and extended dates:
let progressData = orderBy(cycle.progress, "date").map((p) => {
// existing logic for progress entries
});
+let extendedProgress = extendedArray.map((date) => ({
+ date: format(new Date(date), "yyyy-MM-dd"),
+ scope: date <= cycle.end_date! ? scopeToday : null,
+ // other properties set to default or null
+}));
-progress = [...progressData, ...extendedArray].map((p) => { /* ... */ });
+progress = [...progressData, ...extendedProgress];
progress = uniqBy(progress, "date");This ensures that the properties are correctly assigned based on whether p is a progress entry or an extended date.
Committable suggestion was skipped due to low confidence.
| const formatV2Data = (isTypeIssue: boolean, cycle: ICycle, isBurnDown: boolean, endDate: Date | string) => { | ||
| if (!cycle.progress) return []; | ||
| let today: Date | string = startOfToday(); | ||
|
|
||
| const scopeToday = scope(cycle?.progress[cycle?.progress.length - 1]); | ||
| const idealToday = ideal(cycle?.progress[cycle?.progress.length - 1]); | ||
| const extendedArray = endDate > today ? generateDateArray(today as Date, endDate) : []; | ||
| if (isEmpty(cycle.progress)) return extendedArray; | ||
| today = format(startOfToday(), "yyyy-MM-dd"); | ||
| const todaysData = cycle?.progress[cycle?.progress.length - 1]; | ||
| const scopeToday = scope(todaysData, isTypeIssue); | ||
| const idealToday = ideal(todaysData.date, scopeToday, cycle); | ||
|
|
||
| const progress = [...orderBy(cycle?.progress, "date"), ...extendedArray].map((p) => { | ||
| let progress = [...orderBy(cycle?.progress, "date"), ...extendedArray].map((p) => { | ||
| const pending = isTypeIssue | ||
| ? p.total_issues - p.completed_issues - p.cancelled_issues | ||
| : p.total_estimate_points - p.completed_estimate_points - p.cancelled_estimate_points; | ||
| const completed = isTypeIssue ? p.completed_issues : p.completed_estimate_points; | ||
| const dataDate = p.progress_date ? format(new Date(p.progress_date), "yyyy-MM-dd") : p.date; | ||
|
|
||
| return { | ||
| date: p.date, | ||
| scope: p.date! < today ? scope(p) : p.date! < cycle.end_date! ? scopeToday : null, | ||
| date: dataDate, | ||
| scope: dataDate! < today ? scope(p, isTypeIssue) : dataDate! <= cycle.end_date! ? scopeToday : null, | ||
| completed, | ||
| backlog: isTypeIssue ? p.backlog_issues : p.backlog_estimate_points, | ||
| started: isTypeIssue ? p.started_issues : p.started_estimate_points, | ||
| unstarted: isTypeIssue ? p.unstarted_issues : p.unstarted_estimate_points, | ||
| cancelled: isTypeIssue ? p.cancelled_issues : p.cancelled_estimate_points, | ||
| pending: Math.abs(pending), | ||
| // TODO: This is a temporary logic to show the ideal line in the cycle chart | ||
| ideal: p.date! < today ? ideal(p) : p.date! < cycle.end_date! ? idealToday : null, | ||
| actual: p.date! <= today ? (isBurnDown ? Math.abs(pending) : completed) : undefined, | ||
| ideal: | ||
| dataDate! < today | ||
| ? ideal(dataDate, scope(p, isTypeIssue), cycle) | ||
| : dataDate! < cycle.end_date! | ||
| ? idealToday | ||
| : null, | ||
| actual: dataDate! <= today ? (isBurnDown ? Math.abs(pending) : completed) : undefined, | ||
| }; | ||
| }); | ||
| return uniqBy(progress, "date"); | ||
| progress = uniqBy(progress, "date"); | ||
|
|
||
| return progress; | ||
| }; |
There was a problem hiding this comment.
Possible property access on undefined in formatV2Data.
When accessing todaysData.date and other properties of todaysData, there's a risk that todaysData could be undefined if cycle.progress is empty. This would lead to a runtime error.
Apply this diff to add a safety check:
let today: Date | string = startOfToday();
const extendedArray = endDate > today ? generateDateArray(today as Date, endDate) : [];
if (isEmpty(cycle.progress)) return extendedArray;
today = format(startOfToday(), "yyyy-MM-dd");
-const todaysData = cycle?.progress[cycle?.progress.length - 1];
+const todaysData = cycle.progress[cycle.progress.length - 1];
+if (!todaysData) return [];
const scopeToday = scope(todaysData, isTypeIssue);
const idealToday = ideal(todaysData.date, scopeToday, cycle);This ensures that the function returns early if todaysData is undefined.
Committable suggestion was skipped due to low confidence.
SatishGandham
changed the title
Sumary
Community fixes for cycle graphs
Summary by CodeRabbit
Release Notes
New Features
versionproperty in the cycle interface for enhanced versioning information.Bug Fixes
Refactor
ActiveCycleRootcomponent to streamline the active cycle display.Chores