fix: handle expired ZIM downloads with user-friendly error

[Best2Two]

I hope those PRs do something with GSoC :)
Reproduction:
I can see that latest_zim_file_for_builder generates the url based on the latest url for the zim using the function latest_zim_file_url_for, checking that method it calls zim_file_url_for_task_id
so by injecting a bad url like that
def zim_file_url_for_task_id(task_id):
return "https://s3.us-east-1.wasabisys.com/org-kiwix-wikimedia/FAKE_EXPIRED.zim"
and inserting the required records into the tables zim_tasks, selections and builders to simulate a valid selection to prevent Error 404 for my bad fake url and reproduce the targeted error. I managed to reproduce it correctly the same issue as the issuer and I can see it both in the front end and the back end!

Fixes #825

prefetching the wasabi URL with requests, with a HEAD request.

The approach was making head request inside the latest_zim_file_for_builder if it return 404 then it redirects user to create new ZIM.

[Best2Two]

Hello @audiodude, I hope you see this PR
Is this a valid fix, cause I had another solution in my mind where I would update the logic_selection.is_zim_file_deleted inside the builder to include the Head request inside it.
Why?
Because actually, there is an inconsistency in the solution.
You see that ?expired=true query in the endpoint, this happens when the file in the S3 bucket expires before 2 weeks
But if the S3 bucket expires after 2 weeks (Normal Behaviour) it will redirect them to the /zim endpoint without any queries.

This happens mainly because I found that there is a getStatus() function in the ZimFile.vue and this function only evaluates the expiration based on time only, not the head request so updating the logic_selection.is_zim_file_deleted will make it depends on the head request also. But the issue here maybe this will make high network overhead on the client. because getStatus() fires everytime the client visits the #/selections/user endpoint. So I have decided not to do this approach unless I talk to you first and get your opinion.

If you want I can refactor the whole solution to apply this approach. But if you seek a front-end friendly solution only I guess that's works

Edit:
You said in the issue #819

"That's a good point, we shouldn't display the download link if we suspect the ZIM is expired."

I agree with you this seems to be logical but we can't do this without massive overhead because we will need to send head request everytime the /selections/user page loads for each selection. I have a suggestion on where we can use 410 and if the user clicked download and it's expired only then we make the File status to Deleted so it shows the create zim instead of download zim

[audiodude]

This looks really good! This is the appropriate approach that I've been looking for, checking the status on the server at the time of the ZIM request. I also like the idea of reusing the expired message on the ZIM page, because it has the benefit of letting the user immediately request a new ZIM.

Just one small change requested.

[Best2Two]

Hello @audiodude I have submitted the fix now based on your latest review, kindly review, Thank You!

[audiodude]

Tests are failing: https://github.com/openzim/wp1/actions/runs/22513369359/job/65226917959?pr=1090

Please fix.

[Best2Two]

@audiodude Hello, Thanks for the review, I will figure out the tests issue today

[audiodude]

@Best2Two Could you also please file a follow up issue for yourself to clean up this data (https://github.com/openzim/wp1/blob/main/wp1/logic/builder.py#L851-L854), the function it calls, and the frontend functionality that depends on it? Thanks!

[Best2Two]

@audiodude I opened the issue, Now I am fixing the tests that I forgot to update

[Best2Two]

@audiodude Hello, I have fixed the tests, now I assume they will pass, kindly can you please rerun CI/CD checks!

[Best2Two]

Hello @audiodude quick ping, kindly can you review the last changes to make sure if there is something else needs to be modified

[audiodude]

@audiodude Hello, I have fixed the tests, now I assume they will pass, kindly can you please rerun CI/CD checks!

Yes, everyone always assumes their tests will pass, no one intentionally writes broken code.... 😂

But we don't have to assume. We can run the tests ourselves. Are you having trouble running pytest in your dev environment? CI uses an identical environment to the one you would end up with after following README.md.

[Best2Two]

@audiodude I mean I assume they will pass so rerun the ci/cd to test, not merge before testing!😂 they have passed locally after running pipenv run pytest

[Best2Two]

@audiodude

[Best2Two]

Hello @audiodude , I have ran all of the tests and they all had been passed. Kindly can you check. Thanks

[Best2Two]

@audiodude Hello, quick ping can you run the CI/CD please? Thanks

[audiodude]

LGTM

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.78%. Comparing base (c4917dc) to head (e0a28f9).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1090   +/-   ##
=======================================
  Coverage   92.78%   92.78%           
=======================================
  Files          74       74           
  Lines        4308     4312    +4     
=======================================
+ Hits         3997     4001    +4     
  Misses        311      311           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.