-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Move Spree::Address#name attribute to the db #3908
Move Spree::Address#name attribute to the db #3908
Conversation
1d72ede
to
aaf6b9c
Compare
c1caebe
to
b246e59
Compare
3782935
to
b50c24d
Compare
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.
Should we just introduce the name
column first?
This will avoid having automatic getters and setters for these fields and also be present in the list of attributes when the Address is converted into other formats, for example, .as_json
b50c24d
to
a494722
Compare
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.
@filippoliverani @kennyadsl thanks 👍 🎉
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.
Yes, looks good to me.
After solidusio#3908 the table spree_addresses includes the name field, so it can be populated with historical data already present in the table by joining the content of firstname and lastname. Running rake solidus:migrations:migrate_address_names:up will take care of the job. The task is arranged in steps that require user input in order to proceed: * the task will exit no-op when no record with blank name is found; * if records are found, the user is asked to confirm before proceeding; * the user is now asked for the record batch size to be used. My tests showed that 100'000 records per batch should balance the necessity to avoid locking records for long and have the script complete without too much overhead; * before migrating data, the user is asked to confirm once again.
After solidusio#3908 the table spree_addresses includes the name field, so it can be populated with historical data already present in the table by joining the content of firstname and lastname. Running rake solidus:migrations:migrate_address_names:up will take care of the job. The task is arranged in steps that require user input in order to proceed: * the task will exit no-op when no record with blank name is found; * if records are found, the user is asked to confirm before proceeding; * the user is now asked for the record batch size to be used. My tests showed that 100'000 records per batch should balance the necessity to avoid locking records for long and have the script complete without too much overhead; * before migrating data, the user is asked to confirm once again.
After solidusio#3908 the table spree_addresses includes the name field, so it can be populated with historical data already present in the table by joining the content of firstname and lastname. Running rake solidus:migrations:migrate_address_names:up will take care of the job. The task is arranged in steps that require user input in order to proceed: * the task will exit no-op when no record with blank name is found; * if records are found, the user is asked to confirm before proceeding; * the user is now asked for the record batch size to be used. My tests showed that 100'000 records per batch should balance the necessity to avoid locking records for long and have the script complete without too much overhead; * before migrating data, the user is asked to confirm once again.
After solidusio#3908 the table spree_addresses includes the name field, so it can be populated with historical data already present in the table by joining the content of firstname and lastname. Running `rails solidus:migrations:migrate_address_names:up` will take care of the job. The task is arranged in steps that require user input in order to proceed: * the task will exit no-op when no record with blank name is found; * if records are found, the user is asked to confirm before proceeding; * the user is now asked for the record batch size to be used. My tests showed that 100'000 records per batch should balance the necessity to avoid locking records for long and have the script complete without too much overhead; * before migrating data, the user is asked to confirm once again. Running this task is required when upgrading to Solidus 3.0.
After solidusio#3908 the table spree_addresses includes the name field, so it can be populated with historical data already present in the table by joining the content of firstname and lastname. Running `rails solidus:migrations:migrate_address_names:up` will take care of the job. The task is arranged in steps that require user input in order to proceed: * the task will exit no-op when no record with blank name is found; * if records are found, the user is asked to confirm before proceeding; * the user is now asked for the record batch size to be used. My tests showed that 100'000 records per batch should balance the necessity to avoid locking records for long and have the script complete without too much overhead; * before migrating data, the user is asked to confirm once again. Running this task is required when upgrading to Solidus 3.0.
After solidusio#3908 the table spree_addresses includes the name field, so it can be populated with historical data already present in the table by joining the content of firstname and lastname. Running `rails solidus:migrations:migrate_address_names:up` will take care of the job. The task is arranged in steps that require user input in order to proceed: * the task will exit no-op when no record with blank name is found; * if records are found, the user is asked to confirm before proceeding; * the user is now asked for the record batch size to be used. My tests showed that 100'000 records per batch should balance the necessity to avoid locking records for long and have the script complete without too much overhead; * before migrating data, the user is asked to confirm once again. Running this task is required when upgrading to Solidus 3.0.
After solidusio#3908 the table spree_addresses includes the name field, so it can be populated with historical data already present in the table by joining the content of firstname and lastname. Running `rails solidus:migrations:migrate_address_names:up` will take care of the job. The task is arranged in steps that require user input in order to proceed: * the task will exit no-op when no record with blank name is found; * if records are found, the user is asked to confirm before proceeding; * the user is now asked for the record batch size to be used. My tests showed that 100'000 records per batch should balance the necessity to avoid locking records for long and have the script complete without too much overhead; * before migrating data, the user is asked to confirm once again. Running this task is required when upgrading to Solidus 3.0.
After solidusio#3908 the table spree_addresses includes the name field, so it can be populated with historical data already present in the table by joining the content of firstname and lastname. Running `rails solidus:migrations:migrate_address_names:up` will take care of the job. The task is arranged in steps that require user input in order to proceed: * the task will exit no-op when no record with blank name is found; * if records are found, the user is asked to confirm before proceeding; * the user is now asked for the record batch size to be used. My tests showed that 100'000 records per batch should balance the necessity to avoid locking records for long and have the script complete without too much overhead; * before migrating data, the user is asked to confirm once again. Running this task is required when upgrading to Solidus 3.0.
After #3908 the table spree_addresses includes the name field, so it can be populated with historical data already present in the table by joining the content of firstname and lastname. Running `rails solidus:migrations:migrate_address_names:up` will take care of the job. The task is arranged in steps that require user input in order to proceed: * the task will exit no-op when no record with blank name is found; * if records are found, the user is asked to confirm before proceeding; * the user is now asked for the record batch size to be used. My tests showed that 100'000 records per batch should balance the necessity to avoid locking records for long and have the script complete without too much overhead; * before migrating data, the user is asked to confirm once again. Running this task is required when upgrading to Solidus 3.0.
Description
Ref #3234
This PR pushes forward the migration to
Spree::Address#name
to replacefirstname
andlastname
attributes. It adds a corresepondingname
column to the DB and replaces all the remaining direct attribute accesses tofirstname
andlastname
withname
.The DB migration contains a blocking warning to force users to evaluate the impact of the upgrade and to choose the correct strategy to handle it in their store. In a future PR we will provide a complementary rake task to migrage existing data fromfirstname
andlastname
columns to the newname
columns. New stores won't receive any warning and will be able to start working with thename
field from scratch.☝️ This piece will be moved into a separate rake task.
Checklist:
[ ] I have updated Guides and README accordingly to this change (if needed)[ ] I have added tests to cover this change (if needed)[ ] I have attached screenshots to this PR for visual changes (if needed)