Fix call_target__attribute handling

Co-authored-by: Johan Lübcke <johan@lubcke.se>

Author: boxed

docs/test_doc_cookbook_queries.py

@@ -118,6 +118,14 @@ def test_how_do_I_set_the_name_for_a_filter(big_discography):
     )
 
     # @test
+    t = track_table.bind(request=req('get', **{'-query/query': 'artist="black sabbath"'}))
+    assert t.query.get_advanced_query_param() == '-query/query'
+    assert t.query.filters.album_artist_name.attr == 'album__artist__name'
+    assert t.query.filters.album_artist_name.query_name == 'artist'
+    assert not t.query.form.get_errors(), t.query.form.get_errors()
+    assert repr(t.query.get_q()) == repr(Q(album__artist__name__iexact='black sabbath'))
+    assert {row.album.artist.name for row in t.rows} == {'Black Sabbath'}, [row.album.artist.name for row in t.rows]
+
     show_output(track_table, '?-query%2Fquery=artist%3D"black+sabbath"')
     # @end
 

iommi/declarative/namespace.py

@@ -105,6 +105,7 @@ def __call__(self, *args, **kwargs):
 
         if isinstance(call_target, Namespace):
             if 'call_target' in call_target:
+                # TODO: HOLY CRAP DELETE
                 # Override of the default
                 call_target.pop('attribute', None)
                 call_target.pop('cls', None)

iommi/edit_table.py

@@ -54,6 +54,7 @@
     bind_member,
     bind_members,
     refine_done_members,
+    reify_conf,
 )
 from iommi.refinable import (
     Refinable,
@@ -178,12 +179,17 @@ class EditColumn(Column):
 
     field: Field = Refinable()
 
+    @classmethod
+    @dispatch
+    def from_model(cls, model=None, model_field_name=None, model_field=None, **kwargs):
+        return reify_conf(cls._from_model(model=model, model_field_name=model_field_name, model_field=model_field, **kwargs))
+
     @classmethod
     @dispatch(
         filter__call_target__attribute='from_model',
         bulk__call_target__attribute='from_model',
     )
-    def from_model(cls, model=None, model_field_name=None, model_field=None, **kwargs):
+    def _from_model(cls, model=None, model_field_name=None, model_field=None, **kwargs):
         return member_from_model(
             cls=cls,
             model=model,

iommi/form.py

@@ -111,6 +111,7 @@
     bind_member,
     bind_members,
     refine_done_members,
+    reify_conf,
 )
 from iommi.page import (
     Page,
@@ -1162,7 +1163,13 @@ def grouped_choice_tuples(self):
             return groups
 
     @classmethod
+    @dispatch
     def from_model(cls, model=None, model_field_name=None, model_field=None, **kwargs):
+        return reify_conf(cls._from_model(model=model, model_field_name=model_field_name, model_field=model_field, **kwargs))
+
+    @classmethod
+    @dispatch
+    def _from_model(cls, model=None, model_field_name=None, model_field=None, **kwargs):
         return member_from_model(
             cls=cls,
             model=model,
@@ -1499,6 +1506,7 @@ def many_to_many_reverse(cls, model_field, **kwargs):
         return cls.many_to_many(model_field=model_field, **kwargs)
 
     @classmethod
+    @with_defaults
     def hardcoded(cls, **kwargs):
         assert (
             'parsed_data' in kwargs
@@ -1764,13 +1772,19 @@ def on_refine_done(self):
         extra_action_defaults = Namespace()
         crud_type = self.extra.get('crud_type')
         if 'title' not in self.iommi_namespace and crud_type is not None:
-            self.title = lambda form, **_: capitalize(
-                gettext_lazy('%(crud_type)s %(model_name)s')
-                % dict(
-                    crud_type=gettext_lazy(form.extra.crud_type),
-                    model_name=(form.model or form.instance)._meta.verbose_name,
+            def default_title(form, **_):
+                model = (form.model or form.instance)
+                if model is None:
+                    return capitalize(gettext_lazy(form.extra.crud_type))
+
+                return capitalize(
+                    gettext_lazy('%(crud_type)s %(model_name)s')
+                    % dict(
+                        crud_type=gettext_lazy(form.extra.crud_type),
+                        model_name=model._meta.verbose_name,
+                    )
                 )
-            )
+            self.title = default_title
             extra_action_defaults = setdefaults_path(
                 extra_action_defaults,
                 submit__display_name=lambda form, **_: gettext_lazy('Save') if form.extra.crud_type == 'edit' else capitalize(gettext_lazy(form.extra.crud_type)),

iommi/form__tests.py

@@ -88,6 +88,7 @@
 from iommi.from_model import (
     member_from_model,
 )
+from iommi.member import reify_conf
 from iommi.page import (
     Page,
 )
@@ -1061,12 +1062,12 @@ def test_choice_not_required():
     class MyForm(Form):
         foo = Field.choice(required=False, choices=['bar'])
 
-    assert do_post(MyForm()).fields.foo.value is None
-    assert do_post(MyForm(), foo='bar').fields.foo.value == 'bar'
-    form = do_post(MyForm(), do_post_key_validation=False, baz='bar')
+    assert do_post(MyForm.create()).fields.foo.value is None
+    assert do_post(MyForm.create(), foo='bar').fields.foo.value == 'bar'
+    form = do_post(MyForm.create(), do_post_key_validation=False, baz='bar')
     assert form.fields.foo.value is None
 
-    form = do_post(MyForm(), foo='baz')
+    form = do_post(MyForm.create(), foo='baz')
     assert form.fields.foo.value is None
     assert form.get_errors() == {'fields': {'foo': {'baz not in available choices'}}}
 
@@ -1400,6 +1401,10 @@ class FooModel(Model):
     assert not Field.from_model(FooModel, 'c').refine_done().required
     assert Field.from_model(FooModel, 'd').refine_done().required
 
+    # Conf overrides default
+    assert Field.from_model(FooModel, 'a', required=True).refine_done().required
+    assert not Field.from_model(FooModel, 'd', required=False).refine_done().required
+
 
 @pytest.mark.django
 @pytest.mark.filterwarnings("ignore:Model 'tests.foomodel' was already registered")
@@ -1549,13 +1554,15 @@ def test_overriding_parse_empty_string_as_none_in_shortcut():
         parse_empty_string_as_none='foo',
     )
     # test overriding parse_empty_string_as_none
-    x = member_from_model(
-        cls=Field,
-        model=Foo,
-        model_field=CharField(blank=True),
-        factory_lookup={CharField: s},
-        factory_lookup_register_function=register_field_factory,
-        defaults_factory=field_defaults_factory,
+    x = reify_conf(
+        member_from_model(
+            cls=Field,
+            model=Foo,
+            model_field=CharField(blank=True),
+            factory_lookup={CharField: s},
+            factory_lookup_register_function=register_field_factory,
+            defaults_factory=field_defaults_factory,
+        )
     ).refine_done()
 
     assert 'foo' == x.parse_empty_string_as_none
@@ -1643,9 +1650,23 @@ def test_field_from_model_many_to_one_foreign_key():
 def test_register_field_factory():
     from tests.models import FooField, RegisterFieldFactoryTest
 
+    f = Field()
+
+    register_field_factory(FooField, factory=lambda **kwargs: f)
+
+    assert Field.from_model(RegisterFieldFactoryTest, 'foo') is f
+
+
+@pytest.mark.django
+def test_register_field_factory_disallow_non_dict_non_field():
+    from tests.models import FooField, RegisterFieldFactoryTest
+
     register_field_factory(FooField, factory=lambda **kwargs: 7)
 
-    assert Field.from_model(RegisterFieldFactoryTest, 'foo') == 7
+    with pytest.raises(AssertionError) as e:
+        assert Field.from_model(RegisterFieldFactoryTest, 'foo')
+
+    assert str(e.value) == 'Factories must return a configuration dict or an instance of Field. Got int: "7"'
 
 
 def shortcut_test(shortcut, raw_and_parsed_data_tuples, normalizing=None, is_list=False):
@@ -2670,7 +2691,7 @@ class Meta:
 def test_shortcut_to_subclass():
     class MyField(Field):
         @classmethod
-        @with_defaults()
+        @with_defaults
         def my_shortcut(cls, **kwargs):
             return cls(**kwargs)  # pragma: no cover: we aren't testing that this shortcut is implemented correctly
 
@@ -3526,12 +3547,14 @@ def test_initial_is_set_to_default_of_model():
 
 
 def test_shoot_config_into_auto_dunder_field():
-    Form(
+    other_display_name = 'something else'
+    form = Form(
         auto__model=FieldFromModelOneToOneTest,
         # attr `foo_one_to_one__foo` creates a field named `foo_one_to_one_foo`. Note that the `__` is collapsed to one `_`!
         auto__include=['foo_one_to_one__foo'],
-        fields__foo_one_to_one_foo__display_name='bar',
+        fields__foo_one_to_one_foo__display_name=other_display_name,
     ).bind(request=req('get'))
+    assert form.fields.foo_one_to_one_foo.display_name == other_display_name
 
 
 @pytest.mark.django_db
@@ -3747,7 +3770,7 @@ def test_action_callbacks_should_be_lazy():
 def test_form_template_override_bug():
     class MyForm(Form):
         @classmethod
-        @with_defaults()
+        @with_defaults
         def case1(cls, **kwargs):
             kwargs['template'] = 'case1'
             return cls(**kwargs)
@@ -4090,3 +4113,42 @@ def test_parsed_data_does_not_crash_on_non_editable():
             parsed_data=lambda **_: 1,
         )
     ).bind(request=req('POST', **{'-submit': ''}))
+
+
+def test_call_target__attribute():
+    class FooForm(Form):
+        class Meta:
+            auto__model = Foo
+            auto__include = ['foo']
+            fields__foo__call_target__attribute = 'hardcoded'
+            fields__foo__parsed_data = 123
+
+    form = FooForm().bind(request=req('post', foo='456'))
+    assert form.fields.foo.value == 123
+
+
+def test_call_target__attribute_override_to_radio_button():
+    class FooForm(Form):
+        class Meta:
+            auto__model = ChoicesModel
+            fields__color__call_target__attribute = 'radio'
+
+    form = FooForm().bind(request=req('get'))
+    assert form.fields.color.iommi_shortcut_stack == [
+        'radio',
+        'choice',
+    ]
+
+
+def test_shoot_config_into_related_field():
+    class FooForm(Form):
+        class Meta:
+            auto__model = Bar
+            auto__include = ['foo__foo']
+            fields__foo_foo__call_target__attribute = 'hardcoded'
+            fields__foo_foo__parsed_data = 123
+
+    form = do_post(FooForm.create(), do_post_key_validation=False, foo='456')
+    assert form.fields.foo_foo.iommi_shortcut_stack == ['hardcoded']
+    assert form.fields.foo_foo.parsed_data == 123
+    assert form.fields.foo_foo.value == 123

iommi/from_model.py

@@ -24,7 +24,10 @@
     Namespace,
     setdefaults_path,
 )
-from iommi.evaluate import evaluate
+from iommi.evaluate import (
+    evaluate,
+    evaluate_strict,
+)
 from iommi.refinable import (
     Refinable,
     RefinableObject,
@@ -52,27 +55,23 @@ def create_members_from_model(
         if exclude is not None and model_field_name in exclude:
             continue
         name = model_field_name.replace('__', '_')
-        definition = Namespace(
+        conf = Namespace(
             _name=name,
-            call_target__cls=member_class,
-            call_target__attribute='from_model',
             model_field_name=model_field_name,
             model=model,
         )
         if default_included is False:
             setdefaults_path(
-                definition,
+                conf,
                 include=False,
             )
         if include is not None and name in include:
             setdefaults_path(
-                definition,
+                conf,
                 include=True,
             )
 
-        member = definition()
-        if member is not None:
-            members[name] = member
+        members[name] = member_class._from_model(**conf)
 
     return members
 
@@ -107,7 +106,7 @@ def member_from_model(
                 model_field_name=field_path_rest,
                 **kwargs,
             )
-            result = result.refine(attr=model_field_name)
+            result.attr = model_field_name
             return result
     else:
         if model is None:
@@ -144,7 +143,17 @@ def no_factory_defined(**_):
         return None
 
     # Not strict evaluate on purpose
-    factory = evaluate(factory, __match_empty=False, model_field=model_field, model_field_name=model_field_name)
+    if isinstance(factory, dict):
+        conf_or_instance = factory
+    else:
+        conf_or_instance = evaluate_strict(factory, model_field=model_field, model_field_name=model_field_name)
+
+    if isinstance(conf_or_instance, cls):
+        return conf_or_instance
+
+    assert isinstance(conf_or_instance, dict), f'Factories must return a configuration dict or an instance of {cls.__name__}. Got {type(conf_or_instance).__name__}: "{conf_or_instance}"'
+    conf = conf_or_instance
+    del conf_or_instance
 
     setdefaults_path(
         kwargs,
@@ -153,16 +162,16 @@ def no_factory_defined(**_):
     )
 
     defaults = defaults_factory(model_field)
-    if isinstance(factory, Namespace):
-        factory = setdefaults_path(
-            Namespace(),
-            factory,
-            defaults,
-        )
-    else:
-        kwargs.update(**defaults)
 
-    return factory(model_field=model_field, model_field_name=model_field_name, model=model, **kwargs)
+    return setdefaults_path(
+        Namespace(),
+        kwargs,
+        conf,
+        defaults,
+        model_field=model_field,
+        model_field_name=model_field_name,
+        model=model,
+    )
 
 
 def get_field_by_name(model: Type[Model]) -> Dict[str, DjangoField]: