2841 Associate customers with their user records #3126
Conversation
When we introduced the Customer model, we didn't associate any existing customers with users that have the same email address. Later we decided to create that association when users sign up. But we didn't update all the existing customers. We do that now for data consistency and to solve several bugs.
There was a problem hiding this comment.
Nice @mkllnk !
I see:
def backup_customers
def recover_customers
We should put this backup/recover thing in the wiki for future reference!!!
| SET user_id = spree_users.id | ||
| FROM spree_users | ||
| WHERE customers.email = spree_users.email | ||
| AND customers.user_id IS NULL;" |
There was a problem hiding this comment.
#strip_heredoc is the perfect method for these multiline queries 👌
| @@ -13,7 +13,7 @@ def add_enterprise_fee(enterprise_fee) | |||
| end | |||
|
|
|||
| def set_order(order) | |||
| ApplicationController.any_instance.stub(:session).and_return({order_id: order.id, access_token: order.token}) | |||
| allow_any_instance_of(ApplicationController).to receive(:session).and_return({order_id: order.id, access_token: order.token}) | |||
There was a problem hiding this comment.
🙈 I really dislike *any_instance* methods. Carelessly stubbing has caused me problems in the past. It's a door open to false positives.
Just sharing feelings, nothing to do in this PR.
| # customers with users that have the same email address. | ||
| # Later we decided to create that association when users sign up. But we didn't | ||
| # update all the existing customers. We do that now for data consistency and to | ||
| # solve several bugs. |
There was a problem hiding this comment.
👏 for caring about the next dev reading this.
|
You can merge it yourself as soon as you run it in an affected instance @mkllnk |
|
Thank you for your reviews. I started a new wiki page for migrations: https://github.com/openfoodfoundation/openfoodnetwork/wiki/Database-migrations |
|
This has been deployed to Australian production. The backup file is 82kb of affected customer records. |
|
Awesome @mkllnk , I linked the new wiki page from here: https://github.com/openfoodfoundation/openfoodnetwork/wiki/Code-Conventions |
|
Nice, thank you. It looks like we still didn't test enough. Paco reported the migration failing in France. I have to look into it. So maybe a new proposal would be to run all migrations against all production data locally. I mean for each OFN instance:
That takes a while and requires access to all these instances, but we can write a little script for it. |
|
For data migrations, a strategy that proved to be battle-tested in the past was to run them in production as soon as they got approved and before merging. Then if something came up, the migration could be fixed, like in this case. |
|
@mkllnk even if we dont do this process for migrations I'd like to see how you guys "download production data and import into local dev database". can you share a script? |
|
@luisramos0 There is a script in the repository that is broken now, because it hasn't been updated for a while: I'm experimenting with a new version, but I haven't fixed the last part of modifying the data for development yet: #!/bin/bash
# Used to pull data from production or staging servers into local dev database
# Useful for when you want to test a migration against production data, or see
# the effect of codebase changes on real-life data
# Usage: script/mirror_db.sh [ofn-staging1|ofn-staging2|ofn-prod]
set -e
if hash zeus 2>/dev/null && [ -e .zeus.sock ]; then
RAILS_RUN='zeus r'
else
RAILS_RUN='bundle exec rails runner'
fi
#if [[ "$1" = 'ofn-prod' ]]; then
# DB_USER='openfoodnetwork'
# DB_USER='openfoodweb'
# DB_DATABASE='openfoodweb_production'
#elif [[ "$1"
#
#else
# DB_USER='ofn_user'
# DB_DATABASE='openfoodnetwork'
#fi
#DB_USER='openfoodnetwork'
DB_USER='ofn_user'
DB_DATABASE='openfoodnetwork'
DB_OPTIONS=(--exclude-table-data=sessions --no-acl)
# -- Mirror database
echo "Mirroring database..."
dropdb -h localhost -U ofn open_food_network_dev
createdb -h localhost -U ofn open_food_network_dev
ssh -C "$1" "pg_dump -h localhost -U $DB_USER $DB_DATABASE ${DB_OPTIONS[@]}" | psql -h localhost -U ofn open_food_network_dev
echo "Please fix me"
exit
# -- Disable S3
echo "Preparing mirrored database..."
$RAILS_RUN script/prepare_imported_db.rb
# -- Mirror images
if hash aws 2>/dev/null; then
echo "Mirroring images..."
BUCKET=`echo $1 | sed s/-/_/ | sed "s/\\([0-9]\\)/_\1/" | sed s/prod/production/`
aws s3 sync s3://$BUCKET/public public/
else
echo "Please install the AWS CLI tools so that I can copy the images from $1 for you."
echo "eg. sudo easy_install awscli"
fi |
I feel embarrased that this is the second follow up of my last migration: openfoodfoundation#3126 The last migration didn't change any database structure, but the schema still needs the latest migration version. Otherwise it will display pending migrations when setting up the database. This commit allows to run `bundle exec rake db:reset` in development without the following message: Run `rake db:migrate` to update your database then try again. You have 1 pending migrations: 20181123012635 AssociateCustomersToUsers The next pull request with a migration would have solved this problem as well.
What? Why?
Closes #2841.
When we introduced the Customer model, we didn't associate any existing
customers with users that have the same email address.
Later we decided to create that association when users sign up. But we didn't
update all the existing customers. We do that now for data consistency and to
solve several bugs.
What should we test?
Our staging data is too new to contain the legacy records that are fixed with this pull request. Only developers can test this against production data on their local machine.
Release notes
Fixed: Some customer records created before May 2018 caused subscriptions to fail and get repaired with this release.
Changelog Category: Fixed
Discourse thread
https://community.openfoodnetwork.org/t/subscriptions-and-customers/1512