delete previous image on cover change #6368
Open
+27
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
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. |
Xazin
reviewed
Oct 21, 2024
| @@ -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?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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