improvement: Autosave settings on change

web/app/components/Header/index.tsx

@@ -678,7 +678,7 @@ interface HeaderProps {
 
 const Header = ({ refPage, transparent }: HeaderProps) => {
   const { t } = useTranslation('common')
-  const { isAuthenticated, user, logout } = useAuth()
+  const { isAuthenticated, logout } = useAuth()
   const [mobileMenuOpen, setMobileMenuOpen] = useState(false)
   const [feedbackOpen, setFeedbackOpen] = useState(false)
   const { theme, setTheme } = useTheme()

web/app/components/NotificationChannels/EnableWebPushButton.tsx

@@ -1,9 +1,10 @@
 import { BellRingingIcon } from '@phosphor-icons/react'
-import { useEffect, useRef, useState } from 'react'
+import { useEffect, useState } from 'react'
 import { useTranslation } from 'react-i18next'
 import { useFetcher } from 'react-router'
 import { toast } from 'sonner'
 
+import { useDeduplicateFetcherResponse } from '~/hooks/useDeduplicateFetcherResponse'
 import type { NotificationChannelActionData } from '~/routes/notification-channel'
 import Button from '~/ui/Button'
 
@@ -44,7 +45,8 @@ const EnableWebPushButton = ({
 }: EnableWebPushButtonProps) => {
   const { t } = useTranslation('common')
   const subscribeFetcher = useFetcher<NotificationChannelActionData>()
-  const lastSubscribeData = useRef<NotificationChannelActionData | null>(null)
+  const shouldHandleSubscribeData =
+    useDeduplicateFetcherResponse<NotificationChannelActionData>()
   const [busy, setBusy] = useState(false)
   const [supported, setSupported] = useState(true)
 
@@ -59,8 +61,7 @@ const EnableWebPushButton = ({
 
   useEffect(() => {
     if (subscribeFetcher.state !== 'idle' || !subscribeFetcher.data) return
-    if (lastSubscribeData.current === subscribeFetcher.data) return
-    lastSubscribeData.current = subscribeFetcher.data
+    if (!shouldHandleSubscribeData(subscribeFetcher.data)) return
     setBusy(false)
     if (subscribeFetcher.data.error) {
       toast.error(subscribeFetcher.data.error)
@@ -70,7 +71,13 @@ const EnableWebPushButton = ({
       toast.success(t('notificationChannels.webpush.enabled'))
       onSubscribed?.()
     }
-  }, [subscribeFetcher.state, subscribeFetcher.data, t, onSubscribed])
+  }, [
+    subscribeFetcher.state,
+    subscribeFetcher.data,
+    t,
+    onSubscribed,
+    shouldHandleSubscribeData,
+  ])
 
   const onClick = async () => {
     if (!supported) {

web/app/components/NotificationChannels/NotificationChannels.tsx

@@ -7,11 +7,12 @@ import {
   TrashIcon,
 } from '@phosphor-icons/react'
 import _map from 'lodash/map'
-import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react'
+import { memo, useCallback, useEffect, useMemo, useState } from 'react'
 import { useTranslation } from 'react-i18next'
 import { useFetcher } from 'react-router'
 import { toast } from 'sonner'
 
+import { useDeduplicateFetcherResponse } from '~/hooks/useDeduplicateFetcherResponse'
 import type {
   NotificationChannel,
   NotificationChannelType,
@@ -133,7 +134,8 @@ const NotificationChannels = ({
 
   const listFetcher = useFetcher<NotificationChannelActionData>()
   const mutateFetcher = useFetcher<NotificationChannelActionData>()
-  const lastMutateData = useRef<NotificationChannelActionData | null>(null)
+  const shouldHandleMutateData =
+    useDeduplicateFetcherResponse<NotificationChannelActionData>()
 
   const [channels, setChannels] = useState<NotificationChannel[]>([])
   const [isLoaded, setIsLoaded] = useState(false)
@@ -183,8 +185,7 @@ const NotificationChannels = ({
 
   useEffect(() => {
     if (mutateFetcher.state !== 'idle' || !mutateFetcher.data) return
-    if (lastMutateData.current === mutateFetcher.data) return
-    lastMutateData.current = mutateFetcher.data
+    if (!shouldHandleMutateData(mutateFetcher.data)) return
     const { intent, success, error } = mutateFetcher.data
 
     if (error) {
@@ -212,7 +213,14 @@ const NotificationChannels = ({
       toast.success(t('notificationChannels.verifyKickedOff'))
       triggerList()
     }
-  }, [mutateFetcher.state, mutateFetcher.data, t, allowedTypes, triggerList])
+  }, [
+    mutateFetcher.state,
+    mutateFetcher.data,
+    t,
+    allowedTypes,
+    triggerList,
+    shouldHandleMutateData,
+  ])
 
   const submitMutate = (formData: FormData) =>
     mutateFetcher.submit(formData, {

web/app/hooks/useDeduplicateFetcherResponse.ts

@@ -0,0 +1,48 @@
+import { useCallback, useRef } from 'react'
+
+const INITIAL = Symbol('initial')
+
+type Comparator<TData> = (previous: TData, current: TData) => boolean
+
+interface Options<TData, TKey> {
+  getKey?: (data: TData) => TKey
+  isEqual?: Comparator<TData>
+}
+
+const isSameReference = <TData>(previous: TData, current: TData) =>
+  Object.is(previous, current)
+
+export const useDeduplicateFetcherResponse = <TData, TKey = unknown>({
+  getKey,
+  isEqual = isSameReference,
+}: Options<TData, TKey> = {}) => {
+  const lastHandledData = useRef<TData | typeof INITIAL>(INITIAL)
+  const lastHandledKey = useRef<TKey | typeof INITIAL>(INITIAL)
+
+  return useCallback(
+    (data: TData | null | undefined): data is TData => {
+      if (data == null) return false
+
+      if (getKey) {
+        const key = getKey(data)
+
+        if (Object.is(lastHandledKey.current, key)) return false
+
+        lastHandledKey.current = key
+        lastHandledData.current = data
+        return true
+      }
+
+      if (
+        lastHandledData.current !== INITIAL &&
+        isEqual(lastHandledData.current, data)
+      ) {
+        return false
+      }
+
+      lastHandledData.current = data
+      return true
+    },
+    [getKey, isEqual],
+  )
+}

web/app/pages/Organisations/Settings/People.tsx

@@ -7,6 +7,7 @@ import {
   Transition,
 } from '@headlessui/react'
 import dayjs from 'dayjs'
+import _filter from 'lodash/filter'
 import _isEmpty from 'lodash/isEmpty'
 import _keys from 'lodash/keys'
 import _map from 'lodash/map'
@@ -17,11 +18,12 @@ import {
   CaretDownIcon,
   CheckIcon,
 } from '@phosphor-icons/react'
-import React, { Fragment, useState, useEffect } from 'react'
+import React, { Fragment, useState, useEffect, useRef } from 'react'
 import { useTranslation } from 'react-i18next'
 import { useFetcher } from 'react-router'
 import { toast } from 'sonner'
 
+import { useDeduplicateFetcherResponse } from '~/hooks/useDeduplicateFetcherResponse'
 import { roles, INVITATION_EXPIRES_IN } from '~/lib/constants'
 import { DetailedOrganisation, Role } from '~/lib/models/Organisation'
 import { useAuth } from '~/providers/AuthProvider'
@@ -83,7 +85,9 @@ const UsersList = ({ members, onRemove, fetcher }: UsersListProps) => {
   } = useTranslation('common')
   const { user } = useAuth()
 
-  const changeRole = (memberId: string, newRole: Role) => {
+  const changeRole = (memberId: string, currentRole: Role, newRole: Role) => {
+    if (newRole === currentRole || fetcher.state !== 'idle') return
+
     const formData = new FormData()
     formData.set('intent', 'update-member-role')
     formData.set('memberId', memberId)
@@ -144,7 +148,9 @@ const UsersList = ({ members, onRemove, fetcher }: UsersListProps) => {
                       <MenuItem key={itRole}>
                         <button
                           type='button'
-                          onClick={() => changeRole(member.id, itRole)}
+                          onClick={() =>
+                            changeRole(member.id, member.role, itRole)
+                          }
                           className='flex w-full items-center justify-between rounded-md px-3 py-2.5 text-left transition-colors hover:bg-gray-100 dark:hover:bg-slate-800'
                         >
                           <div>
@@ -233,18 +239,28 @@ const People = ({ organisation }: PeopleProps) => {
     role?: string
   }>({})
   const [validated, setValidated] = useState(false)
+  const shouldHandleFetcherData =
+    useDeduplicateFetcherResponse<OrganisationSettingsActionData>()
 
   const [showDeleteModal, setShowDeleteModal] = useState(false)
   const [memberToRemove, setMemberToRemove] = useState<
     DetailedOrganisation['members'][number] | null
   >(null)
+  const memberIdToRemove = useRef<string | null>(null)
 
-  const { name, members } = organisation
+  const { name, members: organisationMembers } = organisation
+  const [members, setMembers] = useState<DetailedOrganisation['members']>(
+    () => organisationMembers,
+  )
 
   const isSubmitting = fetcher.state === 'submitting'
   const isDeleting =
     isSubmitting && fetcher.formData?.get('intent') === 'remove-member'
 
+  useEffect(() => {
+    setMembers(organisationMembers)
+  }, [organisationMembers])
+
   const closeModal = () => {
     setShowModal(false)
     setTimeout(() => setForm({ email: '', role: 'viewer' }), 300)
@@ -259,21 +275,46 @@ const People = ({ organisation }: PeopleProps) => {
 
   /* eslint-disable react-hooks/set-state-in-effect */
   useEffect(() => {
+    if (!fetcher.data) return
+    if (!shouldHandleFetcherData(fetcher.data)) return
+
     if (fetcher.data?.success) {
       const { intent } = fetcher.data
       if (intent === 'invite-member') {
         toast.success(t('apiNotifications.userInvited'))
         closeModal()
       } else if (intent === 'remove-member') {
+        const removedMemberId =
+          fetcher.data.memberId ||
+          fetcher.formData?.get('memberId')?.toString() ||
+          memberIdToRemove.current
+        if (removedMemberId) {
+          setMembers((current) =>
+            _filter(current, (member) => member.id !== removedMemberId),
+          )
+        }
+        memberIdToRemove.current = null
         toast.success(t('apiNotifications.orgUserRemoved'))
         closeRemoveUserModal()
       } else if (intent === 'update-member-role') {
+        if (fetcher.data.memberId && fetcher.data.role) {
+          setMembers((current) =>
+            _map(current, (member) =>
+              member.id === fetcher.data!.memberId
+                ? { ...member, role: fetcher.data!.role as Role }
+                : member,
+            ),
+          )
+        }
         toast.success(t('apiNotifications.roleUpdated'))
       }
     } else if (fetcher.data?.error) {
+      if (fetcher.data.intent === 'remove-member') {
+        memberIdToRemove.current = null
+      }
       toast.error(fetcher.data.error)
     }
-  }, [fetcher.data, t])
+  }, [fetcher.data, fetcher.formData, t, shouldHandleFetcherData])
   /* eslint-enable react-hooks/set-state-in-effect */
 
   const validate = () => {
@@ -333,10 +374,11 @@ const People = ({ organisation }: PeopleProps) => {
     }
   }
 
-  const onRemove = (member: DetailedOrganisation['members'][number]) => {
+  const onRemove = (memberId: string) => {
+    memberIdToRemove.current = memberId
     const formData = new FormData()
     formData.set('intent', 'remove-member')
-    formData.set('memberId', member.id)
+    formData.set('memberId', memberId)
     fetcher.submit(formData, { method: 'post' })
   }
 
@@ -390,6 +432,7 @@ const People = ({ organisation }: PeopleProps) => {
                   members={members}
                   onRemove={(member) => {
                     setMemberToRemove(member)
+                    memberIdToRemove.current = member.id
                     setShowDeleteModal(true)
                   }}
                   fetcher={fetcher}
@@ -401,8 +444,9 @@ const People = ({ organisation }: PeopleProps) => {
       </div>
       <Modal
         onClose={closeRemoveUserModal}
-        onSubmit={async () => {
-          await onRemove(memberToRemove!)
+        onSubmit={() => {
+          if (!memberToRemove) return
+          onRemove(memberToRemove.id)
           closeRemoveUserModal()
         }}
         submitText={t('common.yes')}