-
-
Notifications
You must be signed in to change notification settings - Fork 8
feat: add target recalculation functionality and UI controls for saving and resetting targets #224
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
base: main
Are you sure you want to change the base?
Conversation
…ng and resetting targets Also, fixed time savings chart to use configured targets & max values
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
const totalPercent = userSurveys.reduce((sum, survey) => { | ||
const percentTimeSaved = typeof survey.percentTimeSaved === 'number' ? survey.percentTimeSaved : 0; | ||
// Always parse percentTimeSaved as float | ||
const percentTimeSaved = survey.percentTimeSaved != null ? parseFloat(survey.percentTimeSaved as any) : 0; |
Check failure
Code scanning / ESLint
Disallow the `any` type Error
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the issue, we need to replace the any
type with a more specific type that accurately represents the possible values of percentTimeSaved
. Based on the context, percentTimeSaved
appears to be a property that can be a string, number, or null. We can use a union type (string | number | null
) to represent this. Additionally, we should validate the value before parsing it as a float to ensure it is a valid number or numeric string.
Changes to make:
- Replace the
any
type withstring | number | null
forpercentTimeSaved
. - Add validation logic to ensure the value can be safely parsed as a float.
- Update the relevant lines in the
calculateWeeklyTimeSavedHrs
method.
-
Copy modified lines R706-R708
@@ -705,3 +705,5 @@ | ||
// Always parse percentTimeSaved as float | ||
const percentTimeSaved = survey.percentTimeSaved != null ? parseFloat(survey.percentTimeSaved as any) : 0; | ||
const percentTimeSaved = survey.percentTimeSaved != null && (typeof survey.percentTimeSaved === 'string' || typeof survey.percentTimeSaved === 'number') | ||
? parseFloat(survey.percentTimeSaved.toString()) | ||
: 0; | ||
return sum + percentTimeSaved; |
const hoursPerYear = typeof this.settings.hoursPerYear === 'number' ? this.settings.hoursPerYear : 2000; | ||
const percentCoding = typeof this.settings.percentCoding === 'number' ? this.settings.percentCoding : 50; | ||
// Convert settings values to numbers (parse from string if needed) | ||
const hoursPerYear = this.settings.hoursPerYear != null ? parseFloat(this.settings.hoursPerYear as any) : 2000; |
Check failure
Code scanning / ESLint
Disallow the `any` type Error
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the issue, we need to replace the any
type with a more specific type that accurately represents the possible values of settings.hoursPerYear
. Based on the context, it seems that settings.hoursPerYear
can be a string, null
, or undefined
. We can use a union type (string | null | undefined
) to represent this. The same approach should be applied to other instances where any
is used for type casting, such as settings.percentCoding
and settings.percentTimeSaved
.
Changes to make:
- Replace
any
withstring | null | undefined
for type casting. - Ensure that the logic handles all possible values of the property (e.g.,
null
,undefined
, or non-numeric strings).
-
Copy modified lines R716-R717 -
Copy modified line R725
@@ -715,4 +715,4 @@ | ||
// Convert settings values to numbers (parse from string if needed) | ||
const hoursPerYear = this.settings.hoursPerYear != null ? parseFloat(this.settings.hoursPerYear as any) : 2000; | ||
const percentCoding = this.settings.percentCoding != null ? parseFloat(this.settings.percentCoding as any) : 50; | ||
const hoursPerYear = this.settings.hoursPerYear != null ? parseFloat(this.settings.hoursPerYear as string | null | undefined) : 2000; | ||
const percentCoding = this.settings.percentCoding != null ? parseFloat(this.settings.percentCoding as string | null | undefined) : 50; | ||
|
||
@@ -724,3 +724,3 @@ | ||
// Calculate max based on settings | ||
const maxPercentTimeSaved = this.settings.percentTimeSaved != null ? parseFloat(this.settings.percentTimeSaved as any) : 20; | ||
const maxPercentTimeSaved = this.settings.percentTimeSaved != null ? parseFloat(this.settings.percentTimeSaved as string | null | undefined) : 20; | ||
const maxWeeklyTimeSaved = weeklyDevHours * (maxPercentTimeSaved / 100); |
const percentCoding = typeof this.settings.percentCoding === 'number' ? this.settings.percentCoding : 50; | ||
// Convert settings values to numbers (parse from string if needed) | ||
const hoursPerYear = this.settings.hoursPerYear != null ? parseFloat(this.settings.hoursPerYear as any) : 2000; | ||
const percentCoding = this.settings.percentCoding != null ? parseFloat(this.settings.percentCoding as any) : 50; |
Check failure
Code scanning / ESLint
Disallow the `any` type Error
Copilot Autofix
AI 4 months ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
|
||
// Calculate max based on settings | ||
const maxPercentTimeSaved = typeof this.settings.percentTimeSaved === 'number' ? this.settings.percentTimeSaved : 20; | ||
const maxPercentTimeSaved = this.settings.percentTimeSaved != null ? parseFloat(this.settings.percentTimeSaved as any) : 20; |
Check failure
Code scanning / ESLint
Disallow the `any` type Error
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the issue, we need to replace the any
type with a more specific type that accurately represents the possible values of this.settings.percentTimeSaved
. Based on the context, it seems that percentTimeSaved
can be a string, a number, or null. We can use the union type string | number | null
to represent this. This change ensures type safety while maintaining the flexibility to handle different input types.
The fix involves:
- Updating the type definition for
this.settings.percentTimeSaved
(if it exists) tostring | number | null
. - Replacing the
any
type cast with a union type cast (as string | number | null
). - Ensuring that
parseFloat
is applied correctly to handle the union type.
-
Copy modified line R725
@@ -724,3 +724,3 @@ | ||
// Calculate max based on settings | ||
const maxPercentTimeSaved = this.settings.percentTimeSaved != null ? parseFloat(this.settings.percentTimeSaved as any) : 20; | ||
const maxPercentTimeSaved = this.settings.percentTimeSaved != null ? parseFloat(this.settings.percentTimeSaved as string | number | null) : 20; | ||
const maxWeeklyTimeSaved = weeklyDevHours * (maxPercentTimeSaved / 100); |
// Ensure all values are properly typed as numbers | ||
const hoursPerYear = typeof this.settings.hoursPerYear === 'number' ? this.settings.hoursPerYear : 2000; | ||
// Always parse settings values as numbers (from string if needed) | ||
const hoursPerYear = this.settings.hoursPerYear != null ? parseFloat(this.settings.hoursPerYear as any) : 2000; |
Check failure
Code scanning / ESLint
Disallow the `any` type Error
Copilot Autofix
AI 4 months ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
const weeksInYear = Math.round(hoursPerYear / 40) || 50; // Calculate weeks and ensure it's a number | ||
|
||
const devCostPerYear = typeof this.settings.devCostPerYear === 'number' ? this.settings.devCostPerYear : 0; | ||
const devCostPerYear = this.settings.devCostPerYear != null ? parseFloat(this.settings.devCostPerYear as any) : 0; |
Check failure
Code scanning / ESLint
Disallow the `any` type Error
Copilot Autofix
AI 4 months ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
// Convert hours per year to number | ||
const hoursPerYear = typeof this.settings.hoursPerYear === 'number' ? this.settings.hoursPerYear : 2000; | ||
// Always parse hours per year as number | ||
const hoursPerYear = this.settings.hoursPerYear != null ? parseFloat(this.settings.hoursPerYear as any) : 2000; |
Check failure
Code scanning / ESLint
Disallow the `any` type Error
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the issue, we need to replace the any
type with a more specific type that accurately represents the possible values of this.settings.hoursPerYear
. Based on the context, hoursPerYear
is likely a numeric value or a string that can be parsed into a number. Therefore, the type should be string | number | null | undefined
to account for all possible cases. This change ensures that TypeScript can enforce type safety while still allowing flexibility in the input.
The fix involves:
- Updating the type assertion on line 829 to use
string | number | null | undefined
instead ofany
. - Ensuring that
parseFloat
is only called on valid inputs (e.g., strings or numbers). - No additional imports or dependencies are required.
-
Copy modified line R829
@@ -828,3 +828,3 @@ | ||
// Always parse hours per year as number | ||
const hoursPerYear = this.settings.hoursPerYear != null ? parseFloat(this.settings.hoursPerYear as any) : 2000; | ||
const hoursPerYear = this.settings.hoursPerYear != null ? parseFloat(this.settings.hoursPerYear as string | number | null | undefined) : 2000; | ||
const hoursPerWeek = hoursPerYear / 50 || 40; // Default to 40 if undefined |
|
||
resetTargets() { | ||
// Call the backend endpoint to recalculate targets | ||
this.targetsService.recalculateTargets().subscribe((result: any) => { |
Check failure
Code scanning / ESLint
Disallow the `any` type Error
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the issue, we need to replace the any
type with a more specific type that accurately represents the structure of the result
object returned by the recalculateTargets
method. Based on the code, result
appears to have a targets
property containing org
, user
, and impact
objects, or it might directly be the targets
object. We can define a new interface to represent this structure and use it as the type for result
.
The changes will involve:
- Defining a new interface,
RecalculateTargetsResponse
, to represent the structure of theresult
object. - Updating the type of
result
in theresetTargets
method to use this new interface.
-
Copy modified lines R24-R32 -
Copy modified line R162
@@ -23,2 +23,11 @@ | ||
|
||
interface RecalculateTargetsResponse { | ||
targets?: { | ||
org: Record<string, Target>; | ||
user: Record<string, Target>; | ||
impact: Record<string, Target>; | ||
}; | ||
logs?: any; // Optional, if logs are part of the response | ||
} | ||
|
||
@Component({ | ||
@@ -152,3 +161,3 @@ | ||
// Call the backend endpoint to recalculate targets | ||
this.targetsService.recalculateTargets().subscribe((result: any) => { | ||
this.targetsService.recalculateTargets().subscribe((result: RecalculateTargetsResponse) => { | ||
const targets = result.targets || result; // handle both {targets, logs} and just targets |
|
||
recalculateTargets() { | ||
// Calls the backend endpoint to recalculate targets | ||
return this.http.get<any>(`${this.apiUrl}/calculate`); |
Check failure
Code scanning / ESLint
Disallow the `any` type Error
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the issue, we need to replace the any
type in the recalculateTargets
method with a more specific type that matches the expected structure of the API response. If the response structure is known, we should define a new interface or reuse an existing one that accurately represents the data. If the structure is not well-defined, we can use a more generic type like unknown
or a union type to provide some level of type safety.
Steps to fix:
- Define a new interface (e.g.,
RecalculateTargetsResponse
) that matches the expected structure of the API response, or reuse an existing interface if applicable. - Update the
recalculateTargets
method to use the new type instead ofany
. - Ensure the method's implementation and any downstream code handle the response according to the defined type.
-
Copy modified lines R13-R18 -
Copy modified line R89
@@ -12,2 +12,8 @@ | ||
|
||
export interface RecalculateTargetsResponse { | ||
success: boolean; | ||
message?: string; | ||
updatedTargets?: Targets; | ||
} | ||
|
||
export interface Targets { | ||
@@ -82,3 +88,3 @@ | ||
// Calls the backend endpoint to recalculate targets | ||
return this.http.get<any>(`${this.apiUrl}/calculate`); | ||
return this.http.get<RecalculateTargetsResponse>(`${this.apiUrl}/calculate`); | ||
} |
Also, fixed time savings chart to use configured targets & max values