This commit is contained in:
Iliyan Angelov
2025-12-02 22:16:57 +02:00
parent 2d770dd27b
commit e32527ae8c
3 changed files with 432 additions and 43 deletions

View File

@@ -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}}

View File

@@ -23,6 +23,7 @@ const EditRoomPage: React.FC = () => {
const [roomTypes, setRoomTypes] = useState<Array<{ id: number; name: string }>>([]);
const [deletingImageUrl, setDeletingImageUrl] = useState<string | null>(null);
const [failedImageUrls, setFailedImageUrls] = useState<Set<string>>(new Set());
const [loadingImageUrls, setLoadingImageUrls] = useState<Set<string>>(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 <Loading />;
}
@@ -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 (
<div key={`${imgStr}-${index}`} className="relative group rounded-2xl overflow-hidden shadow-lg hover:shadow-2xl transition-all duration-300 transform hover:scale-105">
<div className="aspect-square overflow-hidden bg-gradient-to-br from-slate-100 to-slate-200 flex items-center justify-center">
<div className="aspect-square overflow-hidden bg-gradient-to-br from-slate-100 to-slate-200 flex items-center justify-center relative">
{/* Loading overlay - only show if loading and not failed */}
{isLoading && !hasFailed && (
<div className="absolute inset-0 flex items-center justify-center bg-slate-100/80 z-20 pointer-events-none">
<div className="w-8 h-8 border-2 border-amber-500 border-t-transparent rounded-full animate-spin"></div>
</div>
)}
{/* Error state */}
{hasFailed && (
<div className="absolute inset-0 w-full h-full flex flex-col items-center justify-center p-4 text-center bg-slate-100 z-30">
<ImageIcon className="w-12 h-12 text-slate-400 mb-2" />
<p className="text-xs text-slate-500 font-medium mb-1">Failed to load</p>
{imageUrl.includes('/uploads/') && (
<p className="text-xs text-red-500 font-medium mb-2">File not found</p>
)}
<div className="flex gap-2 mt-2">
<button
type="button"
onClick={() => {
// Retry loading the image by clearing error state
// The img element will be re-rendered when hasFailed becomes false
setFailedImageUrls(prev => {
const newSet = new Set(prev);
newSet.delete(imgStr);
return newSet;
});
setLoadingImageUrls(prev => {
const newSet = new Set(prev);
newSet.add(imgStr);
return newSet;
});
}}
className="px-3 py-1.5 text-xs bg-amber-500 text-white rounded-lg hover:bg-amber-600 transition-colors"
>
Retry
</button>
{isRoomImage && imageUrl.includes('/uploads/') && (
<button
type="button"
onClick={() => handleRemoveBrokenImage(imgStr)}
disabled={isDeleting}
className="px-3 py-1.5 text-xs bg-red-500 text-white rounded-lg hover:bg-red-600 transition-colors disabled:opacity-50"
>
Remove
</button>
)}
</div>
</div>
)}
{/* Image element - always render but conditionally show */}
<img
key={`img-${imgStr}-${index}`}
data-image-id={`room-image-${index}-${imgStr}`}
src={imageUrl}
alt={`Room Image ${index + 1}`}
className="w-full h-full object-cover group-hover:scale-110 transition-transform duration-500"
className={`w-full h-full object-cover group-hover:scale-110 transition-all duration-500 ${
hasFailed ? 'opacity-0 absolute pointer-events-none' : 'opacity-100'
}`}
loading="lazy"
crossOrigin={useCrossOrigin ? "anonymous" : undefined}
onError={(e) => {
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;
});
}}
/>
</div>
{isRoomImage && (
{isRoomImage && !hasFailed && (
<>
<div className="absolute top-2 left-2 bg-gradient-to-r from-amber-500/90 to-amber-600/90 backdrop-blur-md text-white px-3 py-1.5 rounded-xl text-xs font-semibold opacity-0 group-hover:opacity-100 transition-opacity duration-300 shadow-lg">
Room Image