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

Jump to bottom

1430: Add query params to card self service form #1749

Merged
merged 10 commits into from
Nov 25, 2024
Merged

1430: Add query params to card self service form #1749

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bf49cd7
1430: Add query params to card self service form
seluianova Nov 5, 2024
34cb958
1430: Init card from query params in useCardGeneratorSelfService
seluianova Nov 11, 2024
6224bbb
1430: Use PlainDate.safeFrom() function to parse birthday from query …
seluianova Nov 11, 2024
de0acba
1430: Fix code style
seluianova Nov 11, 2024
654280a
Merge branch 'refs/heads/main' into 1430-create-card-query-params
seluianova Nov 20, 2024
64eba61
1430: Resolve merge conflicts
seluianova Nov 20, 2024
ee12261
1430: Refactor card initialization
seluianova Nov 20, 2024
db61b23
1430: Draft
seluianova Nov 23, 2024
26bc352
Merge branch 'refs/heads/main' into 1430-create-card-query-params
seluianova Nov 23, 2024
0dbdae9
1430: Remove card creation params only after the form submission
seluianova Nov 25, 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
20 changes: 15 additions & 5 deletions administration/src/bp-modules/self-service/hooks/useCardGeneratorSelfService.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { ApolloError } from '@apollo/client'
import React, { useCallback, useContext, useState } from 'react'
import React, { useCallback, useContext, useEffect, useState } from 'react'
import { useSearchParams } from 'react-router-dom'

import { Card, generateCardInfo, initializeCard } from '../../../cards/Card'
import { Card, generateCardInfo, initializeCardFromCSV } from '../../../cards/Card'
import { generatePdf } from '../../../cards/PdfFactory'
import { CreateCardsError, CreateCardsResult } from '../../../cards/createCards'
import getMessageFromApolloError from '../../../errors/getMessageFromApolloError'
Expand All @@ -12,6 +13,7 @@ import { base64ToUint8Array, uint8ArrayToBase64 } from '../../../util/base64'
import downloadDataUri from '../../../util/downloadDataUri'
import getCustomDeepLinkFromQrCode from '../../../util/getCustomDeepLinkFromQrCode'
import { useAppToaster } from '../../AppToaster'
import { getHeaders } from '../../cards/ImportCardsController'
import FormErrorMessage from '../components/FormErrorMessage'

export enum CardSelfServiceStep {
Expand All @@ -35,13 +37,21 @@ type UseCardGeneratorSelfServiceReturn = {
const useCardGeneratorSelfService = (): UseCardGeneratorSelfServiceReturn => {
const projectConfig = useContext(ProjectConfigContext)
const appToaster = useAppToaster()
const [selfServiceCard, setSelfServiceCard] = useState(
initializeCard(projectConfig.card, undefined, { expirationDate: null })
)
const [cardQueryParams, setSearchParams] = useSearchParams()
Copy link
Member

Choose a reason for hiding this comment

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

🔧

Suggested change
const [cardQueryParams, setSearchParams] = useSearchParams()
const [searchParams, setSearchParams] = useSearchParams()

const [selfServiceCard, setSelfServiceCard] = useState(() => {
const headers = getHeaders(projectConfig)
const values = headers.map(header => cardQueryParams.get(header))
return initializeCardFromCSV(projectConfig.card, values, headers, undefined, true)
})
Copy link
Member

Choose a reason for hiding this comment

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

🙃 I really like your implementation. If you feel motivated, feel free to add the same functionality to the useCardGenerator hook and remove the old implementation from AddCardsFrom

Copy link
Contributor

Choose a reason for hiding this comment

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

i think we should merge this soon to create the next beta artifact and i would prefer to implement this in a separate task

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have created a new ticket for that #1799
@steffenkleinle could you please check and adjust the description (if needed), because I'm not sure I could describe this task nicely, and wouldn't want to mislead with the description.

const [isLoading, setIsLoading] = useState<boolean>(false)
const [selfServiceState, setSelfServiceState] = useState<CardSelfServiceStep>(CardSelfServiceStep.form)
const [deepLink, setDeepLink] = useState<string>('')
const [code, setCode] = useState<CreateCardsResult | null>(null)

useEffect(() => {
setSearchParams(undefined, { replace: true })
Copy link
Member

Choose a reason for hiding this comment

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

🔧
Could you perhaps move this inside the initializing method to avoid the extra use effect and possible side effects?

Copy link
Contributor

@f1sh1918 f1sh1918 Nov 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

i think its better to move this to the createKoblenzPass function to only remove them if the card was created successfully, so the params will not be removed if there is a page refresh (maybe by accident)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@f1sh1918 pls check if that's what you mean

Copy link
Contributor

@f1sh1918 f1sh1918 Nov 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

i think you can just reuse the hook in CardSelfServiceForm @seluianova

const [_, setSearchParams] = useSearchParams()

and execute the function as you did. Then you don't have to pass down the function neither to CardSelfServiceForm nor adjust the useCardSelfService hook

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hopefully, it's good to go now, thx 🙈

}, [setSearchParams])

const [createCardsSelfService] = useCreateCardsFromSelfServiceMutation()

const handleErrors = useCallback(
Expand Down
4 changes: 2 additions & 2 deletions administration/src/cards/Card.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,12 @@ export const initializeCardFromCSV = (
cardConfig: CardConfig,
line: (string | null)[],
headers: string[],
region: Region,
region: Region | undefined,
withDefaults = false
): Card => {
const defaultCard = withDefaults
? initializeCard(cardConfig, region)
: { fullName: '', expirationDate: null, extensions: { [REGION_EXTENSION_NAME]: region.id } }
: { fullName: '', expirationDate: null, extensions: region ? { [REGION_EXTENSION_NAME]: region.id } : {} }
const extensions = headers.reduce((acc, header, index) => {
const value = line[index]
const extension = Extensions.find(extension => extension.name === getExtensionNameByCSVHeader(cardConfig, header))
Expand Down
2 changes: 1 addition & 1 deletion administration/src/cards/extensions/BirthdayExtension.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const BirthdayExtension: Extension<BirthdayExtensionState> = {
},
}),
isValid: ({ birthday }: BirthdayExtensionState) => {
if (birthday === null) {
if (!birthday) {
return false
}
const today = PlainDate.fromLocalDate(new Date())
Expand Down