diff --git a/Backend/src/rooms/routes/__pycache__/room_routes.cpython-312.pyc b/Backend/src/rooms/routes/__pycache__/room_routes.cpython-312.pyc index 9d26b61e..586b0efb 100644 Binary files a/Backend/src/rooms/routes/__pycache__/room_routes.cpython-312.pyc and b/Backend/src/rooms/routes/__pycache__/room_routes.cpython-312.pyc differ diff --git a/Backend/src/rooms/routes/room_routes.py b/Backend/src/rooms/routes/room_routes.py index 4aa90463..a38b900f 100644 --- a/Backend/src/rooms/routes/room_routes.py +++ b/Backend/src/rooms/routes/room_routes.py @@ -595,14 +595,20 @@ async def delete_room_images(id: int, image_url: str=Query(..., description='Ima room = db.query(Room).filter(Room.id == id).first() if not room: raise HTTPException(status_code=404, detail='Room not found') + # For external URLs, keep the full URL for matching + # For local files, normalize to path + is_external_url = image_url.startswith('http://') or image_url.startswith('https://') normalized_url = image_url - if image_url.startswith('http://') or image_url.startswith('https://'): - from urllib.parse import urlparse - parsed = urlparse(image_url) - normalized_url = parsed.path - if not normalized_url.startswith('/'): - normalized_url = f'/{normalized_url}' - filename = Path(normalized_url).name + filename = None + + if is_external_url: + # For external URLs, use the full URL as-is for matching + normalized_url = image_url + else: + # For local files, normalize the path + if not normalized_url.startswith('/'): + normalized_url = f'/{normalized_url}' + filename = Path(normalized_url).name # Handle existing_images - it might be a list, a JSON string, or None existing_images = room.images or [] @@ -620,13 +626,24 @@ async def delete_room_images(id: int, image_url: str=Query(..., description='Ima updated_images = [] for img in existing_images: - stored_path = img if img.startswith('/') else f'/{img}' - stored_filename = Path(stored_path).name - if img != normalized_url and stored_path != normalized_url and (stored_filename != filename): - updated_images.append(img) - file_path = Path(__file__).parent.parent.parent / 'uploads' / 'rooms' / filename - if file_path.exists(): - file_path.unlink() + # For external URLs, match by full URL (keep images that don't match) + if is_external_url: + # Keep the image if it doesn't match the URL we're deleting + if img != normalized_url: + updated_images.append(img) + else: + # For local files, match by path or filename (keep images that don't match) + stored_path = img if img.startswith('/') else f'/{img}' + stored_filename = Path(stored_path).name if '/' in str(stored_path) else stored_path + # Keep the image if it doesn't match any of the comparison criteria + if img != normalized_url and stored_path != normalized_url and (not filename or stored_filename != filename): + updated_images.append(img) + + # Only try to delete the file if it's a local file (filename exists) + if filename: + file_path = Path(__file__).parent.parent.parent / 'uploads' / 'rooms' / filename + if file_path.exists(): + file_path.unlink() room.images = updated_images db.commit() return {'status': 'success', 'message': 'Image deleted successfully', 'data': {'images': updated_images}} diff --git a/Frontend/src/pages/admin/EditRoomPage.tsx b/Frontend/src/pages/admin/EditRoomPage.tsx index ff1060bb..2bb0fcf7 100644 --- a/Frontend/src/pages/admin/EditRoomPage.tsx +++ b/Frontend/src/pages/admin/EditRoomPage.tsx @@ -23,6 +23,7 @@ const EditRoomPage: React.FC = () => { const [roomTypes, setRoomTypes] = useState>([]); const [deletingImageUrl, setDeletingImageUrl] = useState(null); const [failedImageUrls, setFailedImageUrls] = useState>(new Set()); + const [loadingImageUrls, setLoadingImageUrls] = useState>(new Set()); const [formData, setFormData] = useState({ room_number: '', @@ -46,6 +47,44 @@ const EditRoomPage: React.FC = () => { fetchRoomTypes(); }, [id]); + // Reset image loading states when room data changes + useEffect(() => { + if (editingRoom) { + // Clear failed images when room data is loaded/refreshed + setFailedImageUrls(new Set()); + setLoadingImageUrls(new Set()); + + // Check for cached images after a brief delay to allow DOM to render + setTimeout(() => { + const allImageElements = document.querySelectorAll('img[data-image-id^="room-image-"]'); + allImageElements.forEach((img) => { + const imgElement = img as HTMLImageElement; + if (imgElement.complete && imgElement.naturalWidth > 0) { + const imageId = imgElement.getAttribute('data-image-id'); + if (imageId) { + // Extract the original path from the image ID + const match = imageId.match(/room-image-\d+-(.+)/); + if (match) { + const originalPath = match[1]; + console.log('EditRoomPage - Found cached image on mount:', originalPath); + setFailedImageUrls(prev => { + const newSet = new Set(prev); + newSet.delete(originalPath); + return newSet; + }); + setLoadingImageUrls(prev => { + const newSet = new Set(prev); + newSet.delete(originalPath); + return newSet; + }); + } + } + } + }); + }, 100); + } + }, [editingRoom?.id]); + const fetchRoomTypes = async () => { try { const response = await roomService.getRooms({ limit: 100, page: 1 }); @@ -98,7 +137,7 @@ const EditRoomPage: React.FC = () => { }; const fetchRoomData = async () => { - if (!id) return; + if (!id) return null; try { setLoading(true); @@ -136,10 +175,13 @@ const EditRoomPage: React.FC = () => { view: room.view || '', amenities: amenitiesArray, }); + + return room; } catch (error: any) { logger.error('Failed to fetch room data', error); toast.error(error.response?.data?.message || 'Failed to load room data'); navigate('/admin/advanced-rooms'); + return null; } finally { setLoading(false); } @@ -299,24 +341,110 @@ const EditRoomPage: React.FC = () => { // Immediately mark as failed to prevent error handler from firing setFailedImageUrls(prev => new Set([...prev, imageUrl])); + // For external URLs (like Unsplash), keep the full URL + // For local files, extract the path let imagePath = imageUrl; - if (imageUrl.startsWith('http://') || imageUrl.startsWith('https://')) { - try { - const url = new URL(imageUrl); - imagePath = url.pathname; - } catch (e) { - const match = imageUrl.match(/(\/uploads\/.*)/); - imagePath = match ? match[1] : imageUrl; + const isExternalUrl = imageUrl.startsWith('http://') || imageUrl.startsWith('https://'); + + if (isExternalUrl) { + // For external URLs, use the full URL as stored in database + imagePath = imageUrl; + } else { + // For local files, extract the path + if (imageUrl.startsWith('http://') || imageUrl.startsWith('https://')) { + try { + const url = new URL(imageUrl); + imagePath = url.pathname; + } catch (e) { + const match = imageUrl.match(/(\/uploads\/.*)/); + imagePath = match ? match[1] : imageUrl; + } } } - - await apiClient.delete(`/rooms/${editingRoom.id}/images`, { - params: { image_url: imagePath }, + + // Also try the normalized format for comparison + const normalizedForDb = normalizeForComparison(imageUrl); + + // Log what we're trying to delete + console.log('EditRoomPage - Deleting image:', { + originalUrl: imageUrl, + imagePath, + normalizedForDb, + isExternalUrl, + roomImages: editingRoom.images }); + // Try deleting with the full URL/path first (most likely to match database) + let deleteSuccess = false; + try { + const response = await apiClient.delete(`/rooms/${editingRoom.id}/images`, { + params: { image_url: imagePath }, + }); + console.log('EditRoomPage - Delete successful with imagePath:', response.data); + deleteSuccess = true; + } catch (firstError: any) { + console.warn('EditRoomPage - Delete failed with imagePath, trying normalizedForDb:', firstError.response?.data); + // If that fails and formats are different, try with the normalized format + if (normalizedForDb !== imagePath && !isExternalUrl) { + try { + const response = await apiClient.delete(`/rooms/${editingRoom.id}/images`, { + params: { image_url: normalizedForDb }, + }); + console.log('EditRoomPage - Delete successful with normalizedForDb:', response.data); + deleteSuccess = true; + } catch (secondError: any) { + console.error('EditRoomPage - Delete failed with both formats:', secondError.response?.data); + throw secondError; + } + } else { + throw firstError; + } + } + + if (!deleteSuccess) { + throw new Error('Failed to delete image'); + } + toast.success('Image deleted successfully'); + + // Clear the image from state immediately + setFailedImageUrls(prev => { + const newSet = new Set(prev); + newSet.delete(imageUrl); + newSet.delete(imagePath); + newSet.delete(normalizedForDb); + return newSet; + }); + setLoadingImageUrls(prev => { + const newSet = new Set(prev); + newSet.delete(imageUrl); + newSet.delete(imagePath); + newSet.delete(normalizedForDb); + return newSet; + }); + + // Refetch room data to get updated image list await refreshRooms(); - await fetchRoomData(); + const updatedRoom = await fetchRoomData(); + + // Verify the image was actually removed + if (updatedRoom) { + const stillExists = (updatedRoom.images || []).some((img: any) => { + const imgStr = String(img); + return imgStr === imageUrl || + imgStr === imagePath || + imgStr === normalizedForDb || + normalizeForComparison(imgStr) === normalizedForDb; + }); + + if (stillExists) { + console.warn('EditRoomPage - Image still exists after deletion:', { + imageUrl, + updatedImages: updatedRoom.images + }); + toast.warning('Image may still be in the list. Please refresh the page.'); + } + } } catch (error: any) { logger.error('Error deleting image', error); toast.error(error.response?.data?.message || error.response?.data?.detail || 'Unable to delete image'); @@ -331,6 +459,78 @@ const EditRoomPage: React.FC = () => { } }; + const handleRemoveBrokenImage = async (imageUrl: string) => { + if (!editingRoom) return; + if (!window.confirm('This image file is missing. Remove it from the room?')) return; + + try { + setDeletingImageUrl(imageUrl); + + // For external URLs (like Unsplash), keep the full URL + // For local files, extract the path + let imagePath = imageUrl; + const isExternalUrl = imageUrl.startsWith('http://') || imageUrl.startsWith('https://'); + + if (isExternalUrl) { + // For external URLs, use the full URL as stored in database + imagePath = imageUrl; + } else { + // For local files, extract the path + if (imageUrl.startsWith('http://') || imageUrl.startsWith('https://')) { + try { + const url = new URL(imageUrl); + imagePath = url.pathname; + } catch (e) { + const match = imageUrl.match(/(\/uploads\/.*)/); + imagePath = match ? match[1] : imageUrl; + } + } + } + + // Also try the normalized format for comparison + const normalizedForDb = normalizeForComparison(imageUrl); + + // Try deleting with the normalized path first + try { + await apiClient.delete(`/rooms/${editingRoom.id}/images`, { + params: { image_url: imagePath }, + }); + } catch (firstError: any) { + // If that fails, try with the database format + if (normalizedForDb !== imagePath) { + await apiClient.delete(`/rooms/${editingRoom.id}/images`, { + params: { image_url: normalizedForDb }, + }); + } else { + throw firstError; + } + } + + toast.success('Broken image reference removed'); + + // Clear the image from state immediately before refetching + setFailedImageUrls(prev => { + const newSet = new Set(prev); + newSet.delete(imageUrl); + return newSet; + }); + setLoadingImageUrls(prev => { + const newSet = new Set(prev); + newSet.delete(imageUrl); + return newSet; + }); + + // Refetch room data to get updated image list + await refreshRooms(); + await fetchRoomData(); + } catch (error: any) { + logger.error('Error removing broken image', error); + toast.error(error.response?.data?.message || error.response?.data?.detail || 'Unable to remove image reference'); + } finally { + setDeletingImageUrl(null); + } + }; + if (loading) { return ; } @@ -379,8 +579,44 @@ const EditRoomPage: React.FC = () => { return ''; } - // Otherwise, construct the full URL - const cleanPath = img.startsWith('/') ? img : `/${img}`; + // Handle local file paths (like room-*.png) + let cleanPath = img.trim(); + + // If it's just a filename (like "room-xxx.png"), add the uploads/rooms path + if (cleanPath.match(/^room-[\w-]+\.(png|jpg|jpeg|gif|webp|webm)$/i)) { + cleanPath = `/uploads/rooms/${cleanPath}`; + } + // If it starts with / but not /uploads, check if it's a room filename + else if (cleanPath.startsWith('/') && !cleanPath.startsWith('/uploads/')) { + if (cleanPath.match(/^\/room-[\w-]+\.(png|jpg|jpeg|gif|webp|webm)$/i)) { + cleanPath = `/uploads/rooms${cleanPath}`; + } else { + // Assume it should be in uploads + cleanPath = `/uploads${cleanPath}`; + } + } + // If it doesn't start with /, add it + else if (!cleanPath.startsWith('/')) { + // Check if it looks like a room filename + if (cleanPath.match(/^room-[\w-]+\.(png|jpg|jpeg|gif|webp|webm)$/i)) { + cleanPath = `/uploads/rooms/${cleanPath}`; + } else { + cleanPath = `/uploads/${cleanPath}`; + } + } + // If it already has /uploads/rooms/, use it as is + else if (cleanPath.startsWith('/uploads/rooms/')) { + // Already correct + } + // If it has /uploads/ but not /uploads/rooms/, check if it's a room file + else if (cleanPath.startsWith('/uploads/') && !cleanPath.startsWith('/uploads/rooms/')) { + const filename = cleanPath.split('/').pop() || ''; + if (filename.match(/^room-[\w-]+\.(png|jpg|jpeg|gif|webp|webm)$/i)) { + cleanPath = `/uploads/rooms/${filename}`; + } + } + + // Construct the full URL return `${apiBaseUrl}${cleanPath}`; }; @@ -417,7 +653,9 @@ const EditRoomPage: React.FC = () => { roomTypeImages: roomTypeImages.length, allImages: allImages.length, sampleImage: allImages[0], - normalizedUrl: allImages[0] ? normalizeImageUrl(String(allImages[0])) : 'none' + normalizedUrl: allImages[0] ? normalizeImageUrl(String(allImages[0])) : 'none', + allImagePaths: allImages.map((img: any) => String(img)), + normalizedUrls: allImages.map((img: any) => normalizeImageUrl(String(img))) }); } @@ -880,8 +1118,8 @@ const EditRoomPage: React.FC = () => { const filteredImages = allImages.filter((img) => { // Convert to string for comparison const imgStr = String(img); - // Filter out failed and deleting images - if (failedImageUrls.has(imgStr) || deletingImageUrl === imgStr) { + // Only filter out deleting images (not failed ones - we'll show them with error state) + if (deletingImageUrl === imgStr) { return false; } // Filter out empty or null images @@ -908,46 +1146,180 @@ const EditRoomPage: React.FC = () => { const isRoomImage = normalizedRoomImgs.includes(normalizedImg); const isDeleting = deletingImageUrl === imgStr; const hasFailed = failedImageUrls.has(imgStr); + const isLoading = loadingImageUrls.has(imgStr); // Safety check - should not happen due to filter, but double-check - if (hasFailed || isDeleting || !imageUrl) { + if (isDeleting || !imageUrl) { return null; } + // Determine if we should use crossOrigin for CORS + // Only use crossOrigin for external URLs (different origin) + // Same-origin requests don't need crossOrigin and it can cause OpaqueResponseBlocking errors + const isExternalUrl = imageUrl.startsWith('http://') || imageUrl.startsWith('https://'); + const isSameOrigin = isExternalUrl && ( + imageUrl.startsWith(apiBaseUrl) || + imageUrl.startsWith(window.location.origin) + ); + const useCrossOrigin = isExternalUrl && !isSameOrigin; + return (
-
+
+ {/* Loading overlay - only show if loading and not failed */} + {isLoading && !hasFailed && ( +
+
+
+ )} + + {/* Error state */} + {hasFailed && ( +
+ +

Failed to load

+ {imageUrl.includes('/uploads/') && ( +

File not found

+ )} +
+ + {isRoomImage && imageUrl.includes('/uploads/') && ( + + )} +
+
+ )} + + {/* Image element - always render but conditionally show */} {`Room { const target = e.target as HTMLImageElement; - // Only handle error once to prevent infinite loop + const retryCount = parseInt(target.dataset.retryCount || '0'); + + // Check if it's a 404 (file not found) + const is404 = target.src && (target.src.includes('404') || target.complete === false); + console.warn('EditRoomPage - Image load error:', { + imageUrl, + originalPath: imgStr, + retryCount, + hasCrossOrigin: !!target.crossOrigin, + is404: is404 || 'unknown', + note: is404 ? 'File may not exist on server. Check if file was deleted or path is incorrect.' : 'CORS or network error' + }); + + // Try retrying without crossOrigin if it was set and we haven't retried yet + if (useCrossOrigin && target.crossOrigin && retryCount === 0) { + target.dataset.retryCount = '1'; + target.removeAttribute('crossorigin'); + target.src = ''; + // Force reload by setting src again + setTimeout(() => { + target.src = imageUrl; + }, 100); + return; + } + + // Only mark as failed after retries are exhausted if (!target.dataset.errorHandled) { target.dataset.errorHandled = 'true'; - // Clear the src to prevent further load attempts - target.src = ''; - // Hide the image element - target.style.display = 'none'; - // Immediately mark as failed to remove from display + + // Mark as failed setFailedImageUrls(prev => { const newSet = new Set(prev); newSet.add(imgStr); return newSet; }); + setLoadingImageUrls(prev => { + const newSet = new Set(prev); + newSet.delete(imgStr); + return newSet; + }); + // Stop event propagation to prevent further errors e.stopPropagation(); e.preventDefault(); } }} - onLoad={() => { - console.log('EditRoomPage - Image loaded successfully:', imageUrl); + onLoad={(e) => { + const target = e.target as HTMLImageElement; + console.log('EditRoomPage - Image loaded successfully:', { + imageUrl, + originalPath: imgStr, + naturalWidth: target.naturalWidth, + naturalHeight: target.naturalHeight, + complete: target.complete, + cached: target.complete && target.naturalWidth > 0 + }); + // Clear failed state if image loads successfully + setFailedImageUrls(prev => { + if (prev.has(imgStr)) { + console.log('EditRoomPage - Clearing failed state for:', imgStr); + } + const newSet = new Set(prev); + newSet.delete(imgStr); + return newSet; + }); + setLoadingImageUrls(prev => { + const newSet = new Set(prev); + newSet.delete(imgStr); + return newSet; + }); + // Ensure image is visible + target.style.opacity = '1'; + target.style.display = 'block'; + }} + onLoadStart={() => { + // Mark as loading when image starts loading + setLoadingImageUrls(prev => { + const newSet = new Set(prev); + newSet.add(imgStr); + return newSet; + }); + // Clear failed state when starting to load + setFailedImageUrls(prev => { + const newSet = new Set(prev); + newSet.delete(imgStr); + return newSet; + }); }} />
- {isRoomImage && ( + {isRoomImage && !hasFailed && ( <>
Room Image