-
-
Notifications
You must be signed in to change notification settings - Fork 353
feat: integration info during event creation #2292
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,21 @@ | ||
| import { Button, Checkbox, Heading, Grid, Text } from '@chakra-ui/react'; | ||
| import { | ||
| Button, | ||
| Checkbox, | ||
| Container, | ||
| Grid, | ||
| Heading, | ||
| Spinner, | ||
| Text, | ||
| } from '@chakra-ui/react'; | ||
| import React, { useMemo } from 'react'; | ||
| import { useForm, FormProvider } from 'react-hook-form'; | ||
| import { add } from 'date-fns'; | ||
|
|
||
| import { InfoIcon, WarningTwoIcon } from '@chakra-ui/icons'; | ||
| import { | ||
| useChapterQuery, | ||
| useCalendarIntegrationStatusQuery, | ||
| useChapterVenuesQuery, | ||
| useDashboardChapterQuery, | ||
| useSponsorsQuery, | ||
| VenueType, | ||
| } from '../../../../generated/graphql'; | ||
|
|
@@ -20,6 +30,57 @@ import EventSponsorsForm from './EventSponsorsForm'; | |
| import EventVenueForm from './EventVenueForm'; | ||
| import { EventFormProps, fields, EventFormData } from './EventFormUtils'; | ||
|
|
||
| interface IntegrationInfoData { | ||
| isAuthenticated: boolean | null | undefined; | ||
| hasCalendar: boolean; | ||
| } | ||
|
|
||
| const IntegrationInfo = ({ | ||
| isAuthenticated, | ||
| hasCalendar, | ||
| }: IntegrationInfoData) => { | ||
| const infoData = ({ isAuthenticated, hasCalendar }: IntegrationInfoData) => { | ||
| if (isAuthenticated) { | ||
| if (hasCalendar) { | ||
| return { | ||
| Icon: InfoIcon, | ||
| text: 'Instance is authenticated with calendar api, and chapter has created calendar. Chapter will attempt creating event in the calendar.', | ||
| }; | ||
| } | ||
| return { | ||
| Icon: WarningTwoIcon, | ||
| text: "Instance is authenticated with calendar api, but chapter doesn't have calendar created. Event will not be created in the calendar.", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about the following? "This chapter does not have a calendar. To allow calendar event creation, go to |
||
| }; | ||
| } | ||
|
|
||
| const isBroken = isAuthenticated === null; | ||
| if (isBroken) { | ||
| if (hasCalendar) { | ||
| return { | ||
| Icon: WarningTwoIcon, | ||
| text: 'Chapter has calendar created, but calendar integration is not working. Event will not be created in the calendar.', | ||
| }; | ||
| } | ||
| return { | ||
| Icon: WarningTwoIcon, | ||
| text: 'Calendar integration is not working, and chapter does not have calendar created. Event will not be created in calendar.', | ||
| }; | ||
| } | ||
| return { | ||
| Icon: WarningTwoIcon, | ||
| text: 'Instance is not authenticated with calendar api. Automatic creation of events in calendar is not possible.', | ||
| }; | ||
|
Comment on lines
+56
to
+72
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to distinguish between these three cases? I think it makes sense to handle With that in mind, how about this
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a bit assuming that person seeing these warnings not necessarily has privileges to (re-)authenticate. Otherwise the alert on the top would be catching the attention to re-authenticate. |
||
| }; | ||
|
|
||
| const { Icon, text } = infoData({ isAuthenticated, hasCalendar }); | ||
| return ( | ||
| <Container> | ||
| <Icon boxSize={5} marginRight={1} /> | ||
| {text} | ||
| </Container> | ||
| ); | ||
| }; | ||
|
|
||
| const EventForm: React.FC<EventFormProps> = (props) => { | ||
| const { | ||
| onSubmit, | ||
|
|
@@ -31,18 +92,6 @@ const EventForm: React.FC<EventFormProps> = (props) => { | |
| } = props; | ||
| const isChaptersDropdownNeeded = typeof initialChapterId === 'undefined'; | ||
|
|
||
| const queryOptions = isChaptersDropdownNeeded | ||
| ? { skip: true } | ||
| : { variables: { chapterId: initialChapterId } }; | ||
|
|
||
| const { | ||
| loading: loadingChapter, | ||
| error: errorChapter, | ||
| data: dataChapter, | ||
| } = useChapterQuery(queryOptions); | ||
|
|
||
| const sponsorQuery = useSponsorsQuery(); | ||
|
|
||
| const defaultValues = useMemo(() => { | ||
| if (!data) { | ||
| const date = new Date(); | ||
|
|
@@ -83,6 +132,16 @@ const EventForm: React.FC<EventFormProps> = (props) => { | |
| const inviteOnly = watch('invite_only'); | ||
| const chapterId = watch('chapter_id'); | ||
|
|
||
| const { | ||
| loading: loadingChapter, | ||
| error: errorChapter, | ||
| data: dataChapter, | ||
| } = useDashboardChapterQuery({ variables: { chapterId } }); | ||
| const { loading: loadingStatus, data: dataStatus } = | ||
| useCalendarIntegrationStatusQuery({ skip: !!data }); | ||
|
|
||
| const sponsorQuery = useSponsorsQuery(); | ||
|
|
||
| const chapterVenuesQuery = useChapterVenuesQuery( | ||
| !chapterId ? { skip: true } : { variables: { chapterId } }, | ||
| ); | ||
|
|
@@ -101,10 +160,10 @@ const EventForm: React.FC<EventFormProps> = (props) => { | |
| {!isChaptersDropdownNeeded || data ? ( | ||
| loadingChapter ? ( | ||
| <Text>Loading Chapter</Text> | ||
| ) : errorChapter || !dataChapter?.chapter ? ( | ||
| ) : errorChapter || !dataChapter?.dashboardChapter ? ( | ||
| <Text>Error loading chapter</Text> | ||
| ) : ( | ||
| <Heading>{dataChapter.chapter.name}</Heading> | ||
| <Heading>{dataChapter.dashboardChapter.name}</Heading> | ||
| ) | ||
| ) : ( | ||
| <EventChapterSelect loading={loading} /> | ||
|
|
@@ -160,6 +219,16 @@ const EventForm: React.FC<EventFormProps> = (props) => { | |
| </Checkbox> | ||
| )} | ||
|
|
||
| {!data && | ||
| (loadingStatus || loadingChapter ? ( | ||
| <Spinner /> | ||
| ) : ( | ||
| <IntegrationInfo | ||
| isAuthenticated={dataStatus?.calendarIntegrationStatus} | ||
| hasCalendar={!!dataChapter?.dashboardChapter.has_calendar} | ||
| /> | ||
| ))} | ||
|
|
||
| <Grid | ||
| gridTemplateColumns="repeat(auto-fit, minmax(13rem, 1fr))" | ||
| width="100%" | ||
|
|
||
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.
It doesn't seem necessary to warn people that things are going to work as expected
Uh oh!
There was an error while loading. Please reload this page.
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.
I wanted to include it as a part if something goes wrong, as there isn't clear path to inform client when creating calendar event errors.
This got me thinking, what if creating calendar and calendar event is pulled out of the create chapter/event resolvers? The actual creation can be then run once entry is created. That gives ability to separate and inform about errors on both these steps:
Edit: Or keep it in resolver and just check in object returned by mutation 🤔
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.
I like the sound of that.
Potentially the Google Event creation could fail in a bunch of ways (lack of permissions, rate limits, network error, bad configuration etc. etc.), so rather than trying to warn organisers that the Event will not get created we should focus on handling the errors when it inevitably does fail. Particularly since there are no checks we can carry out that will guarantee that an Event will be created - only that it has been.
Even if the owner has forgotten to authenticate with Google, it should only be a problem for the first event. After which they'll see the warning, authenticate and create the calendar event (once that's implemented).
What do you reckon?
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.
That seems to be better way of handling it. I'll block this PR for now, start with simple informing if calendar/event in calendar was created, and see how to go from there.
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.
Cool, that sounds like the way to go.