Skip to content

[WEB-2681] fix: module progress indicator #5842

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

Merged
pushya22 merged 3 commits into from
Oct 21, 2024
Merged

[WEB-2681] fix: module progress indicator #5842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2375564
fix: module progress indicator
anmolsinghbhatia Oct 16, 2024
5dc9720
Merge branch 'preview' of github.com:makeplane/plane into fix-module-…
anmolsinghbhatia Oct 16, 2024
5852e33
fix: module progress indicator
anmolsinghbhatia Oct 21, 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
67 changes: 29 additions & 38 deletions web/core/components/modules/module-card-item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,21 @@ import { useParams, usePathname, useSearchParams } from "next/navigation";
import { Info, SquareUser } from "lucide-react";
// ui
import { IModule } from "@plane/types";
import { Card, FavoriteStar, LayersIcon, LinearProgressIndicator, TOAST_TYPE, Tooltip, setPromiseToast, setToast } from "@plane/ui";
import {
Card,
FavoriteStar,
LayersIcon,
LinearProgressIndicator,
TOAST_TYPE,
Tooltip,
setPromiseToast,
setToast,
} from "@plane/ui";
// components
import { DateRangeDropdown } from "@/components/dropdowns";
import { ButtonAvatars } from "@/components/dropdowns/member/avatar";
import { ModuleQuickActions } from "@/components/modules";
import { ModuleStatusDropdown } from "@/components/modules/module-status-dropdown";
import { ModuleStatusDropdown } from "@/components/modules/module-status-dropdown";
// constants
import { PROGRESS_STATE_GROUPS_DETAILS } from "@/constants/common";
import { MODULE_FAVORITED, MODULE_UNFAVORITED } from "@/constants/event-tracker";
Expand All @@ -21,11 +30,10 @@ import { MODULE_STATUS } from "@/constants/module";
import { getDate, renderFormattedPayloadDate } from "@/helpers/date-time.helper";
import { generateQueryParams } from "@/helpers/router.helper";
// hooks
import { useEventTracker, useMember, useModule, useProjectEstimates, useUserPermissions } from "@/hooks/store";
import { useEventTracker, useMember, useModule, useUserPermissions } from "@/hooks/store";
import { useAppRouter } from "@/hooks/use-app-router";
import { usePlatformOS } from "@/hooks/use-platform-os";
// plane web constants
import { EEstimateSystem } from "@/plane-web/constants/estimates";
import { EUserPermissions, EUserPermissionsLevel } from "@/plane-web/constants/user-permissions";

