delete previous image on cover change #6368
Conversation
|
|
Ahad-patel
changed the title
|
Hey @LucasXu0, Is my approach to this issue is correct? |
Ahad-patel
force-pushed
the
delete-unecessary-images
branch
from
b795d8a to
bd9be43
Compare
|
@LucasXu0 I also noticed that the local image case was not handled in migrateAttributes so I added it. I am working on test cases now. Please let me know if my approach is correct. |
| @@ -763,6 +763,7 @@ class DocumentCoverState extends State<DocumentCover> { | |||
| } | |||
|
|
|||
| Future<void> onCoverChanged(CoverType type, String? details) async { | |||
| await deletePreviousImageIfNecessary(); | |||
There was a problem hiding this comment.
We can perform this after changing the cover. (Since it's an asynchronous operation)
| @@ -117,3 +117,7 @@ Future<List<ImageBlockData>> extractAndUploadImages( | |||
|
|
|||
| return images; | |||
| } | |||
|
|
|||
| Future<void> deleteImageFromLocalStorage(String localImagePath) async { | |||
| await File(localImagePath).delete(); | |||
There was a problem hiding this comment.
Should we wrap this in try-catch and at least log errors?
|
Hey @Xazin, I was trying to add test case in this file => https://github.com/Ahad-patel/AppFlowy/blob/delete-unecessary-images/frontend/appflowy_flutter/integration_test/desktop/document/document_with_cover_image_test.dart#L8-L9, but stuck on how should I approach the local file upload case, Because the file will be present in my machine but might not be availaible in some one else machine, Is there any similar code shoud I look into? |
|
@Ahad-patel In my opinion, the deletion seems unable to be mocked in the test. A workaounrd is to add the flag for the test to detect when the delete function is called while changing the cover image. If the function is called, we can assume the deletion takes effect. |
|
@LucasXu0 , I have added test case for local image upload and added a variable |
|
@LucasXu0 , please let me know if any more changes required. |
Feature Preview
Closes: #5145
I have created two methods for deletePreviousImageFromCloudStorage and deleteFile in DocumentSevce Class
when ever the user changes cover image the this method will check if there the previous image was file, If so then it will delete the previous image.
I have a question ther is a an method DocumentEventDeleteFile which takes DownloadFilePB payload which has two params localFilePath and url, the url can be obtained from coverDetails but localFilepath is not stored any where how do i delete it from the local storage? @Xazin
PR Checklist