-
Notifications
You must be signed in to change notification settings - Fork 21
[PROD RELEASE] - Copilot Portal tweaks & tech dept clean up #1222
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
Conversation
PM-1612 Fix Applications tab view
…d-diceid PM-1764 - remove all instances of 2fa and diceid
setActiveTab('0') | ||
} | ||
}, [activeTabHash, isAdminOrPM]) | ||
setActiveTab(activeTabHash) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of the isAdminOrPM
condition changes the behavior of the useEffect
hook. Previously, the activeTab
was set to '0'
if the user was not an admin or PM. Now, it always sets activeTab
to activeTabHash
. Ensure this change aligns with the intended functionality and does not introduce any unintended behavior for non-admin/PM users.
defaultActive={activeTab} | ||
onChange={handleTabChange} | ||
tabs={getCopilotDetailsTabsConfig(isAdminOrPM, copilotApplications?.length || 0)} | ||
tabs={getCopilotDetailsTabsConfig(copilotApplications?.length || 0)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getCopilotDetailsTabsConfig
function call has been modified to remove the isAdminOrPM
parameter. Ensure that this change does not affect the logic that determines the tabs configuration, especially if the tabs need to be different for admin or PM users.
} | ||
|
||
export const getCopilotDetailsTabsConfig = (isAdminOrPM: boolean, count: number): TabsNavItem[] => (isAdminOrPM ? [ | ||
export const getCopilotDetailsTabsConfig = (count: number): TabsNavItem[] => ([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter isAdminOrPM
has been removed from the function getCopilotDetailsTabsConfig
. Ensure that this change is intentional and that the logic dependent on this parameter is no longer needed.
@@ -1,12 +1,10 @@ | |||
import { Dispatch, FC, SetStateAction, useCallback, useContext, useState } from 'react' | |||
import { FC, useCallback, useContext } from 'react' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The imports for SetStateAction
and Dispatch
have been removed. Ensure these are not needed elsewhere in the component, as their removal might affect state management if they were previously used.
import { getAuthenticateAndEnrollRoute, getTCACertificationEnrollPath } from '../../learn.routes' | ||
import { LearnConfig } from '../../config' | ||
import { DiceModal } from '../../course-details/course-curriculum/dice-modal' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import for LearnConfig
has been removed. Verify that this configuration is not required elsewhere in the component, as its removal might lead to missing configuration settings.
import { LearnConfig } from '../../config' | ||
import { DiceModal } from '../../course-details/course-curriculum/dice-modal' | ||
|
||
interface EnrollCtaBtnProps { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import for DiceModal
has been removed. Confirm that this modal is not used in the component, as its removal might affect the UI functionality if it was previously utilized.
|
||
navigate(getTCACertificationEnrollPath(props.certification)) | ||
}, [isLoggedIn, profile?.isWipro, profile?.diceEnabled, props, navigate]) | ||
}, [isLoggedIn, props, navigate]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dependency array for the useEffect
hook has been modified. Ensure that removing profile?.isWipro
and profile?.diceEnabled
from the dependencies does not affect the logic that relies on these properties. If the logic depends on changes to these properties, they should be included in the dependency array.
} | ||
CLIENT: string | ||
REQUIRE_DICE_ID: boolean | undefined | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of REQUIRE_DICE_ID
from the LearnConfigModel
interface may affect any existing code that relies on this property. Ensure that all references to REQUIRE_DICE_ID
are updated or removed accordingly in the codebase to prevent runtime errors.
@@ -1,5 +1,4 @@ | |||
import { EnvironmentConfig } from '~/config' | |||
import { getReactEnv } from '~/config/environments/react-env' | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statement for getReactEnv
has been removed. Ensure that this function is no longer needed in the codebase. If it is still required, consider re-adding it or refactoring the code to accommodate its absence.
const [searchParams]: [URLSearchParams, unknown] = useSearchParams() | ||
|
||
const isLoggedIn: boolean = !!props.profile | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable isTcAcademyPolicyModal
is declared but not used in the code. Consider removing it if it's unnecessary to avoid unused state variables.
// if the user is logged in, | ||
// and the user is a either not wipro user or is a wipro user with dice enabled, | ||
// and if the user has accepted the academic honesty policy, | ||
// the user is permitted to take the course, so there's nothing to do. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment on line 424 seems to be incomplete or not aligned with the current code logic. Consider updating the comment to accurately reflect the conditions being checked in the if
statement.
@@ -1,17 +1,9 @@ | |||
import { ReactComponent as MFAImage } from './mfa.svg' | |||
import { ReactComponent as AppleStore } from './apple-store.svg' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statement for MFAImage
has been removed. Ensure that this component is no longer needed in the codebase, or if it has been moved to another file, update the import path accordingly.
import diceIdLogoBig from './dicelogobig.png' | ||
import diceIdLogoSmall from './dicelogosmall.png' | ||
import googlePlay from './google-play.png' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statements for diceIdLogo
, diceIdLogoBig
, and diceIdLogoSmall
have been removed. Verify that these assets are not used elsewhere in the code. If they are still required, ensure they are imported from the correct location.
@@ -1,17 +1,8 @@ | |||
import { xhrGetAsync, xhrPatchAsync } from '../../../xhr' | |||
import { xhrPatchAsync } from '../../../xhr' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statement for xhrGetAsync
has been removed, but ensure that it is not used elsewhere in this file or needed for any functionality.
} | ||
} | ||
|
||
export interface UserPatchRequest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MfaStatusResult
interface has been removed. Verify that this interface is not used elsewhere in the codebase or if it needs to be replaced with another implementation.
const result: MfaStatusResult = await userStoreGetMfaStatusAsync(userId) | ||
return !!result.result.content.mfaEnabled && !!result.result.content.diceEnabled | ||
} | ||
import { UserPatchRequest, userStorePatchAsync } from './user-store' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statement for userStoreGetMfaStatusAsync
and MfaStatusResult
has been removed. If these are no longer needed, ensure that any related functionality is also updated or removed accordingly.
import { UserRole } from './user-role.enum' | ||
|
||
export function create(profile: UserProfile, token?: TokenModel, hasDiceEnabled?: boolean): UserProfile { | ||
export function create(profile: UserProfile, token?: TokenModel): UserProfile { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hasDiceEnabled
parameter has been removed from the function signature. Ensure that this change is intentional and that the parameter is not used elsewhere in the codebase. If it is no longer needed, verify that all references to this parameter have been removed to prevent any runtime errors.
import { ModifyTracksRequest } from '../../modify-tracks.request' | ||
import { ModifyMemberEmailPreferencesRequest } from '../../modify-user-email-preferences.model' | ||
import { ModifyUserMFARequest, ModifyUserMFAResponse } from '../../modify-user-mfa.model' | ||
import { UpdateProfileRequest, UserPhotoUpdateResponse } from '../../modify-user-profile.model' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import for ModifyUserMFARequest
and ModifyUserMFAResponse
has been removed. Ensure that these imports are no longer needed in the codebase. If they are still used elsewhere, consider refactoring the code to accommodate their removal.
countryLookupURL, | ||
memberEmailPreferencesURL, | ||
memberModifyMfaURL, | ||
memberModifyURL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The memberModifyMfaURL
import has been removed. Ensure that this removal is intentional and that the memberModifyMfaURL
is not used elsewhere in the code, as its absence could lead to runtime errors if it is still needed.
@@ -1,9 +1,8 @@ | |||
import { tokenGetAsync, TokenModel, userGetDiceStatusAsync } from '../../auth' | |||
import { tokenGetAsync, TokenModel } from '../../auth' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The userGetDiceStatusAsync
import has been removed. Ensure that this function is no longer needed in the codebase, or if it has been moved to another file, verify that it is correctly imported there.
const dicePromise: Promise<boolean> = userGetDiceStatusAsync(token.userId) | ||
|
||
const [profileResult, diceEnabled]: [UserProfile, boolean] = await Promise.all([profilePromise, dicePromise]) | ||
const profileResult: UserProfile = await profileStoreGet(safeHandle) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The diceEnabled
variable and its associated logic have been removed. Ensure that this change does not affect any functionality that depends on the dice status. If the dice status is no longer needed, consider removing related code and dependencies elsewhere in the codebase.
fix(PM-1836): project object optional
fix(PM-1808): updated academy certification stamps
|
||
const completedDate: string = moment(props.completedDate || new Date()) | ||
.format('MMM D, YYYY') | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of the displaySignature
variable may affect the functionality if it was used elsewhere in the component. Ensure that its removal does not lead to unintended consequences or errors in the component's behavior.
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-1764
https://topcoder.atlassian.net/browse/PM-1612