type Props = {
Expand All @@ -46,7 +54,6 @@ export const ModuleCardItem: React.FC<Props> = observer((props) => {
const { getModuleById, addModuleToFavorites, removeModuleFromFavorites, updateModuleDetails } = useModule();
const { getUserDetails } = useMember();
const { captureEvent } = useEventTracker();
const { currentActiveEstimateId, areEstimateEnabledByProjectId, estimateById } = useProjectEstimates();

// derived values
const moduleDetails = getModuleById(moduleId);
Expand All @@ -57,7 +64,6 @@ export const ModuleCardItem: React.FC<Props> = observer((props) => {
const isDisabled = !isEditingAllowed || !!moduleDetails?.archived_at;
const renderIcon = Boolean(moduleDetails?.start_date) || Boolean(moduleDetails?.target_date);


const { isMobile } = usePlatformOS();
const handleAddToFavorites = (e: React.MouseEvent<HTMLButtonElement>) => {
e.stopPropagation();
Expand Down Expand Up @@ -156,41 +162,26 @@ export const ModuleCardItem: React.FC<Props> = observer((props) => {

if (!moduleDetails) return null;

/**
* NOTE: This completion percentage calculation is based on the total issues count.
* when estimates are available and estimate type is points, we should consider the estimate point count
* when estimates are available and estimate type is not points, then by default we consider the issue count
*/
const isEstimateEnabled =
projectId &&
currentActiveEstimateId &&
areEstimateEnabledByProjectId(projectId.toString()) &&
estimateById(currentActiveEstimateId)?.type === EEstimateSystem.POINTS;

const moduleTotalIssues = isEstimateEnabled
? moduleDetails?.total_estimate_points || 0
: moduleDetails.backlog_issues +
moduleDetails.unstarted_issues +
moduleDetails.started_issues +
moduleDetails.completed_issues +
moduleDetails.cancelled_issues;

const moduleCompletedIssues = isEstimateEnabled
? moduleDetails?.completed_estimate_points || 0
: moduleDetails.completed_issues;
const moduleTotalIssues =
moduleDetails.backlog_issues +
moduleDetails.unstarted_issues +
moduleDetails.started_issues +
moduleDetails.completed_issues +
moduleDetails.cancelled_issues;

const moduleCompletedIssues = moduleDetails.completed_issues;

// const areYearsEqual = startDate.getFullYear() === endDate.getFullYear();

const moduleStatus = MODULE_STATUS.find((status) => status.value === moduleDetails.status);

const issueCount = module
? !moduleTotalIssues || moduleTotalIssues === 0
? `0 ${isEstimateEnabled ? `Point` : `Issue`}`
? `0 Issue`
: moduleTotalIssues === moduleCompletedIssues
? `${moduleTotalIssues} Issue${moduleTotalIssues > 1 ? `s` : ``}`
: `${moduleCompletedIssues}/${moduleTotalIssues} ${isEstimateEnabled ? `Points` : `Issues`}`
: `0 ${isEstimateEnabled ? `Point` : `Issue`}`;
: `${moduleCompletedIssues}/${moduleTotalIssues} Issues`
: `0 Issue`;

const moduleLeadDetails = moduleDetails.lead_id ? getUserDetails(moduleDetails.lead_id) : undefined;

Expand All @@ -213,9 +204,9 @@ export const ModuleCardItem: React.FC<Props> = observer((props) => {
<div className="flex items-center gap-2" onClick={handleEventPropagation}>
{moduleStatus && (
<ModuleStatusDropdown
isDisabled={isDisabled}
moduleDetails={moduleDetails}
handleModuleDetailsChange={handleModuleDetailsChange}
isDisabled={isDisabled}
moduleDetails={moduleDetails}
handleModuleDetailsChange={handleModuleDetailsChange}
/>
)}
<button onClick={openModuleOverview}>
Expand Down Expand Up @@ -252,9 +243,9 @@ export const ModuleCardItem: React.FC<Props> = observer((props) => {
}}
onSelect={(val) => {
handleModuleDetailsChange({
start_date: (val?.from ? renderFormattedPayloadDate(val.from) : null),
target_date: (val?.to ? renderFormattedPayloadDate(val.to) : null)
})
start_date: val?.from ? renderFormattedPayloadDate(val.from) : null,
target_date: val?.to ? renderFormattedPayloadDate(val.to) : null,
});
}}
placeholder={{
from: "Start date",
Expand Down Expand Up @@ -288,4 +279,4 @@ export const ModuleCardItem: React.FC<Props> = observer((props) => {
</div>
</div>
);
});
});
21 changes: 3 additions & 18 deletions web/core/components/modules/module-list-item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,9 @@ import { ModuleListItemAction, ModuleQuickActions } from "@/components/modules";
// helpers
import { generateQueryParams } from "@/helpers/router.helper";
// hooks
import { useModule, useProjectEstimates } from "@/hooks/store";
import { useModule } from "@/hooks/store";
import { useAppRouter } from "@/hooks/use-app-router";
import { usePlatformOS } from "@/hooks/use-platform-os";
// plane web constants
import { EEstimateSystem } from "@/plane-web/constants/estimates";

type Props = {
moduleId: string;
Expand All @@ -35,27 +33,14 @@ export const ModuleListItem: React.FC<Props> = observer((props) => {
// store hooks
const { getModuleById } = useModule();
const { isMobile } = usePlatformOS();
const { currentActiveEstimateId, areEstimateEnabledByProjectId, estimateById } = useProjectEstimates();

// derived values
const moduleDetails = getModuleById(moduleId);

if (!moduleDetails) return null;

/**
* NOTE: This completion percentage calculation is based on the total issues count.
* when estimates are available and estimate type is points, we should consider the estimate point count
* when estimates are available and estimate type is not points, then by default we consider the issue count
*/
const isEstimateEnabled =
projectId &&
currentActiveEstimateId &&
areEstimateEnabledByProjectId(projectId?.toString()) &&
estimateById(currentActiveEstimateId)?.type === EEstimateSystem.POINTS;

const completionPercentage = isEstimateEnabled
? ((moduleDetails?.completed_estimate_points || 0) / (moduleDetails?.total_estimate_points || 0)) * 100
: ((moduleDetails.completed_issues + moduleDetails.cancelled_issues) / moduleDetails.total_issues) * 100;
const completionPercentage =
((moduleDetails.completed_issues + moduleDetails.cancelled_issues) / moduleDetails.total_issues) * 100;
Comment on lines +42 to +43
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 16, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Verification Completed: Simplified Calculation Consistent Across Codebase

The updated completion percentage calculation using completed_issues, cancelled_issues, and total_issues is consistently applied across the codebase, aligning with the PR objective to simplify and enhance maintainability.

However, there are still several references to estimate_point and related variables. Please ensure that no progress calculations are still dependent on estimate points to maintain uniform behavior.

  • Review instances of estimate_point to confirm they do not impact progress calculations.
  • If estimate-based calculations are no longer needed, consider removing or refactoring them accordingly.
🔗 Analysis chain

LGTM: Simplified completion percentage calculation

The new calculation for completionPercentage correctly implements the PR objective by basing progress on issue state (completed and cancelled issues) rather than estimate points. This change simplifies the logic and improves maintainability.

To ensure consistency across the codebase, please run the following script:

This will help identify any other locations where similar calculations might need to be updated for consistency.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other occurrences of module progress calculation

# Search for similar progress calculations in other files
rg --type typescript --type javascript -e 'completed_issues' -e 'cancelled_issues' -e 'total_issues' --glob '!**/module-list-item.tsx'

# Search for any remaining uses of estimate points for progress calculation
rg --type typescript --type javascript -e 'estimate_point' -e 'useProjectEstimates'

Length of output: 282

Script:

#!/bin/bash
# Description: Check for other occurrences of module progress calculation with corrected file types

# Search for similar progress calculations in other files
rg --type ts --type tsx --type js --type jsx -e 'completed_issues' -e 'cancelled_issues' -e 'total_issues' --glob '!**/module-list-item.tsx'

# Search for any remaining uses of estimate points for progress calculation
rg --type ts --type tsx --type js --type jsx -e 'estimate_point' -e 'useProjectEstimates'

Length of output: 280

Script:

#!/bin/bash
# Description: Check for other occurrences of module progress calculation using file extensions

# Search for similar progress calculations in other files
rg -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' 'completed_issues|cancelled_issues|total_issues' --glob '!**/module-list-item.tsx'

# Search for any remaining uses of estimate points for progress calculation
rg -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' 'estimate_point|useProjectEstimates'

Length of output: 29494


const progress = isNaN(completionPercentage) ? 0 : Math.floor(completionPercentage);

Expand Down