standardebooks / web

The source code for the Standard Ebooks website.
https://standardebooks.org
Creative Commons Zero v1.0 Universal
234 stars 61 forks source link

Merge with master and complete the DB write path #363

Closed colagrosso closed 3 months ago

colagrosso commented 3 months ago

Most of my work is on this commit, which might be easier to review separately:

Complete the DB write path: https://github.com/standardebooks/web/commit/f535e9f62355e30f14d7904d62cc100d171b2268

  1. Rename Created and Updated in PHP code to EbookCreated and EbookUpdated to match the schema
  2. Added GetFoo() methods for all the derived properties like GetUrl(), GetHasDownloads(), etc. Removed that logic from the constructor.
  3. Rename the constructor Ebook::__construct() to static Ebook::FromFilesystem()
  4. Adds a validator in ./scripts/update-ebook-db to compare. It compares objects from Ebook::FromFilesystem() and Ebook::GetByIdentifier() to confirm there are no differences.

With these changes, we have a fully populated DB. Running deploy-ebook-to-www like this:

for BOOK in $(find /standardebooks.org/ebooks -maxdepth 1 -type d)
do
  tsp nice  /standardebooks.org/web/scripts/deploy-ebook-to-www --verbose --no-images --no-build --no-epubcheck --no-recompose --no-feeds --no-bulk-downloads $BOOK
done

will populate the DB then read back the DB records with Ebook::GetByIdentifier() and confirm they match the Ebook object from the filesystem. Now that everything is written correctly, I can start on the read path, e.g., ebook.php.

Sorry for all the other commits in this PR. That's what it took to bring this branch up to master and fix some merge conflicts. I confirmed there are no PHPStan errors. If it's too hard to review, I'll do something different.

acabal commented 3 months ago

I think this PR imports many of the changes that were made to master, which is causing merge conflicts and also makes it really hard to review.

Can you rebase your branch onto master first, then create a new PR that only includes changes from your branch? You can force push to this branch if necessary.

colagrosso commented 3 months ago

Yep, will do. I will have to force push to standardebooks:db-rewrite, but that's fine.