Repository navigation
Demo/Portal example add-on update - #76
Conversation
📝 WalkthroughWalkthroughMultiple documentation and code example updates reflecting widespread renaming of controller and service classes/files (e.g., Forum → ForumController, Creator → CreatorService, Editor → EditorService), an Entity method signature change to Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/lets-build-an-add-on.md`:
- Line 753: Line 753 contains a hard tab before the function declaration public
function canFeatureUnfeature(&$error = null): bool — replace the hard tab with
spaces to match surrounding indentation and satisfy MD010; update the whitespace
at the start of that line (the declaration for canFeatureUnfeature) so it uses
the same number of spaces as other lines in the file.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/files/Demo-Portal-1.0.0 Alpha.zipis excluded by!**/*.zip
📒 Files selected for processing (1)
docs/lets-build-an-add-on.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/lets-build-an-add-on.md
753-753: Hard tabs
Column: 1
(MD010, no-hard-tabs)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/lets-build-an-add-on.md`:
- Around line 828-830: The lines inside the setupThreadEdit function use hard
tabs for indentation which violates MD010; open the function definition for
public function setupThreadEdit(\XF\Entity\Thread $thread) and the docblock line
referencing \Demo\Portal\XF\Service\Thread\EditorService and replace tabs with
spaces so all indentation is space-based (consistent with the project's 2- or
4-space style).
- Around line 369-371: The prose instructs creating `Forum.php` but the example
code block and title use `ForumController.php`; update the instructional text to
reference `ForumController.php` so the filename in the narrative matches the
code block (path: src/addons/Demo/Portal/XF/Admin/Controller and the file name
shown in the code block/Title), ensuring the description, the example filename,
and any subsequent mentions consistently use `ForumController.php`.
- Line 367: Update the wording to use the correct indefinite article before
"Extension class name": change "a Extension class name" to "an Extension class
name" in the sentence that references the base class
`XF\Admin\Controller\ForumController` and the extension
`Demo\Portal\XF\Admin\Controller\ForumController` (the sentence ending with "We
should create that class before clicking Save.").
♻️ Duplicate comments (1)
docs/lets-build-an-add-on.md (1)
754-754: Replace hard tab indentation to satisfy MD010.
This is the same hard‑tab issue previously reported for this line.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/files/Demo-Portal-1.0.0 Alpha.zipis excluded by!**/*.zip
📒 Files selected for processing (1)
docs/lets-build-an-add-on.md
🧰 Additional context used
🪛 LanguageTool
docs/lets-build-an-add-on.md
[misspelling] ~367-~367: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...orumController`. And we need to specify a "Extension class name" which is the cla...
(EN_A_VS_AN)
[typographical] ~433-~433: There might be a comma missing.
Context: ...CP_CreatorService { } ``` While we're here we will also create another extension. ...
(IF_PRP_PRP_COMMA)
[style] ~765-~765: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e and feature or unfeature accordingly. We are going to need two new class extensi...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~767-~767: Consider removing “of” to be more concise
Context: ... that in the relevant location. Here is all of the code for the extended class: ```php ti...
(ALL_OF_THE)
🪛 markdownlint-cli2 (0.18.1)
docs/lets-build-an-add-on.md
754-754: Hard tabs
Column: 1
(MD010, no-hard-tabs)
828-828: Hard tabs
Column: 1
(MD010, no-hard-tabs)
829-829: Hard tabs
Column: 1
(MD010, no-hard-tabs)
830-830: Hard tabs
Column: 1
(MD010, no-hard-tabs)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/lets-build-an-add-on.md (1)
870-899: Fix the file path in the code block title to match the forum controller section.
The prose says to edit the extended forum controller, but the code block title showsXF/Pub/Controller/ThreadController.php. This can misdirect readers.Proposed fix
-```php title="src/addons/Demo/Portal/XF/Pub/Controller/ThreadController.php" +```php title="src/addons/Demo/Portal/XF/Pub/Controller/ForumController.php"
🤖 Fix all issues with AI agents
In `@docs/lets-build-an-add-on.md`:
- Line 433: Update the sentence in the documentation text to include a comma
after "here": change "While we're here we will also create another extension."
to "While we're here, we will also create another extension." in the
docs/lets-build-an-add-on.md file (the line describing creating the extension
class Demo\Portal\XF\Pub\Controller\ForumController and the file path
src/addons/Demo/Portal/XF/Pub/Controller/ForumController.php).
♻️ Duplicate comments (1)
docs/lets-build-an-add-on.md (1)
754-754: Hard tabs remain in code block indentation.
Please replace hard tabs with spaces to satisfy MD010.Also applies to: 828-829
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/lets-build-an-add-on.md
🧰 Additional context used
🪛 LanguageTool
docs/lets-build-an-add-on.md
[typographical] ~433-~433: There might be a comma missing.
Context: ...CP_CreatorService { } ``` While we're here we will also create another extension. ...
(IF_PRP_PRP_COMMA)
[style] ~765-~765: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e and feature or unfeature accordingly. We are going to need two new class extensi...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~767-~767: Consider removing “of” to be more concise
Context: ... that in the relevant location. Here is all of the code for the extended class: ```php ti...
(ALL_OF_THE)
🪛 markdownlint-cli2 (0.18.1)
docs/lets-build-an-add-on.md
754-754: Hard tabs
Column: 1
(MD010, no-hard-tabs)
828-828: Hard tabs
Column: 1
(MD010, no-hard-tabs)
829-829: Hard tabs
Column: 1
(MD010, no-hard-tabs)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
I have gone through this documentation and built the add-on along with it. I committed all the fixes and I believe it is now up-to-date. That said in the method
I downloaded the 2.0.0 Xenforo files and checked the Replier and couldn't find this |
dc4af2a to
b56391c
Compare
The snippet omitted the error reference and return type, so an extended thread entity could not safely override the core method.
The docblock named a class that does not exist, so editors and static analysis could not resolve the repository returned by `XF:Attachment`.
The archive had drifted from the tutorial, so readers who downloaded it saw different metadata and option definitions. Replace it with an export that matches the guide.
XenForo names controllers and services with a `Controller` or `Service` suffix. Rename the extension classes and code block titles to match, so the copied examples extend real classes.
The text still pointed at `Forum.php` after the class rename, and used "a" before "Extension". Align both with the surrounding instructions.
The index page route lives under "Basic options", not "Basic board information", so the instructions sent readers to the wrong page.
The snippets extend the forum controller but were titled as the thread controller, so the file path in the tutorial was misleading.
The permission group is a literal identifier, so render it as code like the other values in the same step.
The admin option editor calls the field "Number box", and "Spin box" no longer appears in the UI, so readers could not find it.
The finder method in the example no longer exists. Point readers at `sliceToPage()`, which the surrounding code already uses.
The tutorial and downloadable add-on relied on outdated XenForo APIs. The demo permission check also overrode the core feature permission, and the package did not declare its minimum XenForo version.
b56391c to
2196b46
Compare
|
I’ve addressed the reply issue you found by extending Merging this now, thanks for bringing it up to date! |
The add-on was a tad outdated, so I rewrote it using a Xenforo 2.3.8 Preview installation.
I changed as little as I could, only fixing errors as I saw them.
If I missed any, please let me know. :)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.