diff --git a/invokeai/app/api/routers/boards.py b/invokeai/app/api/routers/boards.py index c6adeab850e..6d6dea4fb92 100644 --- a/invokeai/app/api/routers/boards.py +++ b/invokeai/app/api/routers/boards.py @@ -38,6 +38,14 @@ class DeleteBoardResult(BaseModel): default_factory=list, description="The names of videos that could not be deleted and became uncategorized.", ) + starred_images_skipped: list[str] = Field( + default_factory=list, + description="The names of starred images that were protected and became uncategorized.", + ) + starred_videos_skipped: list[str] = Field( + default_factory=list, + description="The names of starred videos that were protected and became uncategorized.", + ) @boards_router.post( @@ -125,6 +133,7 @@ def delete_board( include_images: Optional[bool] = Query( description="Permanently delete all images and videos on the board", default=False ), + delete_starred: bool = Query(default=True, description="Whether to allow deletion of starred media"), ) -> DeleteBoardResult: """Deletes a board (user must have access to it)""" try: @@ -141,6 +150,10 @@ def delete_board( cascade_user_id: Optional[str] = None if current_user.is_admin else current_user.user_id deleted_images: list[str] = [] deleted_videos: list[str] = [] + failed_images: list[str] = [] + failed_videos: list[str] = [] + starred_images_skipped: list[str] = [] + starred_videos_skipped: list[str] = [] try: if include_images is True: @@ -151,11 +164,19 @@ def delete_board( # truth — reconstructing failures by diffing a router-side board listing # against the deleted names would double the DB work and misreport items # moved or deleted concurrently between the two queries. - deleted_images, failed_images = ApiDependencies.invoker.services.images.delete_images_on_board( - board_id=board_id, user_id=cascade_user_id + ( + deleted_images, + failed_images, + starred_images_skipped, + ) = ApiDependencies.invoker.services.images.delete_images_on_board( + board_id=board_id, user_id=cascade_user_id, delete_starred=delete_starred ) - deleted_videos, failed_videos = ApiDependencies.invoker.services.videos.delete_videos_on_board( - board_id=board_id, user_id=cascade_user_id + ( + deleted_videos, + failed_videos, + starred_videos_skipped, + ) = ApiDependencies.invoker.services.videos.delete_videos_on_board( + board_id=board_id, user_id=cascade_user_id, delete_starred=delete_starred ) ApiDependencies.invoker.services.boards.delete(board_id=board_id) return DeleteBoardResult( @@ -166,6 +187,8 @@ def delete_board( deleted_videos=deleted_videos, failed_images=failed_images, failed_videos=failed_videos, + starred_images_skipped=starred_images_skipped, + starred_videos_skipped=starred_videos_skipped, ) else: deleted_board_images = ApiDependencies.invoker.services.board_images.get_all_board_image_names_for_board( @@ -198,6 +221,10 @@ def delete_board( "message": "Failed to delete board after partially deleting media", "deleted_images": deleted_images, "deleted_videos": deleted_videos, + "failed_images": failed_images, + "failed_videos": failed_videos, + "starred_images_skipped": starred_images_skipped, + "starred_videos_skipped": starred_videos_skipped, "board_deleted": False, }, ) diff --git a/invokeai/app/api/routers/images.py b/invokeai/app/api/routers/images.py index b9e06befb9c..7861c9b4485 100644 --- a/invokeai/app/api/routers/images.py +++ b/invokeai/app/api/routers/images.py @@ -202,27 +202,34 @@ async def create_image_upload_entry( async def delete_image( current_user: CurrentUserOrDefault, image_name: str = Path(description="The name of the image to delete"), + delete_starred: bool = Query(default=True, description="Whether to allow deletion of starred images"), ) -> DeleteImagesResult: """Deletes an image""" _assert_image_owner(image_name, current_user) assert_image_move_maintenance_inactive() deleted_images: set[str] = set() + failed_images: set[str] = set() affected_boards: set[str] = set() + starred_skipped: set[str] = set() try: - image_dto = ApiDependencies.invoker.services.images.get_dto(image_name) - board_id = image_dto.board_id or "none" - ApiDependencies.invoker.services.images.delete(image_name) - deleted_images.add(image_name) + board_id = ApiDependencies.invoker.services.board_image_records.get_board_for_image(image_name) or "none" + was_deleted = ApiDependencies.invoker.services.images.delete(image_name, delete_starred=delete_starred) + if was_deleted: + deleted_images.add(image_name) + else: + starred_skipped.add(image_name) affected_boards.add(board_id) - except Exception: - # TODO: Does this need any exception handling at all? - pass + except Exception as error: + failed_images.add(image_name) + ApiDependencies.invoker.services.logger.error(f"Failed to delete image {image_name}: {error}") return DeleteImagesResult( deleted_images=list(deleted_images), + failed_images=list(failed_images), affected_boards=list(affected_boards), + starred_skipped=list(starred_skipped), ) @@ -489,6 +496,7 @@ async def list_image_dtos( async def delete_images_from_list( current_user: CurrentUserOrDefault, image_names: list[str] = Body(description="The list of names of images to delete", embed=True), + delete_starred: bool = Body(default=True, description="Whether to allow deletion of starred images"), ) -> DeleteImagesResult: try: assert_image_move_maintenance_inactive() @@ -505,27 +513,34 @@ async def delete_images_from_list( deleted_images: set[str] = set() failed_images: set[str] = set() affected_boards: set[str] = set() + starred_skipped: set[str] = set() # Dedup while preserving order: a name repeated in the request would otherwise # be processed twice, and the second pass's not-found error would land the same # name in both deleted_images and failed_images. for image_name in dict.fromkeys(image_names): try: _assert_image_owner(image_name, current_user) - image_dto = ApiDependencies.invoker.services.images.get_dto(image_name) - board_id = image_dto.board_id or "none" - ApiDependencies.invoker.services.images.delete(image_name) - deleted_images.add(image_name) + board_id = ( + ApiDependencies.invoker.services.board_image_records.get_board_for_image(image_name) or "none" + ) + was_deleted = ApiDependencies.invoker.services.images.delete(image_name, delete_starred=delete_starred) + if was_deleted: + deleted_images.add(image_name) + else: + starred_skipped.add(image_name) affected_boards.add(board_id) except HTTPException: continue - except Exception: + except Exception as error: # A genuine deletion failure (not an auth/404 skip) — report it so the # client can surface a partial-failure warning, matching the video path. failed_images.add(image_name) + ApiDependencies.invoker.services.logger.error(f"Failed to delete image {image_name}: {error}") return DeleteImagesResult( deleted_images=list(deleted_images), failed_images=list(failed_images), affected_boards=list(affected_boards), + starred_skipped=list(starred_skipped), ) except HTTPException: raise @@ -536,6 +551,7 @@ async def delete_images_from_list( @images_router.delete("/uncategorized", operation_id="delete_uncategorized_images", response_model=DeleteImagesResult) async def delete_uncategorized_images( current_user: CurrentUserOrDefault, + delete_starred: bool = Query(default=True, description="Whether to allow deletion of starred images"), ) -> DeleteImagesResult: """Deletes all uncategorized images owned by the current user (or all if admin)""" assert_image_move_maintenance_inactive() @@ -548,21 +564,27 @@ async def delete_uncategorized_images( deleted_images: set[str] = set() failed_images: set[str] = set() affected_boards: set[str] = set() + starred_skipped: set[str] = set() for image_name in image_names: try: _assert_image_owner(image_name, current_user) - ApiDependencies.invoker.services.images.delete(image_name) - deleted_images.add(image_name) + was_deleted = ApiDependencies.invoker.services.images.delete(image_name, delete_starred=delete_starred) + if was_deleted: + deleted_images.add(image_name) + else: + starred_skipped.add(image_name) affected_boards.add("none") except HTTPException: # Skip images not owned by the current user - pass - except Exception: + continue + except Exception as error: failed_images.add(image_name) + ApiDependencies.invoker.services.logger.error(f"Failed to delete image {image_name}: {error}") return DeleteImagesResult( deleted_images=list(deleted_images), failed_images=list(failed_images), affected_boards=list(affected_boards), + starred_skipped=list(starred_skipped), ) except Exception: raise HTTPException(status_code=500, detail="Failed to delete images") diff --git a/invokeai/app/api/routers/videos.py b/invokeai/app/api/routers/videos.py index c535aa3b931..27bc3f2669a 100644 --- a/invokeai/app/api/routers/videos.py +++ b/invokeai/app/api/routers/videos.py @@ -74,6 +74,10 @@ class VideoNamesBatch(BaseModel): ) +class DeleteVideosBatch(VideoNamesBatch): + delete_starred: bool = Field(default=True, description="Whether to allow deletion of starred videos") + + def _get_video_cache_control() -> str: if ApiDependencies.invoker.services.configuration.multiuser: return "private, no-store" @@ -350,36 +354,37 @@ async def upload_video( async def delete_video( current_user: CurrentUserOrDefault, video_name: str = PathParam(description="The name of the video to delete"), + delete_starred: bool = Query(default=True, description="Whether to allow deletion of starred videos"), ) -> DeleteVideosResult: _assert_video_owner(video_name, current_user) - # Let service-level failures surface as 500s rather than swallowing them and returning a - # success-shaped response. A previous version of this handler caught everything and - # returned an empty ``deleted_videos`` list with HTTP 200; the frontend treated that as - # success, dropped the item from its cache, and the video stayed on disk — a silent - # data-consistency failure that only became visible on the next page reload. - try: - video_dto = ApiDependencies.invoker.services.videos.get_dto(video_name) - except Exception: - raise HTTPException(status_code=404, detail="Video not found") - - board_id = video_dto.board_id or "none" + # Report service and relation-read failures as explicit failed items. The frontend + # must only evict names confirmed in ``deleted_videos``; otherwise a partial failure + # can silently hide a video that is still present on disk. + board_id: str | None = None try: - ApiDependencies.invoker.services.videos.delete(video_name) - except Exception: - raise HTTPException(status_code=500, detail="Failed to delete video") + board_id = ApiDependencies.invoker.services.board_video_records.get_board_for_video(video_name) or "none" + was_deleted = ApiDependencies.invoker.services.videos.delete(video_name, delete_starred=delete_starred) + except Exception as error: + ApiDependencies.invoker.services.logger.error(f"Failed to delete video {video_name}: {error}") + return DeleteVideosResult( + deleted_videos=[], + failed_videos=[video_name], + affected_boards=[board_id] if board_id is not None else [], + ) return DeleteVideosResult( - deleted_videos=[video_name], + deleted_videos=[video_name] if was_deleted else [], failed_videos=[], affected_boards=[board_id], + starred_skipped=[] if was_deleted else [video_name], ) @videos_router.post("/delete", operation_id="delete_videos_from_list", response_model=DeleteVideosResult) def delete_videos_from_list( current_user: CurrentUserOrDefault, - batch: VideoNamesBatch, + batch: DeleteVideosBatch, ) -> DeleteVideosResult: # Skip — but do not re-raise — auth failures so a foreign name mid-batch doesn't # discard the response payload for items the caller had already legitimately deleted. @@ -393,31 +398,39 @@ def delete_videos_from_list( deleted_videos: set[str] = set() failed_videos: set[str] = set() affected_boards: set[str] = set() + starred_skipped: set[str] = set() # Dedup while preserving order: a name repeated in the request would otherwise be # processed twice, and the second pass's not-found error would land the same name # in both deleted_videos and failed_videos. for video_name in dict.fromkeys(batch.video_names): try: _assert_video_owner(video_name, current_user) - video_dto = ApiDependencies.invoker.services.videos.get_dto(video_name) - board_id = video_dto.board_id or "none" - ApiDependencies.invoker.services.videos.delete(video_name) - deleted_videos.add(video_name) + board_id = ApiDependencies.invoker.services.board_video_records.get_board_for_video(video_name) or "none" + was_deleted = ApiDependencies.invoker.services.videos.delete( + video_name, delete_starred=batch.delete_starred + ) + if was_deleted: + deleted_videos.add(video_name) + else: + starred_skipped.add(video_name) affected_boards.add(board_id) except HTTPException: continue - except Exception: + except Exception as error: failed_videos.add(video_name) + ApiDependencies.invoker.services.logger.error(f"Failed to delete video {video_name}: {error}") return DeleteVideosResult( deleted_videos=list(deleted_videos), failed_videos=list(failed_videos), affected_boards=list(affected_boards), + starred_skipped=list(starred_skipped), ) @videos_router.delete("/uncategorized", operation_id="delete_uncategorized_videos", response_model=DeleteVideosResult) def delete_uncategorized_videos( current_user: CurrentUserOrDefault, + delete_starred: bool = Query(default=True, description="Whether to allow deletion of starred videos"), ) -> DeleteVideosResult: """Deletes all uncategorized videos owned by the current user (or all if admin). @@ -432,22 +445,28 @@ def delete_uncategorized_videos( deleted_videos: set[str] = set() failed_videos: set[str] = set() affected_boards: set[str] = set() + starred_skipped: set[str] = set() for video_name in names_result.video_names: try: _assert_video_owner(video_name, current_user) - ApiDependencies.invoker.services.videos.delete(video_name) - deleted_videos.add(video_name) + was_deleted = ApiDependencies.invoker.services.videos.delete(video_name, delete_starred=delete_starred) + if was_deleted: + deleted_videos.add(video_name) + else: + starred_skipped.add(video_name) affected_boards.add("none") except HTTPException: # Skip videos not owned by the current user — an intentional skip, not a # failed deletion, so it must not be reported (and toasted) as one. continue - except Exception: + except Exception as error: failed_videos.add(video_name) + ApiDependencies.invoker.services.logger.error(f"Failed to delete video {video_name}: {error}") return DeleteVideosResult( deleted_videos=list(deleted_videos), failed_videos=list(failed_videos), affected_boards=list(affected_boards), + starred_skipped=list(starred_skipped), ) diff --git a/invokeai/app/services/images/images_base.py b/invokeai/app/services/images/images_base.py index 000c8a43fc7..4a956d6cb7d 100644 --- a/invokeai/app/services/images/images_base.py +++ b/invokeai/app/services/images/images_base.py @@ -133,8 +133,8 @@ def get_many( pass @abstractmethod - def delete(self, image_name: str): - """Deletes an image.""" + def delete(self, image_name: str, delete_starred: bool = True) -> bool: + """Deletes an image, returning ``False`` when a starred image is protected.""" pass @abstractmethod @@ -148,8 +148,10 @@ def get_intermediates_count(self, user_id: Optional[str] = None) -> int: pass @abstractmethod - def delete_images_on_board(self, board_id: str, user_id: Optional[str] = None) -> tuple[list[str], list[str]]: - """Deletes all images on a board; returns ``(deleted_names, failed_names)``. + def delete_images_on_board( + self, board_id: str, user_id: Optional[str] = None, delete_starred: bool = True + ) -> tuple[list[str], list[str], list[str]]: + """Deletes images on a board; returns ``(deleted_names, failed_names, starred_skipped_names)``. When ``user_id`` is provided, only images owned by that user are deleted (other users' contributions to a public/shared board are preserved). Pass ``None`` for the admin @@ -158,6 +160,9 @@ def delete_images_on_board(self, board_id: str, user_id: Optional[str] = None) - ``failed_names`` is the service's ground truth for per-image failures — callers must not reconstruct it by diffing their own board listing against ``deleted_names``, which races with concurrent moves/deletes. + + When ``delete_starred`` is ``False``, starred images are preserved and reported in + ``starred_skipped_names``. """ pass diff --git a/invokeai/app/services/images/images_common.py b/invokeai/app/services/images/images_common.py index 51679b43f4c..d21710c70c2 100644 --- a/invokeai/app/services/images/images_common.py +++ b/invokeai/app/services/images/images_common.py @@ -51,6 +51,10 @@ class DeleteImagesResult(ResultWithAffectedBoards): default_factory=list, description="The names of authorized images that could not be deleted", ) + starred_skipped: list[str] = Field( + default_factory=list, + description="The names of starred images that were skipped because deletion protection was enabled", + ) class StarredImagesResult(ResultWithAffectedBoards): diff --git a/invokeai/app/services/images/images_default.py b/invokeai/app/services/images/images_default.py index 9fded083cbc..a766657f52e 100644 --- a/invokeai/app/services/images/images_default.py +++ b/invokeai/app/services/images/images_default.py @@ -275,12 +275,15 @@ def get_many( self.__invoker.services.logger.error("Problem getting paginated image DTOs") raise e - def delete(self, image_name: str): + def delete(self, image_name: str, delete_starred: bool = True) -> bool: try: record = self.__invoker.services.image_records.get(image_name) + if not delete_starred and record.starred: + return False self.__invoker.services.image_files.delete(image_name, image_subfolder=record.image_subfolder) self.__invoker.services.image_records.delete(image_name) self._on_deleted(image_name) + return True except ImageRecordDeleteException: self.__invoker.services.logger.error("Failed to delete image record") raise @@ -291,7 +294,9 @@ def delete(self, image_name: str): self.__invoker.services.logger.error("Problem deleting image record and file") raise e - def delete_images_on_board(self, board_id: str, user_id: Optional[str] = None) -> tuple[list[str], list[str]]: + def delete_images_on_board( + self, board_id: str, user_id: Optional[str] = None, delete_starred: bool = True + ) -> tuple[list[str], list[str], list[str]]: try: # When ``user_id`` is set the lookup filters to images owned by that user so the # cascade doesn't destroy other users' contributions to a public/shared board. @@ -303,10 +308,14 @@ def delete_images_on_board(self, board_id: str, user_id: Optional[str] = None) - ) deleted_image_names: list[str] = [] failed_image_names: list[str] = [] + starred_skipped_image_names: list[str] = [] staged_deletes: list[tuple[str, object]] = [] for image_name in image_names: try: record = self.__invoker.services.image_records.get(image_name) + if not delete_starred and record.starred: + starred_skipped_image_names.append(image_name) + continue token = self.__invoker.services.image_files.stage_delete( image_name, image_subfolder=record.image_subfolder ) @@ -335,7 +344,7 @@ def delete_images_on_board(self, board_id: str, user_id: Optional[str] = None) - self.__invoker.services.logger.error(f"Failed to purge staged image files: {cleanup_error}") for image_name in deleted_image_names: self._on_deleted(image_name) - return deleted_image_names, failed_image_names + return deleted_image_names, failed_image_names, starred_skipped_image_names except ImageRecordDeleteException: self.__invoker.services.logger.error("Failed to delete image records") raise diff --git a/invokeai/app/services/videos/videos_base.py b/invokeai/app/services/videos/videos_base.py index 542e2cdff5d..3de4a131225 100644 --- a/invokeai/app/services/videos/videos_base.py +++ b/invokeai/app/services/videos/videos_base.py @@ -129,13 +129,15 @@ def get_many( pass @abstractmethod - def delete(self, video_name: str) -> None: - """Deletes a video.""" + def delete(self, video_name: str, delete_starred: bool = True) -> bool: + """Deletes a video, returning ``False`` when a starred video is protected.""" pass @abstractmethod - def delete_videos_on_board(self, board_id: str, user_id: Optional[str] = None) -> tuple[list[str], list[str]]: - """Deletes all videos on a board; returns ``(deleted_names, failed_names)``. + def delete_videos_on_board( + self, board_id: str, user_id: Optional[str] = None, delete_starred: bool = True + ) -> tuple[list[str], list[str], list[str]]: + """Deletes videos on a board; returns ``(deleted_names, failed_names, starred_skipped_names)``. When ``user_id`` is provided, only videos owned by that user are deleted (other users' contributions to a public/shared board are preserved). Pass ``None`` for the admin @@ -146,6 +148,9 @@ def delete_videos_on_board(self, board_id: str, user_id: Optional[str] = None) - are the authoritative ``deleted_videos``/``failed_videos`` for the caller's response — callers must not reconstruct failures by diffing their own board listing, which races with concurrent moves/deletes. + + When ``delete_starred`` is ``False``, starred videos are preserved and reported in + ``starred_skipped_names``. """ pass diff --git a/invokeai/app/services/videos/videos_common.py b/invokeai/app/services/videos/videos_common.py index f6d687715c3..9070d80c5e5 100644 --- a/invokeai/app/services/videos/videos_common.py +++ b/invokeai/app/services/videos/videos_common.py @@ -44,6 +44,10 @@ class VideoResultWithAffectedBoards(BaseModel): class DeleteVideosResult(VideoResultWithAffectedBoards): deleted_videos: list[str] = Field(description="The names of the videos that were deleted") failed_videos: list[str] = Field(description="The names of videos that were not deleted") + starred_skipped: list[str] = Field( + default_factory=list, + description="The names of starred videos that were skipped because deletion protection was enabled", + ) class StarredVideosResult(VideoResultWithAffectedBoards): diff --git a/invokeai/app/services/videos/videos_default.py b/invokeai/app/services/videos/videos_default.py index 46e3711dbb5..2a78bc12ebd 100644 --- a/invokeai/app/services/videos/videos_default.py +++ b/invokeai/app/services/videos/videos_default.py @@ -289,11 +289,13 @@ def get_many( self.__invoker.services.logger.error("Problem getting paginated video DTOs") raise e - def delete(self, video_name: str) -> None: + def delete(self, video_name: str, delete_starred: bool = True) -> bool: token: object | None = None record_deleted = False try: record = self.__invoker.services.video_records.get(video_name) + if not delete_starred and record.starred: + return False token = self.__invoker.services.video_files.stage_delete(video_name, video_subfolder=record.video_subfolder) self.__invoker.services.video_records.delete(video_name) record_deleted = True @@ -302,6 +304,7 @@ def delete(self, video_name: str) -> None: except Exception as cleanup_error: self.__invoker.services.logger.error(f"Failed to purge staged video files: {cleanup_error}") self._on_deleted(video_name) + return True except VideoRecordDeleteException: if token is not None: self.__invoker.services.video_files.rollback_delete(token) @@ -319,7 +322,9 @@ def delete(self, video_name: str) -> None: self.__invoker.services.logger.error("Problem deleting video record and file") raise e - def delete_videos_on_board(self, board_id: str, user_id: Optional[str] = None) -> tuple[list[str], list[str]]: + def delete_videos_on_board( + self, board_id: str, user_id: Optional[str] = None, delete_starred: bool = True + ) -> tuple[list[str], list[str], list[str]]: try: # When ``user_id`` is set the lookup filters to videos owned by that user so the # cascade doesn't destroy other users' contributions to a public/shared board. @@ -334,10 +339,14 @@ def delete_videos_on_board(self, board_id: str, user_id: Optional[str] = None) - # board_videos FK. deleted_video_names: list[str] = [] failed_video_names: list[str] = [] + starred_skipped_video_names: list[str] = [] staged_deletes: list[tuple[str, object]] = [] for video_name in video_names: try: record = self.__invoker.services.video_records.get(video_name) + if not delete_starred and record.starred: + starred_skipped_video_names.append(video_name) + continue token = self.__invoker.services.video_files.stage_delete( video_name, video_subfolder=record.video_subfolder ) @@ -366,7 +375,7 @@ def delete_videos_on_board(self, board_id: str, user_id: Optional[str] = None) - self.__invoker.services.logger.error(f"Failed to purge staged video files: {cleanup_error}") for video_name in deleted_video_names: self._on_deleted(video_name) - return deleted_video_names, failed_video_names + return deleted_video_names, failed_video_names, starred_skipped_video_names except VideoRecordDeleteException: self.__invoker.services.logger.error("Failed to delete video records") raise diff --git a/invokeai/frontend/web/openapi.json b/invokeai/frontend/web/openapi.json index e19d0163e31..55b0d71913f 100644 --- a/invokeai/frontend/web/openapi.json +++ b/invokeai/frontend/web/openapi.json @@ -5101,6 +5101,18 @@ "title": "Image Name" }, "description": "The name of the image to delete" + }, + { + "name": "delete_starred", + "in": "query", + "required": false, + "schema": { + "type": "boolean", + "description": "Whether to allow deletion of starred images", + "default": true, + "title": "Delete Starred" + }, + "description": "Whether to allow deletion of starred images" } ], "responses": { @@ -5668,6 +5680,25 @@ "summary": "Delete Uncategorized Images", "description": "Deletes all uncategorized images owned by the current user (or all if admin)", "operationId": "delete_uncategorized_images", + "security": [ + { + "HTTPBearer": [] + } + ], + "parameters": [ + { + "name": "delete_starred", + "in": "query", + "required": false, + "schema": { + "type": "boolean", + "description": "Whether to allow deletion of starred images", + "default": true, + "title": "Delete Starred" + }, + "description": "Whether to allow deletion of starred images" + } + ], "responses": { "200": { "description": "Successful Response", @@ -5678,13 +5709,18 @@ } } } + }, + "422": { + "description": "Validation Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/HTTPValidationError" + } + } + } } - }, - "security": [ - { - "HTTPBearer": [] - } - ] + } } }, "/api/v1/images/star": { @@ -6195,6 +6231,18 @@ "title": "Video Name" }, "description": "The name of the video to delete" + }, + { + "name": "delete_starred", + "in": "query", + "required": false, + "schema": { + "type": "boolean", + "description": "Whether to allow deletion of starred videos", + "default": true, + "title": "Delete Starred" + }, + "description": "Whether to allow deletion of starred videos" } ], "responses": { @@ -6331,7 +6379,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/VideoNamesBatch" + "$ref": "#/components/schemas/DeleteVideosBatch" } } }, @@ -6372,6 +6420,25 @@ "summary": "Delete Uncategorized Videos", "description": "Deletes all uncategorized videos owned by the current user (or all if admin).\n\nMirrors ``delete_uncategorized_images`` so the \"Delete All Uncategorized\nImages/Videos\" board action covers both media kinds.", "operationId": "delete_uncategorized_videos", + "security": [ + { + "HTTPBearer": [] + } + ], + "parameters": [ + { + "name": "delete_starred", + "in": "query", + "required": false, + "schema": { + "type": "boolean", + "description": "Whether to allow deletion of starred videos", + "default": true, + "title": "Delete Starred" + }, + "description": "Whether to allow deletion of starred videos" + } + ], "responses": { "200": { "description": "Successful Response", @@ -6382,13 +6449,18 @@ } } } + }, + "422": { + "description": "Validation Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/HTTPValidationError" + } + } + } } - }, - "security": [ - { - "HTTPBearer": [] - } - ] + } } }, "/api/v1/videos/i/{video_name}/metadata": { @@ -7901,6 +7973,18 @@ "title": "Include Images" }, "description": "Permanently delete all images and videos on the board" + }, + { + "name": "delete_starred", + "in": "query", + "required": false, + "schema": { + "type": "boolean", + "description": "Whether to allow deletion of starred media", + "default": true, + "title": "Delete Starred" + }, + "description": "Whether to allow deletion of starred media" } ], "responses": { @@ -15701,6 +15785,12 @@ "type": "array", "title": "Image Names", "description": "The list of names of images to delete" + }, + "delete_starred": { + "type": "boolean", + "title": "Delete Starred", + "description": "Whether to allow deletion of starred images", + "default": true } }, "type": "object", @@ -23622,6 +23712,22 @@ "type": "array", "title": "Failed Videos", "description": "The names of videos that could not be deleted and became uncategorized." + }, + "starred_images_skipped": { + "items": { + "type": "string" + }, + "type": "array", + "title": "Starred Images Skipped", + "description": "The names of starred images that were protected and became uncategorized." + }, + "starred_videos_skipped": { + "items": { + "type": "string" + }, + "type": "array", + "title": "Starred Videos Skipped", + "description": "The names of starred videos that were protected and became uncategorized." } }, "type": "object", @@ -23666,6 +23772,14 @@ "type": "array", "title": "Failed Images", "description": "The names of authorized images that could not be deleted" + }, + "starred_skipped": { + "items": { + "type": "string" + }, + "type": "array", + "title": "Starred Skipped", + "description": "The names of starred images that were skipped because deletion protection was enabled" } }, "type": "object", @@ -23712,6 +23826,29 @@ "title": "DeleteOrphanedModelsResponse", "description": "Response from deleting orphaned models." }, + "DeleteVideosBatch": { + "properties": { + "video_names": { + "items": { + "type": "string", + "maxLength": 255 + }, + "type": "array", + "maxItems": 1000, + "title": "Video Names", + "description": "The list of video names to process" + }, + "delete_starred": { + "type": "boolean", + "title": "Delete Starred", + "description": "Whether to allow deletion of starred videos", + "default": true + } + }, + "type": "object", + "required": ["video_names"], + "title": "DeleteVideosBatch" + }, "DeleteVideosResult": { "properties": { "affected_boards": { @@ -23737,6 +23874,14 @@ "type": "array", "title": "Failed Videos", "description": "The names of videos that were not deleted" + }, + "starred_skipped": { + "items": { + "type": "string" + }, + "type": "array", + "title": "Starred Skipped", + "description": "The names of starred videos that were skipped because deletion protection was enabled" } }, "type": "object", diff --git a/invokeai/frontend/web/public/locales/en.json b/invokeai/frontend/web/public/locales/en.json index a4c7986b10c..b988aea72dd 100644 --- a/invokeai/frontend/web/public/locales/en.json +++ b/invokeai/frontend/web/public/locales/en.json @@ -138,6 +138,8 @@ "changeBoardVideo_one": "Move Video to Board", "changeBoardVideo_other": "Move {{count}} Videos to Board", "clearSearch": "Clear Search", + "containsStarredMediaConfirm": "This board contains starred media. Starred items will be kept and moved to Uncategorized; all other media and the board will be permanently deleted. Continue?", + "containsStarredMediaTitle": "This Board Contains Starred Media", "deleteBoard": "Delete Board", "deleteBoardAndImages": "Delete Board and Images", "deleteBoardAndAssets": "Delete Board, Images & Videos", @@ -1945,6 +1947,7 @@ "enableModelDescriptions": "Enable Model Descriptions in Dropdowns", "enableHighlightFocusedRegions": "Highlight Focused Regions", "middleClickOpenInNewTab": "Use Middle Click to Open Images/Videos in New Tab", + "protectStarredMedia": "Protect Starred Media", "modelDescriptionsDisabled": "Model Descriptions in Dropdowns Disabled", "modelDescriptionsDisabledDesc": "Model descriptions in dropdowns have been disabled. Enable them in Settings.", "enableInvisibleWatermark": "Enable Invisible Watermark", @@ -2055,6 +2058,9 @@ "mediaDeleteFailedDesc": "A delete request failed. Some media may not have been deleted.", "mediaDeletePartial": "{{count}} media item could not be deleted.", "mediaDeletePartial_other": "{{count}} media items could not be deleted.", + "starredMediaProtected": "Starred Media Protected", + "starredMediaProtectedDesc": "{{count}} starred media item was not deleted.", + "starredMediaProtectedDesc_other": "{{count}} starred media items were not deleted.", "importFailed": "Import Failed", "importSuccessful": "Import Successful", "invalidUpload": "Invalid Upload", diff --git a/invokeai/frontend/web/public/locales/ru.json b/invokeai/frontend/web/public/locales/ru.json index d23150a7164..57fc5f16c7c 100644 --- a/invokeai/frontend/web/public/locales/ru.json +++ b/invokeai/frontend/web/public/locales/ru.json @@ -1181,6 +1181,7 @@ "models": "Модели", "displayInProgress": "Показывать процесс генерации", "confirmOnDelete": "Подтверждать удаление", + "protectStarredMedia": "Защищать избранные медиафайлы", "resetWebUI": "Сброс настроек веб-интерфейса", "resetWebUIDesc1": "Сброс настроек веб-интерфейса удаляет только локальный кэш браузера с вашими изображениями и настройками. Он не удаляет изображения с диска.", "resetWebUIDesc2": "Если изображения не отображаются в галерее или не работает что-то еще, пожалуйста, попробуйте сбросить настройки, прежде чем сообщать о проблеме на GitHub.", @@ -1256,6 +1257,11 @@ }, "toast": { "uploadFailed": "Загрузка не удалась", + "starredMediaProtected": "Избранные медиафайлы защищены", + "starredMediaProtectedDesc_one": "{{count}} избранный медиафайл не был удалён.", + "starredMediaProtectedDesc_few": "{{count}} избранных медиафайла не были удалены.", + "starredMediaProtectedDesc_many": "{{count}} избранных медиафайлов не были удалены.", + "starredMediaProtectedDesc_other": "{{count}} избранных медиафайлов не были удалены.", "imageCopied": "Изображение скопировано", "parametersNotSet": "Параметры не заданы", "serverError": "Ошибка сервера", @@ -1603,6 +1609,8 @@ "changeBoard": "Сменить коллекцию", "loading": "Загрузка...", "clearSearch": "Очистить поиск", + "containsStarredMediaConfirm": "В коллекции есть избранные медиафайлы. Они будут сохранены и перемещены в раздел «Без категории», а остальные медиафайлы и коллекция будут удалены без возможности восстановления. Продолжить?", + "containsStarredMediaTitle": "В коллекции есть избранные медиафайлы", "deleteBoardOnly": "Удалить только коллекцию", "movingImagesToBoard_one": "Перемещение {{count}} изображения в коллекцию:", "movingImagesToBoard_few": "Перемещение {{count}} изображений в коллекцию:", diff --git a/invokeai/frontend/web/src/features/deleteImageModal/store/state.test.ts b/invokeai/frontend/web/src/features/deleteImageModal/store/state.test.ts index 385482447cc..7d97ad58032 100644 --- a/invokeai/frontend/web/src/features/deleteImageModal/store/state.test.ts +++ b/invokeai/frontend/web/src/features/deleteImageModal/store/state.test.ts @@ -17,9 +17,10 @@ vi.mock('services/api/endpoints/images', () => ({ imagesApi: { endpoints: { deleteImages: { - initiate: vi.fn((arg: { image_names: string[] }) => ({ + initiate: vi.fn((arg: { image_names: string[]; delete_starred: boolean }) => ({ type: 'imagesApi/deleteImages', image_names: arg.image_names, + delete_starred: arg.delete_starred, })), }, }, @@ -36,8 +37,11 @@ vi.mock('features/gallery/store/gallerySlice', () => ({ vi.mock('features/system/store/systemSlice', () => ({ selectSystemShouldConfirmOnDelete: vi.fn(() => false), + selectSystemShouldProtectStarredMedia: vi.fn(() => false), })); +vi.mock('features/toast/toast', () => ({ toast: vi.fn() })); + // The canvas/ref-image usage sweeps aren't under test — give them empty state. vi.mock('features/controlLayers/store/selectors', () => ({ selectCanvasSlice: vi.fn(() => ({ controlLayers: { entities: [] }, rasterLayers: { entities: [] } })), @@ -59,20 +63,33 @@ vi.mock('features/gallery/store/selectCachedGalleryItemNames', async (importOrig import type { AppStore } from 'app/store/store'; import { selectLastSelectedItem } from 'features/gallery/store/gallerySelectors'; import { selectCachedGalleryItemNames } from 'features/gallery/store/selectCachedGalleryItemNames'; +import { + selectSystemShouldConfirmOnDelete, + selectSystemShouldProtectStarredMedia, +} from 'features/system/store/systemSlice'; +import { toast } from 'features/toast/toast'; +import { imagesApi } from 'services/api/endpoints/images'; import { handleDeletions } from './state'; -const buildStore = (selection: string[], failingNames: Set) => { +const buildStore = (selection: string[], failingNames: Set, protectedNames: Set = new Set()) => { const dispatched: unknown[] = []; const dispatch = vi.fn((action: unknown) => { dispatched.push(action); - const typed = action as { type?: string; image_names?: string[] }; + const typed = action as { type?: string; image_names?: string[]; delete_starred?: boolean }; if (typed?.type === 'imagesApi/deleteImages') { return { unwrap: () => Promise.resolve({ - deleted_images: (typed.image_names ?? []).filter((name) => !failingNames.has(name)), + deleted_images: (typed.image_names ?? []).filter( + (name) => !failingNames.has(name) && (typed.delete_starred !== false || !protectedNames.has(name)) + ), + failed_images: (typed.image_names ?? []).filter((name) => failingNames.has(name)), affected_boards: [], + starred_skipped: + typed.delete_starred === false + ? (typed.image_names ?? []).filter((name) => protectedNames.has(name)) + : [], }), }; } @@ -91,6 +108,8 @@ const getSelectionChange = (dispatched: unknown[]) => describe('handleDeletions selection behavior', () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(selectSystemShouldConfirmOnDelete).mockReturnValue(false); + vi.mocked(selectSystemShouldProtectStarredMedia).mockReturnValue(false); vi.mocked(selectCachedGalleryItemNames).mockReturnValue(['a.png', 'b.png', 'c.mp4']); }); @@ -144,4 +163,20 @@ describe('handleDeletions selection behavior', () => { expect(getSelectionChange(dispatched)?.payload).toBe('a.png'); }); + + it('passes protection to the backend and keeps a protected image selected', async () => { + vi.mocked(selectSystemShouldConfirmOnDelete).mockReturnValue(true); + vi.mocked(selectSystemShouldProtectStarredMedia).mockReturnValue(true); + vi.mocked(selectLastSelectedItem).mockReturnValue('a.png'); + const { store, dispatched } = buildStore(['a.png'], new Set(), new Set(['a.png'])); + + await handleDeletions(['a.png'], store); + + expect(imagesApi.endpoints.deleteImages.initiate).toHaveBeenCalledWith( + { image_names: ['a.png'], delete_starred: false }, + { track: false } + ); + expect(getSelectionChange(dispatched)).toBeUndefined(); + expect(toast).toHaveBeenCalledWith(expect.objectContaining({ status: 'warning' })); + }); }); diff --git a/invokeai/frontend/web/src/features/deleteImageModal/store/state.ts b/invokeai/frontend/web/src/features/deleteImageModal/store/state.ts index 8169a54bafc..d69b9dfba00 100644 --- a/invokeai/frontend/web/src/features/deleteImageModal/store/state.ts +++ b/invokeai/frontend/web/src/features/deleteImageModal/store/state.ts @@ -23,7 +23,12 @@ import type { NodesState } from 'features/nodes/store/types'; import { isImageFieldCollectionInputInstance, isImageFieldInputInstance } from 'features/nodes/types/field'; import { isInvocationNode } from 'features/nodes/types/invocation'; import { selectUpscaleSlice, type UpscaleState } from 'features/parameters/store/upscaleSlice'; -import { selectSystemShouldConfirmOnDelete } from 'features/system/store/systemSlice'; +import { + selectSystemShouldConfirmOnDelete, + selectSystemShouldProtectStarredMedia, +} from 'features/system/store/systemSlice'; +import { toast } from 'features/toast/toast'; +import { t } from 'i18next'; import { atom } from 'nanostores'; import { useMemo } from 'react'; import { imagesApi } from 'services/api/endpoints/images'; @@ -90,16 +95,38 @@ export const handleDeletions = async (image_names: string[], store: AppStore) => // cache will have shifted, so the index computed afterwards would be wrong. const galleryItemNames = selectCachedGalleryItemNames(state); const lastSelected = selectLastSelectedItem(state); + const shouldConfirmOnDelete = selectSystemShouldConfirmOnDelete(state); + const shouldProtectStarredMedia = selectSystemShouldProtectStarredMedia(state); const lastSelectedIndex = lastSelected && image_names.includes(lastSelected) ? galleryItemNames.indexOf(lastSelected) : -1; const result = await dispatch( - imagesApi.endpoints.deleteImages.initiate({ image_names }, { track: false }) + imagesApi.endpoints.deleteImages.initiate( + { image_names, delete_starred: !shouldProtectStarredMedia }, + { track: false } + ) ).unwrap(); // Only the images the server confirmed deleted count: a partial failure means the // survivor still exists, so the selection must not jump away from it and it remains // a valid replacement candidate. Mirrors deleteVideoModal/store/state.ts. const deletedNames = new Set(result.deleted_images); + const failedImages = result.failed_images ?? []; + const starredSkipped = result.starred_skipped ?? []; + + if (failedImages.length > 0) { + toast({ + status: 'warning', + title: t('toast.mediaDeleteFailed'), + description: t('toast.mediaDeletePartial', { count: failedImages.length }), + }); + } + if (shouldConfirmOnDelete && starredSkipped.length > 0) { + toast({ + status: 'warning', + title: t('toast.starredMediaProtected'), + description: t('toast.starredMediaProtectedDesc', { count: starredSkipped.length }), + }); + } if (intersection(getState().gallery.selection, [...deletedNames]).length > 0) { if (lastSelected && !deletedNames.has(lastSelected)) { @@ -125,7 +152,11 @@ export const handleDeletions = async (image_names: string[], store: AppStore) => deleteRasterLayerImages(state, dispatch, image_name); } } catch { - // no-op + toast({ + status: 'error', + title: t('toast.mediaDeleteFailed'), + description: t('toast.mediaDeleteFailedDesc'), + }); } }; diff --git a/invokeai/frontend/web/src/features/deleteVideoModal/store/state.test.ts b/invokeai/frontend/web/src/features/deleteVideoModal/store/state.test.ts index 82fdfeb3ff2..1d413a9d8df 100644 --- a/invokeai/frontend/web/src/features/deleteVideoModal/store/state.test.ts +++ b/invokeai/frontend/web/src/features/deleteVideoModal/store/state.test.ts @@ -21,9 +21,10 @@ vi.mock('services/api/endpoints/videos', () => ({ videosApi: { endpoints: { deleteVideos: { - initiate: vi.fn((arg: { video_names: string[] }) => ({ + initiate: vi.fn((arg: { video_names: string[]; delete_starred: boolean }) => ({ type: 'videosApi/deleteVideos', video_names: arg.video_names, + delete_starred: arg.delete_starred, })), }, }, @@ -44,6 +45,7 @@ vi.mock('features/nodes/store/nodesSlice', () => ({ vi.mock('features/system/store/systemSlice', () => ({ selectSystemShouldConfirmOnDelete: vi.fn(() => false), + selectSystemShouldProtectStarredMedia: vi.fn(() => false), })); vi.mock('features/toast/toast', () => ({ toast: vi.fn() })); @@ -59,7 +61,10 @@ import type { AppStore } from 'app/store/store'; import { selectLastSelectedItem } from 'features/gallery/store/gallerySelectors'; import { imageSelected } from 'features/gallery/store/gallerySlice'; import { selectCachedGalleryItemNames } from 'features/gallery/store/selectCachedGalleryItemNames'; -import { selectSystemShouldConfirmOnDelete } from 'features/system/store/systemSlice'; +import { + selectSystemShouldConfirmOnDelete, + selectSystemShouldProtectStarredMedia, +} from 'features/system/store/systemSlice'; import { toast } from 'features/toast/toast'; import { videosApi } from 'services/api/endpoints/videos'; @@ -75,20 +80,32 @@ const buildVideoFieldNode = (nodeId: string, videoName: string) => ({ }, }); -const buildStore = (selection: string[], failingNames: Set, nodes: unknown[] = [], rejectAll = false) => { +const buildStore = ( + selection: string[], + failingNames: Set, + nodes: unknown[] = [], + rejectAll = false, + protectedNames: Set = new Set() +) => { const dispatched: unknown[] = []; const dispatch = vi.fn((action: unknown) => { dispatched.push(action); - const typed = action as { type?: string; video_names?: string[] }; + const typed = action as { type?: string; video_names?: string[]; delete_starred?: boolean }; if (typed?.type === 'videosApi/deleteVideos') { return { unwrap: () => rejectAll ? Promise.reject(new Error('delete failed')) : Promise.resolve({ - deleted_videos: (typed.video_names ?? []).filter((name) => !failingNames.has(name)), + deleted_videos: (typed.video_names ?? []).filter( + (name) => !failingNames.has(name) && (typed.delete_starred !== false || !protectedNames.has(name)) + ), failed_videos: (typed.video_names ?? []).filter((name) => failingNames.has(name)), affected_boards: ['none'], + starred_skipped: + typed.delete_starred === false + ? (typed.video_names ?? []).filter((name) => protectedNames.has(name)) + : [], }), }; } @@ -110,6 +127,11 @@ const getVideoFieldChanges = (dispatched: unknown[]) => !!action && typeof action === 'object' && (action as { type?: string }).type === 'nodes/fieldVideoValueChanged' ); +beforeEach(() => { + vi.mocked(selectSystemShouldConfirmOnDelete).mockReturnValue(false); + vi.mocked(selectSystemShouldProtectStarredMedia).mockReturnValue(false); +}); + describe('handleDeletions batching', () => { beforeEach(() => { vi.clearAllMocks(); @@ -124,11 +146,25 @@ describe('handleDeletions batching', () => { expect(videosApi.endpoints.deleteVideos.initiate).toHaveBeenCalledTimes(1); expect(videosApi.endpoints.deleteVideos.initiate).toHaveBeenCalledWith( - { video_names: ['a.mp4', 'b.mp4'] }, + { video_names: ['a.mp4', 'b.mp4'], delete_starred: true }, { track: false } ); expect(toast).not.toHaveBeenCalled(); }); + + it('passes protection to the backend and reports skipped starred videos', async () => { + vi.mocked(selectSystemShouldConfirmOnDelete).mockReturnValue(true); + vi.mocked(selectSystemShouldProtectStarredMedia).mockReturnValue(true); + const { store } = buildStore([], new Set(), [], false, new Set(['a.mp4'])); + + await handleDeletions(['a.mp4'], store); + + expect(videosApi.endpoints.deleteVideos.initiate).toHaveBeenCalledWith( + { video_names: ['a.mp4'], delete_starred: false }, + { track: false } + ); + expect(toast).toHaveBeenCalledWith(expect.objectContaining({ status: 'warning' })); + }); }); describe('handleDeletions selection behavior on partial failure', () => { diff --git a/invokeai/frontend/web/src/features/deleteVideoModal/store/state.ts b/invokeai/frontend/web/src/features/deleteVideoModal/store/state.ts index 9821267088e..e324334fcea 100644 --- a/invokeai/frontend/web/src/features/deleteVideoModal/store/state.ts +++ b/invokeai/frontend/web/src/features/deleteVideoModal/store/state.ts @@ -11,7 +11,10 @@ import { import { fieldVideoValueChanged } from 'features/nodes/store/nodesSlice'; import { isVideoFieldInputInstance } from 'features/nodes/types/field'; import { isInvocationNode } from 'features/nodes/types/invocation'; -import { selectSystemShouldConfirmOnDelete } from 'features/system/store/systemSlice'; +import { + selectSystemShouldConfirmOnDelete, + selectSystemShouldProtectStarredMedia, +} from 'features/system/store/systemSlice'; import { toast } from 'features/toast/toast'; import { t } from 'i18next'; import { atom } from 'nanostores'; @@ -81,6 +84,8 @@ export const handleDeletions = async (video_names: string[], store: AppStore) => // Snapshot the polymorphic gallery list and the currently-displayed item *before* the // delete fires; once the network call resolves the cache will already have shifted. const stateBefore = getState(); + const shouldConfirmOnDelete = selectSystemShouldConfirmOnDelete(stateBefore); + const shouldProtectStarredMedia = selectSystemShouldProtectStarredMedia(stateBefore); const galleryItemNames = selectCachedGalleryItemNames(stateBefore); const lastSelected = selectLastSelectedItem(stateBefore); const lastSelectedIndex = @@ -91,9 +96,13 @@ export const handleDeletions = async (video_names: string[], store: AppStore) => let deletedNames = new Set(); try { const result = await dispatch( - videosApi.endpoints.deleteVideos.initiate({ video_names }, { track: false }) + videosApi.endpoints.deleteVideos.initiate( + { video_names, delete_starred: !shouldProtectStarredMedia }, + { track: false } + ) ).unwrap(); deletedNames = new Set(result.deleted_videos); + const starredSkipped = result.starred_skipped ?? []; if (result.failed_videos.length > 0) { toast({ status: 'warning', @@ -101,6 +110,13 @@ export const handleDeletions = async (video_names: string[], store: AppStore) => description: t('toast.videoDeletePartial', { count: result.failed_videos.length }), }); } + if (shouldConfirmOnDelete && starredSkipped.length > 0) { + toast({ + status: 'warning', + title: t('toast.starredMediaProtected'), + description: t('toast.starredMediaProtectedDesc', { count: starredSkipped.length }), + }); + } } catch { // The whole request failed — nothing was confirmed deleted, so leave selection and // node references untouched. The mutation is untracked, so this toast is the only diff --git a/invokeai/frontend/web/src/features/gallery/components/Boards/DeleteBoardModal.tsx b/invokeai/frontend/web/src/features/gallery/components/Boards/DeleteBoardModal.tsx index fd1480defd0..56ec50bc87d 100644 --- a/invokeai/frontend/web/src/features/gallery/components/Boards/DeleteBoardModal.tsx +++ b/invokeai/frontend/web/src/features/gallery/components/Boards/DeleteBoardModal.tsx @@ -23,11 +23,15 @@ import { getImageUsage } from 'features/deleteImageModal/store/state'; import type { ImageUsage } from 'features/deleteImageModal/store/types'; import { selectNodesSlice } from 'features/nodes/store/selectors'; import { selectUpscaleSlice } from 'features/parameters/store/upscaleSlice'; +import { + selectSystemShouldConfirmOnDelete, + selectSystemShouldProtectStarredMedia, +} from 'features/system/store/systemSlice'; import { toast } from 'features/toast/toast'; import { atom } from 'nanostores'; -import { memo, useCallback, useMemo, useRef } from 'react'; +import { memo, useCallback, useMemo, useRef, useState } from 'react'; import { useTranslation } from 'react-i18next'; -import { useListAllImageNamesForBoardQuery } from 'services/api/endpoints/boards'; +import { useGetGalleryItemNamesQuery } from 'services/api/endpoints/gallery'; import { useDeleteBoardAndImagesMutation, useDeleteBoardMutation, @@ -44,25 +48,31 @@ const DeleteBoardModal = () => { useAssertSingleton('DeleteBoardModal'); const boardToDelete = useStore($boardToDelete); const { t } = useTranslation(); + const shouldConfirmOnDelete = useAppSelector(selectSystemShouldConfirmOnDelete); + const shouldProtectStarredMedia = useAppSelector(selectSystemShouldProtectStarredMedia); const boardId = useMemo(() => (boardToDelete === 'none' ? 'none' : boardToDelete?.board_id), [boardToDelete]); - - const { currentData: boardImageNames, isFetching: isFetchingBoardNames } = useListAllImageNamesForBoardQuery( + const { currentData: boardMedia, isFetching: isFetchingBoardNames } = useGetGalleryItemNamesQuery( boardId ? { board_id: boardId, categories: undefined, is_intermediate: undefined, + starred_first: true, } : skipToken ); + const boardImageNames = useMemo( + () => boardMedia?.items.filter((item) => item.kind === 'image').map((item) => item.name) ?? [], + [boardMedia?.items] + ); const selectImageUsageSummary = useMemo( () => createMemoizedSelector( [selectNodesSlice, selectCanvasSlice, selectUpscaleSlice, selectRefImagesSlice], (nodes, canvas, upscale, refImages) => { - const allImageUsage = (boardImageNames ?? []).map((imageName) => + const allImageUsage = boardImageNames.map((imageName) => getImageUsage(nodes, canvas, upscale, refImages, imageName) ); @@ -83,71 +93,136 @@ const DeleteBoardModal = () => { ); const [deleteBoardOnly, { isLoading: isDeleteBoardOnlyLoading }] = useDeleteBoardMutation(); - const [deleteBoardAndImages, { isLoading: isDeleteBoardAndImagesLoading }] = useDeleteBoardAndImagesMutation(); - const [deleteUncategorizedImages, { isLoading: isDeleteUncategorizedImagesLoading }] = useDeleteUncategorizedImagesMutation(); - const [deleteUncategorizedVideos, { isLoading: isDeleteUncategorizedVideosLoading }] = useDeleteUncategorizedVideosMutation(); const imageUsageSummary = useAppSelector(selectImageUsageSummary); + const [starredConfirmationBoardId, setStarredConfirmationBoardId] = useState(null); + const isStarredConfirmationOpen = + boardToDelete !== null && boardToDelete !== 'none' && starredConfirmationBoardId === boardToDelete.board_id; - const handleDeleteBoardOnly = useCallback(() => { - if (!boardToDelete || boardToDelete === 'none') { - return; - } - deleteBoardOnly({ board_id: boardToDelete.board_id }); + const handleClose = useCallback(() => { + setStarredConfirmationBoardId(null); $boardToDelete.set(null); - }, [boardToDelete, deleteBoardOnly]); + }, []); - const handleDeleteBoardAndImages = useCallback(async () => { + const reportDeletionSummary = useCallback( + (summary: ReturnType, showProtectedWarning: boolean) => { + if (summary.requestFailed) { + toast({ + status: 'error', + title: t('toast.mediaDeleteFailed'), + description: t('toast.mediaDeleteFailedDesc'), + }); + return; + } + if (summary.failedCount > 0) { + toast({ + status: 'warning', + title: t('toast.mediaDeleteFailed'), + description: t('toast.mediaDeletePartial', { count: summary.failedCount }), + }); + } + if (showProtectedWarning && summary.protectedCount > 0) { + toast({ + status: 'warning', + title: t('toast.starredMediaProtected'), + description: t('toast.starredMediaProtectedDesc', { count: summary.protectedCount }), + }); + } + }, + [t] + ); + + const handleDeleteBoardOnly = useCallback(async () => { if (!boardToDelete || boardToDelete === 'none') { return; } - const result = await Promise.allSettled([deleteBoardAndImages({ board_id: boardToDelete.board_id }).unwrap()]); - const summary = getMediaDeletionSummary(result); - if (summary.requestFailed || summary.failedCount > 0) { + try { + await deleteBoardOnly({ board_id: boardToDelete.board_id }).unwrap(); + handleClose(); + } catch { toast({ - status: summary.requestFailed ? 'error' : 'warning', + status: 'error', title: t('toast.mediaDeleteFailed'), - description: summary.requestFailed - ? t('toast.mediaDeleteFailedDesc') - : t('toast.mediaDeletePartial', { count: summary.failedCount }), + description: t('toast.mediaDeleteFailedDesc'), }); } - $boardToDelete.set(null); - }, [boardToDelete, deleteBoardAndImages, t]); + }, [boardToDelete, deleteBoardOnly, handleClose, t]); - const handleDeleteUncategorizedMedia = useCallback(async () => { - if (!boardToDelete || boardToDelete !== 'none') { + const deleteBoardWithMedia = useCallback(async () => { + if (!boardToDelete || boardToDelete === 'none') { return; } - // The uncategorized bucket is polymorphic (the button says "Images/Videos"), so both - // media kinds are deleted. The mutations are independent — a failure in one doesn't - // block the other. const results = await Promise.allSettled([ - deleteUncategorizedImages().unwrap(), - deleteUncategorizedVideos().unwrap(), + deleteBoardAndImages({ + board_id: boardToDelete.board_id, + delete_starred: !shouldProtectStarredMedia, + }).unwrap(), ]); const summary = getMediaDeletionSummary(results); - if (summary.requestFailed || summary.failedCount > 0) { - toast({ - status: summary.requestFailed ? 'error' : 'warning', - title: t('toast.mediaDeleteFailed'), - description: summary.requestFailed - ? t('toast.mediaDeleteFailedDesc') - : t('toast.mediaDeletePartial', { count: summary.failedCount }), - }); + reportDeletionSummary(summary, shouldConfirmOnDelete); + if (!summary.requestFailed) { + handleClose(); } - $boardToDelete.set(null); - }, [boardToDelete, deleteUncategorizedImages, deleteUncategorizedVideos, t]); + }, [ + boardToDelete, + deleteBoardAndImages, + handleClose, + reportDeletionSummary, + shouldConfirmOnDelete, + shouldProtectStarredMedia, + ]); - const handleClose = useCallback(() => { - $boardToDelete.set(null); + const handleDeleteBoardAndMedia = useCallback(() => { + if (!boardToDelete || boardToDelete === 'none') { + return; + } + if (shouldProtectStarredMedia && (boardMedia?.starred_count ?? 0) > 0) { + setStarredConfirmationBoardId(boardToDelete.board_id); + return; + } + void deleteBoardWithMedia(); + }, [boardMedia?.starred_count, boardToDelete, deleteBoardWithMedia, shouldProtectStarredMedia]); + + const handleConfirmStarredDelete = useCallback(() => { + if (!boardToDelete || boardToDelete === 'none' || starredConfirmationBoardId !== boardToDelete.board_id) { + return; + } + void deleteBoardWithMedia(); + }, [boardToDelete, deleteBoardWithMedia, starredConfirmationBoardId]); + + const handleCancelStarredDelete = useCallback(() => { + setStarredConfirmationBoardId(null); }, []); + const handleDeleteUncategorizedMedia = useCallback(async () => { + if (!boardToDelete || boardToDelete !== 'none') { + return; + } + const params = shouldProtectStarredMedia ? { delete_starred: false } : undefined; + const results = await Promise.allSettled([ + deleteUncategorizedImages(params).unwrap(), + deleteUncategorizedVideos(params).unwrap(), + ]); + const summary = getMediaDeletionSummary(results); + reportDeletionSummary(summary, shouldConfirmOnDelete); + if (!summary.requestFailed) { + handleClose(); + } + }, [ + boardToDelete, + deleteUncategorizedImages, + deleteUncategorizedVideos, + handleClose, + reportDeletionSummary, + shouldConfirmOnDelete, + shouldProtectStarredMedia, + ]); + const cancelRef = useRef(null); const isLoading = useMemo( @@ -171,58 +246,88 @@ const DeleteBoardModal = () => { } return ( - - - - - {t('common.delete')} {boardToDelete === 'none' ? t('boards.uncategorizedImages') : boardToDelete.board_name} - - - - - {isFetchingBoardNames ? ( - - - - ) : ( - - )} - {boardToDelete !== 'none' ? ( - {t('boards.deletedBoardsCannotbeRestored')} - ) : ( - {t('gallery.deleteMediaPermanent')} - )} - - - - - - {boardToDelete !== 'none' && ( - - )} - {boardToDelete !== 'none' && ( - + )} + {boardToDelete !== 'none' && ( + + )} + {boardToDelete === 'none' && ( + + )} + + + + + + + + + + {t('boards.containsStarredMediaTitle')} + + + {t('boards.containsStarredMediaConfirm')} + + + + - )} - {boardToDelete === 'none' && ( - - )} - - - - - + + + + + + ); }; diff --git a/invokeai/frontend/web/src/features/gallery/components/Boards/getMediaDeletionSummary.test.ts b/invokeai/frontend/web/src/features/gallery/components/Boards/getMediaDeletionSummary.test.ts index 4714b263c66..c38f0290c32 100644 --- a/invokeai/frontend/web/src/features/gallery/components/Boards/getMediaDeletionSummary.test.ts +++ b/invokeai/frontend/web/src/features/gallery/components/Boards/getMediaDeletionSummary.test.ts @@ -5,7 +5,7 @@ import { getMediaDeletionSummary } from './getMediaDeletionSummary'; describe('getMediaDeletionSummary', () => { it('reports a complete deletion as successful', () => { expect(getMediaDeletionSummary([{ status: 'fulfilled', value: { failed_images: [], failed_videos: [] } }])).toEqual( - { failedCount: 0, requestFailed: false } + { failedCount: 0, protectedCount: 0, requestFailed: false } ); }); @@ -14,7 +14,19 @@ describe('getMediaDeletionSummary', () => { getMediaDeletionSummary([ { status: 'fulfilled', value: { failed_images: ['image.png'], failed_videos: ['video.mp4'] } }, ]) - ).toEqual({ failedCount: 2, requestFailed: false }); + ).toEqual({ failedCount: 2, protectedCount: 0, requestFailed: false }); + }); + + it('counts protected resource and board media separately', () => { + expect( + getMediaDeletionSummary([ + { status: 'fulfilled', value: { starred_skipped: ['image.png'] } }, + { + status: 'fulfilled', + value: { starred_images_skipped: ['board-image.png'], starred_videos_skipped: ['board-video.mp4'] }, + }, + ]) + ).toEqual({ failedCount: 0, protectedCount: 3, requestFailed: false }); }); it('reports a rejected image deletion request', () => { @@ -23,6 +35,6 @@ describe('getMediaDeletionSummary', () => { { status: 'rejected', reason: new Error('image delete failed') }, { status: 'fulfilled', value: { failed_videos: [] } }, ]) - ).toEqual({ failedCount: 0, requestFailed: true }); + ).toEqual({ failedCount: 0, protectedCount: 0, requestFailed: true }); }); }); diff --git a/invokeai/frontend/web/src/features/gallery/components/Boards/getMediaDeletionSummary.ts b/invokeai/frontend/web/src/features/gallery/components/Boards/getMediaDeletionSummary.ts index ef02fc28922..8c9b177e793 100644 --- a/invokeai/frontend/web/src/features/gallery/components/Boards/getMediaDeletionSummary.ts +++ b/invokeai/frontend/web/src/features/gallery/components/Boards/getMediaDeletionSummary.ts @@ -1,6 +1,9 @@ type MediaDeletionResult = { failed_images?: readonly string[]; failed_videos?: readonly string[]; + starred_skipped?: readonly string[]; + starred_images_skipped?: readonly string[]; + starred_videos_skipped?: readonly string[]; }; export const getMediaDeletionSummary = (results: PromiseSettledResult[]) => ({ @@ -11,5 +14,15 @@ export const getMediaDeletionSummary = (results: PromiseSettledResult + result.status === 'fulfilled' + ? count + + (result.value.starred_skipped?.length ?? 0) + + (result.value.starred_images_skipped?.length ?? 0) + + (result.value.starred_videos_skipped?.length ?? 0) + : count, + 0 + ), requestFailed: results.some((result) => result.status === 'rejected'), }); diff --git a/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsModal.tsx b/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsModal.tsx index 0b5602febcb..0b995df52da 100644 --- a/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsModal.tsx +++ b/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsModal.tsx @@ -41,6 +41,7 @@ import { selectSystemShouldEnableHighlightFocusedRegions, selectSystemShouldEnableInformationalPopovers, selectSystemShouldEnableModelDescriptions, + selectSystemShouldProtectStarredMedia, selectSystemShouldShowInvocationProgressDetail, selectSystemShouldUseMiddleClickToOpenInNewTab, selectSystemShouldUseNSFWChecker, @@ -50,6 +51,7 @@ import { setShouldEnableInformationalPopovers, setShouldEnableModelDescriptions, setShouldHighlightFocusedRegions, + setShouldProtectStarredMedia, setShouldShowInvocationProgressDetail, setShouldUseMiddleClickToOpenInNewTab, shouldAntialiasProgressImageChanged, @@ -99,6 +101,7 @@ const SettingsModal = (props: { children: ReactElement<{ onClick?: () => void }> const prefersNumericAttentionWeights = useAppSelector(selectSystemPrefersNumericAttentionWeights); const shouldUseCpuNoise = useAppSelector(selectShouldUseCPUNoise); const shouldConfirmOnDelete = useAppSelector(selectSystemShouldConfirmOnDelete); + const shouldProtectStarredMedia = useAppSelector(selectSystemShouldProtectStarredMedia); const shouldShowProgressInViewer = useAppSelector(selectShouldShowProgressInViewer); const shouldAntialiasProgressImage = useAppSelector(selectSystemShouldAntialiasProgressImage); const shouldUseNSFWChecker = useAppSelector(selectSystemShouldUseNSFWChecker); @@ -187,6 +190,12 @@ const SettingsModal = (props: { children: ReactElement<{ onClick?: () => void }> }, [dispatch] ); + const handleChangeShouldProtectStarredMedia = useCallback( + (e: ChangeEvent) => { + dispatch(setShouldProtectStarredMedia(e.target.checked)); + }, + [dispatch] + ); const handleChangeShouldUseNSFWChecker = useCallback( (e: ChangeEvent) => { dispatch(shouldUseNSFWCheckerChanged(e.target.checked)); @@ -298,6 +307,10 @@ const SettingsModal = (props: { children: ReactElement<{ onClick?: () => void }> {t('settings.confirmOnDelete')} + + {t('settings.protectStarredMedia')} + + {t('settings.confirmOnNewSession')} diff --git a/invokeai/frontend/web/src/features/system/store/systemSlice.test.ts b/invokeai/frontend/web/src/features/system/store/systemSlice.test.ts new file mode 100644 index 00000000000..68124fb05eb --- /dev/null +++ b/invokeai/frontend/web/src/features/system/store/systemSlice.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from 'vitest'; + +import { systemSliceConfig } from './systemSlice'; + +describe('systemSliceConfig persisted state migration', () => { + const migrate = systemSliceConfig.persistConfig?.migrate; + + it('adds starred media protection disabled when migrating the current main state', () => { + expect(migrate).toBeDefined(); + const state: Record = { + ...systemSliceConfig.getInitialState(), + _version: 3, + }; + delete state.shouldProtectStarredMedia; + + const result = migrate?.(state); + + expect(result?._version).toBe(4); + expect(result?.shouldProtectStarredMedia).toBe(false); + }); + + it('preserves the branch-only starred image preference under the media setting', () => { + expect(migrate).toBeDefined(); + const state: Record = { + ...systemSliceConfig.getInitialState(), + _version: 4, + shouldProtectStarredImages: true, + }; + delete state.shouldProtectStarredMedia; + + const result = migrate?.(state); + + expect(result?._version).toBe(4); + expect(result?.shouldProtectStarredMedia).toBe(true); + }); +}); diff --git a/invokeai/frontend/web/src/features/system/store/systemSlice.ts b/invokeai/frontend/web/src/features/system/store/systemSlice.ts index f1bc126d877..e0eaa16d832 100644 --- a/invokeai/frontend/web/src/features/system/store/systemSlice.ts +++ b/invokeai/frontend/web/src/features/system/store/systemSlice.ts @@ -12,8 +12,9 @@ import { assert } from 'tsafe'; import { type Language, type SystemState, zSystemState } from './types'; const getInitialState = (): SystemState => ({ - _version: 3, + _version: 4, shouldConfirmOnDelete: true, + shouldProtectStarredMedia: false, shouldAntialiasProgressImage: false, shouldConfirmOnNewSession: true, language: 'en', @@ -37,6 +38,9 @@ const slice = createSlice({ setShouldConfirmOnDelete: (state, action: PayloadAction) => { state.shouldConfirmOnDelete = action.payload; }, + setShouldProtectStarredMedia: (state, action: PayloadAction) => { + state.shouldProtectStarredMedia = action.payload; + }, logIsEnabledChanged: (state, action: PayloadAction) => { state.logIsEnabled = action.payload; }, @@ -88,6 +92,7 @@ const slice = createSlice({ export const { setShouldConfirmOnDelete, + setShouldProtectStarredMedia, logIsEnabledChanged, logLevelChanged, logNamespaceToggled, @@ -122,6 +127,14 @@ export const systemSliceConfig: SliceConfig = { state.shouldUseMiddleClickToOpenInNewTab = false; state._version = 3; } + if (state._version === 3) { + state.shouldProtectStarredMedia = false; + state._version = 4; + } + if (state._version === 4 && !('shouldProtectStarredMedia' in state)) { + const legacyValue = (state as Record).shouldProtectStarredImages; + state.shouldProtectStarredMedia = typeof legacyValue === 'boolean' ? legacyValue : false; + } return zSystemState.parse(state); }, }, @@ -136,6 +149,7 @@ export const selectSystemLogNamespaces = createSystemSelector((system) => ); export const selectSystemLogIsEnabled = createSystemSelector((system) => system.logIsEnabled); export const selectSystemShouldConfirmOnDelete = createSystemSelector((system) => system.shouldConfirmOnDelete); +export const selectSystemShouldProtectStarredMedia = createSystemSelector((system) => system.shouldProtectStarredMedia); export const selectSystemShouldUseNSFWChecker = createSystemSelector((system) => system.shouldUseNSFWChecker); export const selectSystemShouldUseWatermarker = createSystemSelector((system) => system.shouldUseWatermarker); export const selectSystemShouldAntialiasProgressImage = createSystemSelector( diff --git a/invokeai/frontend/web/src/features/system/store/types.ts b/invokeai/frontend/web/src/features/system/store/types.ts index 106cb5d7094..ce6f855d86e 100644 --- a/invokeai/frontend/web/src/features/system/store/types.ts +++ b/invokeai/frontend/web/src/features/system/store/types.ts @@ -30,8 +30,9 @@ export type Language = z.infer; export const isLanguage = (v: unknown): v is Language => zLanguage.safeParse(v).success; export const zSystemState = z.object({ - _version: z.literal(3), + _version: z.literal(4), shouldConfirmOnDelete: z.boolean(), + shouldProtectStarredMedia: z.boolean(), shouldAntialiasProgressImage: z.boolean(), shouldConfirmOnNewSession: z.boolean(), language: zLanguage, diff --git a/invokeai/frontend/web/src/services/api/endpoints/boards.ts b/invokeai/frontend/web/src/services/api/endpoints/boards.ts index 70d1cfda002..a83bccf7bc9 100644 --- a/invokeai/frontend/web/src/services/api/endpoints/boards.ts +++ b/invokeai/frontend/web/src/services/api/endpoints/boards.ts @@ -152,5 +152,4 @@ export const { useGetBoardVideosTotalQuery, useCreateBoardMutation, useUpdateBoardMutation, - useListAllImageNamesForBoardQuery, } = boardsApi; diff --git a/invokeai/frontend/web/src/services/api/endpoints/images.ts b/invokeai/frontend/web/src/services/api/endpoints/images.ts index cd4ec8f39b3..7c9fa603e93 100644 --- a/invokeai/frontend/web/src/services/api/endpoints/images.ts +++ b/invokeai/frontend/web/src/services/api/endpoints/images.ts @@ -107,11 +107,13 @@ export const imagesApi = api.injectEndpoints({ }), deleteImage: build.mutation< paths['/api/v1/images/i/{image_name}']['delete']['responses']['200']['content']['application/json'], - paths['/api/v1/images/i/{image_name}']['delete']['parameters']['path'] + paths['/api/v1/images/i/{image_name}']['delete']['parameters']['path'] & + NonNullable >({ - query: ({ image_name }) => ({ + query: ({ image_name, delete_starred }) => ({ url: buildImagesUrl(`i/${image_name}`), method: 'DELETE', + params: { delete_starred }, }), invalidatesTags: (result) => { if (!result) { @@ -152,9 +154,9 @@ export const imagesApi = api.injectEndpoints({ }), deleteUncategorizedImages: build.mutation< paths['/api/v1/images/uncategorized']['delete']['responses']['200']['content']['application/json'], - void + NonNullable | void >({ - query: () => ({ url: buildImagesUrl('uncategorized'), method: 'DELETE' }), + query: (params) => ({ url: buildImagesUrl('uncategorized'), method: 'DELETE', params: params ?? undefined }), invalidatesTags: (result) => { if (!result) { return []; @@ -292,7 +294,8 @@ export const imagesApi = api.injectEndpoints({ }), deleteBoard: build.mutation< paths['/api/v1/boards/{board_id}']['delete']['responses']['200']['content']['application/json'], - paths['/api/v1/boards/{board_id}']['delete']['parameters']['path'] + paths['/api/v1/boards/{board_id}']['delete']['parameters']['path'] & + NonNullable >({ query: ({ board_id }) => ({ url: buildBoardsUrl(board_id), method: 'DELETE' }), invalidatesTags: (result) => [ @@ -327,12 +330,13 @@ export const imagesApi = api.injectEndpoints({ deleteBoardAndImages: build.mutation< paths['/api/v1/boards/{board_id}']['delete']['responses']['200']['content']['application/json'], - paths['/api/v1/boards/{board_id}']['delete']['parameters']['path'] + paths['/api/v1/boards/{board_id}']['delete']['parameters']['path'] & + NonNullable >({ - query: ({ board_id }) => ({ + query: ({ board_id, delete_starred }) => ({ url: buildBoardsUrl(board_id), method: 'DELETE', - params: { include_images: true }, + params: { include_images: true, delete_starred }, }), // The backend now also cascade-deletes videos on the board, so the unified gallery // and the video list both need invalidation in addition to the board tag. @@ -351,6 +355,8 @@ export const imagesApi = api.injectEndpoints({ ...getTagsToInvalidateForVideoMutation(result?.deleted_videos ?? []), ...getTagsToInvalidateForImageMutation(result?.failed_images ?? []), ...getTagsToInvalidateForVideoMutation(result?.failed_videos ?? []), + ...getTagsToInvalidateForImageMutation(result?.starred_images_skipped ?? []), + ...getTagsToInvalidateForVideoMutation(result?.starred_videos_skipped ?? []), ], }), addImageToBoard: build.mutation< diff --git a/invokeai/frontend/web/src/services/api/endpoints/videos.ts b/invokeai/frontend/web/src/services/api/endpoints/videos.ts index 49043d763a0..efb192016f1 100644 --- a/invokeai/frontend/web/src/services/api/endpoints/videos.ts +++ b/invokeai/frontend/web/src/services/api/endpoints/videos.ts @@ -102,11 +102,13 @@ export const videosApi = api.injectEndpoints({ deleteVideo: build.mutation< paths['/api/v1/videos/i/{video_name}']['delete']['responses']['200']['content']['application/json'], - paths['/api/v1/videos/i/{video_name}']['delete']['parameters']['path'] + paths['/api/v1/videos/i/{video_name}']['delete']['parameters']['path'] & + NonNullable >({ - query: ({ video_name }) => ({ + query: ({ video_name, delete_starred }) => ({ url: buildVideosUrl(`i/${video_name}`), method: 'DELETE', + params: { delete_starred }, }), invalidatesTags: (result) => { if (!result) { @@ -149,11 +151,12 @@ export const videosApi = api.injectEndpoints({ * board action fires both so the polymorphic uncategorized bucket is fully cleared. */ deleteUncategorizedVideos: build.mutation< paths['/api/v1/videos/uncategorized']['delete']['responses']['200']['content']['application/json'], - void + NonNullable | void >({ - query: () => ({ + query: (params) => ({ url: buildVideosUrl('uncategorized'), method: 'DELETE', + params: params ?? undefined, }), invalidatesTags: (result) => { if (!result) { diff --git a/invokeai/frontend/web/src/services/api/schema.ts b/invokeai/frontend/web/src/services/api/schema.ts index 596472a0d43..ba958db4780 100644 --- a/invokeai/frontend/web/src/services/api/schema.ts +++ b/invokeai/frontend/web/src/services/api/schema.ts @@ -4826,6 +4826,12 @@ export type components = { * @description The list of names of images to delete */ image_names: string[]; + /** + * Delete Starred + * @description Whether to allow deletion of starred images + * @default true + */ + delete_starred?: boolean; }; /** Body_do_hf_login */ Body_do_hf_login: { @@ -9028,6 +9034,16 @@ export type components = { * @description The names of videos that could not be deleted and became uncategorized. */ failed_videos?: string[]; + /** + * Starred Images Skipped + * @description The names of starred images that were protected and became uncategorized. + */ + starred_images_skipped?: string[]; + /** + * Starred Videos Skipped + * @description The names of starred videos that were protected and became uncategorized. + */ + starred_videos_skipped?: string[]; }; /** * DeleteByDestinationResult @@ -9057,6 +9073,11 @@ export type components = { * @description The names of authorized images that could not be deleted */ failed_images?: string[]; + /** + * Starred Skipped + * @description The names of starred images that were skipped because deletion protection was enabled + */ + starred_skipped?: string[]; }; /** * DeleteOrphanedModelsRequest @@ -9087,6 +9108,20 @@ export type components = { [key: string]: string; }; }; + /** DeleteVideosBatch */ + DeleteVideosBatch: { + /** + * Video Names + * @description The list of video names to process + */ + video_names: string[]; + /** + * Delete Starred + * @description Whether to allow deletion of starred videos + * @default true + */ + delete_starred?: boolean; + }; /** DeleteVideosResult */ DeleteVideosResult: { /** @@ -9104,6 +9139,11 @@ export type components = { * @description The names of videos that were not deleted */ failed_videos: string[]; + /** + * Starred Skipped + * @description The names of starred videos that were skipped because deletion protection was enabled + */ + starred_skipped?: string[]; }; /** * Denoise - SD1.5, SDXL @@ -42994,7 +43034,10 @@ export interface operations { }; delete_image: { parameters: { - query?: never; + query?: { + /** @description Whether to allow deletion of starred images */ + delete_starred?: boolean; + }; header?: never; path: { /** @description The name of the image to delete */ @@ -43354,7 +43397,10 @@ export interface operations { }; delete_uncategorized_images: { parameters: { - query?: never; + query?: { + /** @description Whether to allow deletion of starred images */ + delete_starred?: boolean; + }; header?: never; path?: never; cookie?: never; @@ -43370,6 +43416,15 @@ export interface operations { "application/json": components["schemas"]["DeleteImagesResult"]; }; }; + /** @description Validation Error */ + 422: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["HTTPValidationError"]; + }; + }; }; }; star_images_in_list: { @@ -43670,7 +43725,10 @@ export interface operations { }; delete_video: { parameters: { - query?: never; + query?: { + /** @description Whether to allow deletion of starred videos */ + delete_starred?: boolean; + }; header?: never; path: { /** @description The name of the video to delete */ @@ -43745,7 +43803,7 @@ export interface operations { }; requestBody: { content: { - "application/json": components["schemas"]["VideoNamesBatch"]; + "application/json": components["schemas"]["DeleteVideosBatch"]; }; }; responses: { @@ -43771,7 +43829,10 @@ export interface operations { }; delete_uncategorized_videos: { parameters: { - query?: never; + query?: { + /** @description Whether to allow deletion of starred videos */ + delete_starred?: boolean; + }; header?: never; path?: never; cookie?: never; @@ -43787,6 +43848,15 @@ export interface operations { "application/json": components["schemas"]["DeleteVideosResult"]; }; }; + /** @description Validation Error */ + 422: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["HTTPValidationError"]; + }; + }; }; }; get_video_metadata: { @@ -44444,6 +44514,8 @@ export interface operations { query?: { /** @description Permanently delete all images and videos on the board */ include_images?: boolean | null; + /** @description Whether to allow deletion of starred media */ + delete_starred?: boolean; }; header?: never; path: { diff --git a/tests/app/routers/test_boards_multiuser.py b/tests/app/routers/test_boards_multiuser.py index f14bdad3814..3a29511669d 100644 --- a/tests/app/routers/test_boards_multiuser.py +++ b/tests/app/routers/test_boards_multiuser.py @@ -92,12 +92,12 @@ def enable_multiuser_for_tests(monkeypatch: Any, mock_invoker: Invoker): # delete_videos_on_board now returns the authoritative ``deleted_videos`` list (only the # videos whose file deletion actually succeeded). Default to an empty list so pydantic # validation on ``DeleteBoardResult`` doesn't reject the MagicMock auto-return. - mock_invoker.services.videos.delete_videos_on_board.return_value = ([], []) + mock_invoker.services.videos.delete_videos_on_board.return_value = ([], [], []) # The images service is a real ImageService instance in mock_services; the delete-board # cascade calls ``delete_images_on_board`` on it, which fails without an initialized # invoker. Stub it so the multiuser router tests can assert the cascade args. mock_invoker.services.images = MagicMock() - mock_invoker.services.images.delete_images_on_board.return_value = ([], []) + mock_invoker.services.images.delete_images_on_board.return_value = ([], [], []) mock_deps = MockApiDependencies(mock_invoker) monkeypatch.setattr("invokeai.app.api.routers.auth.ApiDependencies", mock_deps) @@ -718,7 +718,7 @@ def test_delete_board_with_include_images_cascades_videos(client: TestClient, mo # The cascade returns the names of videos it actually deleted; the router must surface # *that* list (not the pre-delete enumeration) so the response can't claim a video was # destroyed when its DB record was preserved due to a file-delete failure. - mock_invoker.services.videos.delete_videos_on_board.return_value = (["video_a.mp4", "video_b.mp4"], []) + mock_invoker.services.videos.delete_videos_on_board.return_value = (["video_a.mp4", "video_b.mp4"], [], []) response = client.delete( f"/api/v1/boards/{board_id}?include_images=true", @@ -753,8 +753,8 @@ def test_delete_board_with_partial_video_file_delete_failure_reports_only_actual # The service deleted "good.mp4" successfully but preserved "stuck.mp4" because its # file delete failed. The router must NOT claim "stuck.mp4" was deleted, and must # report the failure from the service's own accounting (not a racy listing diff). - mock_invoker.services.images.delete_images_on_board.return_value = (["good.png"], ["stuck.png"]) - mock_invoker.services.videos.delete_videos_on_board.return_value = (["good.mp4"], ["stuck.mp4"]) + mock_invoker.services.images.delete_images_on_board.return_value = (["good.png"], ["stuck.png"], []) + mock_invoker.services.videos.delete_videos_on_board.return_value = (["good.mp4"], ["stuck.mp4"], []) response = client.delete( f"/api/v1/boards/{board_id}?include_images=true", @@ -768,6 +768,41 @@ def test_delete_board_with_partial_video_file_delete_failure_reports_only_actual assert body["failed_videos"] == ["stuck.mp4"] +def test_delete_board_preserves_starred_media_when_protected( + client: TestClient, mock_invoker: Invoker, user1_token: str +): + create = client.post( + "/api/v1/boards/?board_name=Protected+Starred+Board", + headers={"Authorization": f"Bearer {user1_token}"}, + ) + assert create.status_code == status.HTTP_201_CREATED + board_id = create.json()["board_id"] + mock_invoker.services.images.delete_images_on_board.return_value = ( + ["normal.png"], + [], + ["starred.png"], + ) + mock_invoker.services.videos.delete_videos_on_board.return_value = ( + ["normal.mp4"], + [], + ["starred.mp4"], + ) + + response = client.delete( + f"/api/v1/boards/{board_id}?include_images=true&delete_starred=false", + headers={"Authorization": f"Bearer {user1_token}"}, + ) + + assert response.status_code == status.HTTP_200_OK + body = response.json() + assert body["deleted_images"] == ["normal.png"] + assert body["deleted_videos"] == ["normal.mp4"] + assert body["starred_images_skipped"] == ["starred.png"] + assert body["starred_videos_skipped"] == ["starred.mp4"] + assert mock_invoker.services.images.delete_images_on_board.call_args.kwargs["delete_starred"] is False + assert mock_invoker.services.videos.delete_videos_on_board.call_args.kwargs["delete_starred"] is False + + @pytest.mark.parametrize("failure_phase", ["videos", "board"]) def test_delete_board_reports_partial_cascade_completion( client: TestClient, mock_invoker: Invoker, user1_token: str, failure_phase: str @@ -778,8 +813,8 @@ def test_delete_board_reports_partial_cascade_completion( ) assert create.status_code == status.HTTP_201_CREATED board_id = create.json()["board_id"] - mock_invoker.services.images.delete_images_on_board.return_value = (["deleted.png"], []) - mock_invoker.services.videos.delete_videos_on_board.return_value = (["deleted.mp4"], []) + mock_invoker.services.images.delete_images_on_board.return_value = (["deleted.png"], [], []) + mock_invoker.services.videos.delete_videos_on_board.return_value = (["deleted.mp4"], [], []) if failure_phase == "videos": mock_invoker.services.videos.delete_videos_on_board.side_effect = RuntimeError("video delete failed") diff --git a/tests/app/routers/test_images.py b/tests/app/routers/test_images.py index 1e4270abff7..c5ed5eba34f 100644 --- a/tests/app/routers/test_images.py +++ b/tests/app/routers/test_images.py @@ -81,6 +81,130 @@ def prepare_image_maintenance_test(monkeypatch: Any, mock_invoker: Invoker) -> N monkeypatch.setattr("invokeai.app.api.auth_dependencies.ApiDependencies", mock_deps) +def prepare_starred_delete_test( + monkeypatch: Any, + mock_invoker: Invoker, + starred_names: set[str], + board_ids: dict[str, str | None], +) -> None: + mock_deps = MockApiDependencies(mock_invoker) + mock_invoker.services.image_moves = MagicMock() + mock_invoker.services.image_moves.is_maintenance_active.return_value = False + mock_invoker.services.board_image_records = MagicMock() + mock_invoker.services.board_image_records.get_board_for_image.side_effect = board_ids.__getitem__ + mock_invoker.services.board_images = MagicMock() + mock_invoker.services.images = MagicMock() + mock_invoker.services.images.delete.side_effect = ( + lambda image_name, delete_starred=True: delete_starred or image_name not in starred_names + ) + monkeypatch.setattr("invokeai.app.api.routers.images.ApiDependencies", mock_deps) + monkeypatch.setattr("invokeai.app.api.routers._access.ApiDependencies", mock_deps) + monkeypatch.setattr("invokeai.app.api.routers.image_move_maintenance.ApiDependencies", mock_deps) + monkeypatch.setattr("invokeai.app.api.auth_dependencies.ApiDependencies", mock_deps) + + +def test_delete_starred_image_is_skipped_when_protected( + monkeypatch: Any, mock_invoker: Invoker, client: TestClient +) -> None: + image_name = "starred.png" + prepare_starred_delete_test( + monkeypatch, + mock_invoker, + starred_names={image_name}, + board_ids={image_name: "board-id"}, + ) + + response = client.delete(f"/api/v1/images/i/{image_name}", params={"delete_starred": False}) + + assert response.status_code == 200 + assert response.json() == { + "deleted_images": [], + "failed_images": [], + "affected_boards": ["board-id"], + "starred_skipped": [image_name], + } + mock_invoker.services.images.delete.assert_called_once_with(image_name, delete_starred=False) + + +def test_bulk_delete_only_deletes_unstarred_images_when_protected( + monkeypatch: Any, mock_invoker: Invoker, client: TestClient +) -> None: + starred_name = "starred.png" + unstarred_name = "unstarred.png" + prepare_starred_delete_test( + monkeypatch, + mock_invoker, + starred_names={starred_name}, + board_ids={starred_name: "board-id", unstarred_name: "board-id"}, + ) + + response = client.post( + "/api/v1/images/delete", + json={"image_names": [starred_name, unstarred_name], "delete_starred": False}, + ) + + assert response.status_code == 200 + assert response.json() == { + "deleted_images": [unstarred_name], + "failed_images": [], + "affected_boards": ["board-id"], + "starred_skipped": [starred_name], + } + assert mock_invoker.services.images.delete.call_count == 2 + mock_invoker.services.images.delete.assert_any_call(starred_name, delete_starred=False) + mock_invoker.services.images.delete.assert_any_call(unstarred_name, delete_starred=False) + + +def test_bulk_delete_deduplicates_image_names(monkeypatch: Any, mock_invoker: Invoker, client: TestClient) -> None: + prepare_starred_delete_test( + monkeypatch, + mock_invoker, + starred_names=set(), + board_ids={"duplicate.png": "board-id", "other.png": "board-id"}, + ) + + response = client.post( + "/api/v1/images/delete", + json={"image_names": ["duplicate.png", "duplicate.png", "other.png"]}, + ) + + assert response.status_code == 200 + assert sorted(response.json()["deleted_images"]) == ["duplicate.png", "other.png"] + assert response.json()["failed_images"] == [] + assert [call.args[0] for call in mock_invoker.services.images.delete.call_args_list] == [ + "duplicate.png", + "other.png", + ] + + +def test_delete_uncategorized_only_deletes_unstarred_images_when_protected( + monkeypatch: Any, mock_invoker: Invoker, client: TestClient +) -> None: + starred_name = "starred.png" + unstarred_name = "unstarred.png" + prepare_starred_delete_test( + monkeypatch, + mock_invoker, + starred_names={starred_name}, + board_ids={starred_name: None, unstarred_name: None}, + ) + mock_invoker.services.board_images.get_all_board_image_names_for_board.return_value = [ + starred_name, + unstarred_name, + ] + + response = client.delete("/api/v1/images/uncategorized", params={"delete_starred": False}) + + assert response.status_code == 200 + assert response.json() == { + "deleted_images": [unstarred_name], + "failed_images": [], + "affected_boards": ["none"], + "starred_skipped": [starred_name], + } + assert mock_invoker.services.images.delete.call_count == 2 + + @pytest.mark.parametrize( ("method", "path", "json_body"), [ diff --git a/tests/app/routers/test_multiuser_authorization.py b/tests/app/routers/test_multiuser_authorization.py index be5d2a61beb..ec10f009f50 100644 --- a/tests/app/routers/test_multiuser_authorization.py +++ b/tests/app/routers/test_multiuser_authorization.py @@ -692,7 +692,7 @@ def test_delete_uncategorized_reports_owned_images_that_failed( monkeypatch.setattr( mock_invoker.services.images, "delete", - MagicMock(side_effect=[None, OSError("file busy")]), + MagicMock(side_effect=[True, OSError("file busy")]), ) response = client.delete("/api/v1/images/uncategorized", headers=_auth(user1_token)) diff --git a/tests/app/routers/test_videos_multiuser.py b/tests/app/routers/test_videos_multiuser.py index 8282346b114..6e8f608575d 100644 --- a/tests/app/routers/test_videos_multiuser.py +++ b/tests/app/routers/test_videos_multiuser.py @@ -237,9 +237,7 @@ def test_delete_videos_from_list_dedupes_repeated_names(client: TestClient, mock ownership bypass), landing the same name in BOTH deleted_videos and failed_videos and toasting a spurious partial-failure warning (JPPhoto non-merge-blocker, 2026-07-22). """ - fake_dto = MagicMock() - fake_dto.board_id = None - mock_invoker.services.videos.get_dto.return_value = fake_dto + mock_invoker.services.board_video_records.get_board_for_video.return_value = None response = client.post( "/api/v1/videos/delete", @@ -256,6 +254,51 @@ def test_delete_videos_from_list_dedupes_repeated_names(client: TestClient, mock assert sorted(delete_calls) == ["dup.mp4", "other.mp4"] +def test_delete_starred_video_is_skipped_when_protected(client: TestClient, mock_invoker: Invoker, admin_token: str): + mock_invoker.services.board_video_records.get_board_for_video.return_value = "board-id" + mock_invoker.services.videos.delete.return_value = False + + response = client.delete( + "/api/v1/videos/i/starred.mp4?delete_starred=false", + headers={"Authorization": f"Bearer {admin_token}"}, + ) + + assert response.status_code == status.HTTP_200_OK + assert response.json() == { + "deleted_videos": [], + "failed_videos": [], + "affected_boards": ["board-id"], + "starred_skipped": ["starred.mp4"], + } + mock_invoker.services.videos.delete.assert_called_once_with("starred.mp4", delete_starred=False) + + +def test_bulk_delete_only_deletes_unstarred_videos_when_protected( + client: TestClient, mock_invoker: Invoker, admin_token: str +): + mock_invoker.services.board_video_records.get_board_for_video.return_value = None + mock_invoker.services.videos.delete.side_effect = ( + lambda video_name, delete_starred=True: delete_starred or video_name != "starred.mp4" + ) + + response = client.post( + "/api/v1/videos/delete", + json={ + "video_names": ["starred.mp4", "normal.mp4"], + "delete_starred": False, + }, + headers={"Authorization": f"Bearer {admin_token}"}, + ) + + assert response.status_code == status.HTTP_200_OK + assert response.json() == { + "deleted_videos": ["normal.mp4"], + "failed_videos": [], + "affected_boards": ["none"], + "starred_skipped": ["starred.mp4"], + } + + def test_video_batch_rejects_too_many_or_overlong_names() -> None: with pytest.raises(ValidationError): VideoNamesBatch(video_names=[f"{index}.mp4" for index in range(1001)]) @@ -292,9 +335,7 @@ def fake_get_user_id(video_name: str): # fallback path doesn't relax permissions for the foreign video. mock_invoker.services.board_video_records.get_board_for_video.return_value = None - fake_dto = MagicMock() - fake_dto.board_id = None - mock_invoker.services.videos.get_dto.return_value = fake_dto + mock_invoker.services.board_video_records.get_board_for_video.return_value = None response = client.post( "/api/v1/videos/delete", @@ -913,6 +954,30 @@ def test_delete_uncategorized_videos_deletes_only_owned(client: TestClient, mock assert delete_calls == {"mine_a.mp4", "mine_b.mp4"} +def test_delete_uncategorized_videos_preserves_starred_when_protected( + client: TestClient, mock_invoker: Invoker, admin_token: str +): + names_result = MagicMock() + names_result.video_names = ["starred.mp4", "normal.mp4"] + mock_invoker.services.videos.get_video_names.return_value = names_result + mock_invoker.services.videos.delete.side_effect = ( + lambda video_name, delete_starred=True: delete_starred or video_name != "starred.mp4" + ) + + response = client.delete( + "/api/v1/videos/uncategorized?delete_starred=false", + headers={"Authorization": f"Bearer {admin_token}"}, + ) + + assert response.status_code == status.HTTP_200_OK + assert response.json() == { + "deleted_videos": ["normal.mp4"], + "failed_videos": [], + "affected_boards": ["none"], + "starred_skipped": ["starred.mp4"], + } + + def test_delete_uncategorized_videos_requires_auth(enable_multiuser_for_videos: Any, client: TestClient): response = client.delete("/api/v1/videos/uncategorized") assert response.status_code == status.HTTP_401_UNAUTHORIZED diff --git a/tests/app/services/images/test_images_default.py b/tests/app/services/images/test_images_default.py index c97916dd139..91adb7e2aca 100644 --- a/tests/app/services/images/test_images_default.py +++ b/tests/app/services/images/test_images_default.py @@ -38,6 +38,7 @@ def _make_record( image_name: str = "abc12345-test.png", image_subfolder: str = "", is_intermediate: bool = False, + starred: bool = False, ) -> ImageRecord: now = get_iso_timestamp() return ImageRecord( @@ -49,7 +50,7 @@ def _make_record( created_at=now, updated_at=now, is_intermediate=is_intermediate, - starred=False, + starred=starred, has_workflow=False, image_subfolder=image_subfolder, ) @@ -223,11 +224,12 @@ def test_record_preserved_when_file_delete_fails(self, image_service: ImageServi # File staging succeeds for first, fails for second invoker.services.image_files.stage_delete.side_effect = [object(), Exception("disk error")] - deleted, failed = image_service.delete_images_on_board("board-1") + deleted, failed, starred_skipped = image_service.delete_images_on_board("board-1") invoker.services.image_records.delete_many.assert_called_once_with(["good.png"]) assert deleted == ["good.png"] assert failed == ["bad.png"] + assert starred_skipped == [] def test_file_cleanup_failure_does_not_raise(self, image_service: ImageService): """File cleanup errors are swallowed, not propagated.""" @@ -238,11 +240,12 @@ def test_file_cleanup_failure_does_not_raise(self, image_service: ImageService): invoker.services.image_records.get.return_value = record invoker.services.image_files.stage_delete.side_effect = Exception("permission denied") - deleted, failed = image_service.delete_images_on_board("board-1") + deleted, failed, starred_skipped = image_service.delete_images_on_board("board-1") invoker.services.image_records.delete_many.assert_called_once_with([]) assert deleted == [] assert failed == ["img.png"] + assert starred_skipped == [] def test_record_lookup_failure_does_not_block_others(self, image_service: ImageService): """If getting the record for one image fails, other images are still processed.""" @@ -255,13 +258,14 @@ def test_record_lookup_failure_does_not_block_others(self, image_service: ImageS ok_record = _make_record(image_name="ok.png", image_subfolder="") invoker.services.image_records.get.side_effect = [Exception("not found"), ok_record] - deleted, failed = image_service.delete_images_on_board("board-1") + deleted, failed, starred_skipped = image_service.delete_images_on_board("board-1") # File staging was attempted for the second image only invoker.services.image_files.stage_delete.assert_called_once_with("ok.png", image_subfolder="") invoker.services.image_records.delete_many.assert_called_once_with(["ok.png"]) assert deleted == ["ok.png"] assert failed == ["missing.png"] + assert starred_skipped == [] def test_database_failure_restores_staged_files(self, image_service: ImageService): invoker = image_service._ImageService__invoker # type: ignore @@ -276,3 +280,54 @@ def test_database_failure_restores_staged_files(self, image_service: ImageServic invoker.services.image_files.rollback_delete.assert_called_once_with(token) invoker.services.image_files.commit_delete.assert_not_called() + + +class TestStarredProtection: + def test_single_starred_image_is_deleted_by_default(self, image_service: ImageService): + invoker = image_service._ImageService__invoker # type: ignore + invoker.services.image_records.get.return_value = _make_record(starred=True) + + was_deleted = image_service.delete("starred.png") + + assert was_deleted is True + invoker.services.image_files.delete.assert_called_once() + invoker.services.image_records.delete.assert_called_once_with("starred.png") + + def test_single_starred_image_is_preserved(self, image_service: ImageService): + invoker = image_service._ImageService__invoker # type: ignore + invoker.services.image_records.get.return_value = _make_record(starred=True) + + was_deleted = image_service.delete("starred.png", delete_starred=False) + + assert was_deleted is False + invoker.services.image_files.delete.assert_not_called() + invoker.services.image_records.delete.assert_not_called() + + def test_single_unstarred_image_is_deleted_when_protected(self, image_service: ImageService): + invoker = image_service._ImageService__invoker # type: ignore + invoker.services.image_records.get.return_value = _make_record(starred=False) + + was_deleted = image_service.delete("normal.png", delete_starred=False) + + assert was_deleted is True + invoker.services.image_files.delete.assert_called_once() + invoker.services.image_records.delete.assert_called_once_with("normal.png") + + def test_board_delete_reports_protected_starred_images(self, image_service: ImageService): + invoker = image_service._ImageService__invoker # type: ignore + invoker.services.board_image_records.get_all_board_image_names_for_board.return_value = [ + "starred.png", + "normal.png", + ] + invoker.services.image_records.get.side_effect = [ + _make_record(image_name="starred.png", starred=True), + _make_record(image_name="normal.png", starred=False), + ] + + deleted, failed, starred_skipped = image_service.delete_images_on_board("board-1", delete_starred=False) + + assert deleted == ["normal.png"] + assert failed == [] + assert starred_skipped == ["starred.png"] + invoker.services.image_records.delete_many.assert_called_once_with(["normal.png"]) + invoker.services.image_files.stage_delete.assert_called_once_with("normal.png", image_subfolder="") diff --git a/tests/app/services/videos/test_videos_default.py b/tests/app/services/videos/test_videos_default.py index 6cef061522d..292fc46b694 100644 --- a/tests/app/services/videos/test_videos_default.py +++ b/tests/app/services/videos/test_videos_default.py @@ -15,7 +15,7 @@ from invokeai.app.util.misc import get_iso_timestamp -def _make_record(video_name: str = "abc.mp4", video_subfolder: str = "") -> VideoRecord: +def _make_record(video_name: str = "abc.mp4", video_subfolder: str = "", starred: bool = False) -> VideoRecord: now = get_iso_timestamp() return VideoRecord( video_name=video_name, @@ -28,7 +28,7 @@ def _make_record(video_name: str = "abc.mp4", video_subfolder: str = "") -> Vide created_at=now, updated_at=now, is_intermediate=False, - starred=False, + starred=starred, has_workflow=False, video_subfolder=video_subfolder, ) @@ -60,7 +60,7 @@ def test_record_preserved_when_file_delete_fails(self, video_service: VideoServi ] invoker.services.video_files.stage_delete.side_effect = [object(), Exception("disk error")] - deleted, failed = video_service.delete_videos_on_board("board-1") + deleted, failed, starred_skipped = video_service.delete_videos_on_board("board-1") # Only the video whose file we successfully removed should have its record deleted. invoker.services.video_records.delete_many.assert_called_once_with(["good.mp4"]) @@ -69,6 +69,7 @@ def test_record_preserved_when_file_delete_fails(self, video_service: VideoServi # and can report the failure without racily diffing a board listing. assert deleted == ["good.mp4"] assert failed == ["bad.mp4"] + assert starred_skipped == [] def test_file_cleanup_failure_does_not_raise(self, video_service: VideoService): """A single file-delete failure must not surface as a 500 to the user — the rest of @@ -80,12 +81,13 @@ def test_file_cleanup_failure_does_not_raise(self, video_service: VideoService): invoker.services.video_files.stage_delete.side_effect = Exception("permission denied") # Should not raise - deleted, failed = video_service.delete_videos_on_board("board-1") + deleted, failed, starred_skipped = video_service.delete_videos_on_board("board-1") # And the failing video's record must be preserved. invoker.services.video_records.delete_many.assert_called_once_with([]) assert deleted == [] assert failed == ["v.mp4"] + assert starred_skipped == [] def test_all_records_deleted_on_full_success(self, video_service: VideoService): invoker = video_service._VideoService__invoker # type: ignore[attr-defined] @@ -99,11 +101,12 @@ def test_all_records_deleted_on_full_success(self, video_service: VideoService): ] invoker.services.video_files.stage_delete.side_effect = [object(), object()] - deleted, failed = video_service.delete_videos_on_board("board-1") + deleted, failed, starred_skipped = video_service.delete_videos_on_board("board-1") invoker.services.video_records.delete_many.assert_called_once_with(["a.mp4", "b.mp4"]) assert deleted == ["a.mp4", "b.mp4"] assert failed == [] + assert starred_skipped == [] def test_staging_cleanup_failure_is_deferred_after_records_are_deleted(self, video_service: VideoService): invoker = video_service._VideoService__invoker # type: ignore[attr-defined] @@ -112,10 +115,11 @@ def test_staging_cleanup_failure_is_deferred_after_records_are_deleted(self, vid invoker.services.video_files.stage_delete.return_value = object() invoker.services.video_files.commit_delete.side_effect = OSError("staging directory busy") - deleted, failed = video_service.delete_videos_on_board("board-1") + deleted, failed, starred_skipped = video_service.delete_videos_on_board("board-1") assert deleted == ["v.mp4"] assert failed == [] + assert starred_skipped == [] invoker.services.video_records.delete_many.assert_called_once_with(["v.mp4"]) invoker.services.logger.error.assert_called() @@ -149,6 +153,46 @@ def test_single_delete_rolls_files_back_when_record_delete_fails(self, video_ser invoker.services.video_files.rollback_delete.assert_called_once() invoker.services.video_files.commit_delete.assert_not_called() + def test_single_starred_video_is_deleted_by_default(self, video_service: VideoService): + invoker = video_service._VideoService__invoker # type: ignore[attr-defined] + invoker.services.video_records.get.return_value = _make_record(starred=True) + + was_deleted = video_service.delete("starred.mp4") + + assert was_deleted is True + invoker.services.video_files.stage_delete.assert_called_once_with("starred.mp4", video_subfolder="") + invoker.services.video_records.delete.assert_called_once_with("starred.mp4") + invoker.services.video_files.commit_delete.assert_called_once() + + def test_single_starred_video_is_preserved(self, video_service: VideoService): + invoker = video_service._VideoService__invoker # type: ignore[attr-defined] + invoker.services.video_records.get.return_value = _make_record(starred=True) + + was_deleted = video_service.delete("starred.mp4", delete_starred=False) + + assert was_deleted is False + invoker.services.video_files.stage_delete.assert_not_called() + invoker.services.video_records.delete.assert_not_called() + + def test_board_delete_reports_protected_starred_videos(self, video_service: VideoService): + invoker = video_service._VideoService__invoker # type: ignore[attr-defined] + invoker.services.board_video_records.get_all_board_video_names_for_board.return_value = [ + "starred.mp4", + "normal.mp4", + ] + invoker.services.video_records.get.side_effect = [ + _make_record(video_name="starred.mp4", starred=True), + _make_record(video_name="normal.mp4", starred=False), + ] + + deleted, failed, starred_skipped = video_service.delete_videos_on_board("board-1", delete_starred=False) + + assert deleted == ["normal.mp4"] + assert failed == [] + assert starred_skipped == ["starred.mp4"] + invoker.services.video_records.delete_many.assert_called_once_with(["normal.mp4"]) + invoker.services.video_files.stage_delete.assert_called_once_with("normal.mp4", video_subfolder="") + class TestCreateRollback: """Per JPPhoto's PR review (May 22 follow-up): if the video file save fails after the DB