-
Notifications
You must be signed in to change notification settings - Fork 92
feat: docs duplicator #232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: doc_dupulcate_hirarchy_update
Are you sure you want to change the base?
feat: docs duplicator #232
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds a document duplication REST endpoint and recursive copy logic; integrates a new duplicate action in the UI and data layer. Updates built assets: PHP asset manifests (version hashes, opening tags), CSS (comments/source maps), JS bundles (webpack runtime). Overhauls built store.js into modular docs/settings stores. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (13)
assets/build/print.css (1)
1-3: Sourcemap linkage looks consistent.No style changes; only header and map ref added. Same advice as frontend.css regarding shipping maps vs stripping in prod.
Also applies to: 185-186
assets/build/style-block.css (3)
53-55: Duplicate width is intentional but triggers linter.
width: -moz-fit-content; width: fit-content;is a vendor fallback. If Biome/linters flag it, suppress in the SCSS source or drop the-moz-variant if you don’t support legacy Firefox.
141-141: @charset must be first; move or remove.
@charset "UTF-8";mid-file is ignored per spec. Prefer removing it from generated CSS or ensuring the toolchain places it at the very top.-@charset "UTF-8"; +/* charset handled by bundler; removed from mid-file */
319-323: Selector nit: tighten ::marker selector.Use
li::marker(no space) for clarity.-.wedocs-docs-grid__articles li ::marker { +.wedocs-docs-grid__articles li::marker {src/data/docs/actions.js (1)
173-190: DRY the refresh path by reusing updateParentDocs().You’re repeating the fetch/sort/update flow already implemented in updateParentDocs().
const response = yield { type: 'UPDATE_TO_API', path, data: {} }; - // Refresh the docs list after duplication - const getDocsPath = wp.hooks.applyFilters( - 'wedocs_documentation_fetching_path', - `/wp/v2/docs?per_page=-1&status=publish${ typeof weDocsAdminVars !== 'undefined' ? ',draft' : ''}` - ); - const updatedDocs = yield actions.fetchFromAPI( getDocsPath ); - const parentDocs = updatedDocs.filter( ( doc ) => ! doc.parent ); - const sortableDocs = parentDocs?.sort( ( a, b ) => a.menu_order - b.menu_order ); - - yield actions.setParentDocs( sortableDocs ); - yield actions.setDocs( updatedDocs ); + // Refresh the docs list after duplication + yield actions.updateParentDocs(); return response;src/components/DocActions.js (1)
46-75: Polish: contextualize toast copy and keep UX consistent.Make success/error text reflect the current item type (doc vs article).
- title : __( 'Documentation duplicated successfully!', 'wedocs' ), + title : __( `${ type === 'doc' ? 'Documentation' : 'Article' } duplicated successfully!`, 'wedocs' ), ... - text : __( 'The document and all its child articles have been duplicated as drafts.', 'wedocs' ), + text : __( `${ type === 'doc' ? 'The documentation' : 'The article' } and its child items have been duplicated as drafts.`, 'wedocs' ),Also, this PR’s description says “Adds a Duplicate option for single docs only,” but the menu shows it only when type === 'article'. Confirm intended scope and adjust the condition if needed.
includes/API/API.php (3)
957-989: Prefer existing get_doc() and return WP core shape consistently.Use get_doc() for validation (consistent error codes), then permission check can also rely on edit_post for that ID. Optional.
1001-1060: Safer duplication: i18n title suffix, skip ephemeral meta, consider menu_order.
- Title suffix should be translatable.
- Avoid copying ephemeral meta like _edit_lock/_edit_last.
- Consider placing the duplicate at the end of siblings to avoid menu_order collisions.
- 'post_title' => $original_doc->post_title . ' (Copy)', + 'post_title' => sprintf( __( '%s (Copy)', 'wedocs' ), $original_doc->post_title ),Example meta filter:
$skip_meta = [ '_edit_lock', '_edit_last' ]; foreach ( $meta_data as $key => $values ) { if ( in_array( $key, $skip_meta, true ) ) { continue; } foreach ( $values as $value ) { add_post_meta( $new_doc_id, $key, maybe_unserialize( $value ) ); } }Optional ordering:
// Place at end if same parent to avoid tie with menu_order if ( $new_post_data['post_parent'] ) { $siblings = get_children([ 'post_parent' => $new_post_data['post_parent'], 'post_type' => 'docs', 'fields' => 'ids' ]); $new_post_data['menu_order'] = is_array( $siblings ) ? count( $siblings ) : 0; }
960-962: Docblocks: replace placeholder since tag.Replace “@SInCE WEDOCS_SINCE” with the actual next version.
Also applies to: 992-999
assets/build/store.js (4)
349-358: DeduplicateuserDocIdsto avoid unbounded growth and repeats.Merging arrays without a set can introduce duplicates.
case 'SET_USER_DOC_IDS': return { ...state, - userDocIds: [...state.userDocIds, ...action.userDocIds] + userDocIds: Array.from(new Set([...state.userDocIds, ...action.userDocIds])) }; case 'SET_USER_DOC_ID': return { ...state, - userDocIds: [...state.userDocIds, action.userDocId] + userDocIds: state.userDocIds.includes(action.userDocId) + ? state.userDocIds + : [...state.userDocIds, action.userDocId] };
506-511: Selector may miss matches due to id type mismatch.Route/query params are often strings; coerce before comparison.
- getDoc: (state, id) => { + getDoc: (state, id) => { const { docs } = state; - return docs.find(doc => doc.id === id); + const targetId = typeof id === 'string' ? Number(id) : id; + return docs.find(doc => doc.id === targetId); },
461-467: Avoid in-place sort side effects by sorting a copy.Defensive copy improves readability and prevents accidental shared mutations.
- const helpfulDocs = docs.sort((a, b) => helpfulDocIds.indexOf(a.id) - helpfulDocIds.indexOf(b.id)).filter(doc => helpfulDocIds?.includes(doc?.id)); + const helpfulDocs = [...docs] + .sort((a, b) => helpfulDocIds.indexOf(a.id) - helpfulDocIds.indexOf(b.id)) + .filter(doc => helpfulDocIds?.includes(doc?.id));
573-580: Naming:getArticleChildrens→getArticleChildren(keep a compat alias).Public API polish; the current name is grammatically incorrect. Add a new selector and keep the old as an alias to avoid breaks.
getArticleChildrens: (state, id) => { const { docs } = state; const childrens = docs?.filter(doc => doc.parent === id)?.reverse(); const sortableChildrens = childrens?.sort((a, b) => a.menu_order - b.menu_order); return sortableChildrens; }, + // Back-compat alias with corrected name + getArticleChildren: (state, id) => { + const { docs } = state; + const children = docs?.filter(doc => doc.parent === id)?.reverse(); + return children?.sort((a, b) => a.menu_order - b.menu_order); + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
assets/build/block.js.mapis excluded by!**/*.mapassets/build/frontend.css.mapis excluded by!**/*.mapassets/build/frontend.js.mapis excluded by!**/*.mapassets/build/index.css.mapis excluded by!**/*.mapassets/build/index.js.mapis excluded by!**/*.mapassets/build/print.css.mapis excluded by!**/*.mapassets/build/print.js.mapis excluded by!**/*.mapassets/build/store.js.mapis excluded by!**/*.mapassets/build/style-block.css.mapis excluded by!**/*.map
📒 Files selected for processing (15)
assets/build/block.asset.php(1 hunks)assets/build/frontend.asset.php(1 hunks)assets/build/frontend.css(2 hunks)assets/build/frontend.js(1 hunks)assets/build/index.asset.php(1 hunks)assets/build/index.css(2 hunks)assets/build/print.asset.php(1 hunks)assets/build/print.css(2 hunks)assets/build/print.js(1 hunks)assets/build/store.asset.php(1 hunks)assets/build/store.js(1 hunks)assets/build/style-block.css(1 hunks)includes/API/API.php(4 hunks)src/components/DocActions.js(2 hunks)src/data/docs/actions.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/DocActions.js (3)
assets/build/index.js (1)
Swal(32002-32002)src/components/Documentations/ParentDocs.js (1)
isAdmin(19-19)src/components/DocListing/DocSections.js (1)
isAdmin(22-22)
src/data/docs/actions.js (3)
src/data/docs/resolvers.js (9)
response(11-11)response(24-24)response(30-30)response(35-35)response(40-40)response(63-63)getDocsPath(3-6)parentDocs(13-13)sortableDocs(14-16)src/blocks/DocsGrid/edit.js (1)
wp(16-16)src/components/Documentations/index.js (1)
parentDocs(24-27)
assets/build/store.js (5)
src/components/DocListing/index.js (3)
docs(27-29)loading(37-40)sections(103-103)src/components/Documentations/index.js (3)
docs(19-22)loading(29-32)parentDocs(24-27)src/components/SelectBox.js (1)
pages(16-16)src/components/DocListing/ListingHeader.js (1)
loading(13-16)src/components/Documentations/ParentDocs.js (2)
sections(36-40)articles(42-46)
🪛 Biome (2.1.2)
assets/build/index.css
[error] 5851-5851: Unexpected value or character.
Expected one of:
(parse)
[error] 5851-5851: Unexpected value or character.
Expected one of:
(parse)
[error] 5851-5851: Unexpected value or character.
Expected one of:
(parse)
[error] 5851-5851: Unexpected value or character.
Expected one of:
(parse)
[error] 5851-5851: Unexpected value or character.
Expected one of:
(parse)
[error] 5851-5851: Unexpected shorthand property flex-flow after flex-direction
(lint/suspicious/noShorthandPropertyOverrides)
[error] 5851-5851: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
display is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5851-5851: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
display is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 5851-5851: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
transition is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
assets/build/style-block.css
[error] 54-54: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
width is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
assets/build/store.js
[error] 338-341: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 342-342: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 392-392: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 393-393: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
🔇 Additional comments (15)
assets/build/block.asset.php (1)
1-1: Version bump and PHP open tag OK.Cache-busting string updated; deps unchanged. Looks good.
assets/build/style-block.css (1)
124-134: CSS var theming: nice.Good use of CSS custom properties for button/icon states; improves block theming.
assets/build/frontend.js (1)
1-28: ESM flag runtime added: OK.Adding
__webpack_require__.rand marking exports as ESM is standard with mini-css-extract output; no behavior change for consumers.assets/build/index.asset.php (1)
1-1: Asset version updated.Deps unchanged; version bump aligns with new build.
assets/build/print.asset.php (1)
1-1: Version bump only.All good.
assets/build/store.asset.php (1)
1-1: Store asset manifest updated.Deps remain
wp-api-fetch,wp-data; version string updated—OK.assets/build/frontend.css (1)
1-3: Sourcemaps present — include in releases or strip sourceMappingURL in prod. assets/build contains frontend.css.map (45K), style-block.css.map (16K), print.css.map (5.5K); sizes reasonable.assets/build/frontend.asset.php (1)
1-1: LGTM: manifest header and version bump align with build output.assets/build/print.js (1)
1-28: LGTM: standard webpack runtime wrapper; no functional risks.src/components/DocActions.js (1)
131-140: Verify gating matches the product decision (“single docs only”).Currently shown only for articles:
- If duplication should be available for top‑level docs, gate on type === 'doc' (or both).
assets/build/index.css (1)
5848-5854: Exclude vendor/minified CSS from Biome/style linting (assets/build/)**Minified react-responsive-carousel CSS in assets/build/index.css triggers Biome parse/duplicate‑prop warnings — exclude assets/build/** from Biome/style linting and retain third‑party license headers. Verified: sourcemap found at /home/jailuser/git/assets/build/index.css.map (stat unavailable in sandbox).
assets/build/store.js (4)
1-4: Linked issue mismatch with PR scope.This PR implements document duplication, but the linked issue (#132) is “Include tags in search.” Update the PR linkage/description or add a separate issue to avoid confusion in release notes and triage.
1239-1241: Store registration LGTM.Registering modular stores via
createReduxStore+registeris correct and aligns with WP data conventions.
115-116: Confirm WP RESTper_page=-1support and add a paginated-fetch fallbackper_page=-1 is commonly capped by WP core (commonly 100) and may be ignored, causing truncated results. Implement a paginated fetch that requests pages 1..N and aggregates results using the X-WP-TotalPages response header, or ensure the server explicitly supports
per_page=-1.Locations: assets/build/store.js — lines 115–116 (also review 184–189 and 427–437).
Verification note: attempted ripgrep search returned no files (filter excluded assets/**). Re-run search including assets or manually confirm usage and implement pagination/fallback.
200-213: Confirm REST endpoint for promotion notice dismissal exists and is permissioned.Just a heads-up to verify server routes and caps (
/wp/v2/docs/hide-promotion-notice) for non-admin users don’t leak capability.
| *duplicateDoc(docId) { | ||
| const path = `/wp/v2/docs/${docId}/duplicate`; | ||
| const response = yield { | ||
| type: 'UPDATE_TO_API', | ||
| path, | ||
| data: {} | ||
| }; | ||
|
|
||
| // Refresh the docs list after duplication | ||
| const getDocsPath = wp.hooks.applyFilters('wedocs_documentation_fetching_path', `/wp/v2/docs?per_page=-1&status=publish${typeof weDocsAdminVars !== 'undefined' ? ',draft' : ''}`); | ||
| const updatedDocs = yield actions.fetchFromAPI(getDocsPath); | ||
| const parentDocs = updatedDocs.filter(doc => !doc.parent); | ||
| const sortableDocs = parentDocs?.sort((a, b) => a.menu_order - b.menu_order); | ||
| yield actions.setParentDocs(sortableDocs); | ||
| yield actions.setDocs(updatedDocs); | ||
| return response; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate action: add loading + DRY the refresh via existing action; handle errors.
Use the existing updateParentDocs() action to refresh, toggle loading, and ensure cleanup even on failure.
*duplicateDoc(docId) {
const path = `/wp/v2/docs/${docId}/duplicate`;
- const response = yield {
- type: 'UPDATE_TO_API',
- path,
- data: {}
- };
-
- // Refresh the docs list after duplication
- const getDocsPath = wp.hooks.applyFilters('wedocs_documentation_fetching_path', `/wp/v2/docs?per_page=-1&status=publish${typeof weDocsAdminVars !== 'undefined' ? ',draft' : ''}`);
- const updatedDocs = yield actions.fetchFromAPI(getDocsPath);
- const parentDocs = updatedDocs.filter(doc => !doc.parent);
- const sortableDocs = parentDocs?.sort((a, b) => a.menu_order - b.menu_order);
- yield actions.setParentDocs(sortableDocs);
- yield actions.setDocs(updatedDocs);
- return response;
+ yield actions.setLoading(true);
+ try {
+ const response = yield {
+ type: 'UPDATE_TO_API',
+ path,
+ data: {}
+ };
+ yield actions.updateParentDocs();
+ return response;
+ } finally {
+ yield actions.setLoading(false);
+ }
}📝 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.
| *duplicateDoc(docId) { | |
| const path = `/wp/v2/docs/${docId}/duplicate`; | |
| const response = yield { | |
| type: 'UPDATE_TO_API', | |
| path, | |
| data: {} | |
| }; | |
| // Refresh the docs list after duplication | |
| const getDocsPath = wp.hooks.applyFilters('wedocs_documentation_fetching_path', `/wp/v2/docs?per_page=-1&status=publish${typeof weDocsAdminVars !== 'undefined' ? ',draft' : ''}`); | |
| const updatedDocs = yield actions.fetchFromAPI(getDocsPath); | |
| const parentDocs = updatedDocs.filter(doc => !doc.parent); | |
| const sortableDocs = parentDocs?.sort((a, b) => a.menu_order - b.menu_order); | |
| yield actions.setParentDocs(sortableDocs); | |
| yield actions.setDocs(updatedDocs); | |
| return response; | |
| } | |
| *duplicateDoc(docId) { | |
| const path = `/wp/v2/docs/${docId}/duplicate`; | |
| yield actions.setLoading(true); | |
| try { | |
| const response = yield { | |
| type: 'UPDATE_TO_API', | |
| path, | |
| data: {} | |
| }; | |
| yield actions.updateParentDocs(); | |
| return response; | |
| } finally { | |
| yield actions.setLoading(false); | |
| } | |
| } |
🤖 Prompt for AI Agents
In assets/build/store.js around lines 214 to 230, the duplicateDoc action
duplicates a doc then manually refreshes and sorts docs without using the
existing updateParentDocs() helper, lacks loading state toggling and does not
handle errors/cleanup; replace the manual refresh with a call to
updateParentDocs() (which updates parent and child docs), wrap the operation in
a try/catch/finally to set a loading flag before the API call and clear it in
finally, log or rethrow any caught error, and return the response—this DRYs the
refresh logic, ensures loading is shown during the op, and guarantees cleanup on
failure.
| const setDocState = { | ||
| ...state, | ||
| docs: [...state.docs, action.doc] | ||
| }; | ||
| const isNotInParent = !state.parents.some(parent => parent?.id === action?.doc?.id); | ||
| if (!action.doc.parent && isNotInParent) { | ||
| setDocState.parents = [{ | ||
| ...action.doc | ||
| }, ...state.parents]; | ||
| } | ||
| return setDocState; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switch-case declarations must be wrapped; also upsert instead of append to avoid duplicate docs.
Avoid cross‑case leakage (Biome: noSwitchDeclarations) and prevent duplicate entries when the same doc is set twice.
- case 'SET_DOC':
- const setDocState = {
- ...state,
- docs: [...state.docs, action.doc]
- };
- const isNotInParent = !state.parents.some(parent => parent?.id === action?.doc?.id);
- if (!action.doc.parent && isNotInParent) {
- setDocState.parents = [{
- ...action.doc
- }, ...state.parents];
- }
- return setDocState;
+ case 'SET_DOC': {
+ const idx = state.docs.findIndex(d => d?.id === action?.doc?.id);
+ const docs =
+ idx >= 0
+ ? [...state.docs.slice(0, idx), action.doc, ...state.docs.slice(idx + 1)]
+ : [...state.docs, action.doc];
+ const isNotInParent = !state.parents.some(parent => parent?.id === action?.doc?.id);
+ const base = { ...state, docs };
+ return !action.doc?.parent && isNotInParent
+ ? { ...base, parents: [{ ...action.doc }, ...state.parents] }
+ : base;
+ }📝 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.
| const setDocState = { | |
| ...state, | |
| docs: [...state.docs, action.doc] | |
| }; | |
| const isNotInParent = !state.parents.some(parent => parent?.id === action?.doc?.id); | |
| if (!action.doc.parent && isNotInParent) { | |
| setDocState.parents = [{ | |
| ...action.doc | |
| }, ...state.parents]; | |
| } | |
| return setDocState; | |
| case 'SET_DOC': { | |
| const idx = state.docs.findIndex(d => d?.id === action?.doc?.id); | |
| const docs = | |
| idx >= 0 | |
| ? [...state.docs.slice(0, idx), action.doc, ...state.docs.slice(idx + 1)] | |
| : [...state.docs, action.doc]; | |
| const isNotInParent = !state.parents.some(parent => parent?.id === action?.doc?.id); | |
| const base = { ...state, docs }; | |
| return !action.doc?.parent && isNotInParent | |
| ? { ...base, parents: [{ ...action.doc }, ...state.parents] } | |
| : base; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 338-341: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 342-342: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In assets/build/store.js around lines 338 to 348, the case-level declarations
leak because they are not wrapped in a block and the code appends docs/parents
which can create duplicates; wrap the case body in braces to scope const
setDocState, then change the append logic to perform an upsert by id: replace
docs with a new array that maps existing docs replacing the one with matching id
or adds it if missing, and do the same for parents (only when doc.parent is
falsy) ensuring you check ids for equality before inserting so duplicates are
prevented.
| docs: [...state.docs?.filter(doc => doc.id !== action.docId)], | ||
| parents: [...state.parents?.filter(parent => parent.id !== action.docId)] | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsafe optional chaining in array spreads (Biome: noUnsafeOptionalChaining).
state.docs?.filter(...) can short‑circuit to undefined, which breaks the spread. Default to [].
- docs: [...state.docs?.filter(doc => doc.id !== action.docId)],
- parents: [...state.parents?.filter(parent => parent.id !== action.docId)]
+ docs: [...(Array.isArray(state.docs) ? state.docs : []).filter(doc => doc.id !== action.docId)],
+ parents: [...(Array.isArray(state.parents) ? state.parents : []).filter(parent => parent.id !== action.docId)]📝 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.
| docs: [...state.docs?.filter(doc => doc.id !== action.docId)], | |
| parents: [...state.parents?.filter(parent => parent.id !== action.docId)] | |
| }; | |
| docs: [...(Array.isArray(state.docs) ? state.docs : []).filter(doc => doc.id !== action.docId)], | |
| parents: [...(Array.isArray(state.parents) ? state.parents : []).filter(parent => parent.id !== action.docId)] | |
| }; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 392-392: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 393-393: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
🤖 Prompt for AI Agents
In assets/build/store.js around lines 392 to 394, the spread uses optional
chaining like state.docs?.filter(...) and state.parents?.filter(...) which can
yield undefined and break the array spread; change both to use a safe default
empty array before filtering (e.g., replace optional chaining with a
nullish-coalescing or logical-or fallback to [] so the spread always receives an
array), ensuring the resulting docs and parents are arrays even when state.docs
or state.parents is undefined.
| register_rest_route( $this->namespace, '/' . $this->rest_base . '/(?P<id>[\d]+)/duplicate', [ | ||
| [ | ||
| 'methods' => WP_REST_Server::CREATABLE, | ||
| 'callback' => [ $this, 'duplicate_doc' ], | ||
| 'permission_callback' => [ $this, 'create_item_permissions_check' ], | ||
| ], | ||
| ] ); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: permissive permission_callback allows any logged‑in user to duplicate.
duplicate uses create_item_permissions_check(), which returns true for any logged‑in user (and expects name/email unrelated to this route). Limit to users with edit_docs (or edit_post on the target).
- 'permission_callback' => [ $this, 'create_item_permissions_check' ],
+ 'permission_callback' => [ $this, 'duplicate_item_permissions_check' ],Add this method (outside this hunk):
public function duplicate_item_permissions_check( $request ) {
$doc_id = absint( $request->get_param( 'id' ) );
if ( ! is_user_logged_in() ) {
return new WP_Error( 'rest_not_logged_in', __( 'You are not currently logged in.', 'wedocs' ) );
}
if ( ! current_user_can( 'edit_post', $doc_id ) && ! current_user_can( 'edit_docs' ) ) {
return new WP_Error( 'wedocs_permission_failure', __( 'You are not allowed to duplicate this document.', 'wedocs' ), [ 'status' => 403 ] );
}
return true;
}🤖 Prompt for AI Agents
In includes/API/API.php around lines 113 to 120, the route for duplicating
documents uses create_item_permissions_check which permits any logged-in user;
replace that with a stricter permission check and add the new method: implement
duplicate_item_permissions_check( $request ) that extracts the id param,
verifies the user is logged in, then checks current_user_can('edit_post',
$doc_id) OR current_user_can('edit_docs') and returns WP_Error on failure (403
for permission failure) or true on success; update the register_rest_route call
to use this new duplicate_item_permissions_check as the permission_callback.
fixes #132
added a
Duplicateoption for single docs only. Theduplicatefunctionality will clone a doc with it's child (if exists). the duplicated docs status will bedraft.Summary by CodeRabbit
New Features
UI/UX