-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CCOL-2039: Global consumer configurations for replace_association & bulk_id_generation #205
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment - also, please add a line to the CHANGELOG!
|
||
default = MyConfigConsumer2.config | ||
expect(default[:replace_associations]).to eq(true) | ||
expect(default[:bulk_import_id_generator].call).to eq('global') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't actually test that the generator is called when expected (i.e. during the batch updates). Ideally you'd make this change in e.g. the mass updater spec.
context 'with a class defined bulk_import_id_generator' do | ||
|
||
before(:each) do | ||
consumer_class.config[:bulk_import_id_generator] = proc { 'custom' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should still define the global one here - this should override the global.
be07ca9
to
4876045
Compare
Looks good! |
Pull Request Template
Description
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: