GUILD-690: Initial draft of new Active Record Consumer Generator #170
Conversation
| @@ -0,0 +1,30 @@ | |||
| class <%= migration_class_name %> < ActiveRecord::Migration<%= migration_version %> | |||
There was a problem hiding this comment.
Don't we already have this template file? We should be able to reuse it.
There was a problem hiding this comment.
Yes - better idea to reuse the existing code - will fix this.
| config_file_path = "#{initializer_path}/#{config_file}" | ||
| config_file_path = config_path if config_path.present? | ||
|
|
||
| if File.exist?(config_file_path) |
There was a problem hiding this comment.
We should validate that the config file exists if it's passed in. If not, we can create it.
There was a problem hiding this comment.
When I tested it by passing in a config file path that did not already exist, it did go ahead and create that config file.
There was a problem hiding this comment.
Sorry, that was unclear. We should only create it if no path was specified. If a path was specified and doesn't exist, we should assume that means that they made a mistake and raise an error.
Actually, I'm not sure we need to create it at all. If we want to auto-create a config file, it probably would be in an installer or something, but I don't think we have a use case for that. So maybe I'd just say don't create anything, and throw an error if the file doesn't exist.
|
|
||
| if File.exist?(config_file_path) | ||
| config_template = File.expand_path(find_in_source_paths('config.rb')) | ||
| insert_into_file(config_file_path.to_s, CapturableERB.new(::File.binread(config_template)).result(binding)) |
There was a problem hiding this comment.
Wouldn't this insert the whole file? We need a way to just insert the consumer do block.
There was a problem hiding this comment.
Ok, I will change it to just insert the consumer do block. Wasn't sure if we needed to add the other config.
| # @return [String] | ||
| def consumer_name | ||
| "#{schema.classify}Consumer" | ||
| end |
There was a problem hiding this comment.
A lot of this is copied from the migration template. There should be a way to just call the migration generator from this generator rather than having the same code twice.
There was a problem hiding this comment.
I considered extending from the migration template but decided it wasn't the right approach. Will find a better way!
|
|
||
| # @return [String] Returns the name of the first field in the schema, as the key | ||
| def key_field | ||
| fields.first.name |
There was a problem hiding this comment.
This is almost definitely not going to be the right approach. It probably makes more sense to just have the key config specified as part of the input. We'll also need the key schema (if there is one) when generating the schema classes.
There was a problem hiding this comment.
Ok, yes makes sense to include key config as an input.
In terms of the key schema, how does that work? I noticed that the schema glasses generator doesn't take in any arguments. So, if the user has a key schema for the new consumer, do they just have to place it in the SCHEMA_ROOT prior to running the ActiveRecordConsumer generator? And then what will be the difference in output? Will the generated schema classes be different?
There was a problem hiding this comment.
The generator reads the configuration which already knows what the key schema is.
The generator has a method generate_classes(schema_name, namespace, key_config) which you should probably be using.
|
|
||
| # Generates schema classes | ||
| def create_deimos_schema_class | ||
| Deimos::Generators::SchemaClassGenerator.start |
There was a problem hiding this comment.
There should be a way to generate just the one schema class we care about.
There was a problem hiding this comment.
Which is the one schema class that we care about?
There was a problem hiding this comment.
The one that was passed in as an input.
|
This is a great first pass! I do think that having an interactive process will make more sense, especially because right now the templates are making a bunch of assumptions as to what needs to be filled in. Allowing the user to specify that will help a lot so they don't have to go into each of these files and edit them. |
Thanks for your feedback! An interactive process does sound like it would be more helpful to the user. So, the user would have to specify
Is there anything else that they should provide? |
|
I'd look through the template and generator files to see what assumptions are being made in the output, and try to add those as inputs. |
…with new option to skip generating classes for all schema files
… deimos consumer config
Description
To create an ActiveRecordConsumer (one of our most common tasks) there are a number of steps that have to be taken - create a DB migration, Rails model, consumer, consumer config, Deimos schema class. We have now added a new ActiveRecordConsumer Generator, which will streamline this process into a single flow.
Notes for consideration:
Fixes # (GUILD-690)
Type of change
New feature (non-breaking change which adds functionality)
How Has This Been Tested?
Test A: Unit tests via rspec where we are invoking the generator and validating that all the necessary files have been created
See spec/generators/active_record_consumer_generator_spec.rb
Test B: Manual tests that invoke the generator via command line in a ruby project that imports the deimos gem
Ensure you have a Widget schema file located in your schema root and then execute command
rails g deimos:active_record_consumer test.Widgetandrails g deimos:active_record_consumer test.Widget config/initializers/myconfig.rbChecklist: