Add Wix migrate tool#1802
Open
PaulAdamDavis wants to merge 7 commits into
Open
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
WalkthroughThis pull request introduces Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/migrate/sources/wix-csv.js (2)
17-19: ⚡ Quick winGuard
scrapebefore calling.includes()inwix-csvsource
packages/migrate/sources/wix-csv.jsusesctx.options.scrape.includes(...)ininitialize. The CLI (packages/migrate/commands/wix-csv.js) defines--scrapeas an array with defaultValue['assets']and initializescontext.errors = [], soctx.errors.pushis safe during normal command execution. This still throws if the source is invoked directly with a missing/non-arrayoptions.scrape, so normalize/guardscrapeininitialize.Suggested fix
- ctx.allowScrape = { - assets: ctx.options.scrape.includes('assets') || ctx.options.scrape.includes('img') || ctx.options.scrape.includes('media') || ctx.options.scrape.includes('files') - }; + const scrapeOptions = Array.isArray(ctx.options.scrape) ? ctx.options.scrape : []; + ctx.allowScrape = { + assets: scrapeOptions.includes('assets') || scrapeOptions.includes('img') || scrapeOptions.includes('media') || scrapeOptions.includes('files') + };🤖 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 `@packages/migrate/sources/wix-csv.js` around lines 17 - 19, The initialize logic calls ctx.options.scrape.includes(...) without ensuring scrape is an array; update initialize to normalize/guard scrape first (e.g. use Array.isArray on ctx.options.scrape and fall back to an empty array or ['assets']), assign the normalized array back to ctx.options.scrape, then compute ctx.allowScrape (the current property set in initialize) using the normalized variable so includes() is safe when the source is invoked directly.
16-16: ⚡ Quick winDowngrade: ctx.errors is already initialized for the wix-csv CLI path; local hardening is optional
packages/migrate/commands/wix-csv.jspassescontext = { errors: [] }intomigrate.run(context), sopackages/migrate/sources/wix-csv.jswon’t hitctx.errorsbeing undefined in the normal flow. Adding a defensivectx.errorsinitialization insources/wix-csv.jscan still protect any direct/alternate usage of the exported task runner.Suggested local hardening
task: async (ctx, task) => { ctx.options = options; + ctx.errors = Array.isArray(ctx.errors) ? ctx.errors : []; ctx.allowScrape = { assets: ctx.options.scrape.includes('assets') || ctx.options.scrape.includes('img') || ctx.options.scrape.includes('media') || ctx.options.scrape.includes('files') };🤖 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 `@packages/migrate/sources/wix-csv.js` at line 16, Initialize ctx.errors defensively in the wix-csv source before using it: when setting ctx.options (the place where ctx is prepared), add a guard like setting ctx.errors = ctx.errors || [] so the exported task runner in packages/migrate/sources/wix-csv.js can safely push errors even if called directly outside the CLI path; update only the initialization near the code that sets ctx.options to ensure existing CLI flow (which passes context = { errors: [] }) remains unchanged.
🤖 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 `@packages/mg-wix-csv/src/lib/mapper.ts`:
- Around line 194-196: In mapContent, don't return an Error instance when no
rows exist—throw it so callers receive a rejected flow; replace the current
"return new errors.NoContentError({message: 'Input file is empty'})" with "throw
new errors.NoContentError(...)" (referencing mappedPosts.length and
errors.NoContentError in mapContent) so failure surfaces as an exception rather
than a successful return value.
In `@packages/mg-wix-csv/src/lib/rich-content.ts`:
- Around line 103-105: The code uses escapeHtml(linkUrl) when writing href but
escapeHtml does not block dangerous schemes (e.g., javascript:, data:);
validate/sanitize linkUrl before embedding by parsing the URL (or checking its
scheme) and only allow safe schemes (e.g., http, https, mailto, tel) or
explicitly reject/strip others, then continue to call escapeHtml on the cleaned
URL; update the logic around the anchor generation (the block that references
linkUrl, linkText and uses escapeHtml in this file, and the similar occurrence
at the other location around line 157) to perform this scheme whitelist check
and fall back to outputting plain text (or omitting href) when the URL is
disallowed.
In `@packages/mg-wix-csv/src/lib/wix-image.ts`:
- Around line 24-25: In parseWixImageUri, guard calls to decodeURIComponent for
match[1] and match[2] by wrapping each in try/catch and return null if a
URIError (malformed percent-encoding) is thrown; specifically catch URIError (or
inspect error.name) and avoid rethrowing so parseWixImageUri returns null
instead of crashing. Update any callers if they assume non-null. Add a
regression test that supplies a wix-image-like string that matches the regex but
contains malformed % sequences (e.g., "%ZZ") to assert parseWixImageUri(...)
returns null and does not throw.
---
Nitpick comments:
In `@packages/migrate/sources/wix-csv.js`:
- Around line 17-19: The initialize logic calls ctx.options.scrape.includes(...)
without ensuring scrape is an array; update initialize to normalize/guard scrape
first (e.g. use Array.isArray on ctx.options.scrape and fall back to an empty
array or ['assets']), assign the normalized array back to ctx.options.scrape,
then compute ctx.allowScrape (the current property set in initialize) using the
normalized variable so includes() is safe when the source is invoked directly.
- Line 16: Initialize ctx.errors defensively in the wix-csv source before using
it: when setting ctx.options (the place where ctx is prepared), add a guard like
setting ctx.errors = ctx.errors || [] so the exported task runner in
packages/migrate/sources/wix-csv.js can safely push errors even if called
directly outside the CLI path; update only the initialization near the code that
sets ctx.options to ensure existing CLI flow (which passes context = { errors:
[] }) remains unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f7d8d2f1-ec59-4fbc-8e1b-f063a7d77cb4
⛔ Files ignored due to path filters (3)
packages/mg-wix-csv/src/test/fixtures/empty.csvis excluded by!**/*.csvpackages/mg-wix-csv/src/test/fixtures/posts.csvis excluded by!**/*.csvpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
packages/mg-wix-csv/.eslintrc.cjspackages/mg-wix-csv/.gitignorepackages/mg-wix-csv/README.mdpackages/mg-wix-csv/package.jsonpackages/mg-wix-csv/src/index.tspackages/mg-wix-csv/src/lib/mapper.tspackages/mg-wix-csv/src/lib/rich-content.tspackages/mg-wix-csv/src/lib/wix-image.tspackages/mg-wix-csv/src/test/mapper.test.tspackages/mg-wix-csv/src/test/rich-content.test.tspackages/mg-wix-csv/src/test/wix-image.test.tspackages/mg-wix-csv/src/types.d.tspackages/mg-wix-csv/tsconfig.jsonpackages/migrate/bin/cli.jspackages/migrate/commands/wix-csv.jspackages/migrate/package.jsonpackages/migrate/sources/wix-csv.jspackages/migrate/test/wix-csv-command.test.js
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.
No description provided.