[#256] Migrated blog articles onto a dedicated 'blog' content type. - #269
Merged
Conversation
…current one. A host node references one specific paragraph revision, which is not necessarily the paragraph's current one, so saving the paragraph left the host still rendering the old bundle. Setting the value across every revision also keeps older node revisions rendering a populated list.
📝 WalkthroughWalkthroughChangesBlog content type
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant DeployHook
participant DrupalStorage
participant SearchAPI
participant Cache
DeployHook->>DrupalStorage: Find Blog-topic pages and automated lists
DeployHook->>DrupalStorage: Migrate nodes and repoint paragraph targets
DeployHook->>SearchAPI: Track migrated node items
DeployHook->>Cache: Reset caches and invalidate cache tags
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
AlexSkrypnyk
added
A1
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/default/core.entity_view_display.node.blog.civictheme_promo_card.yml`:
- Around line 53-58: Update field_c_n_thumbnail in both
config/default/core.entity_view_display.node.blog.civictheme_promo_card.yml
(lines 53-58) and
config/default/core.entity_view_display.node.blog.civictheme_navigation_card.yml
(lines 53-58) to use the entity_reference_entity_view formatter, configuring the
media view mode used by the slider display instead of rendering the media
label/link.
In `@config/default/field.field.node.blog.field_c_n_banner_blend_mode.yml`:
- Line 14: Update the field label value in the configuration to use a single
space between “blend” and “mode,” preserving the rest of the label unchanged.
In `@web/modules/custom/do_base/do_base.deploy.php`:
- Around line 425-430: Extend the bundle migration alongside the existing node
and node_field_data updates to also update node_field_revision.type from
civictheme_page to blog for the affected nids, before or alongside the related
table updates. Keep the existing conditions and blog conversion flow unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 786a41a5-ba9b-46a8-9edf-1a879edaaa33
📒 Files selected for processing (38)
config/default/core.entity_form_display.node.blog.default.ymlconfig/default/core.entity_view_display.node.blog.civictheme_navigation_card.ymlconfig/default/core.entity_view_display.node.blog.civictheme_promo_card.ymlconfig/default/core.entity_view_display.node.blog.civictheme_slider_slide.ymlconfig/default/core.entity_view_display.node.blog.default.ymlconfig/default/field.field.node.blog.field_c_n_banner_background.ymlconfig/default/field.field.node.blog.field_c_n_banner_blend_mode.ymlconfig/default/field.field.node.blog.field_c_n_banner_components.ymlconfig/default/field.field.node.blog.field_c_n_banner_components_bott.ymlconfig/default/field.field.node.blog.field_c_n_banner_featured_image.ymlconfig/default/field.field.node.blog.field_c_n_banner_hide_breadcrumb.ymlconfig/default/field.field.node.blog.field_c_n_banner_theme.ymlconfig/default/field.field.node.blog.field_c_n_banner_title.ymlconfig/default/field.field.node.blog.field_c_n_banner_type.ymlconfig/default/field.field.node.blog.field_c_n_components.ymlconfig/default/field.field.node.blog.field_c_n_custom_last_updated.ymlconfig/default/field.field.node.blog.field_c_n_hide_sidebar.ymlconfig/default/field.field.node.blog.field_c_n_hide_tags.ymlconfig/default/field.field.node.blog.field_c_n_show_last_updated.ymlconfig/default/field.field.node.blog.field_c_n_show_toc.ymlconfig/default/field.field.node.blog.field_c_n_site_section.ymlconfig/default/field.field.node.blog.field_c_n_summary.ymlconfig/default/field.field.node.blog.field_c_n_thumbnail.ymlconfig/default/field.field.node.blog.field_c_n_topics.ymlconfig/default/field.field.node.blog.field_c_n_vertical_spacing.ymlconfig/default/field.field.node.blog.field_n_metatags.ymlconfig/default/field.storage.paragraph.field_c_p_list_content_type.ymlconfig/default/language.content_settings.node.blog.ymlconfig/default/metatag.metatag_defaults.node__blog.ymlconfig/default/node.type.blog.ymlconfig/default/pathauto.pattern.blog.ymlconfig/default/user.role.civictheme_content_author.ymlconfig/default/user.role.civictheme_site_administrator.ymlconfig/default/workflows.workflow.civictheme_editorial.ymlconfig/default/xmlsitemap.settings.node.blog.ymldocs/sitemap.mdtests/behat/features/blog_content_type.featureweb/modules/custom/do_base/do_base.deploy.php
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #269 +/- ##
========================================
Coverage 89.16% 89.16%
========================================
Files 15 15
Lines 203 203
========================================
Hits 181 181
Misses 22 22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
AlexSkrypnyk
added
the
Needs review
CivicTheme dispatches its full-view node preprocessing by bundle name, so a bundle it does not ship rendered without a table of contents, without topic tags, and with the title left in place on revision pages. Blog posts now reuse the page preprocessor rather than restating it.
This comment has been minimized.
This comment has been minimized.
…sor. CivicTheme resolves full-view node preprocessing by bundle name so that each content type owns its own theming. Blog posts now supply that per-bundle function in a dedicated include, leaving the theme's generic view-mode hook free for cross-bundle work.
This comment has been minimized.
This comment has been minimized.
Captures the configuration a node bundle needs, the two traps when cloning displays programmatically, the per-bundle theme preprocessor that configuration parity does not cover, and how to move existing nodes onto a new bundle.
This comment has been minimized.
This comment has been minimized.
…ts helpers private. Splits the work into two sandbox-batched deploy hooks, since one sandbox cannot carry two batched operations, and moves atomicity to each item so an interrupted batch is safe to resume. Adds sitemap coverage for blog posts and records the hook agreements in docs/development.md.
|
Code coverage (threshold: 80%) Per-class coverage |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/modules/custom/do_base/do_base.deploy.php`:
- Around line 398-400: Update the node query near lines 398-400 and the
paragraph query near lines 429-432 in
web/modules/custom/do_base/do_base.deploy.php to explicitly disable access
checking with accessCheck(FALSE). Update the corresponding deploy-hook example
at docs/development.md lines 33-38 to include the same explicit access policy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: beadd209-93c2-4b4a-8a18-d5348327e179
📒 Files selected for processing (10)
AGENTS.mdconfig/default/core.entity_form_display.node.blog.default.ymlconfig/default/navigation.settings.ymldocs/README.mddocs/content-types.mddocs/development.mdtests/behat/features/xmlsitemap.featureweb/modules/custom/do_base/do_base.deploy.phpweb/themes/custom/drevops/drevops.themeweb/themes/custom/drevops/includes/blog.inc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #256
Checklist before requesting a review
[#123] Verb in past tense.#123added to descriptionChangedsectionChanged
blogcontent type, labelled "Blog post", cloned fromcivictheme_pageso the two bundles stay interchangeable - same moderation states, same available menus, same revision and submitted-by behaviour.civictheme_page, so this attaches instances rather than creating storages. That is what makes the migration a bundle change in place: no field data is copied or transformed.default_paragraph_type: civictheme_content,edit_mode: open), so a new post opens ready to type into instead of starting empty.default,civictheme_promo_card,civictheme_navigation_card,civictheme_slider_slide), byte-identical to thecivictheme_pageones apart from their UUID. Thedefaultdisplay is Layout Builder-driven, so the bundle embedded in each block plugin ID was rewritten too; the layout keeps the same three rendered blocks the Page layout has, rather than the thirteen that Layout Builder would have produced had the display been saved through the entity API./blog/[node:title], membership of thecivictheme_editorialworkflow,xmlsitemapbundle settings, metatag defaults, and language content settings.blogequivalent of everycivictheme_pagepermission already held by the Content Author and Site Administrator roles. The Content Approver role holds nocivictheme_pagepermissions, so it gains none here.blog/ "Blog post" option to thefield_c_p_list_content_typeallowed values, leaving the existing options and their order intact. The automated list view already takes the bundle as a contextual argument and its exposed content-type filter is unrestricted, so the view itself needed no change.do_base.deploy.php,do_base_deploy_migrate_blog_posts()anddo_base_deploy_repoint_blog_lists(), covered in detail below. Every supporting function is private and underscore-prefixed, so only the two hooks are part of the module's surface._civictheme_preprocess_node__blog__full()in a dedicated theme include,web/themes/custom/drevops/includes/blog.inc. CivicTheme resolves full-view node preprocessing by bundle name, so each content type owns its own theming and a bundle CivicTheme does not ship renders without a table of contents, without topic tags, and with the node title left in place on revision pages. Blog posts now supply that per-bundle function themselves, keeping the type's theming self-contained and independently changeable, and leaving the theme's generic view-mode hook free for cross-bundle work. Configuration parity alone did not cover this - it lives in the theme layer.tests/behat/features/blog_content_type.featureover the authoring form, the pre-added Content paragraph, the/blogalias, the editorial states, draft visibility, component rendering, topic tags (both shown and hidden), the table of contents (both on and off), the automated list option, and a list set to Blog post returning only blog posts. One asserts the Page form still opens with no paragraph pre-added, so the Page type is provably unchanged. A fifteenth scenario intests/behat/features/xmlsitemap.featureasserts a published blog post reaches/sitemap.xmlat its/blogalias while a draft does not.docs/development.md, recording the project's hook agreements so they are not rediscovered each time: hooks are the only public functions and every helper is private and underscore-prefixed;drupal_helpersis reached for before hand-rolling, with its README named as the authoritative list of what exists; and the mechanics that follow from batching, namely the nullable sandbox parameter, one batched operation per hook, batch sizing, per-item atomicity, reporting, and the bootstrap an update hook needs. The longdrupal_helpersrules inAGENTS.mdnow point here rather than restating it.docs/content-types.md, capturing the approach so the next custom content type does not have to rediscover it: the full configuration surface a node bundle needs, the two traps when cloning displays programmatically (Layout Builder rewriting a display saved through the entity API, and the simplified runtime shape oflist_stringallowed values), the per-bundle theme preprocessor that configuration parity does not cover, and how to move existing nodes onto a new bundle. Linked fromdocs/README.mdandAGENTS.md.blognodes in the sitemap coverage table indocs/sitemap.md.How the migration works
Both hooks batch through
Helper::entity($sandbox)->batchQuery(), so the work spreads across requests instead of risking a timeout. They are two hooks rather than one because a sandbox uses fixed keys and cannot carry two batched operations; deploy hooks run alphabetically, so the names put the migration before the repointing. The node hook uses a batch size of 10 because each article rewrites rows in more than forty field tables.The migration changes each article's bundle in storage rather than through the entity API. Drupal treats a loaded entity's bundle as immutable, and the only entity-API route - delete and recreate - would discard exactly what the acceptance criteria require to survive: the node IDs, the 242 revisions, and the paragraph references. So it updates
node.type,node_field_data.type, and thebundlecolumn of every dedicated field table. That table list is derived from the entity table mapping at runtime rather than naming 42 tables literally, so a field attached to either bundle later cannot be silently missed.Each article commits or rolls back on its own rather than the run sharing one transaction. A sandbox spans several requests, so a single transaction could not wrap it anyway, and per-item atomicity combined with a query that only matches unmigrated articles is what makes an interrupted batch safe to resume. The cost is that the two hooks are not atomic with each other: if the repointing fails, the lists render empty until
drush deploy:hookruns again, which completes them.Because no node is ever re-saved, pathauto never fires, and every hand-crafted alias survives verbatim - including
/vortex-138-released, which sits outside/blog/and would otherwise have been rewritten by the new pattern.Storage-level updates fire no entity hooks, so the hook explicitly repairs the three things that would otherwise desync: the
xmlsitemaplink subtype (the existing rebuild hook runs only once per environment and will not re-run), search index tracking, and the node and list cache tags.The second hook repoints the two automated lists, scoped by the Blog topic rather than by target bundle, so the case studies list - which targets Pages under a different topic - is left alone. The repoint is applied across every revision of those paragraphs rather than by saving the entity: a host node references one specific paragraph revision, which is not necessarily the paragraph's current one, so saving would have left the host still rendering the old bundle. Older revisions want the new value too - left on
civictheme_pagethey would render an empty list once no page carries the Blog topic any more.Idempotency comes from the queries themselves: they match
civictheme_pagenodes carrying the Blog topic, and lists still pointing atcivictheme_page. After a successful run neither matches, and the hook reports that there is nothing to migrate and nothing to repoint.Verified against restored production content
Run locally against a restored database: 24 articles migrated across 242 revisions in three batch passes, with every node ID, alias, revision count and moderation state asserted unchanged one by one; the archived article still archived and still returning 403 to anonymous users; zero field rows left on the old bundle; the
/blogpager reaching all 23 published articles with no duplicates; the homepage teasers and the case studies list intact; and a second run of both hooks reporting nothing to process.Topic tags and the table of contents were confirmed on a rendered article, and the two new positive scenarios were checked against a build with the preprocessor removed to confirm they fail without it rather than passing vacuously.
Screenshots
Before / After
Summary by CodeRabbit