c/ezB1gKx5/208: corrected metatag settings for the Page content type. #38
Conversation
…tent type. Reason: missing <description> because of missing body field.
…ags presence on the Page node view.
WalkthroughA new metatag defaults configuration for the "CivicTheme Page" content type has been added, specifying how meta descriptions are generated dynamically. Additionally, a Behat feature test was introduced to verify that these default metatags, including title, description, and canonical link, are correctly rendered on CivicTheme Page content. The existing tracking feature test was updated to change the user login condition for verifying the absence of Google Analytics 4 scripts. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Drupal
participant MetatagModule
User ->> Drupal: Request CivicTheme Page
Drupal ->> MetatagModule: Retrieve metatag defaults for node__civictheme_page
MetatagModule ->> Drupal: Provide metatags (title, description, canonical)
Drupal ->> User: Serve page with metatags in HTML
sequenceDiagram
participant BehatTest
participant Drupal
participant MetatagModule
BehatTest ->> Drupal: Create CivicTheme Page with summary
BehatTest ->> Drupal: Visit created page as anonymous user
Drupal ->> MetatagModule: Retrieve metatag defaults
MetatagModule ->> Drupal: Provide metatags
Drupal ->> BehatTest: Return page HTML with metatags
BehatTest ->> BehatTest: Assert presence of title, description, canonical tags
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
📒 Files selected for processing (2)
config/default/metatag.metatag_defaults.node__civictheme_page.yml(1 hunks)tests/behat/features/metatags.feature(1 hunks)
🔇 Additional comments (2)
config/default/metatag.metatag_defaults.node__civictheme_page.yml (1)
1-8:Details
✅ Verification successful
Confirm whether additional metatags are needed.
The configuration only defines the description metatag, but standard SEO best practices typically include other tags like title and canonical URL that are being tested in the Behat feature. If these are inherited from a parent configuration, that's fine, but it's worth confirming.
🏁 Script executed:
#!/bin/bash # Check if other metatag configurations exist that might provide default values echo "Checking for parent metatag configurations:" fd 'metatag.metatag_defaults' --type f --exec cat {} \; | grep -v "node__civictheme_page" | grep -E "title|canonical" # List content types to verify CivicTheme Page definition echo -e "\nListing content types configurations:" fd 'node.type.civictheme_page' --type fLength of output: 775
No additional metatags needed – inherited from global defaults
I’ve checked the other
metatag.metatag_defaults.*configurations and confirmed that bothtitleandcanonical_urlare defined in the global defaults, so this content‐type override only needs to specifydescription. No further tags are required here; inheritance will satisfy the Behat feature tests.tests/behat/features/metatags.feature (1)
1-16: LGTM: Well-structured BDD test.The feature file follows good BDD practices with clear tags, feature description, background setup, and scenario steps. The test properly creates test content and verifies it from an anonymous user's perspective.
| uuid: 9510eb68-5939-4944-9151-fe114b68ea88 | ||
| langcode: en | ||
| status: true | ||
| dependencies: { } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding dependencies.
Typically, a content type-specific metatag configuration would have dependencies on the content type itself. This helps Drupal track configuration dependencies correctly.
- dependencies: { }
+ dependencies:
+ config:
+ - node.type.civictheme_page| @seo @metatags | ||
| Feature: Page content metatags | ||
| As a site user, I want to verify all default metatags appear for CivicTheme Page content type | ||
|
|
||
| Background: | ||
| Given civictheme_page content: | ||
| | title | status | field_c_n_summary | | ||
| | Test Metatags Page | 1 | This is a test summary for metatags testing | | ||
|
|
||
| @api | ||
| Scenario: CivicTheme page content type contains default metatags | ||
| Given I am an anonymous user | ||
| When I visit civictheme_page "Test Metatags Page" | ||
| Then the response should contain "<title>Test Metatags Page | " | ||
| And the response should contain "<meta name=\"description\" content=\"This is a test summary for metatags testing\"" | ||
| And the response should contain "<link rel=\"canonical\" href=\"" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider including additional important metatags.
The test verifies title, description, and canonical URL, which are essential. However, you might want to consider testing other important metatags like robots, og:title, og:description, etc., if they're expected to be present by default.
| Scenario: CivicTheme page content type contains default metatags | ||
| Given I am an anonymous user | ||
| When I visit civictheme_page "Test Metatags Page" | ||
| Then the response should contain "<title>Test Metatags Page | " |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Make title assertion more specific.
The current title assertion checks for the beginning of the title tag but doesn't verify the complete title. Consider making this more specific by including the site name or using a regex pattern to match the entire title format.
- Then the response should contain "<title>Test Metatags Page | "
+ Then the response should contain "<title>Test Metatags Page | Site Name</title>"Alternatively, use a more flexible regex approach:
Then the response should match "/<title>Test Metatags Page \| .+<\/title>/"
| When I visit civictheme_page "Test Metatags Page" | ||
| Then the response should contain "<title>Test Metatags Page | " | ||
| And the response should contain "<meta name=\"description\" content=\"This is a test summary for metatags testing\"" | ||
| And the response should contain "<link rel=\"canonical\" href=\"" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Enhance canonical URL assertion.
The current assertion only checks for the presence of a canonical tag but not its actual value. Consider strengthening this test by verifying the complete URL.
- And the response should contain "<link rel=\"canonical\" href=\""
+ And the response should contain "<link rel=\"canonical\" href=\"http://localhost/test-metatags-page\""Or use a pattern matching approach if the domain varies across environments:
And the response should match "/<link rel=\"canonical\" href=\"[^\"]+\/test-metatags-page\"/"
… testing not related feature
nsineok
added
the
Needs review
Checklist before requesting a review
[#123] Verb in past tense.#123added to descriptionChangedsectionChanged
Screenshots
Summary by CodeRabbit
New Features
Tests