From b965a5f3ae4e6af03145af2082ab4cf4417c5d4c Mon Sep 17 00:00:00 2001 From: Ahad Patel <69256193+Ahad-patel@users.noreply.github.com> Date: Tue, 31 Dec 2024 07:06:14 +0530 Subject: [PATCH] feat: delete the previous image when the cover changes in local mode (#6368) * remove unecessary images from localstorage * feat: Add handler for deleting previous cover image on cover image change * fix: add local image case for versions after 0.5.5 * fix: add try catch block and delete action to bottom of function * chore: add test case for uploading and deleting image in localmode --------- Co-authored-by: Lucas.Xu --- .../document_with_cover_image_test.dart | 61 +++++++++++++++++++ .../header/document_cover_widget.dart | 20 +++++- .../editor_plugins/image/image_util.dart | 13 ++++ .../migration/editor_migration.dart | 8 +++ 4 files changed, 99 insertions(+), 3 deletions(-) diff --git a/frontend/appflowy_flutter/integration_test/desktop/document/document_with_cover_image_test.dart b/frontend/appflowy_flutter/integration_test/desktop/document/document_with_cover_image_test.dart index c9f844f374..84b6790403 100644 --- a/frontend/appflowy_flutter/integration_test/desktop/document/document_with_cover_image_test.dart +++ b/frontend/appflowy_flutter/integration_test/desktop/document/document_with_cover_image_test.dart @@ -1,15 +1,23 @@ +import 'dart:io'; + import 'package:appflowy/generated/locale_keys.g.dart'; import 'package:appflowy/plugins/document/presentation/editor_plugins/header/document_cover_widget.dart'; +import 'package:appflowy/plugins/document/presentation/editor_plugins/image/image_util.dart'; import 'package:appflowy/shared/icon_emoji_picker/flowy_icon_emoji_picker.dart'; import 'package:appflowy/shared/icon_emoji_picker/recent_icons.dart'; import 'package:appflowy_backend/protobuf/flowy-folder/view.pb.dart'; import 'package:easy_localization/easy_localization.dart'; +import 'package:flutter/gestures.dart'; import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_emoji_mart/flutter_emoji_mart.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:integration_test/integration_test.dart'; +import 'package:path/path.dart' as p; +import 'package:path_provider/path_provider.dart'; import '../../shared/emoji.dart'; +import '../../shared/mock/mock_file_picker.dart'; import '../../shared/util.dart'; void main() { @@ -60,6 +68,59 @@ void main() { tester.expectToSeeNoDocumentCover(); }); + testWidgets('document cover local image tests', (tester) async { + await tester.initializeAppFlowy(); + await tester.tapAnonymousSignInButton(); + + tester.expectToSeeNoDocumentCover(); + + // Hover over cover toolbar to show 'Add Cover' and 'Add Icon' buttons + await tester.editor.hoverOnCoverToolbar(); + + // Insert a document cover + await tester.editor.tapOnAddCover(); + tester.expectToSeeDocumentCover(CoverType.asset); + + // Hover over the cover to show the 'Change Cover' and delete buttons + await tester.editor.hoverOnCover(); + tester.expectChangeCoverAndDeleteButton(); + + // Change cover to a local image image + final imagePath = await rootBundle.load('assets/test/images/sample.jpeg'); + final tempDirectory = await getTemporaryDirectory(); + final localImagePath = p.join(tempDirectory.path, 'sample.jpeg'); + final imageFile = File(localImagePath) + ..writeAsBytesSync(imagePath.buffer.asUint8List()); + + await tester.editor.hoverOnCover(); + await tester.editor.tapOnChangeCover(); + + final uploadButton = find.findTextInFlowyText( + LocaleKeys.document_imageBlock_upload_label.tr(), + ); + await tester.tapButton(uploadButton); + + mockPickFilePaths(paths: [localImagePath]); + await tester.tapButtonWithName( + LocaleKeys.document_imageBlock_upload_placeholder.tr(), + ); + + await tester.pumpAndSettle(); + tester.expectToSeeDocumentCover(CoverType.file); + + // Remove the cover + await tester.editor.hoverOnCover(); + await tester.editor.tapOnRemoveCover(); + tester.expectToSeeNoDocumentCover(); + + // Test if deleteImageFromLocalStorage(localImagePath) function is called once + await tester.pump(kDoubleTapTimeout); + expect(deleteImageTestCounter, 1); + + // delete temp files + await imageFile.delete(); + }); + testWidgets('document icon tests', (tester) async { await tester.initializeAppFlowy(); await tester.tapAnonymousSignInButton(); diff --git a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/header/document_cover_widget.dart b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/header/document_cover_widget.dart index d28020d0fe..3a827b7ce0 100644 --- a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/header/document_cover_widget.dart +++ b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/header/document_cover_widget.dart @@ -763,15 +763,29 @@ class DocumentCoverState extends State { } Future onCoverChanged(CoverType type, String? details) async { - if (type == CoverType.file && details != null && !isURL(details)) { + final previousType = CoverType.fromString( + widget.node.attributes[DocumentHeaderBlockKeys.coverType], + ); + final previousDetails = + widget.node.attributes[DocumentHeaderBlockKeys.coverDetails]; + + bool isFileType(CoverType type, String? details) => + type == CoverType.file && details != null && !isURL(details); + + if (isFileType(type, details)) { if (_isLocalMode()) { - details = await saveImageToLocalStorage(details); + details = await saveImageToLocalStorage(details!); } else { // else we should save the image to cloud storage - (details, _) = await saveImageToCloudStorage(details, widget.view.id); + (details, _) = await saveImageToCloudStorage(details!, widget.view.id); } } widget.onChangeCover(type, details); + + // After cover change,delete from localstorage if previous cover was image type + if (isFileType(previousType, previousDetails) && _isLocalMode()) { + await deleteImageFromLocalStorage(previousDetails); + } } void setOverlayButtonsHidden(bool value) { diff --git a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/image/image_util.dart b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/image/image_util.dart index efa5721382..74d1955312 100644 --- a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/image/image_util.dart +++ b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/image/image_util.dart @@ -117,3 +117,16 @@ Future> extractAndUploadImages( return images; } + +@visibleForTesting +int deleteImageTestCounter = 0; + +Future deleteImageFromLocalStorage(String localImagePath) async { + try { + await File(localImagePath) + .delete() + .whenComplete(() => deleteImageTestCounter++); + } catch (e) { + Log.error('cannot delete image file', e); + } +} diff --git a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/migration/editor_migration.dart b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/migration/editor_migration.dart index 8463030667..dcda2941ff 100644 --- a/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/migration/editor_migration.dart +++ b/frontend/appflowy_flutter/lib/plugins/document/presentation/editor_plugins/migration/editor_migration.dart @@ -226,6 +226,14 @@ class EditorMigration { }, }; } + } else { + extra = { + ViewExtKeys.coverKey: { + ViewExtKeys.coverTypeKey: + PageStyleCoverImageType.localImage.toString(), + ViewExtKeys.coverValueKey: coverDetails, + }, + }; } break; default: