-
Notifications
You must be signed in to change notification settings - Fork 17
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
refactor: move current seeds.rb
script to new DevStarter
module (lib/seed/seeder.rb
)
#767
Conversation
…ded for the seed script
…s involved in the (current) seeds.rb file
wraps the constants in a new module
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.
Approving, left a small comment.
self.class.send(:create_admin_user) | ||
self.class.send(:create_owner_user) | ||
self.class.send(:create_developer_user) |
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.
issue this needn't be called like this, we can just call it like self.class.create_admin_user
and remove the private
. But better yet, since this class doesn't have state yet, the point of the class is sort of very low value. Classes encapsulate state. But it may in the future, so I would recommend just do something like:
def self.call
new.call
end
def call
create_admin_user
..
end
And make all those class methods, instance methods (and you can keep them private).
Link to comment: #767 (review)
Ref ticket: #762
This pull request refactors the database seeding process by moving the logic into a new module and class, and organizing constants into a separate file.
db/seeds.rb
: It now just directly callsSeed::DevStarter.call
.lib/seed/dev_starter.rb
: Created a newSeed::DevStarter
class that encapsulates the seeding logic for admin, owner, and developer users (as it was present in the earlierseeds.rb
file)lib/seed/constants.rb
: Moved constants for admin, owner, and developer user details into a separate fileThis starts a new series of PRs that are meant to eventually help resolve #762