Add ContentBlocks by mrcasals · Pull Request #3839 · decidim/decidim

mrcasals · 2018-07-12T11:01:12Z

🎩 What? Why?

This PR adds the concept of "content blocks" to Decidim. As of now, content blocks are only used in the home page.

A content block is a sortable, configurable layout block registered by a Decidim module.

Differences with view hooks?

On a view hook you can only hook a partial. There's no possibility of manual configuration, and even though you can set a priority, hooked views in the same priority cannot get sorted.

Examples of content blocks usage

# Registering a content block with static hard-coded options
Decidim.content_blocks.register(:homepage, :highlighted_processes) do |content_block|
  content_block.option :count, :integer, default: 4, values: [4, 8, 12]
  content_block.cell "decidim/participatory_processes/homepage_processes_block"
end

# A content block where options are analysed on runtime
Decidim.content_blocks.register(:homepage, :global_stats) do |content_block|
  content_block.option :minimum_priority_level,
                    :integer
                    default: lambda { StatsRegistry::HIGH_PRIORITY }
                    values: lambda { [StatsRegistry::HIGH_PRIORITY, StatsRegistry::MEDIUM_PRIORITY] }
  content_block.cell "decidim/content_blocks/stats_block"
end

# A content block with images
Decidim.content_blocks.register(:homepage, :carousel) do |content_block|
  content_block.image :image_1
  content_block.image :image_2
  content_block.image :image_3
  content_block.cell "decidim/carousel_block"
end

📌 Related Issues

Fixes Ways to customize home blocks #3672

📋 Subtasks

In later PRs:

Add the admin section Content blocks: use settings and images #3968
Add options/images to the content blocks and migrate the ContentBlocks Content blocks: use settings and images #3968
Move the "Appearance" settings section to each content block
Add documentation regarding the feature

josepjaume · 2018-07-16T08:05:50Z

@mrcasals nice approach 👍. We should definitely build this in a way that it doesn't become dramatic to generalize in the future. Maybe adding a scope when registering a content block might make the API more flexible?

Something like:

Decidim.content_blocks.register(:home_page, :global_stats) do
  # ...
end

Or even:

Decidim.content_blocks[:home_page].register(:global_stats) do
  # ...
end

mrcasals · 2018-07-16T08:14:47Z

@josepjaume that's what I wanted feedback of. I think we don't have a use case right now where this could be useful except for the homepage, and abstracting and reusing the admin section is not that simple.

Still, I'd go with the first suggestion.

josepjaume · 2018-07-16T08:16:52Z

Yeah, I definitely understand the reasons behind it. But using this API makes the use case more explicit from the reader's point of view, and also will potentially prevent us from having to modify this part of the code when we extend it.

mrcasals · 2018-07-16T09:14:59Z

@josepjaume done! I've modified the PR description and the code docs accordingly.

mrcasals · 2018-07-27T07:05:04Z

Following @deivid-rodriguez's suggestion (#2476 (comment)), I'm gonna add a few things a send this PR for review.

This PR adds the backbones for ContentBlocks, and uses them in the homepage. I need to add a migration to create the default content blocks for existing organizations, and create the default content blocks on organization registration.

oriolgual · 2018-07-30T07:58:20Z

decidim-core/app/cells/decidim/content_blocks/sub_hero_cell.rb

+        render
+      end
+
+      delegate :current_organization, to: :controller


Maybe we can add this to Decidim::ViewModel?

I won't do it by now as it might affect other cells.

oriolgual · 2018-07-30T07:58:59Z

decidim-core/app/models/decidim/content_block.rb

+    belongs_to :organization, foreign_key: :decidim_organization_id, class_name: "Decidim::Organization"
+
+    def manifest
+      Decidim.content_blocks.for(scope).find { |manifest| manifest.name.to_s == manifest_name }


Should we memoize this?

oriolgual · 2018-07-30T07:59:51Z

decidim-core/db/migrate/20180724103814_add_content_blocks.rb

+      t.string :manifest_name, null: false, index: true
+      t.string :scope, null: false, index: true
+      t.jsonb :options
+      t.datetime :published_at


Add an index here (I guess we're only showing the published ones)

Maybe we need a combined index with decidim_organization_id, can you try an run an explain on the query to see which is better?

mrcasals · 2018-07-30T08:11:44Z

decidim-core/lib/decidim/content_block_manifest.rb

+# frozen_string_literal: true
+
+module Decidim
+  class ContentBlockManifest


oriolgual · 2018-07-30T08:03:41Z

decidim-core/app/views/decidim/pages/home.html.erb

-
-<% if !translated_attribute(current_organization.description).blank? %>
-  <%= render partial: "decidim/pages/home/sub_hero" %>
+<% Decidim::ContentBlock.where(organization: current_organization, scope: :homepage).where.not(published_at: nil).order(weight: :asc).each do |content_block| %>


Maybe move all these conditions to a scope? (Also, related to the index comment for the database)

oriolgual · 2018-07-30T09:08:30Z

decidim-core/lib/decidim/content_block_registry.rb

  #       content_block.cell "decidim/content_blocks/carousel_block"
  #     end
+  #
+  # You will probably want to register your content blocs in an initializer in


s/blocs/blocks

deivid-rodriguez · 2018-07-30T11:40:07Z

@deivid-rodriguez I know your comment wasn't targeting this PR, but still! What I dislike of my approach is that if I later find that I need to refactor some things, context gets a little lost. For this PR I'd rather use different PRs:

This one could be considered an internal refactor

A later PR will come with the admin side that allows adding/removing content blocks, and reordering them

Another PR will add the options to content blocks, and the admin layouts to handle that.

I see, you're right about refactorings losing content. Your approach for this feature looks good!

mrcasals added lot: core status: WIP labels Jul 12, 2018

mrcasals self-assigned this Jul 12, 2018

mrcasals force-pushed the content-blocks branch 5 times, most recently from 2807add to a751655 Compare July 12, 2018 14:04

mrcasals force-pushed the content-blocks branch 3 times, most recently from c361021 to c11a6cd Compare July 16, 2018 09:08

mrcasals force-pushed the content-blocks branch 9 times, most recently from 3fcb666 to c586f73 Compare July 17, 2018 10:53

mrcasals force-pushed the content-blocks branch 5 times, most recently from 19b7e66 to e9f8ce9 Compare July 25, 2018 13:32

mrcasals added 6 commits July 30, 2018 09:36

Add ContentBlock model

c9f89f6

Create default content blocks on seed

68312c6

Use content blocks from DB on the homepage

b710edc

Fix specs

2bc9040

Add default content blocks

9c6751d

Add changelog

30a4ab2

mrcasals force-pushed the content-blocks branch from e9f8ce9 to 30a4ab2 Compare July 30, 2018 07:37

mrcasals added in-review and removed status: WIP labels Jul 30, 2018

mrcasals changed the title ~~[WIP] Add ContentBlocks~~ Add ContentBlocks Jul 30, 2018

Fix rubocop

725855f

ghost added the status: WIP label Jul 30, 2018

oriolgual suggested changes Jul 30, 2018

View reviewed changes

Address feedback

f8a51f3

oriolgual reviewed Jul 30, 2018

View reviewed changes

Fix typo

561daca

oriolgual approved these changes Jul 30, 2018

View reviewed changes

mrcasals removed the status: WIP label Jul 30, 2018

mrcasals merged commit a58eb46 into master Jul 30, 2018

mrcasals deleted the content-blocks branch July 30, 2018 09:56

This was referenced Jul 30, 2018

Content blocks admin #3941

Merged

MVP participatory space of Conference/events #3781

Merged

oriolgual mentioned this pull request Aug 1, 2018

Data Visualization #3603

Merged

5 tasks

This was referenced Aug 1, 2018

Add docs for content blocks #3948

Merged

Content blocks: use settings manifest #3950

Merged

Content blocks: use settings and images #3968

Merged

mrcasals added this to the CDP4 milestone Dec 10, 2018

This was referenced Sep 30, 2020

Move CTA from Appearence to 'Hero' Content Block #6574

Closed

Remove show_statistics checkbox in Appearance #6575

Merged

Uh oh!

Conversation

mrcasals commented Jul 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎩 What? Why?

Differences with view hooks?

Examples of content blocks usage

📌 Related Issues

📋 Subtasks

Uh oh!

josepjaume commented Jul 16, 2018

Uh oh!

mrcasals commented Jul 16, 2018

Uh oh!

josepjaume commented Jul 16, 2018

Uh oh!

mrcasals commented Jul 16, 2018

Uh oh!

mrcasals commented Jul 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deivid-rodriguez commented Jul 30, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mrcasals commented Jul 12, 2018 •

edited

Loading

mrcasals commented Jul 27, 2018 •

edited

Loading