Skip to content
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

Add CSV support for studio_people script. #17440

Merged
merged 2 commits into from Aug 31, 2017
Merged

Conversation

ashercodeorg
Copy link
Contributor

@ashercodeorg ashercodeorg commented Aug 31, 2017

My intention is to run the script on staging, test, and levelbuilder in the no argument mode and on production in the argument mode.

It has already been run on test before these changes. Though slow, it isn't unreasonably slow and saves me from needing to generate the CSV.

Unfortunately, I'm not sure how best to get this run in all developer environments. Given the lack of index on User.user_type, I'm not sure how to generate an appropriate migration.

MANUAL TESTING: Created a CSV, eliminated existing StudioPersons (avoiding the before_save callback on User), and ran the script with the CSV.

next
end

user.update!(studio_person: StudioPerson.create!(emails: user.email))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that if there was a convenient way to touch a record without changing it, this could alternately be done by user.touch && user.save!. Unfortunately, newer versions of Rails seem not to support touch as expected.

next
end

user.update!(studio_person: StudioPerson.create!(emails: user.email))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this will update the updated_at timestamp. This seems desirable, though if desired, I can disable that.

Copy link
Contributor

@Hamms Hamms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you just create a standard rails migration, but gate it to run only on the development environment? It won't be performant, but dev dbs are usually small enough that that shouldn't be a huge issue

next
end

user.update!(studio_person: StudioPerson.create!(emails: user.email))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

worth adding a safeguard for teachers who converted to students in between CSV generation and script run?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Doing so.

@ashercodeorg
Copy link
Contributor Author

A development only migration might be necessary. There was discussion at some point on Slack that we didn't want to have environment specific migrations, though.

@ashercodeorg ashercodeorg merged commit 2fbd035 into staging Aug 31, 2017
@ashercodeorg ashercodeorg deleted the studioPersonScript branch August 31, 2017 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants