-
Notifications
You must be signed in to change notification settings - Fork 194
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
Have CompoundBlock extract subblocks #9756
Conversation
7c1ddbc
to
3623869
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
assert subject.valid? | ||
end | ||
|
||
test "invalid when featured content blocks are invalid" do | ||
invalid_blocks_config = [{ "invalid" => "because I do not have a type" }] | ||
subject = LandingPage::FeaturedBlock.new(@valid_featured_block_config, @valid_featured_images, invalid_blocks_config) | ||
subject = LandingPage::FeaturedBlock.new( | ||
@valid_featured_block_config.tap { _1["featured_content"]["blocks"] = invalid_blocks_config }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this _1
really unpleasant, excited for being able to use it
instead in Ruby 3.4 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've come to quite like it, mostly through trying to golf my way through advent of code. Agree it
will be nicer.
I'd be happy to do this if it's less smelly:
@valid_featured_block_config.tap { _1["featured_content"]["blocks"] = invalid_blocks_config }, | |
@valid_featured_block_config.tap { |config| config["featured_content"]["blocks"] = invalid_blocks_config }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or I could do this I think:
@valid_featured_block_config.tap { _1["featured_content"]["blocks"] = invalid_blocks_config }, | |
@valid_featured_block_config.merge("featured_content" => { "blocks" => invalid_blocks_config }), |
... instead of doing it with pattern matching in BlockFactory. We want these to be optional in most cases, but putting them in the patterns means we only use the correct block type if they're present. Instead, we just use type: in the pattern, and have the blocks take care of finding their config inside source.
3623869
to
a46d2b9
Compare
... instead of doing it with pattern matching in BlockFactory.
We want these to be optional in most cases, but putting them in the patterns means we only use the correct block type if they're present. Instead, we just use type: in the pattern, and have the blocks take care of finding their config inside source.
This cleans up the following piece of tech debt: https://trello.com/c/zfUEbZdo/224-simplify-compound-block-models-in-whitehall