How to limit targets in move page view

[barsch]

It would be nice to have a way to disallow certain targets in the move view - e.g. moving pages outside of a "Home" node or disallow moving pages before fixed pages (read: node stays always on top in menu).

I tried to monkey patch feincms3.admin.MoveForm.__generate_choices method and filter only for allowed pages but to no success.

I somehow got it working now by overwriting the feincms3.admin.MoveForm.clean() method - but that feels wrong and is not very user friendly as all the move targets are still shown in the admin UI:

class MyMoveForm(MoveForm):
    def clean(self):
        data = super().clean()
        if not data.get("new_location"):
            return data
        if self.request.user.is_superuser:
           return data

        # don't allow to create a new root node
        relative = data["relative"]
        first_or_right = data["first_or_right"]
        if not relative:
            raise ValidationError("You do not have the permission to move this page here!")
        elif first_or_right =='right' and not relative.parent:
            raise ValidationError("You do not have the permission to move this page here!")


@admin.register(models.Page)
class PageAdmin(ContentEditor, TreeAdmin):
    ...

    def move_view(self, request, obj):
        return self.action_form_view(
            request, obj, form_class=MyMoveForm, title=f"Move {obj}"
        )

An option to disallow certain pages as move target or any idea how to tackle that in a smart way is very appreciated.

[matthiask]

I don't immediately have a good idea for this. Maybe you could try overriding MoveForm.__init__ and add self.fields["new_location"].choices = _generate_choices(self.get_queryset(self.request).filter(...your...filters...).with_tree_fields()) or something like that after super.__init__()?

I'm happy to change the code, and I'd also appreciate a pull request if you find a solution yourself!

[barsch]

Alright I tinkered a bit with it and its definitively not trivial to find a good solution especially without a non database heavy way to get the previous or next sibling of any given node during the choice generation.

For now I overwrote MoveForm._generate_choices to use three new methods can_move_root, can_move_right and can_move_first, which are used to customize the _generate_choice logic. To have the original behaviour all three methods should just return True. Those methods are also used in the clean method to prevent any malicious pampering with the move form.

One can now use the self.queryset and self.request properties in those 3 methods to limit the move targets. However, I opted for two new BooleanField fields can_move_right and can_move_first within my Page model. This way I can exactly define what move targets are allowed for fixed pages without extra database queries or additional logic.

Don't know if this is to convoluted/obscure to be included into feincms3 - decision is up to you - I'm happy to create a PR if wanted.

admin.py

class MyMoveForm(MoveForm):
    """
    Custom move form
    """

    def can_move_root(self):
        # custom code here - default: return True
        if self.request.user.is_superuser:
            return True
        return False

    def can_move_right(self, node):
        # custom code here - default: return True
        if self.request.user.is_superuser:
            return True
        return getattr(node, "can_move_right", True)

    def can_move_first(self, node):
        # custom code here - default: return True
        if self.request.user.is_superuser:
            return True
        return getattr(node, "can_move_first", True)

    def _generate_choices(self, queryset):
        children = defaultdict(list)
        for node in queryset:
            children[node.parent_id].append(node)

        def _text_indent(depth):
            return mark_safe(f' style="text-indent:{depth * 30}px"')

        choices = []

        def _iterate(parent_id):
            for index, node in enumerate(children[parent_id]):
                if node == self.instance:
                    choice = (
                        "",
                        format_html(
                            '<div class="mv is-self"{}><strong>{}</strong>',
                            _text_indent(node.tree_depth),
                            node,
                        ),
                    )
                    if index == 0 and parent_id:
                        # Moving the first child of parent_id; do not remove parent_id
                        choices[-1] = (
                            choices[-1][0],
                            mark_safe(choices[-1][1].replace("mv-mark", "hidden")),
                        )
                        choices.append(choice)
                    else:
                        choices[-1] = choice
                    continue

                if self.can_move_first(node):
                    choices.append(
                        (
                            f"{node.id}:first",
                            format_html(
                                '<div class="mv to-first"{}><strong>{}</strong>'
                                '<div class="mv-mark"{}>&rarr; {}</div></div>',
                                _text_indent(node.tree_depth),
                                node,
                                _text_indent(node.tree_depth + 1),
                                _("move here"),
                            ),
                        )
                    )
                else:
                    choices.append(
                        (
                            f"{node.id}:first",
                            format_html(
                                '<div class="mv to-first"{}><strong>{}</strong>',
                                _text_indent(node.tree_depth),
                                node,
                            ),
                        )
                    )
                _iterate(node.id)
                if self.can_move_right(node):
                    choices.append(
                        (
                            f"{node.id}:right",
                            format_html(
                                '<div class="mv to-right mv-mark"{}>&rarr; {}</div>',
                                _text_indent(node.tree_depth),
                                _("move here"),
                            ),
                        )
                    )

        if self.can_move_root():
            choices.append(
                (
                    "0:first",
                    format_html(
                        '<div class="mv to-root mv-mark">&rarr; {}</div>',
                        _("move here"),
                    ),
                )
            )
        _iterate(None)
        return choices

    def clean(self):
        data = super().clean()
        if not data.get("new_location"):
            return data

        if self.request.user.is_superuser:
            return data

        relative = data["relative"]
        first_or_right = data["first_or_right"]

        if not relative and not self.can_move_root():
            raise ValidationError(
                "You do not have the permission to move this page here!"
            )
        elif first_or_right == "first" and not self.can_move_first(relative):
            raise ValidationError(
                "You do not have the permission to move this page here!"
            )
        elif first_or_right == "right" and not self.can_move_right(relative):
            raise ValidationError(
                "You do not have the permission to move this page here!"
            )
        return data

@admin.register(models.Page)
class PageAdmin(ContentEditor, TreeAdmin):
    ...

    def move_view(self, request, obj):
        return self.action_form_view(
            request, obj, form_class=MyMoveForm, title=f"Move {obj}"
        )

models.py

class Page(AbstractPage, RedirectMixin):
    can_move_first = models.BooleanField(
        verbose_name="Sub page allowed",
        default=True,
    )
    can_move_right = models.BooleanField(
        verbose_name="Subsequent page allowed",
        default=True,
    )

    ...