Fix: Creatability of page types incorrectly affecting the can_reorder_subpages() check

[ababic]

Fixes #3465

Where all of the page types in allowed_subpage_types have is_creatable=False on them, it is currently not possible to reorder pages that exist below instances of that page type.

I'm classifying this as a bug, because I have always interpreted the is_creatable=False mechanism as a way to prevent manual creation ONLY. Not only do I feel it is perfectly valid for programatically-created pages to exist at certain locations (and remain editable/publishable/deletable). But, projects also have a life outside of the current state of the codebase... it's quite possible for pages to have been created before the current code state (where is_creatable or allowed_subpage_types have changed); In which case, the ability to manage those pages (including 'reordering') should not be impacted.

[squash-labs]

Manage this branch in Squash

Test this branch here: https://ababicfixcan-reorder-children-80vsi.squash.io

[laymonage]

Thanks, I haven't looked into this but it seems like it would fix #3465

[laymonage]

Thanks @ababic!

I was also testing with subpage_types = [] which would now show the "Sort menu order" option as well, even though you cannot actually reorder the children (since the operation would basically still do a "move" to the parent page). However, this is an edge case that doesn't seem worth considering because this only happens if the child pages were created prior to setting the subpage_types = []. If there's a child page despite the model has subpage_types = [], that sounds like an unrelated data integrity issue.

That said, could you also add a test case on the reordering view itself, to ensure that this works when the action is performed?

We have a similar test case that relates to the parent_page_types restriction here:

wagtail/wagtail/admin/tests/pages/test_reorder_page.py

Lines 114 to 142 in 8ddf472

	class TestPageReorderWithParentPageRestrictions(TestPageReorder):
	"""
	This testCase is the same as the TestPageReorder class above, but with a different
	page type: BusinessChild has a parent_page_types restriction.
	This ensures that this restriction doesn't affect the ability to reorder pages.
	"""
	def setUp(self):
	# Find root page
	self.root_page = Page.objects.get(id=2)
	# root
	# |- index_page (BusinessIndex)
	# | |- child_1 (BusinessChild)
	# | |- child_2 (BusinessChild)
	# | |- child_3 (BusinessChild)
	self.index_page = BusinessIndex(title="Simple", slug="simple")
	self.root_page.add_child(instance=self.index_page)
	self.child_1 = BusinessChild(title="Child 1 of BusinessIndex", slug="child-1")
	self.index_page.add_child(instance=self.child_1)
	self.child_2 = BusinessChild(title="Child 2 of BusinessIndex", slug="child-2")
	self.index_page.add_child(instance=self.child_2)
	self.child_3 = BusinessChild(title="Child 3 of BusinessIndex", slug="child-3")
	self.index_page.add_child(instance=self.child_3)
	# Login
	self.user = self.login()

Would be nice to have the same for subpage_types 🙂