Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Allow developers to extend StreamValue #11830

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

ababic
Copy link
Contributor

@ababic ababic commented Apr 6, 2024

I have encountered a couple of situations where I wanted to customise the behaviour of StreamValue:

  1. In a version of StreamField that doesn't include block definitions in migrations, I wanted to override the logic in StreamValue.to_python() that strips out values for blocks that are not recognised.
  2. In a client project, I wanted to add functionality to StreamValue to autogenerate unique IDs for heading blocks and use them (and the heading text) to generate a structured 'minimap' for users.

However, using anything other than the built-in StreamValue class is currently quite difficult, due to various assumptions in the codebase. This PR aims to address these assumptions, allowing the value class to be more easily substituted.

@ababic ababic marked this pull request as draft April 6, 2024 19:36
Copy link

squash-labs bot commented Apr 6, 2024

Manage this branch in Squash

Test this branch here: https://ababiccustom-streamvalue-suppo-xetco.squash.io

@ababic ababic force-pushed the custom-streamvalue-support branch 2 times, most recently from 0cee20a to 5b1c9d8 Compare April 6, 2024 19:53
@ababic ababic marked this pull request as ready for review April 6, 2024 20:32
wagtail/blocks/stream_block.py Outdated Show resolved Hide resolved
@@ -111,6 +111,10 @@ def __init__(self, block_types, use_json_field=True, **kwargs):

self.stream_block.set_meta_options(block_opts)

@property
def value_class(self):
return self.stream_block.value_class
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in __init__ we have:

if isinstance(block_types, Block):
    # use the passed block as the top-level block
    self.stream_block = block_types

but StreamField really expectes BaseStreamBlock (which inherits from Block). I wonder if the check should change to BaseStreamBlock, or perhaps this to use getattr.

I couldn't find any tests of what happens if you try to initialize StreamField with a non-StreamBlock block. Perhaps I am overthinking this

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find any tests of what happens if you try to initialize StreamField with a non-StreamBlock block. Perhaps I am overthinking this

This isn't currently supported (but might be in future, once #9746 lands).

@ababic ababic force-pushed the custom-streamvalue-support branch from 77a115c to 70cd03e Compare April 9, 2024 13:23
Copy link
Collaborator

@gasman gasman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ababic! A few suggestions, nothing too major. (Unfortunately #9746 / #11850 have conflicted with this, but hopefully the changes are actually for the better as they mean more of the logic is contained in wagtail.blocks.stream_block rather than leaking into wagtail.fields.StreamField...)

wagtail/blocks/stream_block.py Outdated Show resolved Hide resolved
def __init__(self, local_blocks=None, search_index=True, **kwargs):
self._constructor_kwargs = kwargs
self.search_index = search_index
self.value_class = kwargs.get("value_class", self.default_value_class)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should define this as part of the inner Meta class, like BaseStructBlock does - that way, subclasses can define this in their own Meta class, which is probably a more natural way of working than passing it to the StructBlock constructor.

wagtail/tests/test_streamfield.py Outdated Show resolved Hide resolved
@ababic ababic force-pushed the custom-streamvalue-support branch from 70cd03e to b2c79f2 Compare April 20, 2024 14:52
@ababic ababic force-pushed the custom-streamvalue-support branch from 17d6a66 to c75b7f9 Compare April 20, 2024 15:01
@ababic ababic marked this pull request as draft April 20, 2024 16:18
@ababic ababic force-pushed the custom-streamvalue-support branch from 2bf0bd0 to 24f63dd Compare April 22, 2024 08:20
@ababic ababic force-pushed the custom-streamvalue-support branch from 24f63dd to 020c3a5 Compare April 22, 2024 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants