Copy new migrations as part of the update task #4957

kennyadsl · 2023-02-27T09:41:17Z

Summary

Running bin/rails solidus:update, we are not currently copying the new migrations introduced with the new Solidus version. This new install step will copy migrations and it's left to the user to run db:migrate, before checking them.

This commit also updates some copy of the task to reflect the new content of the file.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

I have written a thorough PR description.
I have kept my commits small and atomic.
I have used clear, explanatory commit messages.

The following are not always needed:

📖 I have updated the README to account for my changes.
📑 I have documented new code with YARD.
🛣️ I have opened a PR to update the guides.
✅ I have added automated tests to cover my changes.
📸 I have attached screenshots to demo visual changes.

codecov · 2023-02-27T09:59:13Z

Codecov Report

Merging #4957 (241f6bc) into master (aa3063e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4957   +/-   ##
=======================================
  Coverage   86.68%   86.68%           
=======================================
  Files         578      578           
  Lines       14684    14687    +3     
=======================================
+ Hits        12729    12732    +3     
  Misses       1955     1955

Impacted Files	Coverage Δ
.../lib/generators/solidus/update/update_generator.rb	`100.00% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

waiting-for-dev

Thanks, @kennyadsl!

Did you try to add a test for the new behavior? That generator is tested and it'd be good to add it. However, if that's going to require a complex setup (which I'm afraid could be the case) I'm also ok with not doing that.

waiting-for-dev · 2023-02-27T10:41:48Z

core/lib/generators/solidus/update/update_generator.rb

@@ -40,7 +40,7 @@ class UpdateGenerator < ::Rails::Generators::Base
    def create_new_defaults_initializer
      previous_version_prompt = options[:previous_version_prompt]
      return if previous_version_prompt && !yes?(<<~MSG, :red)
-        The update process is only supported if you are coming from version #{FROM}. If this is not the case, please, skip it and update your application to use Solidus #{FROM} before retrying.
+        The defaults update process is only supported if you are coming from version #{FROM}. If this is not the case, please, skip it and update your application to use Solidus #{FROM} before retrying.


Suggested change

The defaults update process is only supported if you are coming from version #{FROM}. If this is not the case, please, skip it and update your application to use Solidus #{FROM} before retrying.

The default update process is only supported if you are coming from version #{FROM}. If this is not the case, please, skip it and update your application to use Solidus #{FROM} before retrying.

It was actually defaults. It refers to new_solidus_defaults. I'll try to see if there's a way to make it more clear.

Changed to "The default preferences update process..". Thoughts?

core/lib/generators/solidus/update/update_generator.rb

kennyadsl · 2023-02-27T11:44:07Z

I didn't remember we have tests for that task, thanks for pointing out. I'll definitely try!

kennyadsl · 2023-03-02T15:42:22Z

@waiting-for-dev I'm losing any hope that we can test copying the migrations. My only idea was to remove the last existing migration from the dummy app in the test setup and then run the update task to see it generated again. But this won't work because the dummy app actually doesn't hold any migration; they are used directly from the engine, injected programmatically in the App via config.paths['db/migrate'].

Right now, we are not copying the new migrations introduced with the new version. This new install step will copy migrations and it's left to the user to run db:migrate, before checking them. This commit also update some copy of the task to reflect the new content of the file.

waiting-for-dev

@waiting-for-dev I'm losing any hope that we can test copying the migrations. My only idea was to remove the last existing migration from the dummy app in the test setup and then run the update task to see it generated again. But this won't work because the dummy app actually doesn't hold any migration; they are used directly from the engine, injected programmatically in the App via config.paths['db/migrate'].

No worries. The only clean way I can think would be to have the install:migrations accept an injectable directory and use tmp/ for the tests, but that comes from Rails.

Nice work!!

waiting-for-dev · 2023-03-03T05:03:18Z

core/lib/generators/solidus/update/update_generator.rb

@@ -57,6 +57,11 @@ def create_new_defaults_initializer
               File.join(options[:initializer_directory], "#{options[:initializer_basename]}.rb")
    end

+    def install_migrations
+      say_status :copying, "migrations"
+      rake 'spree:install:migrations'


Using spree: as the namespace is even better than using the FROM env variable 👏 👏

waiting-for-dev

Oh, sorry, I thought I had already approve this one 👍

github-actions · 2023-03-09T09:12:17Z

💚 All backports created successfully

Status	Branch	Result
✅	v3.2
✅	v3.3

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

kennyadsl added backport-v3.2 backport-v3.3 Backport this pull-request to v3.3 labels Feb 27, 2023

kennyadsl requested a review from a team as a code owner February 27, 2023 09:41

kennyadsl self-assigned this Feb 27, 2023

github-actions bot added the changelog:solidus_core Changes to the solidus_core gem label Feb 27, 2023

waiting-for-dev reviewed Feb 27, 2023

View reviewed changes

kennyadsl force-pushed the kennyadsl/update-command branch from 0886241 to 241f6bc Compare March 2, 2023 15:45

waiting-for-dev reviewed Mar 3, 2023

View reviewed changes

kennyadsl requested review from waiting-for-dev and a team March 8, 2023 11:05

waiting-for-dev approved these changes Mar 8, 2023

View reviewed changes

adammathys approved these changes Mar 8, 2023

View reviewed changes

kennyadsl merged commit c29e8fc into master Mar 9, 2023

kennyadsl deleted the kennyadsl/update-command branch March 9, 2023 09:11

This was referenced Mar 9, 2023

[v3.2] Copy new migrations as part of the update task #4972

Merged

[v3.3] Copy new migrations as part of the update task #4973

Merged

kennyadsl mentioned this pull request Mar 13, 2023

Remove rake aborted error running the update generator in specs #4978

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Copy new migrations as part of the update task #4957

Copy new migrations as part of the update task #4957

kennyadsl commented Feb 27, 2023 •

edited

Loading

codecov bot commented Feb 27, 2023 •

edited

Loading

waiting-for-dev left a comment

waiting-for-dev Feb 27, 2023

kennyadsl Mar 2, 2023

kennyadsl Mar 2, 2023

kennyadsl commented Feb 27, 2023

kennyadsl commented Mar 2, 2023

waiting-for-dev left a comment

waiting-for-dev Mar 3, 2023

waiting-for-dev left a comment

github-actions bot commented Mar 9, 2023

	The defaults update process is only supported if you are coming from version #{FROM}. If this is not the case, please, skip it and update your application to use Solidus #{FROM} before retrying.
	The default update process is only supported if you are coming from version #{FROM}. If this is not the case, please, skip it and update your application to use Solidus #{FROM} before retrying.

Copy new migrations as part of the update task #4957

Copy new migrations as part of the update task #4957

Conversation

kennyadsl commented Feb 27, 2023 • edited Loading

Summary

Checklist

codecov bot commented Feb 27, 2023 • edited Loading

Codecov Report

waiting-for-dev left a comment

Choose a reason for hiding this comment

waiting-for-dev Feb 27, 2023

Choose a reason for hiding this comment

kennyadsl Mar 2, 2023

Choose a reason for hiding this comment

kennyadsl Mar 2, 2023

Choose a reason for hiding this comment

kennyadsl commented Feb 27, 2023

kennyadsl commented Mar 2, 2023

waiting-for-dev left a comment

Choose a reason for hiding this comment

waiting-for-dev Mar 3, 2023

Choose a reason for hiding this comment

waiting-for-dev left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 9, 2023

💚 All backports created successfully

Questions ?

kennyadsl commented Feb 27, 2023 •

edited

Loading

codecov bot commented Feb 27, 2023 •

edited

Loading