Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Make LinkFormField not annoying #345

Open
gavinwahl opened this issue Aug 4, 2015 · 0 comments
Open

Make LinkFormField not annoying #345

gavinwahl opened this issue Aug 4, 2015 · 0 comments

Comments

@gavinwahl
Copy link
Member

I don't want to have to use LinkFormField and LinkFormMixin. I just want to use a regular model form.

I verified this is is possible by making LinkField inherit from Field, which means model forms will discover it. We can add a formfield method to LinkField that passes the db field into the form field, ei return LinkFormField(model_field=self).

I attempted this but had to give up because it was too hard to get choices to be populated lazily. I tried to put it in LinkFormField.init, but that doesn't work because we need to do the query to get the list of choices every time the form is rendered, not just when the form is created.

Here's my partial attempt if anyone's interested in finishing it

diff --git a/widgy/contrib/page_builder/models.py b/widgy/contrib/page_builder/models.py
index f711129..a4dfbc4 100644
--- a/widgy/contrib/page_builder/models.py
+++ b/widgy/contrib/page_builder/models.py
@@ -529,8 +529,9 @@ class Video(StrDisplayNameMixin, Content):
         return self.video


-class ButtonForm(LinkFormMixin, forms.ModelForm):
-    link = LinkFormField(label=_('Link'), required=False)
+class ButtonForm(forms.ModelForm):
+    class Meta:
+        fields = ['text', 'link']


 @widgy.register
diff --git a/widgy/models/links.py b/widgy/models/links.py
index a42ea06..ba76157 100644
--- a/widgy/models/links.py
+++ b/widgy/models/links.py
@@ -1,5 +1,6 @@
 import copy
 from operator import or_
+import warnings
 import itertools
 from six.moves import reduce

@@ -52,15 +53,14 @@ def points_to_links(linker, linkable):
                         if isinstance(field, LinkField)))


-class LinkField(WidgyGenericForeignKey):
-    """
-    TODO: Explore the consequences of using add_field in contribute_to_class to
-    make this a real field instead of a virtual_field.
-    """
+class LinkField(WidgyGenericForeignKey, Field):
     def __init__(self, ct_field=None, fk_field=None, *args, **kwargs):
         self.null = kwargs.pop('null', False)
         self._link_registry = kwargs.pop('link_registry', link_registry)
+        editable = kwargs.pop('editable', True)
         super(LinkField, self).__init__(ct_field, fk_field, *args, **kwargs)
+        # GenericForeignKey.__init__ forces editable to False
+        self.editable = editable

     def get_choices(self):
         return itertools.chain.from_iterable(choices for _, choices in self.get_choices_by_class())
@@ -72,6 +72,8 @@ class LinkField(WidgyGenericForeignKey):
                 for Model in sorted(self._link_registry, key=key))

     def contribute_to_class(self, cls, name):
+        self.attname = name
+
         if self.ct_field is None:
             self.ct_field = '%s_content_type' % name

@@ -98,6 +100,9 @@ class LinkField(WidgyGenericForeignKey):
         memodict[id(self)] = obj
         return obj

+    def formfield(self):
+        return LinkFormField(model_field=self)
+

 def get_composite_key(linkable):
     content_type = ContentType.objects.get_for_model(linkable)
@@ -116,9 +121,16 @@ def convert_linkable_to_choice(linkable):


 class LinkFormField(forms.ChoiceField):
-    def __init__(self, choices=(), empty_label="---------", *args, **kwargs):
-        self.empty_label = empty_label
-        super(LinkFormField, self).__init__(choices, *args, **kwargs)
+    def __init__(self, *args, **kwargs):
+        self.model_field = kwargs.pop('model_field', None)
+        if self.model_field is None:
+            warnings.warn(
+                "You no longer need to explicitly declare a LinkFormField. Just use a regular model form or pass model_field.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+        # skip ChoiceField, because it will try to set self.choices
+        super(forms.ChoiceField, self).__init__(*args, **kwargs)

     def clean(self, value):
         content_type_pk, _, object_pk = value.partition('-')
@@ -128,7 +140,9 @@ class LinkFormField(forms.ChoiceField):
         else:
             return None

-    def populate_choices(self, choice_map):
+    @property
+    def choices(self):
+        choice_map = self.model_field.get_choices_by_class()
         keyfn = lambda x: x[1].lower()

         choices = [(capfirst(Model._meta.verbose_name_plural),
@@ -139,7 +153,13 @@ class LinkFormField(forms.ChoiceField):
         if not self.required and self.empty_label is not None:
             choices.insert(0, ('', self.empty_label))

-        self.choices = choices
+        return choices
+
+    def __deepcopy__(self, memo):
+        # skip ChoiceField, because it will try to access _choices
+        result = super(forms.ChoiceField, self).__deepcopy__(memo)
+        result.model_field = self.model_field
+        return result


 def get_link_field_from_model(model, name):
@@ -150,23 +170,21 @@ def get_link_field_from_model(model, name):

 class LinkFormMixin(object):
     def __init__(self, *args, **kwargs):
+        warnings.warn(
+            "You no longer need to use LinkFormMixin. Just use a regular model form.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
         super(LinkFormMixin, self).__init__(*args, **kwargs)
         for name, field in self.get_link_form_fields():
             value = getattr(self.instance, name, None)
             model_field = get_link_field_from_model(self.instance, name)
-            field.initial = get_composite_key(value) if value else None
-            field.populate_choices(model_field.get_choices_by_class())
+            if not field.model_field:
+                # old style declaration of LinkFormField
+                field.initial = get_composite_key(value) if value else None
+                field.model_field = model_field

     def get_link_form_fields(self):
         return ((name, field)
                 for name, field in self.fields.items()
                 if isinstance(field, LinkFormField))
-
-    def save(self, *args, **kwargs):
-        if not self.errors:
-            cleaned_data = self.cleaned_data
-
-            for name, field in self.get_link_form_fields():
-                setattr(self.instance, name, cleaned_data[name])
-
-        return super(LinkFormMixin, self).save(*args, **kwargs)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant