wagtail / wagtail-factories

Factory boy classes for wagtail
http://wagtail-factories.readthedocs.io/en/latest/
MIT License
102 stars 40 forks source link

Add support for stream block #55

Closed jams2 closed 2 years ago

jams2 commented 2 years ago

Overview

This pull request adds a StreamBlockFactory, which supersedes the existing StreamFieldFactory functionality, adding supported for nested streams. Support for current functionality is maintained, and the new functionality uses the same syntax as StreamFieldFactory for instantiating a stream value's inner blocks (e.g. MyPageFactory(body__0__struct__title="foobar").

With the new functionality, StreamBlockFactory subclasses can be defined and used as follows:

class StreamBlockFactoryA(StreamBlockFactory):
    struct = factory.SubFactory(MyStructBlock)
    text = factory.SubFactory(MyTextBlock)

    class Meta:
        model = StreamBlockA

class MyPageFactory(PageFactory):
    body = StreamFieldFactory(StreamBlockFactoryA)

    class Meta:
        model = MyPage

StreamBlockFactories can be used as sub-factories of StructBlockFactory, ListBlockFactory and StreamBlockFactory, for example:

class StreamBlockFactoryB(StreamBlockFactory):
    inner_stream = factory.SubFactory(StreamBlockFactoryA)

    class Meta:
        model = StreamBlockB

Breaking changes

This PR significantly changes the way that StreamFieldFactory works under the hood. The changes provide a deeper integration with factory_boy's object generation logic, resulting in the requirement that all BlockFactory values passed to an old-style StreamFieldFactory (dictionary-style) declaration must be wrapped in SubFactories (this is covered in more detail in the changes to the readme). I propose a new major version release (3.0)

jams2 commented 2 years ago

Thanks for your initial review @bcdickinson ! I've marked the items that are no longer relevant as resolved.

bcdickinson commented 2 years ago

I'm encountering an issue testing this locally when calling factory classes that contain StreamFieldFactory declarations and not passing any kwargs (e.g. just MyPageFactory(parent=some_page) without a body__0="my_struct_block" kwarg or some such). It seems like the step builder code can't handle an absence of block data and I get this (truncated stack trace to only show factory code frames):

  File "/venv/lib/python3.8/site-packages/factory/base.py", line 40, in __call__
    return cls.create(**kwargs)
  File "/venv/lib/python3.8/site-packages/factory/base.py", line 528, in create
    return cls._generate(enums.CREATE_STRATEGY, kwargs)
  File "/venv/lib/python3.8/site-packages/factory/django.py", line 117, in _generate
    return super()._generate(strategy, params)
  File "/venv/lib/python3.8/site-packages/factory/base.py", line 465, in _generate
    return step.build()
  File "/venv/lib/python3.8/site-packages/factory/builder.py", line 258, in build
    step.resolve(pre)
  File "/venv/lib/python3.8/site-packages/factory/builder.py", line 199, in resolve
    self.attributes[field_name] = getattr(self.stub, field_name)
  File "/venv/lib/python3.8/site-packages/factory/builder.py", line 344, in __getattr__
    value = value.evaluate_pre(
  File "/venv/lib/python3.8/site-packages/factory/declarations.py", line 48, in evaluate_pre
    return self.evaluate(instance, step, context)
  File "/venv/lib/python3.8/site-packages/wagtail_factories/blocks.py", line 113, in evaluate
    return self.stream_block_factory(**extra)
  File "/venv/lib/python3.8/site-packages/factory/base.py", line 40, in __call__
    return cls.create(**kwargs)
  File "/venv/lib/python3.8/site-packages/factory/base.py", line 528, in create
    return cls._generate(enums.CREATE_STRATEGY, kwargs)
  File "/venv/lib/python3.8/site-packages/wagtail_factories/blocks.py", line 44, in _generate
    step = cls._builder_class(cls._meta, params, strategy)
  File "/venv/lib/python3.8/site-packages/wagtail_factories/builder.py", line 46, in __init__
    indexed_block_names, extra_declarations = self.get_block_declarations(
  File "/venv/lib/python3.8/site-packages/wagtail_factories/builder.py", line 100, in get_block_declarations
    self.validate_block_indexes(indexed_block_names, factory_meta)
  File "/venv/lib/python3.8/site-packages/wagtail_factories/builder.py", line 108, in validate_block_indexes
    for declared, expected in zip_longest(indexes, range(max(indexes) + 1)):
ValueError: max() arg is an empty sequence

As discussed on our call just now, I think in this instance it would be sensible to just handle this by leaving the body empty and not throwing an error. Also, as we noted, there's currently no way of defining default data for the StreamBlockFactory, so I'll raise a new issue for that.

jams2 commented 2 years ago

@bcdickinson good catch, the same issue also existed with requesting the default value for a StreamBlockFactory in a StreamBlockFactory, e.g.

MyPageFactory(body__0="inner_stream")

Fixed in https://github.com/wagtail/wagtail-factories/pull/55/commits/e893a8a47f421437239c288ce1d493b87000d8d8

bcdickinson commented 2 years ago

Nice, I'd say this is there now! Happy for this to be merged and released.