466 add duplicate button #471
Conversation
WalkthroughThe pull request introduces enhancements across several default page components (Banners, Hints, Links, and Popups) by incorporating new service methods for item duplication and retrieval. The Changes
Suggested reviewers
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
frontend/src/scenes/links/LinksDefaultPage.jsx (1)
3-3: There's inconsistency on his sweater already! 🤔While the implementation is solid, the function naming
createHelperbreaks the consistency with other similar functions in the codebase (addHint,addPopup). Consider renaming for consistency:- import { createHelper, getHelperById, deleteHelper, getHelpers } from "../../services/helperLinkService"; + import { addHelper, getHelperById, deleteHelper, getHelpers } from "../../services/helperLinkService"; // ... getItemById={getHelperById} - duplicateItem={createHelper} + duplicateItem={addHelper}Also applies to: 44-45
frontend/src/templates/DefaultPageTemplate/DefaultPageTemplate.jsx (2)
52-74: Knees weak! Let's enhance error handling in duplicateHandlerThe error handling could use some love to be more specific about what went wrong.
} catch (error) { - const errorMessage = error.response?.data?.message - ? `Error: ${error.response.data.message}` - : 'An unexpected error occurred. Please try again.'; + const errorMessage = error.response?.data?.message + ? `Failed to duplicate ${itemType.slice(0, -1)}: ${error.response.data.message}` + : `Failed to duplicate ${itemType.slice(0, -1)}. Please try again.`; toastEmitter.emit(TOAST_EMITTER_KEY, errorMessage); + console.error(`Duplication error for ${itemType}:`, error); }
54-59: Arms are heavy! Consider extracting helper links logicThe special handling for helper links could be cleaner if extracted into a separate function.
+ const duplicateHelperLinks = async (id) => { + const { createdBy, id: fetchedId, links, ...helper } = await getItemById(id); + const updatedLinks = links.map(({ id, ...data }) => data); + return duplicateItem(helper, updatedLinks); + }; const duplicateHandler = async (id) => { try { if (itemType === 'helper links') { - const { createdBy, id: fetchedId, links, ...helper } = await getItemById(id); - const updatedLinks = links.map(({ id, ...data }) => data); - await duplicateItem(helper, updatedLinks); + await duplicateHelperLinks(id); } else {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/src/scenes/banner/BannerDefaultPage.jsx(2 hunks)frontend/src/scenes/hints/HintDefaultPage.jsx(2 hunks)frontend/src/scenes/links/LinksDefaultPage.jsx(2 hunks)frontend/src/scenes/popup/PopupDefaultPage.jsx(2 hunks)frontend/src/templates/DefaultPageTemplate/DefaultPageTemplate.jsx(5 hunks)frontend/src/templates/GuideMainPageTemplate/GuideMainPageComponents/List/List.jsx(1 hunks)frontend/src/templates/GuideMainPageTemplate/GuideMainPageComponents/List/ListItem/ListItem.jsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (22.x)
🔇 Additional comments (3)
frontend/src/scenes/hints/HintDefaultPage.jsx (1)
6-6: Mom's spaghetti approves! 🍝Clean implementation of the duplication feature. The code follows the established pattern and maintains good separation of concerns.
Also applies to: 32-33
frontend/src/scenes/popup/PopupDefaultPage.jsx (1)
5-5: Knees weak, arms heavy, but this code is ready! 💪The implementation is solid and consistent with the pattern established in other components.
Also applies to: 33-34
frontend/src/scenes/banner/BannerDefaultPage.jsx (1)
33-34: There's vomit on his sweater already! But this code is clean!The integration of duplicate functionality follows the established pattern consistently. Props are correctly passed through to the template.
frontend/src/templates/GuideMainPageTemplate/GuideMainPageComponents/List/List.jsx
Show resolved
Hide resolved
frontend/src/templates/GuideMainPageTemplate/GuideMainPageComponents/List/ListItem/ListItem.jsx
Show resolved
Hide resolved
frontend/src/templates/GuideMainPageTemplate/GuideMainPageComponents/List/ListItem/ListItem.jsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/src/templates/GuideMainPageTemplate/GuideMainPageComponents/List/ListItem/ListItem.jsx (1)
33-36:⚠️ Potential issueYo! That event bubbling issue is still there!
This is a duplicate of a previous comment that wasn't addressed. The duplicate button click will bubble up to the parent div's onClick.
- <IconButton onClick={onDuplicate}> + <IconButton onClick={(e) => { + e.stopPropagation(); + onDuplicate(); + }}>
🧹 Nitpick comments (2)
frontend/src/templates/GuideMainPageTemplate/GuideMainPageComponents/List/ListItem/ListItem.jsx (2)
12-12: Mom's spaghetti! Add default prop value for onDuplicateTo prevent those nasty undefined errors when onDuplicate isn't provided, let's add a default prop value.
- const ListItem = ({ title, text, id, onClick, onDelete, onEdit, onDuplicate }) => { + const ListItem = ({ title, text, id, onClick, onDelete, onEdit, onDuplicate = () => {} }) => {
35-35: Keep it consistent with MUI theming, dawg!Instead of using CSS variables, let's stick to MUI's theme system for better consistency.
- <ContentCopyTwoToneIcon sx={{ color: 'var(--main-text-color)' }} /> + <ContentCopyTwoToneIcon sx={{ color: theme.palette.text.primary }} />
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/templates/GuideMainPageTemplate/GuideMainPageComponents/List/ListItem/ListItem.jsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (22.x)
🔇 Additional comments (2)
frontend/src/templates/GuideMainPageTemplate/GuideMainPageComponents/List/ListItem/ListItem.jsx (2)
1-1: Yo dawg, these imports are clean!The removal of unused React hooks and addition of the ContentCopyTwoTone icon import is on point! 🍝
Also applies to: 7-7
57-57: Props validation is on point!The onDuplicate PropType has been added correctly. Nice work! 🍝
|
I will update those icons in issue #472. |
Describe your changes
Added duplicate button in DefaultPageTemplate to create a copy of any guide.
Issue number
#466
Please ensure all items are checked off before requesting a review:
Preview