[5 / 7] - Unstuck / Scroll of Resurrection / RAF#202
Merged
Conversation
📝 WalkthroughWalkthroughThis PR adds a new ChangesCharacter UI Overhaul and Security Hardening
Dark Mode Feature
Sequence Diagram(s)sequenceDiagram
rect rgba(30, 100, 200, 0.5)
Note over Browser,WP_REST: Item Restoration Flow
end
participant Browser
participant ItemRestorationPage
participant WP_REST
participant ToolsApi
participant ACoreServices
participant DB
Browser->>ItemRestorationPage: click character row
ItemRestorationPage->>WP_REST: GET /v1/item-restore/list/{guid} (nonce header)
WP_REST->>ToolsApi: ItemRestoreList (is_user_logged_in check)
ToolsApi->>ACoreServices: getRestorableItemsByCharacter(guid)
ACoreServices->>DB: verify ownership + query recovery_item
DB-->>ACoreServices: items with Count, DeleteDate
ACoreServices-->>ToolsApi: item rows
ToolsApi-->>Browser: JSON items
ItemRestorationPage->>Browser: render item cards with Wowhead icons
Browser->>WP_REST: POST /v1/item-restore {item, cname} (nonce header)
WP_REST->>ToolsApi: ItemRestore (is_user_logged_in check)
ToolsApi->>ToolsApi: filter_var validate item >= 1
ToolsApi->>ACoreServices: currentAccountOwnsCharacterName(cname)
ACoreServices->>DB: SELECT characters WHERE name+account+deleteDate IS NULL
DB-->>ACoreServices: owned?
ToolsApi->>ToolsApi: SOAP restore command
ToolsApi-->>Browser: success
ItemRestorationPage->>Browser: remove restored card, renumber remaining
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 7
🤖 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 `@src/acore-wp-plugin/src/Components/CharactersMenu/CharactersController.php`:
- Line 26: There is a typo in the success notice message within the div with
class "updated" in the CharactersController.php file. The word "succesfully" is
misspelled and should be corrected to "successfully". Locate the success message
text "Character settings succesfully saved." and fix the spelling of the word to
display the correct message to users.
In `@src/acore-wp-plugin/src/Components/MailReturnMenu/MailReturnView.php`:
- Around line 58-59: The src attributes of the race-icon and class-icon img
elements are outputting dynamically constructed URLs without proper escaping.
While the alt and title attributes are correctly using esc_attr() for escaping,
the src attributes need to be wrapped with esc_url() to properly escape the
generated file paths. Apply esc_url() around the concatenated path expressions
in both the race icon src (constructed with ACORE_URL_PLG,
intval($char['race']), gender condition, and '.webp') and the class icon src
(constructed with ACORE_URL_PLG, intval($char['class']), and '.webp') to harden
the URL output.
In `@src/acore-wp-plugin/src/Components/Tools/ToolsApi.php`:
- Around line 19-22: The ItemRestore method in ToolsApi.php only validates that
the character name belongs to the current user, but does not verify that the
recovery_item itself is associated with that character and account. To fix this,
create a new service method in ACoreServices (similar to the example provided in
the comment) that validates both the recovery_item and character together by
querying the recovery_item table joined with the characters table, ensuring the
item ID matches a valid recovery for the given account and character name. Then
update the ItemRestore method to call this new validation method before
executing the SOAP command. Additionally, update the WooCommerce fulfillment
caller that invokes ItemRestore to pass the purchaser's account ID explicitly
instead of relying on the current WordPress session context.
In `@src/acore-wp-plugin/src/Components/UserPanel/Pages/ItemRestorationPage.php`:
- Around line 217-235: The success condition in the ItemRestorationPage.php file
incorrectly gates on checking if the response data contains the substring
'mail'. Since parseJsonResponse() already guarantees HTTP-success for responses
that reach this point, remove the data.toLowerCase().includes('mail') check from
the if statement. The entire block containing
cardEl.parentElement.removeChild(), successBox updates, card renumbering logic,
and the remaining.length check should execute unconditionally for all successful
HTTP responses, not just those with 'mail' in the payload.
In `@src/acore-wp-plugin/src/Components/UserPanel/Pages/RafProgressPage.php`:
- Line 35: The UserController::showRafProgress() method is missing server-side
nonce verification for the CSRF protection token generated by wp_nonce_field().
Add a nonce verification call using wp_verify_nonce() immediately after checking
if the request method is POST and before processing any data from the
$_POST["recruited"] variable. The nonce verification should use the action
'acore_raf_recruit' and the nonce field name 'acore_raf_nonce' that match the
wp_nonce_field() call on line 35. If verification fails, the request should be
rejected to prevent CSRF attacks.
In `@src/acore-wp-plugin/web/assets/css/main.css`:
- Around line 555-557: The selector body.profile-php `#wpbody-content` > .wrap
applies the max-width: 900px rule too broadly, and its higher specificity
overrides the later full-width rules for `#acore-mail-return-page` and
`#acore-item-restoration-page`. Scope the max-width rule more specifically by
adding an additional selector constraint that excludes or targets only the pages
that should have the 900px constraint, while allowing `#acore-mail-return-page`
and `#acore-item-restoration-page` to maintain their full-width behavior. This may
involve using a more specific child or descendant selector, or creating a
separate rule that explicitly targets only non-full-width profile pages.
In `@src/acore-wp-plugin/web/assets/css/theme.css`:
- Around line 600-616: The CSS selectors for dark mode styling of `.subsubsub`
links, checkboxes, and `.btn` elements are scoped too broadly under `#wpcontent
`#wpbody-content``, causing these styles to apply across all admin pages when dark
mode is active. Scope these selectors to only apply within myCRED pages by
adding `#myCRED-wrap` (or `.mycred-log-page`) as a parent selector in the chain
for all the rules between lines 600-616. This ensures the color overrides for
subsubsub links, input checkboxes, and button styling only affect the myCRED
plugin pages and do not bleed into unrelated admin screens.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b0d0c4f-157d-420d-8a34-b862b03ab056
📒 Files selected for processing (19)
src/acore-wp-plugin/src/Components/CharactersMenu/CharactersController.phpsrc/acore-wp-plugin/src/Components/CharactersMenu/CharactersView.phpsrc/acore-wp-plugin/src/Components/MailReturnMenu/MailReturnController.phpsrc/acore-wp-plugin/src/Components/MailReturnMenu/MailReturnView.phpsrc/acore-wp-plugin/src/Components/ResurrectionScrollMenu/ResurrectionScrollMenu.phpsrc/acore-wp-plugin/src/Components/ResurrectionScrollMenu/ResurrectionScrollView.phpsrc/acore-wp-plugin/src/Components/Tools/ToolsApi.phpsrc/acore-wp-plugin/src/Components/UnstuckMenu/UnstuckView.phpsrc/acore-wp-plugin/src/Components/UserPanel/Pages/ItemRestorationPage.phpsrc/acore-wp-plugin/src/Components/UserPanel/Pages/RafProgressPage.phpsrc/acore-wp-plugin/src/Hooks/Various/DarkMode.phpsrc/acore-wp-plugin/src/Manager/ACoreServices.phpsrc/acore-wp-plugin/src/Manager/Soap/SmartstoneService.phpsrc/acore-wp-plugin/src/Utils/AcoreCharColors.phpsrc/acore-wp-plugin/src/boot.phpsrc/acore-wp-plugin/web/assets/css/dark-mode.csssrc/acore-wp-plugin/web/assets/css/main.csssrc/acore-wp-plugin/web/assets/css/theme.csssrc/acore-wp-plugin/web/assets/mail-return/mail-return.js
TheSCREWEDSoftware
added a commit
to TheSCREWEDSoftware/acore-cms
that referenced
this pull request
Jun 24, 2026
(Resume generated by Claude) **PR azerothcore#199 — Characters Menu** - Fixed typo: "succesfully" → "successfully" in save confirmation - Fixed broken sentence in the Order Character Screen description - Extended `user-select: none` to cover all `.acore-char-row` elements, not just `button` **PR azerothcore#200 — Mail Return** - *(issues covered by later PRs — no unique fixes)* **PR azerothcore#201 — Item Restoration** - Added `alt=` attributes to race/class icons in `CharactersView.php` - `ToolsApi`: `ItemRestoreList` now catches exceptions and returns a 403 instead of a 500 - `ToolsApi` + `ACoreServices`: **Security fix** — `ItemRestore` now verifies the recovery item belongs to the character, not just that the user owns the character name (IDOR vulnerability) **PR azerothcore#202 — Unstuck / Scroll / RAF** - `ItemRestorationPage`: removed brittle `data.includes('mail')` success check — any HTTP success now triggers the success path **PR azerothcore#203 — Security Tab** - `CharactersController`: `resetCharacterOrder()` now guards against invalid/null account ID before running UPDATE - `Tools.php`: added `esc_attr()` on banned-names table input (XSS fix) - `Tools.php`: added `is_array()` guard on thresholds `foreach` - `Tools.php` + `SettingsController`: added sentinel input so deleting all thresholds actually clears them in the DB - `Tools.php`: GeoIP hidden fallback `value="0"` is now always rendered, not conditionally **PR azerothcore#204 — Admin Settings / CSRF** - `AcoreCharColors`: removed unused `$fDark`, unknown race IDs now get a neutral color instead of Horde red - `MailReturnView`: added `esc_url()` on race/class icon `src` attributes - `DarkMode`: added `dmToggleInFlight` flag to prevent overlapping AJAX requests on rapid dark mode clicks
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split from the main PR: #197
Split by Claude
Unstuck restyled to the new rows; Scroll of Resurrection gets green/red/yellow status colours; RAF (recruit-a-friend) gets a CSRF nonce + validation. Minor shared-service tidy.
Not testde the RAF page, as the changes were made by Rabbit's review.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes