Defect #31116
closed
Database migrations don't run correctly for plugins when specifying the `VERSION` env variable
0%
Description
tl;dr: When comparing versions, the current_version is set to the core's latest migration instead of the plugin's latest.
Steps to reproduce:
1. Install any plugin with migrations, for example:
cd redmine/plugins git clone https://github.com/ixti/redmine_tags.git bundle install rake redmine:plugins:migrate NAME=redmine_tags
2. Undo the last migration of that plugin:
rake redmine:plugins:migrate NAME=redmine_tags VERSION=20151209153801
3. Migrate to the latest version again, but specifying the version this time:
rake redmine:plugins:migrate NAME=redmine_tags VERSION=20180122193833
Expected behaviour: the 20180122193833 migration is executed.
Actual behaviour: the 20180122193833 is not executed.
This is because redmine core's latest migration is 20180923091603 which is greather that the plugin's 20180122193833.
The attached fix defines a `migrate` method for the `Redmine::Plugin::MigrationContext` class that makes sure to pass along the current plugin's latest migration. Using the default migrate method defined in activerecord executes a `current_version` that looks for the most current migration in the list of all migrations (core and plugins).
Note: I have not run any tests with this patch.
Files
Updated by Anonymous over 5 years ago
Test run available here: https://gitlab.com/sdwolfz/redmine/-/jobs/187237412
Although... I am not sure how to add a test that checks for this exact error.
Updated by Go MAEDA over 5 years ago
- Category set to Plugin API
- Target version set to Candidate for next major release
Updated by Martijn Vernooij over 5 years ago
I noticed the same issue (with 4.0.2 though) and 'fixed' it another way.
Updated by crypto gopher about 2 years ago
The problem stems from fact, that Redmine::Plugin::MigrationContext does not override current_version() method. So whenever ActiveRecord::MigrationContext.migrate() is executed, it calls ActiveRecord::MigrationContext.current_version() which does not differentiate between Redmine's migrations and plugin's migrations - yielding erroneous result.
The most straightforward change is to override ActiveRecord::MigrationContext.migrate() by using Redmine::Plugin::Migrator.current_version() which already provides required functionality:
class MigrationContext < ActiveRecord::MigrationContext
...
def current_version
Migrator.current_version
end
end
Updated by Go MAEDA about 2 years ago
crypto gopher wrote:
The most straightforward change is to override
ActiveRecord::MigrationContext.migrate()by usingRedmine::Plugin::Migrator.current_version()which already provides required functionality:
Thank you for posting the patch.
I wrote a test for your patch as follows. Do you think the following code tests the right way?
diff --git a/test/unit/lib/redmine/plugin_test.rb b/test/unit/lib/redmine/plugin_test.rb
index a5a1b2aa3..24c06240c 100644
--- a/test/unit/lib/redmine/plugin_test.rb
+++ b/test/unit/lib/redmine/plugin_test.rb
@@ -217,4 +217,17 @@ class Redmine::PluginTest < ActiveSupport::TestCase
assert Redmine::Plugin.migrate('foo_plugin')
end
+
+ def test_migration_context_should_override_current_version
+ plugin = @klass.register :foo_plugin do
+ name 'Foo plugin'
+ version '0.0.1'
+ end
+ migration_dir = File.join(@klass.directory, 'db', 'migrate')
+
+ Redmine::Plugin::Migrator.current_plugin = plugin
+ context = Redmine::Plugin::MigrationContext.new(migration_dir, ::ActiveRecord::Base.connection.schema_migration)
+ # current_version should be zero because Foo plugin has no migration
+ assert_equal 0, context.current_version
+ end
end
Updated by Go MAEDA about 2 years ago
- Target version changed from Candidate for next major release to 5.1.0
Setting the target version to 5.1.0.
Updated by Go MAEDA about 2 years ago
- Status changed from Confirmed to Closed
- Assignee set to Go MAEDA
- Resolution set to Fixed
Committed the patch. Thank you for your contribution.