1. Gustavo Picón
  2. django-treebeard
  3. Issues
Issue #31 resolved

Incorrect ManyToMany field handling

Sardar Yumatov
created an issue

If the node has many-to-many field, then this can not be assigned until the node is saved first. Django forms always defer relational fields until post save()

Treebeard creates nodes with * *kwargs, assigning many-to-many values directly in the constructor. Standard django model fails here. Yes, it is not the problem of treebeard, but rather how django handles relational fields (consistency).

Current solution is to override init and save_base and defer all many-to-many fields until object is saved. This requires extra code.

Better fix: instead of node( kwargs) iterate over Model._meta.many_to_many, pop all these fields from kwargs. Then instance.save() and then assign all popped field values.

Comments (8)

  1. Alejandro Peralta

    alexei Vlasov I think the self.save() in line 35 of your patch:

    +                self.instance = self.Meta.model.add_root(** cl_data)
    +            self.save()
    

    makes the node be saved in the root position. That is it creates the node ok, but in level 1

    also what is the point of the m2m_fl in the following bit:

    +            m2m_fl = False
    +            for field in self.cleaned_data:
    +                if type(self.cleaned_data[field]) == list:
    +                    m2m_fl = True
    +                else:
    +                    cl_data[field] = self.cleaned_data[field]
    

    same as:

    +            for field in self.cleaned_data:
    +                if type(self.cleaned_data[field]) != list:
    +                    cl_data[field] = self.cleaned_data[field]
    

    Just a friendly advice,

    Cheers!

  2. Alejandro Peralta

    I tried this as save method, it seems to work:

        def save(self, commit=True):
            reference_node_id = 0
            if '_ref_node_id' in self.cleaned_data:
                reference_node_id = self.cleaned_data['_ref_node_id']
                # delete auxilary fields not belonging to node model
                del self.cleaned_data['_ref_node_id']
    
            if '_position' in self.cleaned_data:
                position_type = self.cleaned_data['_position']
                # delete auxilary fields not belonging to node model
                del self.cleaned_data['_position']          
    
            if self.instance.pk is None:
                cl_data = {}
                for field in self.cleaned_data:
                    if not isinstance(self.cleaned_data[field], (list, QuerySet)):
                        cl_data[field] = self.cleaned_data[field]
                if reference_node_id:
                    reference_node = self.Meta.model.objects.get(
                        pk=reference_node_id)
                    self.instance = reference_node.add_child(** cl_data)
                    self.instance.move(reference_node, pos=position_type)
                else:
                    self.instance = self.Meta.model.add_root(** cl_data)
    
            else:
                # this is needed in django >= 1.2
    ...
    
  3. Anonymous

    Ran into this problem moments ago, wrote a quick fix:

    (in mp_tree.MP_Node)

        @classmethod
        def add_root(cls, **kwargs):
            """
            Adds a root node to the tree.
    
            :raise PathOverflow: when no more root objects can be added
            """
    
            # do we have a root node already?
            last_root = cls.get_last_root_node()
    
            if last_root and last_root.node_order_by:
                # there are root nodes and node_order_by has been set
                # delegate sorted insertion to add_sibling
                return last_root.add_sibling('sorted-sibling', **kwargs)
    
            if last_root:
                # adding the new root node as the last one
                newpath = cls._inc_path(last_root.path)
            else:
                # adding the first root node
                newpath = cls._get_path(None, 1, 1)
            # Pop ManyToManyFields to add them later
            m2m = {}
            for field in cls._meta.many_to_many:
                if field.name not in kwargs:
                    continue
                m2m[field.name] = kwargs.pop(field.name)
            
            # creating the new object
            newobj = cls(**kwargs)
            newobj.depth = 1
            newobj.path = newpath
            # saving the instance before returning it
            newobj.save()
            # save m2m
            for field, value in m2m.items():
                setattr(newobj, field, value)
            newobj.save()
            transaction.commit_unless_managed()
            return newobj
    
  4. Anonymous

    Bump. Why is this still an open issue after over a year with a provided patch? First time out of the gate using django-treebeard, and I ran into this bug. Essentially, treebeard only works on models that have no m2m relations, which is useless, unless you add complicated overrides to each model or manually patch the source.

    I'd heard good things about treebeard, but I'm rethinking my options at this point. Something this core shouldn't just be sitting in the pipeline, so it doesn't instill much faith in the rest of the codebase for me.

  5. Log in to comment