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

1953: Avoid non-null assertions for WhoAmI #1954

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+58 −46
Open

1953: Avoid non-null assertions for WhoAmI #1954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions administration/src/KeepAliveToken.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { useTranslation } from 'react-i18next'
import { useNavigate } from 'react-router-dom'

import { TokenPayload } from './AuthProvider'
import { WhoAmIContext } from './WhoAmIProvider'
import { useWhoAmI } from './WhoAmIProvider'
import { useAppToaster } from './bp-modules/AppToaster'
import PasswordInput from './bp-modules/PasswordInput'
import getMessageFromApolloError from './errors/getMessageFromApolloError'
Expand All @@ -24,7 +24,7 @@ const KeepAliveToken = ({ authData, onSignOut, onSignIn, children }: Props): Rea
const { t } = useTranslation('auth')
const navigate = useNavigate()
const projectId = useContext(ProjectConfigContext).projectId
const email = useContext(WhoAmIContext).me!.email
const email = useWhoAmI().me.email
const [secondsLeft, setSecondsLeft] = useState(computeSecondsLeft(authData))
useEffect(() => {
setSecondsLeft(computeSecondsLeft(authData))
Expand Down
16 changes: 14 additions & 2 deletions administration/src/WhoAmIProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,26 @@ import StandaloneCenter from './bp-modules/StandaloneCenter'
import { WhoAmIQuery, useWhoAmIQuery } from './generated/graphql'
import { ProjectConfigContext } from './project-configs/ProjectConfigContext'

export const WhoAmIContext = createContext<{
type WhoAmIContextType = {
me: WhoAmIQuery['me'] | null
refetch: () => void
}>({
}

export const WhoAmIContext = createContext<WhoAmIContextType>({
me: null,
refetch: () => undefined,
})

type UseWhoAmIReturn = WhoAmIContextType & { me: WhoAmIQuery['me'] }

export const useWhoAmI = (): UseWhoAmIReturn => {
const { me, ...context } = useContext(WhoAmIContext)
Copy link
Member

Choose a reason for hiding this comment

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

I think we should throw an error, if me is null.
Also, I think it's not necessary to create a new object, we can just return whatever useContext(WhoAmIContext) returns.

I am also a bit confused about the @ts-expect-errror on line 58. 🤔

Copy link
Member Author

@steffenkleinle steffenkleinle Mar 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think we should throw an error, if me is null.

👍

Also, I think it's not necessary to create a new object, we can just return whatever useContext(WhoAmIContext) returns.

I am also a bit confused about the @ts-expect-errror on line 58. 🤔

Typescript seems to have problems when checking object properties for nullish values and raises an error in that case. This is why we have the @ts-expect-error. Same for directly returning the context without destructuring/creating a new object like this:

  const context = useContext(WhoAmIContext)
  if (context.me === null) {
    throw new Error('')
  }
  // TS 2322: Type null is not assignable to type ...
  return context

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, this seems to be a known limitation of typescript 🤔 . Probably, it's better to do

  const context = useContext(WhoAmIContext)
  if (context.me === null) {
    throw new Error('WhoAmI context is not available')
  }
  return context as UseWhoAmIReturn

(and below we can probably do value as WhoAmIContextType instead of ignoring the error)

Copy link
Member Author

@steffenkleinle steffenkleinle Mar 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, I really don't like typecasts if we can avoid them, do you think that is a better solution here instead of destructuring the object in this case? I agree for below, doesn't really matter if we have a @ts-expect-error or a typecast, but looks cleaner.

Copy link
Member

@michael-markl michael-markl Mar 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

You could also abstract away this typecast in a type guard like

const hasProp= <
  P extends PropertyKey,
  O extends { [p in P]: unknown }
>(obj: O, p: P): obj is (O & { [p in P]: NonNullable<unknown> }) => obj[p] !== undefined && obj[p] !== null


export const useWhoAmI = (): UseWhoAmIReturn => {
  const context = useContext(WhoAmIContext)
  if (!hasProp(context, 'me')) {
    throw Error("WhoAmI context is not available")
  }
  return context
}

Maybe, that's the cleanest, but slightly overwhelming solution...

if (!me) {
throw new Error('WhoAmI context is not available')
}
return { me, ...context }
}

const WhoAmIProvider = ({ children }: { children: ReactNode }): ReactElement => {
const { t } = useTranslation('auth')
const { projectId } = useContext(ProjectConfigContext)
Expand Down
4 changes: 2 additions & 2 deletions administration/src/bp-modules/NavigationBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { useTranslation } from 'react-i18next'
import { NavLink } from 'react-router-dom'
import styled from 'styled-components'

import { WhoAmIContext } from '../WhoAmIProvider'
import { useWhoAmI } from '../WhoAmIProvider'
import { Role } from '../generated/graphql'
import { ProjectConfigContext } from '../project-configs/ProjectConfigContext'
import UserMenu from './UserMenu'
Expand All @@ -24,7 +24,7 @@ type Props = {
const Navigation = ({ onSignOut }: Props): ReactElement => {
const { t } = useTranslation('misc')
const config = useContext(ProjectConfigContext)
const { region, role } = useContext(WhoAmIContext).me!
const { region, role } = useWhoAmI().me
const canSeeProjectSettings =
(role === Role.ProjectAdmin && config.userImportApiEnabled) || role === Role.ExternalVerifiedApiUser

Expand Down
6 changes: 3 additions & 3 deletions administration/src/bp-modules/UserMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { Button, Divider, Menu, Popover } from '@blueprintjs/core'
import React, { ReactElement, useContext, useState } from 'react'
import React, { ReactElement, useState } from 'react'
import { useTranslation } from 'react-i18next'
import { NavLink, useNavigate } from 'react-router-dom'
import styled from 'styled-components'

import { WhoAmIContext } from '../WhoAmIProvider'
import { useWhoAmI } from '../WhoAmIProvider'
import roleToText from './users/utils/roleToText'

type UserMenuProps = {
Expand Down Expand Up @@ -41,7 +41,7 @@ const RoleInfo = styled.span`
`

const UserMenu = ({ onSignOut }: UserMenuProps): ReactElement => {
const { role, email } = useContext(WhoAmIContext).me!
const { role, email } = useWhoAmI().me
const { t } = useTranslation('misc')
const [isOpen, setIsOpen] = useState<boolean>(false)
const navigate = useNavigate()
Expand Down
6 changes: 3 additions & 3 deletions administration/src/bp-modules/applications/ApplicationsController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { NonIdealState } from '@blueprintjs/core'
import React, { ReactElement, useContext } from 'react'
import React, { ReactElement } from 'react'
import { useTranslation } from 'react-i18next'

import { WhoAmIContext } from '../../WhoAmIProvider'
import { useWhoAmI } from '../../WhoAmIProvider'
import { Region, useGetApplicationsQuery } from '../../generated/graphql'
import getQueryResult from '../util/getQueryResult'
import ApplicationsOverview from './ApplicationsOverview'
Expand All @@ -21,7 +21,7 @@ const ApplicationsController = ({ region }: { region: Region }) => {
}

const ControllerWithRegion = (): ReactElement => {
const region = useContext(WhoAmIContext).me!.region
const region = useWhoAmI().me.region
const { t } = useTranslation('errors')

if (!region) {
Expand Down
6 changes: 3 additions & 3 deletions administration/src/bp-modules/cards/AddCardsController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { NonIdealState, Spinner } from '@blueprintjs/core'
import React, { ReactElement, useContext } from 'react'
import React, { ReactElement } from 'react'
import { useTranslation } from 'react-i18next'
import { useNavigate } from 'react-router-dom'

import { WhoAmIContext } from '../../WhoAmIProvider'
import { useWhoAmI } from '../../WhoAmIProvider'
import { Region } from '../../generated/graphql'
import useBlockNavigation from '../../util/useBlockNavigation'
import AddCardsForm from './AddCardsForm'
Expand Down Expand Up @@ -65,7 +65,7 @@ const InnerAddCardsController = ({ region }: { region: Region }) => {
}

const AddCardsController = (): ReactElement => {
const { region } = useContext(WhoAmIContext).me!
const { region } = useWhoAmI().me
const { t } = useTranslation('cards')

if (!region) {
Expand Down
4 changes: 2 additions & 2 deletions administration/src/bp-modules/cards/CreateCardsController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { useTranslation } from 'react-i18next'
import { useNavigate } from 'react-router-dom'
import styled from 'styled-components'

import { WhoAmIContext } from '../../WhoAmIProvider'
import { useWhoAmI } from '../../WhoAmIProvider'
import { ProjectConfigContext } from '../../project-configs/ProjectConfigContext'
import StandaloneCenter from '../StandaloneCenter'
import { FREINET_PARAM } from '../constants'
Expand All @@ -15,7 +15,7 @@ const Buttons = styled(ButtonGroup)`
`

const CreateCardsController = (): ReactElement => {
const { region } = useContext(WhoAmIContext).me!
const { region } = useWhoAmI().me
const { freinetCSVImportEnabled } = useContext(ProjectConfigContext)
const { t } = useTranslation('cards')

Expand Down
6 changes: 3 additions & 3 deletions administration/src/bp-modules/cards/ImportCardsController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { NonIdealState, Spinner } from '@blueprintjs/core'
import React, { ReactElement, useContext } from 'react'
import React, { ReactElement } from 'react'
import { useTranslation } from 'react-i18next'
import { useNavigate } from 'react-router-dom'

import { WhoAmIContext } from '../../WhoAmIProvider'
import { useWhoAmI } from '../../WhoAmIProvider'
import { Region } from '../../generated/graphql'
import useBlockNavigation from '../../util/useBlockNavigation'
import GenerationFinished from './CardsCreatedMessage'
Expand Down Expand Up @@ -63,7 +63,7 @@ const InnerImportCardsController = ({ region }: { region: Region }): ReactElemen
}

const ImportCardsController = (): ReactElement => {
const { region } = useContext(WhoAmIContext).me!
const { region } = useWhoAmI().me
const { t } = useTranslation('errors')
if (!region) {
return <NonIdealState icon='cross' title={t('notAuthorized')} description={t('notAuthorizedToCreateCards')} />
Expand Down
4 changes: 2 additions & 2 deletions administration/src/bp-modules/home/HomeController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { useTranslation } from 'react-i18next'
import { NavLink } from 'react-router-dom'
import styled from 'styled-components'

import { WhoAmIContext } from '../../WhoAmIProvider'
import { useWhoAmI } from '../../WhoAmIProvider'
import { Role } from '../../generated/graphql'
import { ProjectConfigContext } from '../../project-configs/ProjectConfigContext'

Expand All @@ -20,7 +20,7 @@ const Container = styled.div`

const HomeController = (): ReactElement => {
const { applicationFeature, cardStatistics, cardCreation, userImportApiEnabled } = useContext(ProjectConfigContext)
const { role } = useContext(WhoAmIContext).me!
const { role } = useWhoAmI().me
const { t } = useTranslation('home')

return (
Expand Down
4 changes: 2 additions & 2 deletions administration/src/bp-modules/project-settings/ProjectSettingsController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React, { ReactElement, useContext } from 'react'
import { useTranslation } from 'react-i18next'
import styled from 'styled-components'

import { WhoAmIContext } from '../../WhoAmIProvider'
import { useWhoAmI } from '../../WhoAmIProvider'
import { Role } from '../../generated/graphql'
import { ProjectConfigContext } from '../../project-configs/ProjectConfigContext'
import ApiTokenSettings from './ApiTokenSettings'
Expand All @@ -18,7 +18,7 @@ const ProjectSettingsContainer = styled.div`

const ProjectSettingsController = (): ReactElement => {
const { userImportApiEnabled } = useContext(ProjectConfigContext)
const { role } = useContext(WhoAmIContext).me!
const { role } = useWhoAmI().me
const { t } = useTranslation('errors')

if ((role !== Role.ProjectAdmin || !userImportApiEnabled) && role !== Role.ExternalVerifiedApiUser) {
Expand Down
6 changes: 3 additions & 3 deletions administration/src/bp-modules/regions/RegionController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { NonIdealState } from '@blueprintjs/core'
import React, { ReactElement, useContext } from 'react'
import React, { ReactElement } from 'react'
import { useTranslation } from 'react-i18next'
import styled from 'styled-components'

import { WhoAmIContext } from '../../WhoAmIProvider'
import { useWhoAmI } from '../../WhoAmIProvider'
import { Role } from '../../generated/graphql'
import DataPrivacyCard from './DataPrivacyCard'
import RegionSettingsController from './RegionSettingsController'
Expand All @@ -17,7 +17,7 @@ const RegionSettingsContainer = styled.div`
`

const RegionController = (): ReactElement => {
const { region, role } = useContext(WhoAmIContext).me!
const { region, role } = useWhoAmI().me
const { t } = useTranslation('errors')
if (!region || role !== Role.RegionAdmin) {
return <NonIdealState icon='cross' title={t('notAuthorized')} description={t('notAuthorizedForRegionSettings')} />
Expand Down
6 changes: 3 additions & 3 deletions administration/src/bp-modules/regions/data-privacy-policy/DataPrivacyController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { NonIdealState } from '@blueprintjs/core'
import React, { ReactElement, useContext } from 'react'
import React, { ReactElement } from 'react'
import { useTranslation } from 'react-i18next'

import { WhoAmIContext } from '../../../WhoAmIProvider'
import { useWhoAmI } from '../../../WhoAmIProvider'
import { Role, useGetDataPolicyQuery } from '../../../generated/graphql'
import getQueryResult from '../../util/getQueryResult'
import DataPrivacyOverview from './DataPrivacyOverview'
Expand All @@ -24,7 +24,7 @@ const DataPrivacyController = ({ regionId }: { regionId: number }) => {
}

const DataPrivacyWithRegion = (): ReactElement => {
const { region, role } = useContext(WhoAmIContext).me!
const { region, role } = useWhoAmI().me
const { t } = useTranslation('errors')
if (!region || role !== Role.RegionAdmin) {
return <NonIdealState icon='cross' title={t('notAuthorized')} description={t('notAuthorizedForDataPrivacy')} />
Expand Down
4 changes: 2 additions & 2 deletions administration/src/bp-modules/statistics/StatisticsController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { NonIdealState } from '@blueprintjs/core'
import React, { ReactElement, useContext } from 'react'
import { useTranslation } from 'react-i18next'

import { WhoAmIContext } from '../../WhoAmIProvider'
import { useWhoAmI } from '../../WhoAmIProvider'
import {
Region,
Role,
Expand Down Expand Up @@ -61,7 +61,7 @@ const ViewRegionStatistics = ({ region }: { region: Region }) => {
)
}
const StatisticsController = (): ReactElement => {
const { role, region } = useContext(WhoAmIContext).me!
const { role, region } = useWhoAmI().me
const { cardStatistics } = useContext(ProjectConfigContext)
const { t } = useTranslation('errors')

Expand Down
4 changes: 2 additions & 2 deletions administration/src/bp-modules/statistics/StatisticsOverview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { ReactElement, useContext } from 'react'
import { useTranslation } from 'react-i18next'

import { WhoAmIContext } from '../../WhoAmIProvider'
import { useWhoAmI } from '../../WhoAmIProvider'
import { CardStatisticsResultModel, Region, Role } from '../../generated/graphql'
import { ProjectConfigContext } from '../../project-configs/ProjectConfigContext'
import downloadDataUri from '../../util/downloadDataUri'
Expand All @@ -18,7 +18,7 @@ type StatisticsOverviewProps = {
}

const StatisticsOverview = ({ statistics, onApplyFilter, region }: StatisticsOverviewProps): ReactElement => {
const { role } = useContext(WhoAmIContext).me!
const { role } = useWhoAmI().me
const appToaster = useAppToaster()
const { cardStatistics } = useContext(ProjectConfigContext)
const { t } = useTranslation('statistics')
Expand Down
4 changes: 2 additions & 2 deletions administration/src/bp-modules/stores/StoresController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { useTranslation } from 'react-i18next'
import { useNavigate } from 'react-router-dom'
import styled from 'styled-components'

import { WhoAmIContext } from '../../WhoAmIProvider'
import { useWhoAmI } from '../../WhoAmIProvider'
import { Role } from '../../generated/graphql'
import { ProjectConfigContext } from '../../project-configs/ProjectConfigContext'
import StandaloneCenter from '../StandaloneCenter'
Expand All @@ -16,7 +16,7 @@ const Buttons = styled(ButtonGroup)`

const StoresController = (): ReactElement => {
const navigate = useNavigate()
const { role } = useContext(WhoAmIContext).me!
const { role } = useWhoAmI().me
const storesManagement = useContext(ProjectConfigContext).storesManagement
const { t } = useTranslation('stores')

Expand Down
4 changes: 2 additions & 2 deletions administration/src/bp-modules/stores/StoresImportController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { useTranslation } from 'react-i18next'
import { useNavigate } from 'react-router-dom'
import styled from 'styled-components'

import { WhoAmIContext } from '../../WhoAmIProvider'
import { useWhoAmI } from '../../WhoAmIProvider'
import getMessageFromApolloError from '../../errors/getMessageFromApolloError'
import { Role, useImportAcceptingStoresMutation } from '../../generated/graphql'
import { ProjectConfigContext } from '../../project-configs/ProjectConfigContext'
Expand Down Expand Up @@ -95,7 +95,7 @@ const StoresImport = ({ fields }: StoreImportProps): ReactElement => {
}

const StoresImportController = (): ReactElement => {
const { role } = useContext(WhoAmIContext).me!
const { role } = useWhoAmI().me
const storesManagement = useContext(ProjectConfigContext).storesManagement
const { t } = useTranslation('errors')
if (role !== Role.ProjectStoreManager || !storesManagement.enabled) {
Expand Down
4 changes: 2 additions & 2 deletions administration/src/bp-modules/user-settings/ChangePasswordForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Button, Callout, H2 } from '@blueprintjs/core'
import React, { ReactElement, useContext, useState } from 'react'
import { useTranslation } from 'react-i18next'

import { WhoAmIContext } from '../../WhoAmIProvider'
import { useWhoAmI } from '../../WhoAmIProvider'
import getMessageFromApolloError from '../../errors/getMessageFromApolloError'
import { useChangePasswordMutation } from '../../generated/graphql'
import { ProjectConfigContext } from '../../project-configs/ProjectConfigContext'
Expand Down Expand Up @@ -36,7 +36,7 @@ const ChangePasswordForm = (): ReactElement => {
})

const project = useContext(ProjectConfigContext).projectId
const email = useContext(WhoAmIContext).me!.email
const email = useWhoAmI().me.email

const isDirty = newPassword !== '' || repeatNewPassword !== ''

Expand Down
4 changes: 2 additions & 2 deletions administration/src/bp-modules/user-settings/UserSettingsController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { ReactElement, useContext } from 'react'
import styled from 'styled-components'

import { WhoAmIContext } from '../../WhoAmIProvider'
import { useWhoAmI } from '../../WhoAmIProvider'
import { Role } from '../../generated/graphql'
import { ProjectConfigContext } from '../../project-configs/ProjectConfigContext'
import ActivityLogCard from './ActivityLogCard'
Expand All @@ -18,7 +18,7 @@ const UserSettingsContainer = styled.div`

const UserSettingsController = (): ReactElement => {
const { applicationFeature, activityLogConfig, projectId } = useContext(ProjectConfigContext)
const { role } = useContext(WhoAmIContext).me!
const { role } = useWhoAmI().me
return (
<UserSettingsContainer>
{applicationFeature && role !== Role.ProjectAdmin && <NotificationSettings projectId={projectId} />}
Expand Down
4 changes: 2 additions & 2 deletions administration/src/bp-modules/users/ManageUsersController.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Card, H3, NonIdealState } from '@blueprintjs/core'
import React, { ReactElement, useContext } from 'react'
import { useTranslation } from 'react-i18next'

import { WhoAmIContext } from '../../WhoAmIProvider'
import { useWhoAmI } from '../../WhoAmIProvider'
import {
Region,
Role,
Expand Down Expand Up @@ -78,7 +78,7 @@ const ManageRegionUsers = ({ region }: { region: Region }) => {

const ManageUsersController = (): ReactElement => {
const { t } = useTranslation('errors')
const { role, region } = useContext(WhoAmIContext).me!
const { role, region } = useWhoAmI().me
if (role === Role.RegionAdmin && region) {
return <ManageRegionUsers region={region} />
}
Expand Down
4 changes: 2 additions & 2 deletions administration/src/bp-modules/users/RoleSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { HTMLSelect } from '@blueprintjs/core'
import React, { ReactElement, useContext } from 'react'
import { useTranslation } from 'react-i18next'

import { WhoAmIContext } from '../../WhoAmIProvider'
import { useWhoAmI } from '../../WhoAmIProvider'
import { Role } from '../../generated/graphql'
import { ProjectConfigContext } from '../../project-configs/ProjectConfigContext'
import roleToText from './utils/roleToText'
Expand All @@ -18,7 +18,7 @@ const RoleSelector = ({
}): ReactElement => {
const { t } = useTranslation('users')
const config = useContext(ProjectConfigContext)
const { role: currentUserRole } = useContext(WhoAmIContext).me!
const { role: currentUserRole } = useWhoAmI().me

return (
<HTMLSelect fill onChange={e => onChange((e.target.value as Role | null) ?? null)} value={role ?? ''} required>
Expand Down