Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

The application does not remove the thumbnail file from the FS if it is removed in the original article #524

Closed
Katarn opened this issue Oct 17, 2022 · 3 comments · Fixed by #683
Closed

The application does not remove the thumbnail file from the FS if it is removed in the original article #524

Katarn opened this issue Oct 17, 2022 · 3 comments · Fixed by #683
Assignees
@Monirzadeh
Labels
component:backend good first issue Good for newcomers priority:high type:bug Something isn't working
Milestone
1.6.0

Comments

@Katarn
Copy link
Contributor

Katarn commented Oct 17, 2022 •
edited
Loading

Data

  • Shiori version: commit 77269b7
  • Database Engine: SQLite
  • Operating system: Windows 8.1
  • CLI/Web interface/Web Extension: CLI

Describe the bug / actual behavior

The problem is described in the title of the issue.

Expected behavior

If a thumbnail is removed from the original article, then it should also be removed from the file system.

To Reproduce

Steps to reproduce the behavior:

  1. Save any HTML page with thumbnail from the internet to the localhost and serve it with any way (e. g. python -m SimpleHTTPServer 8080.
  2. Add a link to this page in Shiori. As you can see, an image file is created in the thumb directory.
  3. Remove all text and all image tags from the saved page.
  4. Click the update archive button. I also check both checkboxes: "Keep the old title and excerpt" and "Update archive as well".
  5. As you can see, the site thumbnail disappeared from Shiori's interface, but it remained in the file system.
@Katarn Katarn added the type:bug Something isn't working label Oct 17, 2022
@fmartingr fmartingr added the good first issue Good for newcomers label Jan 10, 2023
@fmartingr fmartingr added this to the 1.6.3 milestone Jul 21, 2023
@fmartingr
Copy link
Member

Whomever lands checking this out, please also check that all archive and ebook data is properly handled on cleanup as well.

Relates to: #634

@github-project-automation github-project-automation bot moved this to To do in Roadmap Jul 21, 2023
@Monirzadeh
Copy link
Collaborator

Are you sure we should update ebook with archive update (without user request) too?
So if an article has ebook before update, it should generate each time with update archive?

Monirzadeh added a commit to Monirzadeh/shiori that referenced this issue Jul 27, 2023
@Monirzadeh
thumbnail will update in update archive fix go-shiori#524
7634038
This was referenced Jul 27, 2023
Closed
Merged
@fmartingr
Copy link
Member

Are you sure we should update ebook with archive update (without user request) too? So if an article has ebook before update, it should generate each time with update archive?

No, not meaning that. But when this issue is checked we should apply the same logic for archives, thumbs and ebooks:

Monirzadeh added a commit to Monirzadeh/shiori that referenced this issue Aug 5, 2023
@Monirzadeh
update thumbnail based on last status of bookmark fix go-shiori#524
658c80f
@fmartingr fmartingr modified the milestones: 1.6.3, 1.5.6 Aug 20, 2023
fmartingr added a commit that referenced this issue Aug 20, 2023
@Monirzadeh @fmartingr
fix: Ensure bookmark files are correctly downloaded before deleting c…
f4817cb 
…urrent ones (#683)

* generate ebook get dstPath

* Archive and ebook can recover if download faild

* recover thumb if download faild

* thumb image create just if image processing is sucssesful

* create epub in tmp if it sucssesful copy to destination

* archive file create in tmp if it successful move to destination

* move to destination as function

* update ebook download api and remove .epub from file name

* report faild item to user

* not show dialog if error not happen

* update thumbnail based on last status of bookmark fix #524

* better warning massage

Co-authored-by: Felipe Martin <[email protected]>

* tmpFile without .epub

* MoveToDestination change to MoveFileToDestination

* return .epub

* log if downloadBookImage return error

* fix bug remove imgPath just if download last image be unsuccessful

* update old unit test

* add processing.go unit test

* small massage for report failded item to the user

* add some more unit test and samplefile

* use sample image in unit test

* use local sample file and unit test not need internet connection anymore

* update error to user and log that too

* add more comment

* update comment

* change variable name parentDir to dstDir

* more simpler error handling

* remove unneeded defer

* remvoe unneeded epubWriter.Close()

* more readable unit test in processing

* more readable unit test for ebooks

* delete all defer os.RemoveAll from unit tests

* Better comment

Co-authored-by: Felipe Martin <[email protected]>

* Better Error output

Co-authored-by: Felipe Martin <[email protected]>

* fix err.String() method

---------

Co-authored-by: Felipe Martin <[email protected]>
@github-project-automation github-project-automation bot moved this from To do to Done in Roadmap Aug 20, 2023
@fmartingr fmartingr modified the milestones: 1.5.6, 1.6.0 Aug 20, 2023
@Monirzadeh Monirzadeh mentioned this issue Feb 2, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Monirzadeh Monirzadeh

Labels
component:backend good first issue Good for newcomers priority:high type:bug Something isn't working
Projects
Roadmap
Archived in project
Milestone
1.6.0
3 participants
@fmartingr @Katarn @Monirzadeh