Confirm modal on meeting creation #106
Conversation
✅ Deploy Preview for rolebase ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Chloengr
force-pushed
the
chloe/meeting-concurrence
branch
from
2173d36 to
9777e5a
Compare
Godefroy
left a comment
Godefroy
left a comment
There was a problem hiding this comment.
Je t'ai mis quelques commentaires, notamment pour unifier la nomenclature autour du terme "Overlap" (plutôt que "atsametime", "confirm", "match") mais je pense qu'il serait intéressant d'imaginer un changement un peu plus important. Le state newMeeting et le chemin que la data prend avec la modale n'est pas très clair et peut être source d'erreurs lors de changements ultérieurs.
On pourrait calculer les participants ayant une réu au même moment dès qu'on a choisi un rôle et à chaque fois qu'on modifier la date ou la durée. La liste des participants et des meetings concernés pourraient être affichés dans une <Alert> en dessous de la date quand il y a un overlap plutôt que dans une modale.
|
|
||
|
|
||
| query meetingsAtSameTime($NMstartDate: timestamptz!, $NMendDate: timestamptz!) { | ||
| meeting(where: { archived: {_eq: false}, |
There was a problem hiding this comment.
Il manque orgId. Là ça compare avec les réus de toutes les orgs.
| { startDate: { _gt: $NMstartDate }, endDate: { _lt: $NMendDate } }, | ||
| { startDate: { _lte: $NMstartDate }, endDate: { _gte: $NMendDate } }, | ||
| {_and: [{ startDate: { _lte: $NMstartDate }, endDate: { _gte: $NMstartDate } }, { endDate: { _lte: $NMendDate } }]}, | ||
| {_and: [{ startDate: { _gte: $NMstartDate }, endDate: { _gte: $NMendDate } }, { startDate: { _lt: $NMendDate } }]} |
There was a problem hiding this comment.
Tu dois pouvoir faire plus simple et performant avec la fonction postgres OVERLAPS.
https://database.guide/how-to-test-for-overlapping-dates-in-postgresql/
Je ne crois pas qu'Hasura supporte nativement cette fonction, donc tu peux créer une fonction Hasura getOverlappingMeetings :
https://hasura.io/docs/latest/schema/postgres/custom-functions/
| {_and: [{ startDate: { _gte: $NMstartDate }, endDate: { _gte: $NMendDate } }, { startDate: { _lt: $NMendDate } }]} | ||
| ]}) { | ||
| ...MeetingSummary | ||
| archived |
There was a problem hiding this comment.
Pas besoin de récupérer archived
| const [conflictingMeetings, setConflictingMeetings] = useState< | ||
| MeetingSummaryFragment[] | ||
| >([]) | ||
| const [confirmModal, setConfirmModal] = useState(false) |
There was a problem hiding this comment.
Pour faire pareil que pour les autres modales et pour renommer en overlapModal :
const overlapModal = useDisclosure()| onAccept: (currentMeeting: MeetingFormDataValues) => void | ||
| } | ||
|
|
||
| export default function MeetingConfirmModal({ |
There was a problem hiding this comment.
Renommer en MeetingOverlapModal
| export type MeetingFormDataValues = Omit< | ||
| MeetingSummaryFragment, | ||
| 'id' | 'orgId' | 'ended' | ||
| > |
There was a problem hiding this comment.
Tu dois pouvoir utiliser l'interface Values à la place
| const circleId = watch('circleId') | ||
| const circle = useCircle(circleId) | ||
|
|
||
| const [checkConflictingMeetings] = useMeetingsAtSameTimeLazyQuery() |
There was a problem hiding this comment.
Renommer en :
const [getOverlappingMeetings] = useGetOverlappingMeetings()
| } | ||
|
|
||
|
|
||
| query meetingsAtSameTime($NMstartDate: timestamptz!, $NMendDate: timestamptz!) { |
There was a problem hiding this comment.
Je vais te proposer quelques changements de nomenclature pour mieux s'y retrouver.
Ici, renommer en getOverlappingMeetings
| } catch (error) { | ||
| console.error(error) | ||
| } | ||
| } |
There was a problem hiding this comment.
Tu peux réintégrer cette fonction dans onSubmit, ça sera plus lisible à mon avis.
| meeting.participantsScope, | ||
| meeting.participantsMembersIds | ||
| ).map((p) => p.member) | ||
| }) |
There was a problem hiding this comment.
Il ne faut pas appeler un hook dans un map, car le nombre d'exécution peut varier et c'est interdit par React.
Il vaut mieux dans ce cas faire un map dans un useMemo et s'inspirer de la logique de useParticipants.
Tu peux faire le matching par la même occasion.
Ticket : https://app.clickup.com/t/860r3r4am