Allow custom ImportResult subclass via :result_class option#214
Merged
Allow custom ImportResult subclass via :result_class option#214
Conversation
Adds a :result_class option to active_admin_import so users can plug in their own ImportResult subclass to collect adapter-specific data (like inserted ids) without forcing the gem to take on cross-adapter quirks. The DSL action block now receives result, options via instance_exec (matching DEFAULT_RESULT_PROC's signature) so the captured data is actually reachable from user code. Existing zero-arity blocks are unaffected since Ruby blocks ignore extra arguments. A PG-only spec demonstrates the canonical PR #191 use case (capturing result.ids), which is reliable on PostgreSQL via RETURNING but not on MySQL/SQLite without extra setup.
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
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.
Summary
Adds a
:result_classoption toactive_admin_importso users can plug in their ownImportResultsubclass and collect adapter-specific data — most notably the inserted record ids that PR #191 was trying to expose.This is the "extension point" alternative to PR #191's approach of hardcoding
idson the base class. The gem stays minimal and adapter-agnostic; users who need RETURNING-style data own that quirk in their app code.Why
PR #191 proposed adding
idsdirectly toImportResultby forwardingresult.idsfromactiverecord-import. The downside is thatresult.idsonly behaves consistently on PostgreSQL (viaRETURNING); MySQL and SQLite require extra options or simply don't populate it. Hardcoding it would mean the gem owns those cross-adapter quirks forever, and every future "I want X from the batch result" request becomes another PR against this gem.Instead, this PR makes the result class itself pluggable. Users who want
idswrite a 5-line subclass and pass it viaresult_class:. They own the adapter caveats; the gem owns nothing extra.Changes
lib/active_admin_import/options.rb— adds:result_classtoVALID_OPTIONSlib/active_admin_import/importer.rb— adds:result_classto the importer'sOPTIONSwhitelist;import_resultnow usesoptions[:result_class]if provided, falling back to the existingImportResultclass. The narrowerimport_optionsslice is unchanged, so:result_classdoesn't leak into theactiverecord-importcall.lib/active_admin_import/dsl.rb— the user action block is now invoked viainstance_exec(result, options, &block)(matchingDEFAULT_RESULT_PROC's signature) so the captured data is actually reachable from user code. Existing zero-arity blocks are unaffected — Ruby procs ignore extra arguments.spec/import_spec.rb— adds a PG-only spec demonstrating the canonical PR Store result ids on ImportResult (postgres only) #191 use case: a custom subclass that capturesresult.idsand exposes them via the flash, asserted end-to-end through Capybara. The spec is wrapped inif ENV['DB'] == 'postgres'so it only runs on the Postgres CI job, whereRETURNINGmakes the assertion meaningful.README.md— documents:result_classin the options table and adds a "Custom ImportResult" section with the full ids-capturing example, plus a note about adapter-specific behavior.Test plan
bundle exec rspec spec/import_spec.rb→ 49 examples, 0 failures (PG-only context correctly skipped)Follow-up
Once this is merged, PR #191 can be closed with a comment pointing at the new
:result_classoption and the README example as the supported way to surface inserted ids.