Counter part for Avoid purging Revisions #10678

[disconnect821]

This is a counterpart for issue wagtail/wagtail#10678 which changes the foreign key relation of ABTest to PageRevision from models.CASCADE to models.PROTECT

[disconnect821]

I'll be opening PR for this by today itself.
Let me know if I should go ahead or not.

[Stormheg]

Hi @disconnect821, thanks for raising this as an issue first.

Changing the ForeignKey from CASCASE to PROTECT sounds fine to me. Be sure to base your work on my branch that has been updated to work with Wagtail 4.1+: https://github.com/Stormheg/wagtail-ab-testing/tree/support/wagtail42 The current main of this project only works with old versions of Wagtail and a lot has changed, making your change against the current main would be unwise.

When you make this change, could you please also add a test that proves ProtectedError is raised when an attempting to delete a revision in use by a AbTest model. You may find this test helpful. it already does 90% of what you need to test. All you need to do is check ProtectedError is raised when deleting a revision.

Regarding the above reference to 'basing your work on my branch' I recommend you take the following steps:

1. Fork this repository and clone it
2. Add my fork as a git remote (assuming you use git cli): $ git remote add Stormheg https://github.com/Stormheg/wagtail-ab-testing.git
3. Fetch all remotes $ git fetch --all
4. Checkout my branch: $ git checkout support/wagtail42
5. Rename the branch to something more sensible for the change you are making: $ git branch -m fix/mark-revision-relation-as-protected
6. Make your changes and commit your changes!
7. Create a pull request. It will include all changes made on my branch, be sure to mention in the pull request description that your work is dependant on my PR: Wagtail 4.1 and 4.2 support (revised) #52. It is also good practice to reference the issue your PR is fixing, that would be this issue.

Thanks again for wanting to work on this! ❤️

[disconnect821]

Thanks @Stormheg, I'm on it.

[disconnect821]

@Stormheg Can you provide me some tips for writing tests as I'm not very experienced with tests?

[salty-ivy]

@disconnect821 you can try something like this

from django.db.models.deletion import ProtectedError

def test_check_raise_protect_error(self):
        """
        Test to check if ProtectedError is raised when associated revision is deleted
        """
        with self.assertRaises(ProtectedError):
            self.revision.delete()

Also @Stormheg could you let me have some insights of how to set-up the dependencies for this repo and runtests?
I wanted to use wagtail-ab-testing to extending it for some other use cases, was having few difficulties with dependency setup and running tests.

[Stormheg]

@salty-ivy running the tests is not that well documented in the readme, here is how I did it.

Assuming you are inside a virtualenv

# Install wagtail-ab-testing in editable mode along with its testing dependencies
pip install -e ".[testing]"

# Run the tests
python testmanage.py test

[Stormheg]

@disconnect821 If you need some general information on how to write tests I recommend part 5 of the Django tutorial: https://docs.djangoproject.com/en/4.2/intro/tutorial05/.

salty-ivy provided you with an example on how to check if a specific type of exception is raised. Thank you Aman Pandey!

In my earlier comment I outlined what I would expect you to test. I hope this gives you enough guidance on what to do. It is up to you to research and gather the necessary knowledge to complete the goal of writing a test.

Writing tests is part of developing quality software. I hope you see this an opportunity to learn something new that will bring you further in your journey as a software developer.