bookwyrm icon indicating copy to clipboard operation
bookwyrm copied to clipboard

Support trigger migrations

Open dato opened this issue 1 year ago • 5 comments

  • 0299f2e2352773c53155ae815cd0a525d6408fe3 Add functional tests for search_vector triggers
  • e4d688665c7d54912b73752a270c4d4ead2a63b0 Remove index for author.search_vector, which is never used
  • 44ef928c3ceb8ce9b4426113551baa53563630f7 Alter object row IDs to force test failure in original code
  • 416a6caf2d4124b884fd1a04841908e572d92c61 Define author_search_vector_trigger via Author.Meta.triggers
  • bcb3a343d4514fd9bc2674a918755aaa832c8886 Fix JOIN in author_search_vector_trigger, add missing WHERE clause
  • 8df408e07eccefb934373f5c69b5068c1bee5046 Define search_vector_trigger via Book.Meta.triggers
  • 9bcb5b80ea9dd4f11cae26ad1fc752a7cb71dcde Further simplify bookwyrm_author trigger
  • bbfbd1e97ab201cd87c6414127c603f615923ce5 Add tests for trigger code (i.e. how search_vector is computed)
  • b5805accacb497be62cb4e7bb54a52258a1d12e3 Minor improvements to bookwyrm_book trigger code
  • d6eb390ceed8e6f63926aa8b54a4ef9314b93c75 Add test that forces book_authors_search_vector_trigger to execute

This PR uses django-pgtrigger to define triggers as part of model metadata. With this, Django's migration mechanism can be used to keep track of changes.

Done:

  • [x] tests for existing triggers (functional and structural)
  • [x] migrate trigger on bookwyrm_book table
  • [x] migrate trigger on bookwyrm_author table

In a later PR:

  • migrate trigger on bookwyrm_book_authors table

    (pgtriggers docs say the latter probably requires giving Book.authors its own "through" table, which we need anyway for things like #1120. But that's a more pervasive change, and I don't believe it belongs in this PR.)

    Also, about this trigger, see last commit.)

dato avatar Nov 25 '23 01:11 dato

fwiw, I think it might be a good idea to delay merging this until after having tagged the next release. I’m completely fine with that if you agree it makes sense.

dato avatar Jan 02 '24 03:01 dato

I agree! I think I'm going to make a release tomorrow morning with just what's already merged unless you think there's anything that should get in under the wire

mouse-reeve avatar Jan 02 '24 03:01 mouse-reeve

Only #3096 comes to mind (very optional, just to get it out there?; but I didn’t quite understand what it fixes, so I have no technical opinion).

Current main sounds great!

dato avatar Jan 02 '24 03:01 dato

I considered that one and decided to wait because I have zero recollection of why I had it set to the current way, but I know I did it on purpose, and I don't want to have to re-debug it tomorrow. But another day, I will have the willpower to do so 😆

mouse-reeve avatar Jan 02 '24 03:01 mouse-reeve

💯

Thank you for preparing this release.

dato avatar Jan 02 '24 03:01 dato