fix(item-restoration): send the REST nonce from the shop page - #208
Merged
Conversation
The item-restore routes got a permission_callback in #202, but the shop page still calls them without X-WP-Nonce, so WordPress drops the cookie identity and answers 401. The fetch has no error handling either, so that 401 ends up in the same branch as an empty list and the page claims there is nothing to recover.
📝 WalkthroughWalkthroughThe item restoration UI now displays a load-error alert. REST requests include a WordPress REST nonce, validate HTTP status, and parse JSON only after successful responses. Failures hide the item container and stop the loader. ChangesItem restoration loading flow
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 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 |
![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
🤖 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/Hooks/WooCommerce/ItemRestoration.php`:
- Around line 229-246: Update selectCharacter() to track the latest item-list
request with a request identifier or equivalent token, and have every associated
then(), catch(), and finally() callback return without changing shared DOM when
it is no longer current. Apply the same stale-response guard to the request
handling around the fetch call and the additional callbacks near the later
referenced block, while preserving updates for the latest selected character.
🪄 Autofix
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: 12321fc9-a2ff-4b80-b99d-62743a33d17c
📒 Files selected for processing (1)
src/acore-wp-plugin/src/Hooks/WooCommerce/ItemRestoration.php
Comment on lines
+229
to
+246
| loadError.style.display = 'none'; | ||
| loaderIcon.style.display = 'block'; | ||
|
|
||
| itemListLoaders.forEach(element => element.classList.remove('hidden')); | ||
| itemContainer.classList.remove('hidden'); | ||
| const character = charGuid; | ||
| const characterName = charList.options[charList.selectedIndex].innerText; | ||
|
|
||
| fetch('<?= get_rest_url(null, 'acore/v1/item-restore/list/'); ?>' + character) | ||
| .then((response) => response.json()) | ||
| fetch('<?= get_rest_url(null, 'acore/v1/item-restore/list/'); ?>' + character, { | ||
| headers: { 'Accept': 'application/json', 'X-WP-Nonce': restNonce } | ||
| }) | ||
| .then((response) => { | ||
| if (!response.ok) { | ||
| throw new Error('Request failed with status ' + response.status); | ||
| } | ||
|
|
||
| return response.json(); | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Ignore stale responses when the character changes.
When selectCharacter() runs again before the previous request finishes, the older request can still update the shared DOM. A late response can replace the current character’s items. A late failure can hide a successful result and display the error alert. Track the latest request and ignore stale .then(), .catch(), and .finally() callbacks.
Proposed fix
+let latestRequestId = 0;
+
function selectCharacter(charGuid) {
+ const requestId = ++latestRequestId;
+
...
.then((response) => {
+ if (requestId !== latestRequestId) return;
if (!response.ok) {
throw new Error('Request failed with status ' + response.status);
}
return response.json();
})
.then(function(items) {
+ if (requestId !== latestRequestId) return;
...
})
.catch(() => {
+ if (requestId !== latestRequestId) return;
...
})
.finally(() => {
+ if (requestId !== latestRequestId) return;
...
});
}Also applies to: 292-296
🤖 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 `@src/acore-wp-plugin/src/Hooks/WooCommerce/ItemRestoration.php` around lines
229 - 246, Update selectCharacter() to track the latest item-list request with a
request identifier or equivalent token, and have every associated then(),
catch(), and finally() callback return without changing shared DOM when it is no
longer current. Apply the same stale-response guard to the request handling
around the fetch call and the additional callbacks near the later referenced
block, while preserving updates for the latest selected character.
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
1 participant
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.
Item restoration is broken on the shop page: no matter how many deleted items a character has, it always says "There are no items to recover for the selected character".
The list endpoint answers 401 for everyone:
#202 added
permission_callback => is_user_logged_in()to both item-restore routes, which is the right thing to do, but the shop page still calls them with a barefetch(). Being logged in isn't enough for the REST API:rest_cookie_check_errors()callswp_set_current_user(0)when there's no_wpnonceparam and noX-WP-Nonceheader, so the request is anonymous and the permission callback rejects it.The user panel page already got the nonce when it was reworked, so the shop page was the last caller left.
Second half of the fix: the fetch had no
.catchand didn't look at the status, so an error body has no.lengthand lands in the same branch as an empty list. That's why a 401 was reported to players as "you have nothing to recover". Now a failed request shows an error instead.How to test
Buy or open the item-restoration product with a character that has rows in
recovery_item, they should be listed again. To check the failure path, drop theX-WP-Nonceheader in devtools and confirm you get the error message rather than the empty state.Follow-up, not in here
item-restore/liststill takes any character guid from any logged-in user, so you can enumerate someone else's deleted items. The POST already checks ownership viacurrentAccountOwnsCharacterName(), the GET could use the same treatment.Summary by CodeRabbit