From a6fb3c87c4b2ea43723f1c4c8be52495d9e522eb Mon Sep 17 00:00:00 2001 From: Mo Tarbin Date: Sat, 18 Jul 2026 02:16:42 -0400 Subject: [PATCH] Refactor Attchemnt Upload and fix bug with signing --- package-lock.json | 4 +- src/hooks/useDescriptionHtml.js | 26 ++ src/hooks/useFileUpload.js | 15 +- src/queries/ChoreQueries.jsx | 11 +- src/utils/ApiClient.js | 9 +- src/utils/Fetcher.jsx | 27 +- src/utils/Helpers.jsx | 159 ++++----- src/utils/ImageCache.js | 301 ++++++++++++++++++ src/utils/SyncEngine.js | 15 + src/views/ChoreEdit/ChoreEdit.jsx | 42 ++- src/views/ChoreEdit/ChoreView.jsx | 13 +- .../Modals/Inputs/AttachmentBrowserModal.jsx | 31 +- src/views/components/AddTaskModal.jsx | 1 + .../components/AttachmentPickerField.jsx | 41 ++- src/views/components/RichTextEditor.jsx | 69 ++-- 15 files changed, 601 insertions(+), 163 deletions(-) create mode 100644 src/hooks/useDescriptionHtml.js create mode 100644 src/utils/ImageCache.js diff --git a/package-lock.json b/package-lock.json index 645756d..ee5471e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "donetick", - "version": "1.2.8", + "version": "1.2.15", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "donetick", - "version": "1.2.8", + "version": "1.2.15", "dependencies": { "@capacitor-community/sqlite": "^8.0.0", "@capacitor/android": "^8.0.0", diff --git a/src/hooks/useDescriptionHtml.js b/src/hooks/useDescriptionHtml.js new file mode 100644 index 0000000..ca2f237 --- /dev/null +++ b/src/hooks/useDescriptionHtml.js @@ -0,0 +1,26 @@ +import { useEffect, useState } from 'react' +import { patchDescriptionHtml } from '../utils/ImageCache' + +// Returns description HTML safe to render: embedded images with an expired +// signed src are swapped for the offline-cached blob or a freshly signed URL. +// Renders the raw HTML immediately and patches in place when needed. +export const useDescriptionHtml = (html, meta = {}) => { + const [patched, setPatched] = useState(html) + + useEffect(() => { + let cancelled = false + setPatched(html) + if (!html || !html.includes('dt-data-path')) return undefined + patchDescriptionHtml(html, meta).then(result => { + if (!cancelled && result !== html) setPatched(result) + }) + return () => { + cancelled = true + } + // meta is an inline object at call sites; keying on its values would + // re-run every render, so only the html triggers a re-patch. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [html]) + + return patched +} diff --git a/src/hooks/useFileUpload.js b/src/hooks/useFileUpload.js index e44329e..972a11e 100644 --- a/src/hooks/useFileUpload.js +++ b/src/hooks/useFileUpload.js @@ -5,7 +5,11 @@ import { useNotification } from '../service/NotificationProvider' import { apiClient } from '../utils/ApiClient' import { isPlusAccount, resolvePhotoURL } from '../utils/Helpers' -export const useFileUpload = ({ entityType = 'chore_attachment', entityId, draftId } = {}) => { +export const useFileUpload = ({ + entityType = 'chore_attachment', + entityId, + draftId, +} = {}) => { const { showError } = useNotification() const { data: userProfile } = useUserProfile() @@ -76,7 +80,14 @@ export const useFileUpload = ({ entityType = 'chore_attachment', entityId, draft } const data = await response.json() - return resolvePhotoURL(data.url || data.sign) + // url is fetchable now; path is the stable storage key used to + // re-sign, delete, and cache the file later. + return { + url: resolvePhotoURL(data.sign || data.url), + path: data.path, + fileName: data.file_name || file.name, + sizeBytes: data.size_bytes, + } } catch { showError({ title: 'Upload Failed', diff --git a/src/queries/ChoreQueries.jsx b/src/queries/ChoreQueries.jsx index 20186dc..fff13b8 100644 --- a/src/queries/ChoreQueries.jsx +++ b/src/queries/ChoreQueries.jsx @@ -21,6 +21,7 @@ import { UnArchiveChore, UpdateChoreHistory, } from '../utils/Fetcher' +import { cacheChoreImages } from '../utils/ImageCache' import { offlineDB } from '../utils/OfflineDB' import { isOfflineFeatureEnabled } from '../utils/OfflineFeatureToggle' import { syncEngine } from '../utils/SyncEngine' @@ -337,7 +338,11 @@ export const useChore = choreId => { try { const response = await GetChoreByID(choreId) if (response && response.ok) { - return await response.json() + const data = await response.json() + // Fire-and-forget: store this chore's images (incl. attachments, + // which only appear in detail responses) for offline use + if (data?.res) cacheChoreImages(data.res) + return data } throw new Error('Failed to fetch chore') } catch { @@ -595,9 +600,7 @@ export const useMarkChoreComplete = () => { if (!oldData) return oldData return { res: oldData.res.map(chore => - chore.id === choreId - ? { ...chore, _pending: 'complete' } - : chore, + chore.id === choreId ? { ...chore, _pending: 'complete' } : chore, ), } }) diff --git a/src/utils/ApiClient.js b/src/utils/ApiClient.js index a945195..24a54d1 100644 --- a/src/utils/ApiClient.js +++ b/src/utils/ApiClient.js @@ -138,6 +138,13 @@ class ApiClient { } catch (e) { console.error('Error clearing offline data on logout', e) } + try { + // Dynamic import sidesteps the ApiClient <-> ImageCache module cycle + const { clearImageCache } = await import('./ImageCache') + await clearImageCache() + } catch (e) { + console.error('Error clearing image cache on logout', e) + } try { await logout() } catch (e) { @@ -252,7 +259,7 @@ class ApiClient { } return this.request(endpoint, { - options: { ...options }, + ...options, method: 'POST', body: data ? data : undefined, }) diff --git a/src/utils/Fetcher.jsx b/src/utils/Fetcher.jsx index 0e3caae..65375e7 100644 --- a/src/utils/Fetcher.jsx +++ b/src/utils/Fetcher.jsx @@ -728,7 +728,11 @@ const DeleteUser = (password, confirmation, transferOptions = []) => { }) } -const UploadChoreAttachment = (file, entityType, { entityId, draftId } = {}) => { +const UploadChoreAttachment = ( + file, + entityType, + { entityId, draftId } = {}, +) => { const formData = new FormData() formData.append('file', file) formData.append('entityType', entityType) @@ -737,6 +741,22 @@ const UploadChoreAttachment = (file, entityType, { entityId, draftId } = {}) => return apiClient.upload('/assets/chore', formData) } +const DeleteDraftAttachment = filePath => { + return Fetch(`/assets/chore`, { + method: 'DELETE', + headers: HEADERS(), + body: JSON.stringify({ file_path: filePath }), + }) +} + +// Returns a fresh signed URL for a stored asset path the current user may access. +const SignAssetURL = path => { + return Fetch(`/files/sign?path=${encodeURIComponent(path)}`, { + method: 'GET', + headers: HEADERS(), + }) +} + const GetChoreAttachments = choreId => { return Fetch(`/chores/${choreId}/attachments`, { method: 'GET', @@ -958,7 +978,9 @@ const TrackFilterUsage = id => { export { AcceptCircleMemberRequest, DeleteChoreAttachment, + DeleteDraftAttachment, GetChoreAttachments, + SignAssetURL, UploadChoreAttachment, ApproveChore, ArchiveChore, @@ -1060,6 +1082,5 @@ export { UpdateThingState, UpdateTimeSession, UpdateUserDetails, - VerifyMFA + VerifyMFA, } - diff --git a/src/utils/Helpers.jsx b/src/utils/Helpers.jsx index b276778..abf17de 100644 --- a/src/utils/Helpers.jsx +++ b/src/utils/Helpers.jsx @@ -13,111 +13,74 @@ const resolvePhotoURL = url => { return apiClient.getAssetURL(url) } -// Detect cloud storage pre-signed URLs (S3, GCS, Azure) that carry expiry params. -const isCloudSignedUrl = url => { - if (!url) return false +// Returns the expiry of a signed asset URL in epoch ms, or null when the URL +// carries no expiry (public assets, plain paths, OIDC picture URLs). +// Understands the backend's local signer (`expires` in unix seconds) and +// S3 presigned URLs (`X-Amz-Date` + `X-Amz-Expires`). +const getSignedUrlExpiry = url => { + if (!url) return null try { - return ( - url.includes('X-Amz-Signature') || - url.includes('X-Amz-Expires') || - url.includes('X-Goog-Expires') || - url.includes('expires') - ) - } catch(e) { - return false - } -} - -// Extract the storage key from a cloud signed URL so we can route it through -// the backend proxy (which re-signs on every request and never expires). -// -// Handles: -// Virtual-hosted S3: https://{bucket}.s3[.region].amazonaws.com/{key}?... -// Path-style S3: https://s3[.region].amazonaws.com/{bucket}/{key}?... -// Cloudflare R2: https://{bucket}.{accountid}.r2.cloudflarestorage.com/{key}?... -// GCS: https://storage.googleapis.com/{bucket}/{key}?... -// Azure Blob: https://{account}.blob.core.windows.net/{container}/{blob}?... -// -// The app stores files under an "assets/" prefix in the bucket but the backend -// proxy already mounts at /assets/, so we strip that leading segment when present. -const extractStorageKey = url => { - try { - const u = new URL(url) - const host = u.hostname - const rawPath = u.pathname.replace(/^\//, '') - - let key - - if (host.endsWith('.r2.cloudflarestorage.com')) { - // Virtual-hosted R2: bucket is in the host, key is the full path - key = rawPath - } else if (host.endsWith('.amazonaws.com')) { - if (host.startsWith('s3') || host.includes('.s3.')) { - // Path-style S3: first segment is the bucket — strip it - key = rawPath.split('/').slice(1).join('/') - } else { - // Virtual-hosted S3: bucket is in the host, path is the key - key = rawPath + const query = new URL(url, 'http://relative.local').searchParams + if (query.has('expires')) { + const expires = parseInt(query.get('expires'), 10) + return Number.isFinite(expires) ? expires * 1000 : null + } + if (query.has('X-Amz-Expires') && query.has('X-Amz-Date')) { + const iso = query + .get('X-Amz-Date') + .replace( + /^(\d{4})(\d{2})(\d{2})T(\d{2})(\d{2})(\d{2})Z$/, + '$1-$2-$3T$4:$5:$6Z', + ) + const start = Date.parse(iso) + const validFor = parseInt(query.get('X-Amz-Expires'), 10) + if (Number.isFinite(start) && Number.isFinite(validFor)) { + return start + validFor * 1000 } - } else if (host === 'storage.googleapis.com') { - // First segment is the bucket - key = rawPath.split('/').slice(1).join('/') - } else if (host.endsWith('.blob.core.windows.net')) { - // First segment is the container name - key = rawPath.split('/').slice(1).join('/') - } else { - key = rawPath } - - // The bucket stores files under an "assets/" prefix; the backend /assets/ - // endpoint already adds that prefix, so strip it to avoid duplication. - if (key.startsWith('assets/')) { - key = key.slice('assets/'.length) - } - - return key || null + return null } catch { return null } } -// Scan an HTML string for tags whose src is a cloud signed URL and -// replace them with backend proxy URLs (which generate fresh signed URLs on -// each request). Returns the patched HTML, or the original if nothing changed. -const refreshSignedUrlsInHtml = html => { - if (!html) return html - if ( - !html.includes('dt-data-path') && - !html.includes('X-Amz-') && - !html.includes('X-Goog-') && - !html.includes('sig') && - !html.includes('.blob.core.windows.net') - ) { - return html - } - const parser = new DOMParser() - const doc = parser.parseFromString(html, 'text/html') - const imgs = doc.querySelectorAll('img[src]') - let changed = false - - imgs.forEach(img => { - const stablePath = img.getAttribute('dt-data-path') - const src = img.getAttribute('src') - let nextSrc = src - - if (stablePath) { - nextSrc = resolvePhotoURL(stablePath) - } else if (isCloudSignedUrl(src)) { - nextSrc = resolvePhotoURL(extractStorageKey(src)) - } - - if (nextSrc && nextSrc !== src) { - img.setAttribute('src', nextSrc) - changed = true - } - }) - - return changed ? doc.body.innerHTML : html +// One minute of clock skew so we refresh before the server starts rejecting. +const isSignedUrlExpired = url => { + const expiry = getSignedUrlExpiry(url) + return expiry != null && Date.now() > expiry - 60_000 } -export { isPlusAccount, refreshSignedUrlsInHtml, resolvePhotoURL } +// Fetches a fresh signed URL for a stored asset path via the authed sign +// endpoint. Memoized per path until the signed URL nears expiry, with +// in-flight de-duplication so a burst of images signs each path once. +const signedUrlCache = new Map() +const getSignedAssetUrl = async path => { + if (!path) return '' + const hit = signedUrlCache.get(path) + if (hit?.url && !isSignedUrlExpired(hit.url)) return hit.url + if (hit?.promise) return hit.promise + + const promise = (async () => { + const response = await apiClient.get( + `/files/sign?path=${encodeURIComponent(path)}`, + ) + if (!response?.ok) { + throw new Error(`Failed to sign asset path: ${path}`) + } + const data = await response.json() + const url = resolvePhotoURL(data.url) + signedUrlCache.set(path, { url }) + return url + })() + promise.catch(() => signedUrlCache.delete(path)) + signedUrlCache.set(path, { promise }) + return promise +} + +export { + getSignedAssetUrl, + getSignedUrlExpiry, + isPlusAccount, + isSignedUrlExpired, + resolvePhotoURL, +} diff --git a/src/utils/ImageCache.js b/src/utils/ImageCache.js new file mode 100644 index 0000000..b77ccea --- /dev/null +++ b/src/utils/ImageCache.js @@ -0,0 +1,301 @@ +import { + getSignedAssetUrl, + isSignedUrlExpired, + resolvePhotoURL, +} from './Helpers' + +// Offline image store keyed by the *stable* storage path (never the signed +// URL, whose query params change on every re-sign). Backed by the Cache +// Storage API, which works in browsers and the Capacitor WebView alike. +// +// Lifecycle: an image stays cached for as long as something still references +// it. syncOfflineImages() reconciles the store against the freshly synced +// chore data — prefetching referenced images that are missing and evicting +// entries whose chore is gone or whose description no longer embeds them. +// Attachment entries are evicted when the attachment (or its chore) is +// deleted. Everything is dropped on logout. + +const CACHE_NAME = 'dt-images-v1' +const MANIFEST_KEY = 'dt-image-cache-manifest' + +const hasCacheSupport = () => + typeof caches !== 'undefined' && typeof window !== 'undefined' + +// Cache API keys must be request URLs; keep them under a reserved fake path. +const keyForPath = path => `/__dt-image-cache__/${encodeURI(path)}` + +// Manifest maps path -> { choreId, kind: 'description' | 'attachment', cachedAt }. +// It exists so eviction can reason about *why* an image was cached. +const loadManifest = () => { + try { + return JSON.parse(localStorage.getItem(MANIFEST_KEY)) || {} + } catch { + return {} + } +} +const saveManifest = manifest => { + try { + localStorage.setItem(MANIFEST_KEY, JSON.stringify(manifest)) + } catch { + // localStorage full or unavailable — cache still works, eviction degrades + } +} + +// Object URLs are memoized per path so repeated renders reuse one blob URL. +const objectUrlByPath = new Map() + +const getCachedImageUrl = async path => { + if (!path || !hasCacheSupport()) return null + if (objectUrlByPath.has(path)) return objectUrlByPath.get(path) + try { + const cache = await caches.open(CACHE_NAME) + const response = await cache.match(keyForPath(path)) + if (!response) return null + const blob = await response.blob() + const url = URL.createObjectURL(blob) + objectUrlByPath.set(path, url) + return url + } catch { + return null + } +} + +const isImageCached = async path => { + if (!path || !hasCacheSupport()) return false + if (objectUrlByPath.has(path)) return true + try { + const cache = await caches.open(CACHE_NAME) + return !!(await cache.match(keyForPath(path))) + } catch { + return false + } +} + +// Downloads fetchUrl and stores it under the stable path. meta records the +// owning chore so eviction can drop the entry when the reference goes away. +const cacheImageFromUrl = async (path, fetchUrl, meta = {}) => { + if (!path || !fetchUrl || !hasCacheSupport()) return false + try { + const response = await fetch(fetchUrl) + if (!response.ok) return false + const cache = await caches.open(CACHE_NAME) + await cache.put(keyForPath(path), response) + const manifest = loadManifest() + manifest[path] = { ...manifest[path], ...meta, cachedAt: Date.now() } + saveManifest(manifest) + // Invalidate any stale object URL for this path + const stale = objectUrlByPath.get(path) + if (stale) { + URL.revokeObjectURL(stale) + objectUrlByPath.delete(path) + } + return true + } catch { + return false + } +} + +const removeCachedImage = async path => { + if (!path || !hasCacheSupport()) return + try { + const cache = await caches.open(CACHE_NAME) + await cache.delete(keyForPath(path)) + } catch { + // best effort + } + const manifest = loadManifest() + if (manifest[path]) { + delete manifest[path] + saveManifest(manifest) + } + const url = objectUrlByPath.get(path) + if (url) { + URL.revokeObjectURL(url) + objectUrlByPath.delete(path) + } +} + +const clearImageCache = async () => { + if (hasCacheSupport()) { + try { + await caches.delete(CACHE_NAME) + } catch { + // best effort + } + } + try { + localStorage.removeItem(MANIFEST_KEY) + } catch { + // ignore + } + objectUrlByPath.forEach(url => URL.revokeObjectURL(url)) + objectUrlByPath.clear() +} + +// Extracts (path, src) pairs for every dt-data-path image embedded in a +// description HTML string. Attribute values are HTML-escaped in the raw +// string ("&"), so src is decoded for fetching while rawSrc keeps the +// exact attribute text for string replacement. +const extractDescriptionImages = html => { + if (!html || !html.includes('dt-data-path')) return [] + const results = [] + const imgReg = /]+>/g + const pathReg = /dt-data-path="([^"]+)"/ + const srcReg = /src="([^"]*)"/ + const decodeAttr = value => (value ? value.replaceAll('&', '&') : value) + for (const tag of html.match(imgReg) || []) { + const path = tag.match(pathReg)?.[1] + if (!path) continue + const rawSrc = tag.match(srcReg)?.[1] || null + results.push({ path: decodeAttr(path), src: decodeAttr(rawSrc), rawSrc }) + } + return results +} + +// Collects every image reference in a chore: description embeds and (when +// present, i.e. detail responses) attachments. path -> { choreId, kind, src }. +const collectChoreImageRefs = (chore, referenced = new Map()) => { + if (chore?.id == null) return referenced + for (const { path, src } of extractDescriptionImages(chore.description)) { + referenced.set(path, { + choreId: String(chore.id), + kind: 'description', + src, + }) + } + for (const att of chore.attachments || []) { + if (att?.file_path) { + referenced.set(att.file_path, { + choreId: String(chore.id), + kind: 'attachment', + src: att.sign ? resolvePhotoURL(att.sign) : null, + }) + } + } + return referenced +} + +// Downloads every referenced image that is not stored yet. +const prefetchReferenced = async referenced => { + for (const [path, { choreId, kind, src }] of referenced) { + if (await isImageCached(path)) { + // keep manifest ownership fresh (chore may have changed via clone etc.) + const current = loadManifest() + if ( + current[path] && + (current[path].choreId !== choreId || current[path].kind !== kind) + ) { + current[path] = { ...current[path], choreId, kind } + saveManifest(current) + } + continue + } + let fetchUrl = src && !isSignedUrlExpired(src) ? src : null + if (!fetchUrl) { + try { + fetchUrl = await getSignedAssetUrl(path) + } catch { + continue + } + } + await cacheImageFromUrl(path, fetchUrl, { choreId, kind }) + } +} + +// Stores one chore's images for offline use without evicting anything. +// Call with detail responses, which are the only place attachments appear. +const cacheChoreImages = async chore => { + if (!hasCacheSupport()) return + try { + await prefetchReferenced(collectChoreImageRefs(chore)) + } catch (err) { + console.error('Failed to cache chore images', err) + } +} + +// Reconciles the image store with the *complete* synced chore list: every +// image the chores still reference gets downloaded (so offline has +// everything), and cached description images whose chore or reference +// disappeared are evicted. Attachments are only referenced from chore-detail +// responses, so they are evicted here only when their whole chore is gone. +const syncOfflineImages = async chores => { + if (!hasCacheSupport() || !Array.isArray(chores)) return + try { + const referenced = new Map() + const choreIds = new Set() + for (const chore of chores) { + if (chore?.id == null) continue + choreIds.add(String(chore.id)) + collectChoreImageRefs(chore, referenced) + } + + // Evict entries that are provably no longer referenced. + const manifest = loadManifest() + for (const [path, entry] of Object.entries(manifest)) { + if (!entry?.choreId) continue + const choreGone = !choreIds.has(String(entry.choreId)) + const droppedFromDescription = + entry.kind === 'description' && + choreIds.has(String(entry.choreId)) && + !referenced.has(path) + if (choreGone || droppedFromDescription) { + await removeCachedImage(path) + } + } + + await prefetchReferenced(referenced) + } catch (err) { + console.error('Offline image sync failed', err) + } +} + +// Returns a displayable src for a stored asset path: the still-valid signed +// src when one is provided, otherwise the cached blob, otherwise a freshly +// signed URL. Opportunistically stores the image for offline use. +const getImageSrc = async (path, signedSrc = null, meta = {}) => { + if (signedSrc && !isSignedUrlExpired(signedSrc)) { + if (!(await isImageCached(path))) { + cacheImageFromUrl(path, signedSrc, meta) // fire and forget + } + return signedSrc + } + const cached = await getCachedImageUrl(path) + if (cached) return cached + const fresh = await getSignedAssetUrl(path) + cacheImageFromUrl(path, fresh, meta) // fire and forget + return fresh +} + +// Rewrites dt-data-path images in a description HTML string so every img has +// a usable src: valid signed srcs are kept (and stored for offline), expired +// ones are swapped for the cached blob or a freshly signed URL. +const patchDescriptionHtml = async (html, meta = {}) => { + const images = extractDescriptionImages(html) + if (images.length === 0) return html + let patched = html + for (const { path, src, rawSrc } of images) { + try { + const nextSrc = await getImageSrc(path, src, { + kind: 'description', + ...meta, + }) + if (nextSrc && nextSrc !== src) { + patched = patched.replaceAll(`src="${rawSrc}"`, `src="${nextSrc}"`) + } + } catch { + // offline with no cache entry — leave the original src in place + } + } + return patched +} + +export { + cacheChoreImages, + cacheImageFromUrl, + clearImageCache, + getCachedImageUrl, + getImageSrc, + patchDescriptionHtml, + removeCachedImage, + syncOfflineImages, +} diff --git a/src/utils/SyncEngine.js b/src/utils/SyncEngine.js index 2d247a1..8ae2004 100644 --- a/src/utils/SyncEngine.js +++ b/src/utils/SyncEngine.js @@ -15,6 +15,7 @@ import { UpdateChoreHistory, UpdateDueDate, } from './Fetcher' +import { syncOfflineImages } from './ImageCache' import { offlineDB } from './OfflineDB' import { isOfflineFeatureEnabled } from './OfflineFeatureToggle' @@ -58,6 +59,13 @@ class SyncEngine { // Sync succeeded — server is reachable (only sync success restores online status) networkManager.setServerReachable() this._notify({ syncing: false, lastSync: Date.now() }) + // Reconcile the offline image store against the full cached chore list + // (prefetch referenced images, evict ones no longer referenced). + // Fire-and-forget: image downloads must not block or fail the sync. + offlineDB + .getChores(true) + .then(chores => syncOfflineImages(chores || [])) + .catch(() => {}) return true } catch (err) { await commandQueue.resetSyncing() @@ -242,6 +250,13 @@ class SyncEngine { if (!isOfflineFeatureEnabled()) return if (!chores || chores.length === 0) return await offlineDB.saveChores(chores) + // Fire-and-forget: keep the offline image store in step with the data. + // Reconcile against the *full* cached list — the passed list may exclude + // archived chores, and eviction must only run against everything we have. + offlineDB + .getChores(true) + .then(all => syncOfflineImages(all || [])) + .catch(() => {}) } } diff --git a/src/views/ChoreEdit/ChoreEdit.jsx b/src/views/ChoreEdit/ChoreEdit.jsx index 23f3fee..e28efbe 100644 --- a/src/views/ChoreEdit/ChoreEdit.jsx +++ b/src/views/ChoreEdit/ChoreEdit.jsx @@ -53,11 +53,13 @@ import { useNotification } from '../../service/NotificationProvider' import { getTextColorFromBackgroundColor } from '../../utils/Colors.jsx' import { DeleteChoreAttachment, + DeleteDraftAttachment, GetAllCircleMembers, GetThings, UploadChoreAttachment, } from '../../utils/Fetcher' import { isPlusAccount, resolvePhotoURL } from '../../utils/Helpers' +import { getImageSrc, removeCachedImage } from '../../utils/ImageCache' import { generateUUID } from '../../utils/UUID' import Priorities from '../../utils/Priorities.jsx' import { getIconComponent } from '../../utils/ProjectIcons' @@ -725,6 +727,7 @@ const ChoreEdit = () => { onChange={setDescription} entityId={choreId} entityType={'chore_description'} + draftId={draftId} /> {errors.description} @@ -970,12 +973,23 @@ const ChoreEdit = () => { Files attached to this task {attachments.length > 0 && ( - + {attachments.map((att, idx) => ( { - const url = resolvePhotoURL(att.sign || att.file_path) + onClick={async () => { + const url = await getImageSrc( + att.file_path, + att.sign ? resolvePhotoURL(att.sign) : null, + { choreId, kind: 'attachment' }, + ).catch(() => resolvePhotoURL(att.sign || att.file_path)) const ext = (att.file_name || '') .split('.') .pop() @@ -1039,6 +1053,7 @@ const ChoreEdit = () => { event.stopPropagation() DeleteChoreAttachment(choreId, att.file_path) .then(() => { + removeCachedImage(att.file_path) setAttachments(prev => prev.filter(a => a.file_path !== att.file_path), ) @@ -1061,9 +1076,20 @@ const ChoreEdit = () => { color='danger' onClick={event => { event.stopPropagation() - setAttachments(prev => - prev.filter((_, i) => i !== idx), - ) + // Draft uploads live server-side too — delete there + // so they are not promoted onto the chore on save. + DeleteDraftAttachment(att.file_path) + .then(() => { + setAttachments(prev => + prev.filter((_, i) => i !== idx), + ) + }) + .catch(() => { + showError({ + title: 'Delete Failed', + message: 'Failed to delete attachment.', + }) + }) }} > @@ -1078,9 +1104,7 @@ const ChoreEdit = () => { variant='outlined' color='neutral' size='sm' - startDecorator={ - isUploadingAttachment ? null : - } + startDecorator={isUploadingAttachment ? null : } loading={isUploadingAttachment} sx={{ alignSelf: 'flex-start' }} > diff --git a/src/views/ChoreEdit/ChoreView.jsx b/src/views/ChoreEdit/ChoreView.jsx index 0b51fd3..d9ba621 100644 --- a/src/views/ChoreEdit/ChoreView.jsx +++ b/src/views/ChoreEdit/ChoreView.jsx @@ -88,7 +88,7 @@ import RichTextEditor from '../components/RichTextEditor.jsx' import SubTasks from '../components/SubTask.jsx' import TimePassedCard from './TimePassedCard.jsx' import TimerSplitButton from './TimerSplitButton.jsx' -import { refreshSignedUrlsInHtml } from '../../utils/Helpers.jsx' +import { useDescriptionHtml } from '../../hooks/useDescriptionHtml' const isNetworkError = err => err instanceof TypeError && err.message === 'Failed to fetch' @@ -132,6 +132,12 @@ const ChoreView = () => { useCircleMembers() const { data: userProfile } = useUserProfile() const { impersonatedUser } = useImpersonateUser() + const descriptionHtml = useDescriptionHtml(chore?.description || '', { + choreId: chore?.id, + }) + const notesHtml = useDescriptionHtml(chore?.notes || '', { + choreId: chore?.id, + }) const { data: choreData, isLoading: isChoreLoading } = useChoreDetails(choreId) @@ -935,7 +941,7 @@ const ChoreView = () => { whiteSpace: 'pre-wrap', wordBreak: 'break-word', }} - dangerouslySetInnerHTML={{ __html: refreshSignedUrlsInHtml(raw) }} + dangerouslySetInnerHTML={{ __html: descriptionHtml }} /> ) : ( { whiteSpace: 'pre-wrap', wordBreak: 'break-word', }} - dangerouslySetInnerHTML={{ __html: refreshSignedUrlsInHtml(raw) }} + dangerouslySetInnerHTML={{ __html: notesHtml }} /> ) : ( { diff --git a/src/views/Modals/Inputs/AttachmentBrowserModal.jsx b/src/views/Modals/Inputs/AttachmentBrowserModal.jsx index 3de68f5..e96d90a 100644 --- a/src/views/Modals/Inputs/AttachmentBrowserModal.jsx +++ b/src/views/Modals/Inputs/AttachmentBrowserModal.jsx @@ -1,9 +1,18 @@ import { AttachFile, Close, Image } from '@mui/icons-material' -import { Box, Button, CircularProgress, List, ListItem, ListItemButton, Typography } from '@mui/joy' +import { + Box, + Button, + CircularProgress, + List, + ListItem, + ListItemButton, + Typography, +} from '@mui/joy' import { useEffect, useState } from 'react' import { useResponsiveModal } from '../../../hooks/useResponsiveModal' import { GetChoreAttachments } from '../../../utils/Fetcher' import { resolvePhotoURL } from '../../../utils/Helpers' +import { cacheChoreImages, getImageSrc } from '../../../utils/ImageCache' import AttachmentViewerModal from './AttachmentViewerModal' const IMAGE_EXTENSIONS = ['jpg', 'jpeg', 'png', 'gif', 'webp', 'bmp', 'svg'] @@ -38,7 +47,12 @@ function AttachmentBrowserModal({ choreId, isOpen, onClose }) { if (!res.ok) throw new Error('Failed to fetch attachments') return res.json() }) - .then(data => setAttachments(Array.isArray(data) ? data : [])) + .then(data => { + const list = Array.isArray(data) ? data : [] + setAttachments(list) + // Fire-and-forget: store attachments so they open offline later + cacheChoreImages({ id: choreId, attachments: list }) + }) .catch(() => setAttachments([])) .finally(() => setIsLoading(false)) }, [isOpen, choreId]) @@ -48,8 +62,12 @@ function AttachmentBrowserModal({ choreId, isOpen, onClose }) { onClose?.() } - const handleAttachmentClick = attachment => { - const url = resolvePhotoURL(attachment.sign) + const handleAttachmentClick = async attachment => { + const url = await getImageSrc( + attachment.file_path, + attachment.sign ? resolvePhotoURL(attachment.sign) : null, + { choreId: String(choreId), kind: 'attachment' }, + ).catch(() => resolvePhotoURL(attachment.sign)) if (isImageFile(attachment.file_name)) { setViewerConfig({ isOpen: true, @@ -119,7 +137,10 @@ function AttachmentBrowserModal({ choreId, isOpen, onClose }) { {attachment.file_name || `File ${index + 1}`} {attachment.size_bytes > 0 && ( - + {(attachment.size_bytes / 1024).toFixed(1)} KB )} diff --git a/src/views/components/AddTaskModal.jsx b/src/views/components/AddTaskModal.jsx index dc76c6c..30ed6f5 100644 --- a/src/views/components/AddTaskModal.jsx +++ b/src/views/components/AddTaskModal.jsx @@ -1078,6 +1078,7 @@ const TaskInput = ({ onChoreUpdate, isModalOpen, onClose }) => { onChange={setDescription} value={description || ''} entityType={'chore_description'} + draftId={draftId} /> diff --git a/src/views/components/AttachmentPickerField.jsx b/src/views/components/AttachmentPickerField.jsx index fc626a8..4f128a1 100644 --- a/src/views/components/AttachmentPickerField.jsx +++ b/src/views/components/AttachmentPickerField.jsx @@ -11,6 +11,7 @@ import { ClickAwayListener, Popper } from '@mui/material' import { useEffect, useRef, useState } from 'react' import { Z_INDEX } from '../../constants/zIndex' import { useFileUpload } from '../../hooks/useFileUpload' +import { DeleteDraftAttachment } from '../../utils/Fetcher' const AttachmentPickerField = ({ attachments = [], @@ -45,9 +46,12 @@ const AttachmentPickerField = ({ if (!file) return setIsUploading(true) try { - const url = await uploadFile(file) - if (url) { - onChange([...attachments, { url, name: file.name }]) + const uploaded = await uploadFile(file) + if (uploaded) { + onChange([ + ...attachments, + { url: uploaded.url, path: uploaded.path, name: uploaded.fileName }, + ]) } } finally { setIsUploading(false) @@ -55,7 +59,17 @@ const AttachmentPickerField = ({ } } - const handleRemove = index => { + const handleRemove = async index => { + const attachment = attachments[index] + // Draft uploads exist server-side too — delete there so they are not + // promoted onto the chore when it is created. + if (attachment?.path) { + try { + await DeleteDraftAttachment(attachment.path) + } catch { + // file may already be gone; still drop it from the list + } + } const updated = attachments.filter((_, i) => i !== index) onChange(updated) if (updated.length === 0) setIsOpen(false) @@ -91,7 +105,10 @@ const AttachmentPickerField = ({ }} > {isUploading ? ( - + ) : ( )} @@ -162,7 +179,14 @@ const AttachmentPickerField = ({ }} > {attachments.length > 0 && ( - + {attachments.map((attachment, index) => ( + ) : ( ) diff --git a/src/views/components/RichTextEditor.jsx b/src/views/components/RichTextEditor.jsx index fe875eb..28a6eb7 100644 --- a/src/views/components/RichTextEditor.jsx +++ b/src/views/components/RichTextEditor.jsx @@ -12,7 +12,10 @@ class DtImageBlot extends ImageBlot { return node } static value(node) { - return { src: node.getAttribute('src'), path: node.getAttribute('dt-data-path') } + return { + src: node.getAttribute('src'), + path: node.getAttribute('dt-data-path'), + } } static formats(node) { return { 'dt-data-path': node.getAttribute('dt-data-path') } @@ -36,14 +39,12 @@ import { useImperativeHandle, useRef, } from 'react' +import { useDescriptionHtml } from '../../hooks/useDescriptionHtml' import { useUserProfile } from '../../queries/UserQueries' import { useNotification } from '../../service/NotificationProvider' import { apiClient } from '../../utils/ApiClient' -import { - isPlusAccount, - refreshSignedUrlsInHtml, - resolvePhotoURL, -} from '../../utils/Helpers' +import { isPlusAccount, resolvePhotoURL } from '../../utils/Helpers' +import { patchDescriptionHtml } from '../../utils/ImageCache' import './RichTextEditor.css' const RichTextEditor = forwardRef( @@ -56,11 +57,14 @@ const RichTextEditor = forwardRef( variant = 'outlined', entityId, entityType, + draftId, }, ref, ) => { const { showError } = useNotification() const { data: userProfile } = useUserProfile() + // Display-only HTML with expired image srcs swapped for cached/re-signed ones + const displayHtml = useDescriptionHtml(value) const quillRef = useRef(null) const editorRef = useRef(null) const initialContentSet = useRef(false) @@ -132,11 +136,20 @@ const RichTextEditor = forwardRef( `Compressed size: ${(compressedJpegFile.size / 1024 / 1024).toFixed(2)} MB`, ) - // Upload compressed image to backend + // Upload compressed image to backend. Without a saved entity yet, + // upload as a draft tied to draftId — the backend promotes drafts + // to the real entity when the chore is created. const formData = new FormData() formData.append('file', compressedJpegFile) - formData.append('entityId', entityId) - formData.append('entityType', entityType) + if (entityId) { + formData.append('entityId', String(entityId)) + formData.append('entityType', entityType) + } else if (draftId) { + formData.append('entityType', `${entityType}_draft`) + formData.append('draftId', draftId) + } else { + formData.append('entityType', entityType) + } const response = await apiClient.upload('/assets/chore', formData) @@ -152,7 +165,7 @@ const RichTextEditor = forwardRef( message: 'The file you are trying to upload is too large.', }) return - } else if (response.status === 403 && !isPlusAccount()) { + } else if (response.status === 403 && !isPlusAccount(userProfile)) { showError({ title: 'Upgrade Required', message: @@ -173,9 +186,9 @@ const RichTextEditor = forwardRef( return } const data = await response.json() - // Prefer the backend-proxied path (data.sign) over the direct cloud - // signed URL (data.url) — the proxy re-signs on every request so the - // embedded src never expires. + // data.sign is a fetchable signed URL; data.path is the stable + // storage key kept in dt-data-path so the src can be re-signed + // after the URL expires. const path = data.path const url = resolvePhotoURL(data.sign || data.url) // Insert image into Quill with dt-data-path tracked by the custom blot @@ -191,7 +204,7 @@ const RichTextEditor = forwardRef( }) } } - }, [entityId, entityType, showError, userProfile]) // Dependencies for useCallback + }, [entityId, entityType, draftId, showError, userProfile]) // Dependencies for useCallback useEffect(() => { if (!quillRef.current) return @@ -223,27 +236,25 @@ const RichTextEditor = forwardRef( } }) } - // If switching to read-only mode, disable Quill instance - if (editorRef.current && !isEditable) { - // editorRef.current.disable() - editorRef.current.readOnly = true - - // If switching back to editable, enable Quill - if (editorRef.current && isEditable) { - // editorRef.current.enable() - editorRef.current.readOnly = false - } + // Keep Quill's editing state in sync with the isEditable prop + if (editorRef.current) { + editorRef.current.enable(isEditable) } }, [onChange, value, isEditable, variant, handleImageUpload, userProfile]) // Added handleImageUpload and userProfile to dependency array useEffect(() => { if (editorRef.current && isEditable) { if (editorRef.current.root.innerHTML !== value) { - const html = !initialContentSet.current - ? refreshSignedUrlsInHtml(value || '') - : value || '' + editorRef.current.root.innerHTML = value || '' + // On first load, swap expired image srcs for cached/re-signed ones + if (!initialContentSet.current && value) { + patchDescriptionHtml(value).then(html => { + if (editorRef.current && html !== value) { + editorRef.current.root.innerHTML = html + } + }) + } initialContentSet.current = true - editorRef.current.root.innerHTML = html } } }, [value, isEditable]) @@ -268,7 +279,7 @@ const RichTextEditor = forwardRef( boxShadow: 'var(--joy-shadow-xs, 0px 1px 2px 0px rgba(16, 24, 40, 0.05))', }} - dangerouslySetInnerHTML={{ __html: refreshSignedUrlsInHtml(value) }} + dangerouslySetInnerHTML={{ __html: displayHtml }} /> ) }