id summary reporter owner description type status component version severity resolution keywords cc stage has_patch needs_docs needs_tests needs_better_patch easy ui_ux 1407 Clarify add(), create(), remove() on ManyRelatedObjectsDescriptor for ForeignKeys jdunck@… Russell Keith-Magee "---------- Forwarded message ---------- From: Russell Keith-Magee Date: Feb 26, 2006 9:30 PM .... Hi all, In working on an implementation for the remove() and clear() methods for ManyRelatedObjectsDescriptor (i.e., mypolls.choices.remove() and mypoll.choices.clear()), I have noticed that add() on a o2m descriptor creates a new Choice object, whereas add() on a m2m descriptor takes a list of pre-existing objects and adds them to the membership set. I have had a trawl through the wiki and mailing list logs, but I didn't find any discussion covering this issue - it looks like the inconsistency is an accident of porting the old API to the new. o2m field behavior follows the old add_choices() behaviour; add/remove is a new feature for m2m, replacing the set_articles() call. To me, the difference in add() behaviours is a potentially confusing inconsistency. In addition, the m2m interpretation of add() makes more sense (to me, anyway) than the o2m behaviour. To that end, I would like to propose the following: 1) Delete Manager.add(). Poll.objects.add(kwargs) doesn't strike me as a particularly intuitive alternative to p = Poll(kwargs); p.save() - the only reason it seems to be there is to simplify the o2m descriptor. 2) Rename the existing mypoll.choices.add() to mypoll.choices.create() 3) Add a mypoll.choices.add() that will take a list of already constructed objects (to match the m2m add), rather than create a single new object. This would make mypoll.choices.add(choice) an analog of ""choice.poll_id = mypoll.id; choice.save()"" 4) Delete remove() and clear() from the o2m descriptor. I'm not entirely happy with (4) because of the lack of symmetry between o2m and m2m descriptors, but the only alternatives I can see are: - have remove(*objs) delete all the named objects, and clear() delete all objects with the matching key. This seems potentially confusing, since the m2m remove/clear doesn't delete anything but the relationship. Also, there is the potential for error if an object provided to remove() isn't part of the o2m set. - Clear/null the ForeignKey field for all objects in the removed set. This would only work for objects where the field has blank=True set, so blank=False fields would have to throw an error (or not have the method available). This also strikes me as confusing. - rename add() for o2m descriptors to something else, to remove the implication that there should be a corresponding remove() and clear() method. However, I can't think of a reasonable alternative. " enhancement new Core (Other) magic-removal normal Unreviewed 0 0 0 0 0 0