feat(api): add import preflight validation#1755
Conversation
📝 WalkthroughWalkthroughAdds an Import::Preflight service and POST /api/v1/imports/preflight endpoint to validate CSV and NDJSON content without creating imports; exposes class-level size/limit accessors, centralizes MintImport mappings, updates OpenAPI docs, and adds comprehensive tests. ChangesImport Preflight Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95e4dfbc9d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@app/controllers/api/v1/imports_controller.rb`:
- Around line 153-160: The 500 response in ImportsController#preflight returns a
generic message; change the render JSON to match API v1 convention by using
message: "Error: #{e.message}". Keep the existing Rails.logger.error calls and
backtrace logging, but replace the payload's message value so the rescue
StandardError => e block renders { error: "internal_server_error", message:
"Error: #{e.message}" }, status: :internal_server_error.
In `@app/models/import/preflight.rb`:
- Around line 297-300: The apply_import_defaults method currently
unconditionally assigns MintImport.default_column_mappings and overwrites any
client-provided values; change it so defaults are only applied for attributes
that are not already set on the import (i.e., do not clobber explicit
*_col_label, date_format, signage_convention). In apply_import_defaults(import)
(and when dealing with MintImport), merge defaults into the import by assigning
each key from MintImport.default_column_mappings only if the corresponding
attribute on import is nil/blank/unset (or use a reverse_merge-style behavior),
preserving any values already provided by import_config_params.
In `@app/models/sure_import.rb`:
- Around line 42-45: The current counting (in SureImport around the
JSON.parse(line) branch) increments counts for any JSON object that has a "type"
even if it lacks "data"; change the condition so you only increment counts for
fully-formed NDJSON records that are a Hash and have both a "type" and a "data"
key (e.g. check record.is_a?(Hash) && record["type"] && record.key?("data") or
an equivalent presence check), so malformed lines like { "type": "Transaction" }
are not counted by counts[...] and thus dry_run/rows_count/publishable? reflect
only valid records.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 64a81506-cd21-415b-bf87-f9a9171bddf1
📒 Files selected for processing (13)
app/controllers/api/v1/base_controller.rbapp/controllers/api/v1/imports_controller.rbapp/models/import.rbapp/models/import/preflight.rbapp/models/mint_import.rbapp/models/sure_import.rbconfig/routes.rbdocs/api/openapi.yamlspec/requests/api/v1/imports_spec.rbspec/swagger_helper.rbtest/controllers/api/v1/imports_controller_test.rbtest/models/mint_import_test.rbtest/models/sure_import_test.rb
Supersedes #1633. The original PR head was on the old
JSONbored/surefork network; this replacement branch lives onJSONbored/sure-upstream, the current fork ofwe-promise/sure.This recreation uses the latest old-fork branch head (
cc97a12), which includes the shared API UUID validation and public CSV size documentation fixes requested on the old PR.Summary
What changed
POST /api/v1/imports/preflight.Why
API clients and self-hosted operators need a safe way to inspect import content before starting a state-changing import workflow.
Screenshots
Not applicable: this PR adds a public API/OpenAPI import preflight endpoint and does not add or change web UI. The reviewer-facing surface is the generated OpenAPI contract and the JSON response shape covered by controller/model tests.
Validation
Run through the packaged devcontainer stack for this worktree:
devcontainer up --workspace-folder . --mount-git-worktree-common-dir --include-configurationdevcontainer exec --workspace-folder . bash -lc 'cd /workspace && bin/rails test test/controllers/api/v1/imports_controller_test.rb test/models/mint_import_test.rb test/models/sure_import_test.rb'73 runs, 358 assertions, 0 failures, 0 errors, 0 skipsdevcontainer exec --workspace-folder . bash -lc 'cd /workspace && RAILS_ENV=test bundle exec rake rswag:specs:swaggerize'282 examples, 0 failures; regenerateddocs/api/openapi.yamldevcontainer exec --workspace-folder . bash -lc 'cd /workspace && bin/rubocop app/controllers/api/v1/base_controller.rb app/controllers/api/v1/imports_controller.rb app/models/import.rb app/models/import/preflight.rb app/models/mint_import.rb app/models/sure_import.rb spec/requests/api/v1/imports_spec.rb spec/swagger_helper.rb test/controllers/api/v1/imports_controller_test.rb test/models/mint_import_test.rb test/models/sure_import_test.rb'11 files inspected, no offenses detectedgit diff --checkSummary by CodeRabbit
New Features
Improvements
Documentation
Tests