-
Couldn't load subscription status.
- Fork 40
Add content processors properties #8086 #2068
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
base: master
Are you sure you want to change the base?
Add content processors properties #8086 #2068
Conversation
Summary by CodeRabbit
WalkthroughAdds documentation for a new Content Processors Configuration in the Studio reference and expands content-modeling docs with server-side controller variables (including applicationContext access), replaces a controller example to use Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Studio
participant Processor as Content Processor (Groovy)
participant AppCtx as ApplicationContext / Beans
participant Repo as Site Repository
Client->>Studio: call write-content API
Studio->>Processor: invoke lifecycle script at configured scriptLocation
Note right of Processor: if includeApplicationContext=true\nand includedBeans set
Processor->>AppCtx: access configured beans/services
Processor->>Repo: read lifecycleContent
Processor->>Processor: mutate content (e.g., internal-name)
Processor->>Repo: write updated content
Processor-->>Studio: return mutation details
Studio-->>Client: API response (write-content)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
source/by-role/developer/common/content-modeling/content-modeling.rst(1 hunks)source/reference/modules/studio.rst(2 hunks)
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.
Actionable comments posted: 0
🧹 Nitpick comments (6)
source/reference/modules/studio.rst (6)
114-116: Align wording with section titleUse “Content Processors Configuration” (plural) in the description for consistency, e.g., “Configure content processors (content lifecycle) properties.”
3537-3538: Clarify placeholder in scriptLocationDocument what {content-type} expands to (e.g., “/page/entry”), and whether leading slash is required.
3546-3554: Add a security note when enabling applicationContextEnabling applicationContext exposes Engine and site beans to Groovy scripts. Add a short caution and link to the Groovy Sandbox section to encourage keeping includeApplicationContext=false and using includedBeans as a safer alternative.
3557-3566: Provide a concrete includedBeans exampleShow actual values to guide users.
- # List of beans that should be available in the content lifecycle controller script. - studio.contentProcessor.contentLifecycle.includedBeans: [] + # List of beans that should be available in the content lifecycle controller script. + # You can use either inline or block list syntax: + # studio.contentProcessor.contentLifecycle.includedBeans: [contentService, searchService] + studio.contentProcessor.contentLifecycle.includedBeans: + - contentService + - searchService
3578-3611: Document script context variables used in the exampleThe example uses lifecycleContent and path without stating what they are. Add a short preface listing available variables in content lifecycle scripts (e.g., lifecycleContent, path, request/user if applicable) so readers can follow.
3537-3542: Approve contentLifecycle fix; add examples for includedBeans
- Verified no remaining
contentLifeCycletypos.- Include a brief example of
includedBeanssyntax and note lists may be YAML arrays or inline:studio.contentProcessor.contentLifecycle.includedBeans: - beanA - beanBor
studio.contentProcessor.contentLifecycle.includedBeans: [beanA, beanB]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
source/reference/modules/studio.rst(2 hunks)
🔇 Additional comments (2)
source/reference/modules/studio.rst (2)
3523-3525: Version tag vs page version headerThis section is marked “Since 5.0.0” while the page header shows last-updated 4.4.3. Confirm this page is intended to include 5.0 content or update the header accordingly.
3569-3569: Verify cross-reference target existsEnsure :ref:
server-side-form-controllersresolves; otherwise replace with a valid anchor.
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
source/reference/modules/studio.rst (2)
3545-3554: Clarify wording and add a security warning about applicationContext.Current phrasing ties this to “Crafter Engine’s Spring beans,” which may confuse readers; it’s effectively the site Spring application context. Also, enabling applicationContext broadly increases the attack surface.
Suggested edit:
- The ``applicationContext`` variable provides access to Crafter Engine's Spring beans and site beans defined in - ``config/spring/application-context.xml``. The ``applicationContext`` is disabled by default and can be enabled + The ``applicationContext`` variable provides access to the site's Spring application context beans defined in + ``config/spring/application-context.xml``. The ``applicationContext`` is disabled by default and can be enabled by setting the property ``studio.contentProcessor.contentLifecycle.includeApplicationContext`` to ``true``:And add a warning right below:
.. warning:: Enabling ``includeApplicationContext`` exposes the full site application context to lifecycle scripts. Prefer keeping it ``false`` and whitelist only the required beans via ``studio.contentProcessor.contentLifecycle.includedBeans``.
3557-3566: Clarify includedBeans semantics and prefer a readable YAML example.
- Mention that includedBeans is only considered when includeApplicationContext is false.
- Use a multiline YAML list for clarity.
Suggested edits:
- When ``studio.contentProcessor.contentLifecycle.includeApplicationContext`` is set to false, administrators might - decide to include some specific beans from the context, e.g.: contentService, searchService. Use the - ``studio.contentProcessor.contentLifecycle.includedBeans`` property to list the beans to be included: + When ``studio.contentProcessor.contentLifecycle.includeApplicationContext`` is set to ``false``, administrators may + whitelist specific beans (for example, ``contentService``, ``searchService``). Use + ``studio.contentProcessor.contentLifecycle.includedBeans`` to list the beans to expose. This setting is ignored when + ``includeApplicationContext`` is ``true``.And update the example:
- studio.contentProcessor.contentLifecycle.includedBeans: [beanA, beanB] + studio.contentProcessor.contentLifecycle.includedBeans: + - beanA + - beanB
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
source/by-role/developer/common/content-modeling/content-modeling.rst(1 hunks)source/reference/modules/studio.rst(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- source/by-role/developer/common/content-modeling/content-modeling.rst
🔇 Additional comments (3)
source/reference/modules/studio.rst (3)
114-116: Good addition to the configuration index.The link and anchor name look consistent and discoverable.
3517-3541: Defaults block and property names look correct.The keys use contentLifecycle (lowercase “l”), matching the configuration. Defaults read well.
3571-3667: Solid, actionable example. Two small polish suggestions.
- Add a short note linking to the Groovy sandbox section so readers understand execution constraints.
- Ensure the cross‑reference ``:ref:`server-side-form-controllers``` resolves in the build.
Proposed note (place after the example or before it):
.. note:: See :ref:`Groovy Sandbox Configuration <studio-groovy-sandbox-configuration>` for restrictions that apply to lifecycle scripts and how to configure the sandbox.Also, please verify that the
server-side-form-controllersanchor exists and builds without warnings.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
source/by-role/developer/common/content-modeling/content-modeling.rst (1)
1977-1978: Consider adding more context to the applicationContext reference.The note directs readers to
:ref:content-processors-configuration`` for accessingapplicationContext. To help developers immediately understand what this means, consider adding a brief inline note about whether `applicationContext` is available by default or requires configuration, or a short example hint.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
source/by-role/developer/common/content-modeling/content-modeling.rst(4 hunks)
🔇 Additional comments (3)
source/by-role/developer/common/content-modeling/content-modeling.rst (3)
1948-1975: Comprehensive server-side controller variables reference added.The new variables table clearly documents all available variables with concise descriptions. The inclusion of the logger variable with its name format pattern (showing site and content type IDs) is particularly helpful for developers setting up logging and monitoring. The table structure aligns well with the rest of the documentation.
1999-2009: Logger-based example is clean and idiomatic.The updated example demonstrates the proper way to log lifecycle events using
logger.info(). The simple, focused approach makes it easy for developers to understand and adapt for their own use cases.
2027-2031: Log output example format is clear and realistic.The updated log output at line 2030 demonstrates the actual logger name format
[Lifecycle-hello-/page/entry]which aligns well with the documentation in the logger variable description (lines 1971-1974). This consistency helps developers understand what to expect when monitoring their controller scripts.
Ticket reference or full description of what's in the PR
Add content processors properties craftercms/craftercms#8086