405-linkid-potential-bug #412
Conversation
WalkthroughThe pull request encompasses modifications across four frontend React components: Changes
Possibly Related PRs
Suggested Reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 1
🔭 Outside diff range comments (1)
frontend/src/scenes/links/NewLinksPopup.jsx (1)
Line range hint
86-96: Critical: Validation bypass for existing linksYo! The validation check for title and URL is skipped when id is a number, which could allow empty or invalid links to slip through. This might be causing the linkId bug mentioned in the PR title.
Fix the validation flow like this:
const handleLinks = async (item) => { const { id, ...link } = item; if (!link?.title.trim() || !link?.url.trim()) { emitToastError(buildToastError("Title and URL are required")); return null; } try { - if (typeof id === "number") return item; + if (typeof id === "number") return { ...item, title: link.title.trim(), url: link.url.trim() }; return { ...link }; } catch (err) { emitToastError(err); return null; } };
🧹 Nitpick comments (2)
frontend/src/scenes/links/NewLinksPopup.jsx (1)
Line range hint
117-129: Prevent race conditions in link deletionMom's spaghetti moment! 🍝 There's a potential race condition here. If the helper update succeeds but link deletion fails, we'll end up in an inconsistent state.
Consider wrapping the helper update and link deletions in a transaction-like pattern:
if (isEdit && deletedLinks.length) { - await Promise.all( - deletedLinks.map(async (it) => { - try { - return await deleteLink({ ...it, helperId: helperToEdit }); - } catch (err) { - emitToastError(err); - return null; - } - }) - ); + const deletionResults = await Promise.all( + deletedLinks.map((it) => deleteLink({ ...it, helperId: helperToEdit }).catch(err => ({ error: err }))) + ); + const deletionErrors = deletionResults.filter(result => result.error); + if (deletionErrors.length) { + deletionErrors.forEach(({ error }) => emitToastError(error)); + // Consider rolling back helper update here + return null; + } }frontend/src/components/Links/Settings/Settings.jsx (1)
157-159: Improve auto-save indicator accuracyThere's vomit on his sweater already! The "Auto-saved" text is misleading since changes are only saved on form submission.
Consider either:
- Implementing actual auto-save functionality, or
- Changing the text to "Changes will be saved when you close"
- <span className={s["settings__header--info"]}>Auto-saved</span> + <span className={s["settings__header--info"]}>Saved on close</span>Also applies to: 98-99
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/components/Links/Settings/Settings.jsx(5 hunks)frontend/src/scenes/links/LinksDefaultPage.jsx(2 hunks)frontend/src/scenes/links/NewLinksPopup.jsx(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/src/scenes/links/LinksDefaultPage.jsx
🔇 Additional comments (1)
frontend/src/components/Links/Settings/Settings.jsx (1)
84-86: 🛠️ Refactor suggestion
Verify URL format before saving
Arms are heavy! We should validate the URL format before saving to prevent invalid links.
Let's check if there are any URL validation utilities in the codebase:
Consider adding URL validation:
+ const isValidUrl = (url) => {
+ if (url.startsWith('/')) return true;
+ try {
+ new URL(url);
+ return true;
+ } catch {
+ return false;
+ }
+ };
+
if (!info.title.trim() && !info.url.trim()) {
toggleSettings(e);
setLinkToEdit(null);
return;
}
+ if (!isValidUrl(info.url.trim())) {
+ // Use your toast system here
+ alert('Please enter a valid URL');
+ return;
+ }
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/utils/constants.js (1)
3-3: Be mindful of local testing needs
Palms might be sweating when toggling between local and staging. By commenting out the local API, you lose an easy environment for debugging. If local usage is still valuable, consider adding a separate configuration switch.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/scenes/links/NewLinksPopup.jsx(7 hunks)frontend/src/utils/constants.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/scenes/links/NewLinksPopup.jsx
🔇 Additional comments (1)
frontend/src/utils/constants.js (1)
6-6: Verify the staging endpoint
Knees weak, arms heavy—double-check that the staging URL is correct and ready to handle new requests. A misconfigured endpoint could lead to confusion and headaches during QA.
|
I still get this error in the console. It happens when you add a link, delete it, and then clicking save. My guess is that we add that link to the "links to be deleted" list and then try to delete it. However, this error occurs because we haven't created it in the first place. The links that are created before clicking save shouldn't be added to the
|
Describe your changes
Briefly describe the changes you made and their purpose.
Issue number
Mention the issue number(s) this PR addresses (e.g., #123).
Please ensure all items are checked off before requesting a review: