Amsterdam-Music-Lab / MUSCLE

An application to easily set up and run online listening experiments for music research.
https://www.amsterdammusiclab.nl/
MIT License
4 stars 1 forks source link

Fixed(migration): Refactor block content migration script #1256

Closed drikusroor closed 2 months ago

drikusroor commented 2 months ago

This pull request includes a fix for the migration script that was causing some errors as it tried to get the rules of one or more blocks whose configured rules id was invalid on the test environment.

Update:

After closer inspection, the error really seems to be 'Block' object has no attribute 'get_rules'. Ergo, we cannot call the method in the first place. However, it did work in our (Drikus, Berit)'s local environment so we might have a discrepancy in the way we use "Historical models". It's probably best practice to not use class methods in migrations as these can change over time. A better solution, then, might be to copy the get_rules function into the migration and call it directly. However, we'll still be dependent on BLOCK_RULES, and if this list ever gets removed that might also bring us in a pickle.

sentry-io[bot] commented 2 months ago

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: backend/experiment/migrations/0054_migrate_block_content.py

Function Unhandled Issue
migrate_block_content AttributeError: 'Block' object has no attribute 'get_rules' experiment.migrations.0054_migrate_block_content in migrate_bl...
Event Count: 5.1k

Did you find this useful? React with a 👍 or 👎

drikusroor commented 2 months ago

See my Teams comment: perhaps better to continue in the except clause, and log which blocks got caught, so we can identify if specific blocks are the culprit here. (At least I wouldn't be confident that messing around with block.phase further on would actually work if anything with the block object itself is off.)

I could also add the phase earlier in the process and do the rules/consent part afterwards?