Documentation#32
Conversation
Add task-oriented developer guides for the ten Modularity modules under sources/, one file per module in docs/. Each guide covers what a developer can build with the module and how (recipes + public API surface: api.php functions, interfaces/value objects, and the actions/filters to hook into), rather than internal Module wiring. Expand README.md with an Architecture section (modularity, module registration order, interaction data flow) and a Module Documentation TOC linking each guide by capability. The existing storage-drivers.md is kept and referenced from the new storage.md and the TOC. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ule Documentation
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Documentation navigation and runtime integrations README.md, docs/activation.md, docs/api-fetch.md, docs/configuration.md, docs/icons.md |
Adds module navigation and documents lifecycle tasks, API-fetch helpers, configuration payloads, and SVG icon APIs. |
Blocks and REST extension points docs/blocks.md, docs/rest.md |
Documents dynamic block rendering, contexts, hooks, templates, styling, typed routes, middleware, schemas, controllers, and error handling. |
Storage and database model docs/storage.md, docs/database.md |
Documents shared interaction storage, backend replacement, storage contracts, database schema management, and custom table lifecycle tasks. |
User and post interaction domains docs/user.md, docs/post.md |
Documents user interaction APIs, persistence actions, post-indexed counts, and event-driven synchronization between user and post storage. |
Estimated code review effort: 2 (Simple) | ~15 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | ❓ Inconclusive | The title is too vague and doesn't describe the main change beyond saying it's documentation. | Use a concise, specific title that names the docs area or feature set, such as "Add module developer documentation". |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
docs/module-developer-guides
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 @coderabbitai help to get the list of available commands.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@docs/blocks.md`:
- Line 334: Update the REST reference in the block view documentation to use the
registered endpoint names user-reaction and user-bookmark instead of
add-reaction and add-bookmark, while preserving the existing REST documentation
link.
- Around line 243-255: Add the `SpaghettiDojo\Konomi\Blocks` import to the usage
sketch in `docs/blocks.md` so the `Blocks\context()`, `Blocks\renderer()`,
`Blocks\style()`, `Blocks\CustomProperty`, `Blocks\kses()`, and `Blocks\loop()`
references resolve as shown.
- Line 209: Update the pre_render_block fallback description in docs/blocks.md
to state that returning null continues the default rendering, while returning
the incoming $preRender preserves an existing non-null short-circuit; avoid
describing null as the incoming value.
In `@docs/configuration.md`:
- Around line 13-19: Update Configuration::toArray() to omit iconsPath from the
script-module/browser payload while retaining it for server-side use. Remove the
iconsPath entry from the client-facing configuration documentation and adjust
related tests to expect only the exposed client keys.
In `@docs/post.md`:
- Around line 73-79: Update the cross-axis consistency statement in the
documented toggle flow to describe mirroring as best-effort, explicitly
acknowledging that the separate entity-axis write may fail and requires retry or
error handling. Do not claim the indexes always stay consistent unless the
implementation is changed to provide an atomic or outbox-based guarantee.
In `@docs/rest.md`:
- Line 120: Update sources/Rest/Middlewares/ErrorCatch.php so its WP_Error
response retains only the generic translated message, while logging the caught
Throwable server-side instead of exposing code, file, line, message, or
stackTrace in error data. Then revise the ErrorCatch documentation in
docs/rest.md to describe the sanitized response and server-side logging.
- Around line 54-63: Update the REST API example callback to resolve PingSchema
and PingController through the captured $container using $container->get(...)
instead of instantiating them with new, keeping the prose consistent with the
demonstrated container-based collaborator resolution.
In `@docs/storage.md`:
- Around line 114-116: Update the StorageKey documentation to describe
StorageKey::for() as validating the group key against the allowed format, not
stripping invalid characters. State that it returns the original valid value and
throws UnexpectedValueException when sanitization would alter the input, while
retaining the existing empty-value exception behavior.
In `@docs/user.md`:
- Around line 112-124: Update the add_action example’s error_log call to avoid
directly logging the user and item identifiers; use privacy-safe controlled
logging instead, or explicitly state that any identifiers must comply with the
host application’s logging and retention 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1dd48764-8efa-4196-a6b2-e4a8c4836687
📒 Files selected for processing (11)
README.mddocs/activation.mddocs/api-fetch.mddocs/blocks.mddocs/configuration.mddocs/database.mddocs/icons.mddocs/post.mddocs/rest.mddocs/storage.mddocs/user.md
| ); | ||
| ``` | ||
|
|
||
| Return `null` (the incoming `$preRender`) to let rendering continue; return a string to replace the output. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== docs/blocks.md around line 209 ==\n'
sed -n '195,220p' docs/blocks.md
printf '\n== search for pre_render_block usage ==\n'
rg -n "pre_render_block|preRender" docs src -g '!**/vendor/**'
printf '\n== inspect related block rendering implementation ==\n'
fd -a "Blocks.php|Module.php|Render.*php|Block.*php" srcRepository: Spaghetti-Dojo/konomi
Length of output: 2087
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== repo files mentioning pre_render_block ==\n'
git ls-files | rg '(^|/)(docs/blocks\.md|.*\.php|.*\.md)$' | xargs -r rg -n "pre_render_block|preRender" --
printf '\n== docs/blocks.md lines 180-215 ==\n'
sed -n '180,215p' docs/blocks.md
printf '\n== locate implementation files under known paths ==\n'
git ls-files | rg '(^|/)(src|sources)/.*\.(php|md)$' | head -n 200Repository: Spaghetti-Dojo/konomi
Length of output: 5464
🌐 Web query:
WordPress pre_render_block hook returns non-null short-circuits render incoming value null docs
💡 Result:
The WordPress pre_render_block filter hook is designed to allow developers to short-circuit the rendering of a block [1][2]. When a callback function attached to this hook returns a non-null value, WordPress uses that value as the final rendered output for the block, bypassing the standard block rendering process [3][4]. If your callback function receives a non-null value (meaning a previous filter has already short-circuited the process) and you intend to allow that short-circuiting to persist, you must return the value passed into your filter [5]. If you do not explicitly return a value (or if you return null), you effectively reset the short-circuiting mechanism, potentially causing previous overrides to be ignored and allowing the standard rendering process to proceed [5][6]. To correctly participate in this filter chain: 1. Always accept the $pre_render parameter (the first argument passed to the filter) [3]. 2. Check if $pre_render is not null [3][4]. 3. If it is already non-null, return it unchanged to preserve the existing override [5]. 4. If you wish to provide your own override, return your string content [3][4]. 5. If you do not wish to override the block rendering, return the original $pre_render value [5]. Failure to pass along the existing $pre_render value is a common source of bugs where multiple plugins attempt to manipulate block output, as a later filter can unintentionally clear the work of an earlier one [5][6].
Citations:
- 1: https://developer.wordpress.org/reference/hooks/pre_render_block/
- 2: https://make.wordpress.org/core/2021/02/18/wordpress-5-7-a-new-dynamic-hook-to-filter-the-content-of-a-single-block/
- 3: https://developer.wordpress.org/reference/functions/render_block/
- 4: https://developer.wordpress.org/reference/classes/wp_block/render/
- 5: Usage of
pre_render_blockfilter without returning value woocommerce/woocommerce-blocks#11077 - 6: Returning $pre_render_block in the pre_render_block filter woocommerce/woocommerce-blocks#11143
Clarify the pre_render_block fallback text.
$preRender is the incoming value and can already be non-null; returning it preserves a previous short-circuit. Only null continues with the default render, so the sentence should avoid implying that $preRender is always null.
🤖 Prompt for 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.
In `@docs/blocks.md` at line 209, Update the pre_render_block fallback description
in docs/blocks.md to state that returning null continues the default rendering,
while returning the incoming $preRender preserves an existing non-null
short-circuit; avoid describing null as the incoming value.
| Usage sketches: | ||
|
|
||
| ```php | ||
| $context = Blocks\context(Reaction\Context::class); // Blocks\Context | ||
| $html = Blocks\renderer()->render('partials/button', ['label' => 'Save', 'icon' => 'heart']); | ||
| $css = (string) Blocks\style()->add( | ||
| Blocks\CustomProperty::new('--konomi-color--active', '#e2264d', 'sanitize_hex_color') | ||
| ); | ||
| $safe = Blocks\kses($html); | ||
|
|
||
| Blocks\loop([12, 34, 56], static function (\WP_Post $post): void { | ||
| echo esc_html(get_the_title($post)); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- docs/blocks.md around requested lines ---\n'
sed -n '220,275p' docs/blocks.md | cat -n
printf '\n--- search for Blocks import/example in docs ---\n'
rg -n "use SpaghettiDojo\\\\Konomi\\\\Blocks|Blocks\\\context\\(|Blocks\\\renderer\\(|Blocks\\\style\\(" docs . -g '*.md' -g '*.php' -g '*.txt' || true
printf '\n--- repository files mentioning SpaghettiDojo\\\\Konomi\\\\Blocks ---\n'
rg -n "SpaghettiDojo\\\\Konomi\\\\Blocks" . || trueRepository: Spaghetti-Dojo/konomi
Length of output: 250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- docs/blocks.md around requested lines ---'
sed -n '220,275p' docs/blocks.md | cat -n
echo
echo '--- search for Blocks import/example in docs ---'
rg -n "use SpaghettiDojo\\\\Konomi\\\\Blocks|Blocks\\\context\\(|Blocks\\\renderer\\(|Blocks\\\style\\(" docs . -g '*.md' -g '*.php' -g '*.txt' || true
echo
echo '--- repository files mentioning SpaghettiDojo\\\\Konomi\\\\Blocks ---'
rg -n "SpaghettiDojo\\\\Konomi\\\\Blocks" . || trueRepository: Spaghetti-Dojo/konomi
Length of output: 12745
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- docs/blocks.md lines 1-60 ---'
sed -n '1,60p' docs/blocks.md | cat -n
echo
echo '--- docs/blocks.md lines 90-115 ---'
sed -n '90,115p' docs/blocks.md | cat -n
echo
echo '--- docs/blocks.md lines 235-270 ---'
sed -n '235,270p' docs/blocks.md | cat -nRepository: Spaghetti-Dojo/konomi
Length of output: 6193
Add the missing Blocks import. This sketch uses Blocks\context(), Blocks\renderer(), Blocks\style(), and Blocks\CustomProperty without showing use SpaghettiDojo\Konomi\Blocks; (or fully qualified names), so it is not copy-pasteable as written.
🤖 Prompt for 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.
In `@docs/blocks.md` around lines 243 - 255, Add the `SpaghettiDojo\Konomi\Blocks`
import to the usage sketch in `docs/blocks.md` so the `Blocks\context()`,
`Blocks\renderer()`, `Blocks\style()`, `Blocks\CustomProperty`, `Blocks\kses()`,
and `Blocks\loop()` references resolve as shown.
| ## Related | ||
|
|
||
| - [User](./user.md) — the `User\UserFactory` / `User\Item` / `User\ItemGroup` the contexts read favorite state from. | ||
| - [REST](./rest.md) — the `konomi/v1` add-reaction / add-bookmark endpoints the block view stores call. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use the registered REST endpoint names.
The related link refers to add-reaction / add-bookmark, but the documented registrations in docs/rest.md:80-99 and sources/Blocks/Module.php:140-150 are /user-reaction and /user-bookmark. Following this link can lead developers to call nonexistent routes.
-- [REST](./rest.md) — the `konomi/v1` add-reaction / add-bookmark endpoints the block view stores call.
+- [REST](./rest.md) — the `konomi/v1/user-reaction` and `konomi/v1/user-bookmark` endpoints the block view stores call.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [REST](./rest.md) — the `konomi/v1` add-reaction / add-bookmark endpoints the block view stores call. | |
| - [REST](./rest.md) — the `konomi/v1/user-reaction` and `konomi/v1/user-bookmark` endpoints the block view stores call. |
🤖 Prompt for 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.
In `@docs/blocks.md` at line 334, Update the REST reference in the block view
documentation to use the registered endpoint names user-reaction and
user-bookmark instead of add-reaction and add-bookmark, while preserving the
existing REST documentation link.
| `Configuration::toArray()` is the payload attached to the script module. It exposes: | ||
|
|
||
| | Key | Type | Meaning | | ||
| | --- | --- | --- | | ||
| | `iconsUrl` | `string` | Public URL of Konomi's icons directory. | | ||
| | `iconsPath` | `string` | Filesystem path of Konomi's icons directory. | | ||
| | `isDebugMode` | `bool` | Whether the plugin is running in debug mode (from the plugin properties). | |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n "iconsPath|toArray\(\)|Configuration::toArray|config\.iconsPath|iconsUrl" .Repository: Spaghetti-Dojo/konomi
Length of output: 7120
🏁 Script executed:
sed -n '1,140p' sources/Configuration/Configuration.php && printf '\n---\n' && sed -n '1,120p' sources/Configuration/Module.php && printf '\n---\n' && sed -n '1,140p' tests/unit/php/Configuration/ConfigurationTest.php && printf '\n---\n' && sed -n '1,120p' tests/unit/js/configuration/init-configuration.spec.ts && printf '\n---\n' && sed -n '1,140p' docs/configuration.mdRepository: Spaghetti-Dojo/konomi
Length of output: 11113
Keep iconsPath out of the browser payload. Configuration::toArray() still includes a server filesystem path and the docs show it being exposed to client code. Remove iconsPath from the script-module data and keep it server-only; update the client docs/tests to match.
🤖 Prompt for 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.
In `@docs/configuration.md` around lines 13 - 19, Update Configuration::toArray()
to omit iconsPath from the script-module/browser payload while retaining it for
server-side use. Remove the iconsPath entry from the client-facing configuration
documentation and adjust related tests to expect only the exposed client keys.
| So the full flow for a single toggle is: | ||
|
|
||
| 1. A caller builds a `User\Item` and calls `$user->saveItem($item)` (see [user.md](./user.md)). | ||
| 2. `User\Repository::save()` writes the record on `Storage\Axis::User` and fires `konomi.user.repository.save-successfully`. | ||
| 3. This module's subscriber calls `Post\Repository::save($item, $user)`, which writes the *same* record on `Storage\Axis::Entity` — mirroring the interaction onto the post index (and firing its own `konomi.post.collection.save` action before the write). | ||
|
|
||
| Because both axes live in the one `konomi_interactions` table, the user-indexed and post-indexed views stay consistent. See [storage.md](./storage.md) for the axis model. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Qualify the cross-axis consistency guarantee.
The documented flow writes the user-axis record first, then performs a separate entity-axis write through an action callback whose boolean result is ignored. If the mirror write fails, the two indexes can diverge, so “stay consistent” is too strong. Document the flow as best-effort with retry/error handling, or provide an atomic/outbox-based synchronization guarantee.
🤖 Prompt for 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.
In `@docs/post.md` around lines 73 - 79, Update the cross-axis consistency
statement in the documented toggle flow to describe mirroring as best-effort,
explicitly acknowledging that the separate entity-axis write may fail and
requires retry or error handling. Do not claim the indexes always stay
consistent unless the implementation is changed to provide an atomic or
outbox-based guarantee.
| To pull collaborators from Konomi's container instead of `new`-ing them, resolve them inside the callback: | ||
|
|
||
| ```php | ||
| $container = \SpaghettiDojo\Konomi\package()->container(); | ||
|
|
||
| add_action('rest_api_init', static function () use ($container): void { | ||
| Rest\Route::post('my-plugin/v1', '/ping', new PingSchema(), new PingController()) | ||
| ->register(); | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced section and nearby context in docs/rest.md
nl -ba docs/rest.md | sed -n '45,75p'
# Search for the specific example symbols in the docs and codebase
rg -n "PingSchema|PingController|package\(\)->container\(\)|resolve them inside the callback|pull collaborators" docs . -g '!**/node_modules/**'Repository: Spaghetti-Dojo/konomi
Length of output: 199
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path('docs/rest.md')
for i, line in enumerate(path.read_text().splitlines(), 1):
if 45 <= i <= 75:
print(f"{i:>4}: {line}")
PY
rg -n "PingSchema|PingController|package\(\)->container\(\)|resolve them inside the callback|pull collaborators" docs .Repository: Spaghetti-Dojo/konomi
Length of output: 5001
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path('docs/rest.md')
for i, line in enumerate(path.read_text().splitlines(), 1):
if 170 <= i <= 220:
print(f"{i:>4}: {line}")
PY
rg -n "PingSchema::class|PingController::class|bind|register.*PingSchema|register.*PingController|->get\\(PingSchema::class\\)|->get\\(PingController::class\\)" docs sources tests .Repository: Spaghetti-Dojo/konomi
Length of output: 18558
Resolve the collaborators from the container in the example. The prose says to pull them from Konomi's container, but the snippet still instantiates PingSchema and PingController with new. Update the example to call $container->get(...) or soften the wording.
🤖 Prompt for 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.
In `@docs/rest.md` around lines 54 - 63, Update the REST API example callback to
resolve PingSchema and PingController through the captured $container using
$container->get(...) instead of instantiating them with new, keeping the prose
consistent with the demonstrated container-based collaborator resolution.
| Authentication::new(User\User $user, Rest\ErrorFactory $errorFactory): Authentication; | ||
| ``` | ||
|
|
||
| **`Middlewares\ErrorCatch`** — wraps `$next` in a `try/catch` and converts any `\Throwable` into a `500 internal_error` `WP_Error` (with a generic, translated message; exception details ride along in the error `data`). Attach it **first** so it is the outermost wrapper and catches everything downstream: |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== docs/rest.md excerpt ==\n'
nl -ba docs/rest.md | sed -n '110,135p'
printf '\n== docs/rest.md excerpt (351-357) ==\n'
nl -ba docs/rest.md | sed -n '345,360p'
printf '\n== locate ErrorCatch ==\n'
git ls-files | rg 'ErrorCatch|middlewares|rest\.md$'
printf '\n== search for WP_Error usage and rest response handling ==\n'
rg -n --hidden --glob '!**/vendor/**' --glob '!**/node_modules/**' \
'WP_Error|internal_error|ErrorCatch|rest_ensure_response|error_data|data\]' .Repository: Spaghetti-Dojo/konomi
Length of output: 227
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== docs/rest.md excerpt ==\n'
awk 'NR>=110 && NR<=135 { printf "%d:%s\n", NR, $0 }' docs/rest.md
printf '\n== docs/rest.md excerpt (351-357) ==\n'
awk 'NR>=345 && NR<=360 { printf "%d:%s\n", NR, $0 }' docs/rest.md
printf '\n== locate ErrorCatch ==\n'
git ls-files | rg '(^|/)ErrorCatch|rest\.md$'
printf '\n== search for relevant symbols ==\n'
rg -n --hidden --glob '!**/vendor/**' --glob '!**/node_modules/**' \
'ErrorCatch|WP_Error|internal_error|rest_ensure_response|error_data|Throwable' .Repository: Spaghetti-Dojo/konomi
Length of output: 11142
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== sources/Rest/Middlewares/ErrorCatch.php ==\n'
awk 'NR>=1 && NR<=120 { printf "%d:%s\n", NR, $0 }' sources/Rest/Middlewares/ErrorCatch.php
printf '\n== tests/unit/php/Rest/Middlewares/ErrorCatchTest.php ==\n'
awk 'NR>=1 && NR<=140 { printf "%d:%s\n", NR, $0 }' tests/unit/php/Rest/Middlewares/ErrorCatchTest.phpRepository: Spaghetti-Dojo/konomi
Length of output: 3942
Remove exception details from REST error data.
sources/Rest/Middlewares/ErrorCatch.php puts code, file, line, message, and stackTrace into the WP_Error payload, which can leak internal details to REST clients. Keep the response generic and log the throwable server-side; update docs/rest.md to match.
🤖 Prompt for 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.
In `@docs/rest.md` at line 120, Update sources/Rest/Middlewares/ErrorCatch.php so
its WP_Error response retains only the generic translated message, while logging
the caught Throwable server-side instead of exposing code, file, line, message,
or stackTrace in error data. Then revise the ErrorCatch documentation in
docs/rest.md to describe the sanitized response and server-side logging.
| ### `StorageKey` | ||
|
|
||
| Turns a `User\ItemGroup` into the sanitized `groupKey` string used by `read` / `write`. It strips to `[a-z0-9_]` and throws `\InvalidArgumentException` on an empty value or `\UnexpectedValueException` if sanitizing would change the value (invalid characters present). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Describe StorageKey::for() as validation, not stripping.
StorageKey::for() computes a sanitized candidate but throws UnexpectedValueException when sanitization would change the input; it does not return a stripped value. Update this wording so callers do not expect invalid characters to be silently removed.
🤖 Prompt for 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.
In `@docs/storage.md` around lines 114 - 116, Update the StorageKey documentation
to describe StorageKey::for() as validating the group key against the allowed
format, not stripping invalid characters. State that it returns the original
valid value and throws UnexpectedValueException when sanitization would alter
the input, while retaining the existing empty-value exception behavior.
| add_action( | ||
| 'konomi.user.repository.save-successfully', | ||
| static function ( | ||
| User\Item $item, | ||
| User\User $user, | ||
| User\ItemGroup $group, | ||
| Storage\StorageKey $storageKey | ||
| ): void { | ||
| // Runs after the item was written on Axis::User. | ||
| error_log(sprintf('User %d saved post %d (%s)', $user->id(), $item->id(), $group->value)); | ||
| }, | ||
| 10, | ||
| 4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Avoid recommending raw identifiers in error_log().
This example logs both the user ID and post ID. Replace it with privacy-safe, controlled logging or explicitly warn that identifiers must follow the host application's logging and retention policy.
🤖 Prompt for 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.
In `@docs/user.md` around lines 112 - 124, Update the add_action example’s
error_log call to avoid directly logging the user and item identifiers; use
privacy-safe controlled logging instead, or explicitly state that any
identifiers must comply with the host application’s logging and retention
policy.
1 participant
Summary by CodeRabbit