Pattern directory: say why a pattern title is invalid - #782
Conversation
The error now names the reason, e.g. `Pattern titles cannot include "Test".`, instead of a generic "title is invalid". Disallowed words are matched as whole words, so titles like "Latest Posts" and "Contest Banner" are no longer rejected. Fixes #743. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 6 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughTitle validation now returns specific reasons for invalid titles. Disallowed-word checks use case-insensitive whole-word matching, and tests cover valid titles containing those words as substrings and REST error messages. ChangesPattern title validation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Low Suggested reviewers: Merge Risk: ⚪ Minimal · up to The title-validation change appears ready to merge after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Pattern submissions now accept some titles previously rejected and receive more specific error messages. Existing validation and moderation checks appear unchanged, but the public behavior warrants review. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
Copilot review overview
🔵 Needs a closer look
Improve Unicode whole-word matching and assert the interactivity-directive error message.
Review effort: Lite
Findings: None
What changed in this PR
Updates pattern title validation with clearer rejection messages and whole-word matching.
Changes:
- Adds descriptive validation errors.
- Prevents false positives within larger words.
- Expands validation and message tests.
| File | Summary |
|---|---|
public_html/wp-content/plugins/pattern-directory/includes/pattern-validation.php |
Implements detailed errors and whole-word matching; Unicode-aware matching needs adjustment. |
public_html/wp-content/plugins/pattern-directory/tests/phpunit/pattern-title-validation-test.php |
Adds coverage for accepted titles and validation messages. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
LGTM. CI is green and it merges cleanly into trunk.
is_title_valid()had no other callers, so it's safe to rename it toget_title_error(). Nothing else reads the old error text.- I tried the new regex on a few edge cases. "Latest Posts", "Contest Banner" and "Testübersicht" pass. "Test" and "Test-Pattern" are rejected. With
/u,\btreats non-ASCII letters as word characters, so a word like "Testübersicht" isn't flagged by mistake. - Showing the matched text (e.g.
"My Pattern") in the error is safe. Titles with HTML or shortcodes are rejected before this check runs.
Nit, not blocking: matching whole words also lets through plurals that were rejected before, e.g. "My Patterns" and "Examples". "Examples Grid" is intended per the tests. You could still reject "My Patterns" with my patterns? if that's wanted.
Whole-word matching let the plural through, which the old check rejected. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Thanks for testing the edge cases, @bor0. b71268e adds "my patterns" to the list, so "My Patterns" is rejected again like before. It says as little about the pattern as "My Pattern" does. I left "Examples" and "Tests" allowed on purpose. "Examples Grid" can describe a real pattern, and "Tests" on its own rarely comes up as a whole word. |
Fixes #743.
The title error now says why, e.g.
Pattern titles cannot include "Test".Disallowed words only match whole words, so "Latest Posts" and "Contest Banner" are no longer rejected.Testing
Pattern titles cannot include "Test".🤖 Generated with Claude Code
Summary by CodeRabbit