Unable to delete a page after running an A/B test

[danielfmiranda]

If I start an A/B test for a page and then attempt to delete it once the AB test has concluded, I am met with a 500 error mentioning "("Cannot delete some instances of model 'Page' because they are referenced through protected foreign keys: '<Page_Model>.page_ptr'.", {<AbTest: AbTest object (2)>})"

[Stormheg]

This is inadvertent behaviour since #54, which marked the page revision an ab test references as protected to prevent Wagtail's purge_revisions from cleaning up revisions that are used by an ab test.

When deleting a page, Wagtail will try to delete all revisions along with it which we are trying to prevent to avoid data loss but should allow in this case.

We should show some sort of confirmation view when deleting a page that has ab tests. We can use the before_delete_page hook to hijack the delete view.

I think this best be solved by:

1. Hijacking the page delete view by listening for before_delete_page and checking if there are ab tests for that page.
2. If there are, redirect to another admin view showing an overview of AB tests for that page.
3. A confirmation button on the overview will delete the ab test data and redirects back to the actual Wagtail page delete confirmation view.
4. Deleting a page will now succeed as there are no AB tests that references the page anymore.

Workaround

@danielfmiranda as a temporary workaround, I think you can define a delete() method on your page class that looks like this (untested).

from wagtail.models import Page

class MyPageType(Page):
   def delete(self, *args, **kwargs):
        self.ab_tests.all().delete()
        return super().delete(*args, **kwargs)   

[joelwilliam2005]

I was able to reproduce this issue and I'm trying to work on it.

Also found that the time when the test was created is shown in different timezone. Time difference can be seen in my taskbar and the time in the test page. Although this should be addressed in a separate issue, let me know if I should raise an issue for that.

[joelwilliam2005]

Let me know if this is going in the right direction.

Screencast.From.2024-12-13.09-37-07.1.mp4

I used a hook with before_delete_page that shows the new view and when clicked proceed it deletes the ab tests and redirects to the original confirm_delete_page view.

[joelwilliam2005]

Updated ab test delete view page.

[Stormheg]

This looks great @joelwilliam2005! Thank you very much for working on this ❤️

The table format makes sense to me, but I would suggest the following order of columns for the table:

1. Test name (link, takes you to the ab test)
2. Goal event
3. Status (use AbTest.get_status_description for a rich display)
4. Created by
5. Start date (first_started_at)

By default, the tests should be ordered by descending first_started_date (most recently created tests first, oldest tests last)

I would also change the wording of the confirmation to make it clear the data is lost forever and cannot be restored.

All [count] A/B tests associated with this page will be permanently deleted.

Yes, delete A/B tests / No, don't delete

Not sure if we if and for what permissions this should account. @danielfmiranda do you have input for us on this?

[joelwilliam2005]

Updated columns:

• Test name (link, takes you to the ab test)
• Goal event
• Status (use AbTest.get_status_description for a rich display)
• Created by
• Start date (first_started_at)
• Ordered by descending first_started_date
• Updated confirmation statement.
  

Just want to mention that all the changes were made in the bakery demo site and not in the A/B testing code base.
Please let me know how you’d like the PR to be raised, or if it is required at all.

[Stormheg]

@joelwilliam2005 looks amazing! Please raise a PR against wagtail-ab-testing so every project using ab-testing automatically gets this nice overview when deleting a page😄