From b4e14109611fc53e6ac14aaadffa7dd72bf29f6e Mon Sep 17 00:00:00 2001 From: Xavier Abad Date: Thu, 7 May 2026 11:23:22 +0200 Subject: [PATCH] fix: show the dialog only every 30 days --- .../hooks/useDownloadBackupKeys.test.ts | 61 +++++++++++-------- .../hooks/useDownloadBackupKeys.ts | 25 +++++--- src/services/local-storage.service.test.ts | 40 +++++++----- src/services/local-storage.service.ts | 29 +++++---- src/services/storage-keys.ts | 2 +- 5 files changed, 94 insertions(+), 63 deletions(-) diff --git a/src/app/drive/components/DownloadBackupKeysDialog/hooks/useDownloadBackupKeys.test.ts b/src/app/drive/components/DownloadBackupKeysDialog/hooks/useDownloadBackupKeys.test.ts index fd2c18e158..c4d76d4750 100644 --- a/src/app/drive/components/DownloadBackupKeysDialog/hooks/useDownloadBackupKeys.test.ts +++ b/src/app/drive/components/DownloadBackupKeysDialog/hooks/useDownloadBackupKeys.test.ts @@ -1,8 +1,9 @@ import { renderHook, act } from '@testing-library/react'; import { describe, expect, vi, beforeEach, test } from 'vitest'; -import { FOURTEEN_DAYS, useDownloadBackupKeys } from './useDownloadBackupKeys'; +import { THIRTY_DAYS, useDownloadBackupKeys } from './useDownloadBackupKeys'; import { handleExportBackupKey } from 'utils'; import { localStorageService } from 'services'; +import dayjs from 'dayjs'; const mockOpenDialog = vi.fn(); const mockCloseDialog = vi.fn(); @@ -29,17 +30,19 @@ describe('Download Backup Keys - Custom hook', () => { mockIsDialogOpen.mockReturnValue(false); }); - test('When the user has never seen the dialog, then the dialog opens', () => { - vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, remindMeLater: null }); + test('When the user has never seen the dialog, then seenAt is saved and the dialog does not open', () => { + const setBackupKeysSeenAtSpy = vi.spyOn(localStorageService, 'setBackupKeysSeenAt'); + vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, seenAt: null }); const { result } = renderHook(() => useDownloadBackupKeys(translate)); act(() => result.current.openBackupKeysDialog()); - expect(mockOpenDialog).toHaveBeenCalledOnce(); + expect(setBackupKeysSeenAtSpy).toHaveBeenCalledOnce(); + expect(mockOpenDialog).not.toHaveBeenCalled(); }); test('When the user already saved the backup key, then the dialog does not open', () => { - vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: true, remindMeLater: null }); + vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: true, seenAt: null }); const { result } = renderHook(() => useDownloadBackupKeys(translate)); act(() => result.current.openBackupKeysDialog()); @@ -47,10 +50,12 @@ describe('Download Backup Keys - Custom hook', () => { expect(mockOpenDialog).not.toHaveBeenCalled(); }); - describe('Remind me later', () => { - test('When the user clicked remind me later less than 14 days ago, then the dialog does not open', () => { - const recentDate = new Date(Date.now() - FOURTEEN_DAYS + 1000).toISOString(); - vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, remindMeLater: recentDate }); + describe('30-day cycle', () => { + test('When seenAt is less than 30 days ago, then the dialog does not open', () => { + const recentDate = dayjs() + .subtract(THIRTY_DAYS - 1, 'day') + .toISOString(); + vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, seenAt: recentDate }); const { result } = renderHook(() => useDownloadBackupKeys(translate)); act(() => result.current.openBackupKeysDialog()); @@ -58,24 +63,26 @@ describe('Download Backup Keys - Custom hook', () => { expect(mockOpenDialog).not.toHaveBeenCalled(); }); - test('When the user clicked remind me later more than 14 days ago, then the dialog opens again', () => { - const expiredDate = new Date(Date.now() - FOURTEEN_DAYS - 1000).toISOString(); - vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, remindMeLater: expiredDate }); + test('When seenAt is 30 or more days ago, then the dialog opens and seenAt is renewed', () => { + const setBackupKeysSeenAtSpy = vi.spyOn(localStorageService, 'setBackupKeysSeenAt'); + const expiredDate = dayjs().subtract(THIRTY_DAYS, 'day').toISOString(); + vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, seenAt: expiredDate }); const { result } = renderHook(() => useDownloadBackupKeys(translate)); act(() => result.current.openBackupKeysDialog()); + expect(setBackupKeysSeenAtSpy).toHaveBeenCalledOnce(); expect(mockOpenDialog).toHaveBeenCalledOnce(); }); - test('When the user clicks remind me later, then the current date is saved and the dialog closes', () => { - const backupRemindLaterSpy = vi.spyOn(localStorageService, 'setBackupKeysRemindLater'); - vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, remindMeLater: null }); + test('When the user clicks remind me later, then seenAt is renewed and the dialog closes', () => { + const setBackupKeysSeenAtSpy = vi.spyOn(localStorageService, 'setBackupKeysSeenAt'); + vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, seenAt: null }); const { result } = renderHook(() => useDownloadBackupKeys(translate)); act(() => result.current.onRemindMeLaterButtonClicked()); - expect(backupRemindLaterSpy).toHaveBeenCalledOnce(); + expect(setBackupKeysSeenAtSpy).toHaveBeenCalledOnce(); expect(mockCloseDialog).toHaveBeenCalledOnce(); }); }); @@ -83,7 +90,7 @@ describe('Download Backup Keys - Custom hook', () => { describe('Download backup key saved', () => { test('When the user confirms the backup key is saved, then the saved flag is persisted and the dialog closes', () => { const backupSavedSpy = vi.spyOn(localStorageService, 'setBackupKeysAcknowledged'); - vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, remindMeLater: null }); + vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, seenAt: null }); const { result } = renderHook(() => useDownloadBackupKeys(translate)); act(() => result.current.onBackupSavedButtonClicked()); @@ -92,33 +99,33 @@ describe('Download Backup Keys - Custom hook', () => { expect(mockCloseDialog).toHaveBeenCalledOnce(); }); - test('When the user confirms the backup key is saved and had a remind me later, then the remind me later entry is removed', () => { - const removeItem = vi.spyOn(localStorageService, 'removeItem'); + test('When the user confirms the backup key is saved and had a seenAt, then the seenAt entry is removed', () => { + const removeBackupKeysSeenAt = vi.spyOn(localStorageService, 'removeBackupKeysSeenAt'); vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, - remindMeLater: new Date().toISOString(), + seenAt: dayjs().toISOString(), }); const { result } = renderHook(() => useDownloadBackupKeys(translate)); act(() => result.current.onBackupSavedButtonClicked()); - expect(removeItem).toHaveBeenCalledOnce(); + expect(removeBackupKeysSeenAt).toHaveBeenCalledOnce(); }); - test('When the user confirms the backup key is saved without a previous remind me later, then the remind me later entry is not removed', () => { - const removeItem = vi.spyOn(localStorageService, 'removeItem'); - vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, remindMeLater: null }); + test('When the user confirms the backup key is saved without a previous seenAt, then the seenAt entry is not removed', () => { + const removeBackupKeysSeenAt = vi.spyOn(localStorageService, 'removeBackupKeysSeenAt'); + vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, seenAt: null }); const { result } = renderHook(() => useDownloadBackupKeys(translate)); act(() => result.current.onBackupSavedButtonClicked()); - expect(removeItem).not.toHaveBeenCalled(); + expect(removeBackupKeysSeenAt).not.toHaveBeenCalled(); }); }); describe('Downloading keys', () => { test('When the user clicks the download button, then the key download starts', () => { - vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, remindMeLater: null }); + vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, seenAt: null }); const { result } = renderHook(() => useDownloadBackupKeys(translate)); act(() => result.current.onDownloadBackupKeysButtonClicked()); @@ -127,7 +134,7 @@ describe('Download Backup Keys - Custom hook', () => { }); test('When the user clicks the download button, then the downloaded state is marked as true', () => { - vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, remindMeLater: null }); + vi.spyOn(localStorageService, 'getBackupKeys').mockReturnValue({ saved: false, seenAt: null }); const { result } = renderHook(() => useDownloadBackupKeys(translate)); act(() => result.current.onDownloadBackupKeysButtonClicked()); diff --git a/src/app/drive/components/DownloadBackupKeysDialog/hooks/useDownloadBackupKeys.ts b/src/app/drive/components/DownloadBackupKeysDialog/hooks/useDownloadBackupKeys.ts index d86be6f102..547b3f3455 100644 --- a/src/app/drive/components/DownloadBackupKeysDialog/hooks/useDownloadBackupKeys.ts +++ b/src/app/drive/components/DownloadBackupKeysDialog/hooks/useDownloadBackupKeys.ts @@ -2,11 +2,12 @@ import { ActionDialog } from 'app/contexts/dialog-manager/ActionDialogManager.co import { useActionDialog } from 'app/contexts/dialog-manager/useActionDialog'; import { Translate } from 'app/i18n/types'; import notificationsService, { ToastType } from 'app/notifications/services/notifications.service'; +import dayjs from 'dayjs'; import { useState } from 'react'; -import { localStorageService, STORAGE_KEYS } from 'services'; +import { localStorageService } from 'services'; import { handleExportBackupKey } from 'utils'; -export const FOURTEEN_DAYS = 14 * 24 * 60 * 60 * 1000; +export const THIRTY_DAYS = 30; export const useDownloadBackupKeys = (translate: Translate) => { const [isDownloadedKeys, setIsDownloadedKeys] = useState(false); @@ -15,25 +16,31 @@ export const useDownloadBackupKeys = (translate: Translate) => { const isBackupKeysDialogOpen = isDialogOpen(ActionDialog.DownloadBackupKey); const openBackupKeysDialog = () => { - const remindMeLater = backupKeysLocalStorage.remindMeLater; - const remindTimestamp = remindMeLater ? new Date(remindMeLater).getTime() : 0; + if (backupKeysLocalStorage.saved) return; - const hasExpired = !remindTimestamp || Date.now() - remindTimestamp >= FOURTEEN_DAYS; + const { seenAt } = backupKeysLocalStorage; - if (backupKeysLocalStorage.saved || !hasExpired) return; + if (!seenAt) { + localStorageService.setBackupKeysSeenAt(dayjs().toISOString()); + return; + } + const hasExpired = dayjs().diff(dayjs(seenAt), 'day') >= THIRTY_DAYS; + + if (!hasExpired) return; + + localStorageService.setBackupKeysSeenAt(dayjs().toISOString()); openDialog(ActionDialog.DownloadBackupKey); }; const onRemindMeLaterButtonClicked = () => { - const now = new Date(); - localStorageService.setBackupKeysRemindLater(now.toISOString()); + localStorageService.setBackupKeysSeenAt(dayjs().toISOString()); closeDialog(ActionDialog.DownloadBackupKey, { closeAllDialogsFirst: true }); }; const onBackupSavedButtonClicked = () => { localStorageService.setBackupKeysAcknowledged(); - if (backupKeysLocalStorage?.remindMeLater) localStorageService.removeItem(STORAGE_KEYS.BACKUP_KEY.REMIND_LATER_AT); + if (backupKeysLocalStorage?.seenAt) localStorageService.removeBackupKeysSeenAt(); notificationsService.show({ text: translate('modals.downloadBackupKeys.success'), type: ToastType.Success }); closeDialog(ActionDialog.DownloadBackupKey, { closeAllDialogsFirst: true }); }; diff --git a/src/services/local-storage.service.test.ts b/src/services/local-storage.service.test.ts index 9d7bdf829e..61fe2aca77 100644 --- a/src/services/local-storage.service.test.ts +++ b/src/services/local-storage.service.test.ts @@ -333,15 +333,15 @@ describe('Testing the local storage service', () => { describe('Backup key acknowledgment', () => { const userId = mockUserSettings.uuid; - const remindLaterKey = `backup_key_remind_later_at_${userId}`; + const seenAtKey = `backup_key_seen_at_${userId}`; const acknowledgedKey = `backup_key_acknowledged_at_${userId}`; describe('Get backup keys', () => { - test('When neither saved nor remind me later are set, then both are returned as empty', () => { - const { saved, remindMeLater } = localStorageService.getBackupKeys(); + test('When the user has never interacted with the backup keys dialog, then nothing is returned', () => { + const { saved, seenAt } = localStorageService.getBackupKeys(); expect(saved).toBe(false); - expect(remindMeLater).toBeNull(); + expect(seenAt).toBeNull(); }); test('When the user has acknowledged the backup key, then saved is true', () => { @@ -352,13 +352,13 @@ describe('Testing the local storage service', () => { expect(saved).toBe(true); }); - test('When the user set a remind me later date, then the date is returned', () => { + test('When the user has already been shown the dialog before, then the date is returned', () => { const date = new Date().toISOString(); - localStorage.setItem(remindLaterKey, date); + localStorage.setItem(seenAtKey, date); - const { remindMeLater } = localStorageService.getBackupKeys(); + const { seenAt } = localStorageService.getBackupKeys(); - expect(remindMeLater).toBe(date); + expect(seenAt).toBe(date); }); }); @@ -370,25 +370,35 @@ describe('Testing the local storage service', () => { }); }); - describe('setBackupKeysRemindLater', () => { - test('When the user sets remind me later, then the date is persisted for that user', () => { + describe('Track when the dialog was last shown', () => { + test('When the dialog is shown, then the date is persisted for that user', () => { const date = new Date().toISOString(); - localStorageService.setBackupKeysRemindLater(date); + localStorageService.setBackupKeysSeenAt(date); - expect(localStorage.getItem(remindLaterKey)).toBe(date); + expect(localStorage.getItem(seenAtKey)).toBe(date); + }); + }); + + describe('Remove when the dialog was last shown', () => { + test('When the backup key is acknowledged, then the last seen date is removed for that user', () => { + localStorage.setItem(seenAtKey, new Date().toISOString()); + + localStorageService.removeBackupKeysSeenAt(); + + expect(localStorage.getItem(seenAtKey)).toBeNull(); }); }); describe('clear', () => { - test('When the user logs out, then the remind me later date is removed but the acknowledged flag is kept', () => { + test('When the user logs out, then the last seen date is removed but the acknowledged flag is kept', () => { const date = new Date().toISOString(); - localStorage.setItem(remindLaterKey, date); + localStorage.setItem(seenAtKey, date); localStorage.setItem(acknowledgedKey, 'true'); localStorageService.clear(); - expect(localStorage.getItem(remindLaterKey)).toBeNull(); + expect(localStorage.getItem(seenAtKey)).toBeNull(); expect(localStorage.getItem(acknowledgedKey)).toBe('true'); }); }); diff --git a/src/services/local-storage.service.ts b/src/services/local-storage.service.ts index ecc6928c88..a991ae6b22 100644 --- a/src/services/local-storage.service.ts +++ b/src/services/local-storage.service.ts @@ -15,7 +15,7 @@ function getBackupKeyStorageKeys() { const user = getUser(); const userId = user?.uuid; return { - remindLaterAt: `${STORAGE_KEYS.BACKUP_KEY.REMIND_LATER_AT}_${userId}`, + seenAt: `${STORAGE_KEYS.BACKUP_KEY.SEEN_AT}_${userId}`, acknowledgedAt: `${STORAGE_KEYS.BACKUP_KEY.ACKNOWLEDGED_AT}_${userId}`, }; } @@ -25,19 +25,24 @@ function setBackupKeysAcknowledged(): void { localStorage.setItem(acknowledgedAt, 'true'); } -function setBackupKeysRemindLater(date: string): void { - const { remindLaterAt } = getBackupKeyStorageKeys(); - localStorage.setItem(remindLaterAt, date); +function setBackupKeysSeenAt(date: string): void { + const { seenAt } = getBackupKeyStorageKeys(); + localStorage.setItem(seenAt, date); +} + +function removeBackupKeysSeenAt(): void { + const { seenAt } = getBackupKeyStorageKeys(); + localStorage.removeItem(seenAt); } function getBackupKeys(): { - remindMeLater: string | null; + seenAt: string | null; saved: boolean; } { - const { remindLaterAt, acknowledgedAt } = getBackupKeyStorageKeys(); + const { seenAt, acknowledgedAt } = getBackupKeyStorageKeys(); const isAcknowledged = localStorage.getItem(acknowledgedAt) === 'true'; return { - remindMeLater: localStorage.getItem(remindLaterAt), + seenAt: localStorage.getItem(seenAt), saved: isAcknowledged, }; } @@ -85,7 +90,7 @@ function hasCompletedTutorial(id?: string): boolean { function clear(): void { localStorage.setItem('theme', 'system'); - localStorage.removeItem(getBackupKeyStorageKeys().remindLaterAt); + localStorage.removeItem(getBackupKeyStorageKeys().seenAt); Object.values(STORAGE_KEYS.THEMES).forEach((key) => localStorage.removeItem(key)); Object.values(LocalStorageItem).forEach((key) => localStorage.removeItem(key)); localStorage.removeItem('theme:isDark'); @@ -98,7 +103,8 @@ const localStorageService = { set, get, setBackupKeysAcknowledged, - setBackupKeysRemindLater, + setBackupKeysSeenAt, + removeBackupKeysSeenAt, getBackupKeys, getUser, getWorkspace, @@ -116,9 +122,10 @@ export interface LocalStorageService { set: (key: string, value: string) => void; get: (key: string) => string | null; setBackupKeysAcknowledged: () => void; - setBackupKeysRemindLater: (date: string) => void; + setBackupKeysSeenAt: (date: string) => void; + removeBackupKeysSeenAt: () => void; getBackupKeys: () => { - remindMeLater: string | null; + seenAt: string | null; saved: boolean; }; getUser: () => UserSettings | null; diff --git a/src/services/storage-keys.ts b/src/services/storage-keys.ts index 59303deec4..3ce8ec02a9 100644 --- a/src/services/storage-keys.ts +++ b/src/services/storage-keys.ts @@ -7,7 +7,7 @@ export const STORAGE_KEYS = { GCLID: 'gclid', HAS_SEEN_TRASH_DISPOSAL_DIALOG: 'hasSeenTrashDisposalDialog', BACKUP_KEY: { - REMIND_LATER_AT: 'backup_key_remind_later_at', + SEEN_AT: 'backup_key_seen_at', ACKNOWLEDGED_AT: 'backup_key_acknowledged_at', }, THEMES: {