Sets the new user up with an example set of agents #1404
Conversation
|
Great idea to utilize the scenario import for that! What do you think about using that approach for all new users that are created (either via the seeds, registration or the admin user interface)? We could have the default scenario match our current |
TildeWill
force-pushed
the
onboarding
branch
4 times, most recently
from
4238f16
to
655dfb5
Compare
| def self.import(user) | ||
| scenario_import = ScenarioImport.new() | ||
| scenario_import.set_user(user) | ||
| scenario_import.file = File.open(File.join(Rails.root, "data", "default_scenario.json"), "r") |
There was a problem hiding this comment.
Should we close this file afterwards?
|
I think it's a good idea! Probably should be a configuration option in .env if new users get this treatment or not. |
| @@ -54,6 +54,9 @@ def import_confirmed? | |||
| do_import == "1" | |||
| end | |||
|
|
|||
| def import!(options = {}) | |||
There was a problem hiding this comment.
Is this method used anywhere?
There was a problem hiding this comment.
it was the start of a refactor to have import check validity, the same way the ActiveRecord#save and #save! work. I've backed away from that change for the moment, but might go back to it
TildeWill
force-pushed
the
onboarding
branch
13 times, most recently
from
1818cb1
to
6f3b676
Compare
|
I took a crack at using one |
| scenario_file = ENV['DEFAULT_SCENARIO_FILE'] || File.join(Rails.root, "data", "default_scenario.json") | ||
| scenario_import.file = open(scenario_file) | ||
| raise "Import failed" unless scenario_import.valid? && scenario_import.import | ||
| scenario_import.file.close() |
There was a problem hiding this comment.
Maybe
ensure
scenario_import.file.close
to also close the file when the import failed?
We always try to keep the default behavior the same as it was before a change. With no change to the |
|
Ready for review |
|
Very cool! Looks good to me, but waiting for another 👍 for confirm we did not miss anything :) |
| scenario_import = ScenarioImport.new() | ||
| scenario_import.set_user(user) | ||
| default_scenario = File.join(Rails.root, "data", "default_scenario.json") | ||
| scenario_file = ENV['DEFAULT_SCENARIO_FILE'] || default_scenario |
There was a problem hiding this comment.
Should probably be ENV['DEFAULT_SCENARIO_FILE'].presence || File.join(Rails.root, "data", "default_scenario.json") since '' is truthy.
There was a problem hiding this comment.
Sure, then it also guards agains an empty string, instead of just not being set. Thanks
|
Awesome! @dsander, is there an easy way for @TildeWill to test that this didn't mess up Docker upgrades in some way? |
- gives them a place to start from - leverages existing code to import scenarios - adds test coverage to seeds to ensure sees can run multiple times against the same db as is the case for Docker - adds env variable to set if importing is turned on for new user - adds env variable to point to a custom scenario for new users
|
With #1359 merged building docker images locally is possible with should ensure everything works. But I think the specs ensure, that running the seeds multiple times does not raise any exceptions. |
|
Docker seems to work just fine. Thanks for the instructions @dsander - I'm still very new to Docker |
|
Awesome, LGTM too. Should we go ahead and merge? |
|
Sounds good to me. On Sun, Apr 10, 2016 at 7:08 PM Andrew Cantino notifications@github.com
|
|
That's a very neat addition, thanks @TildeWill! |
I wanted to run this up the flag pole before I got too far, it still needs tests and there's a refactor in
ScenarioImporterthat I want to do. On Omniscope, I see a lot of users who are signing up but never create any agents. My hope is that if we give them some agents by default like the admin seed then people will be more likely to engage and modify their agents.The weather agent is nice because it could be relevant to everyone (weather), but needs additional set up by the user (zip code and API key). Maybe that's a good thing because it gets new users involved and thinking about how to adapt it to their own needs?