fix migration file for rails 5 #985

jonhue · 2017-08-24T15:33:08Z

The migration file generated with rails generate doorkeeper:migration did not have the appropriate [5.x] when I was installing it.

Rails version: 5.1.3

So here is a fix.

houndci-bot · 2017-08-24T15:33:56Z

lib/generators/doorkeeper/migration_generator.rb

-      migration_version: migration_version
-    )
+    @migration_version = Rails::VERSION::STRING[0..2].to_f
+    migration_template 'migration.rb.erb', 'db/migrate/create_doorkeeper_tables.rb'


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Line is too long. [83/80]

houndci-bot · 2017-08-24T16:17:32Z

lib/generators/doorkeeper/application_owner_generator.rb

-      'add_owner_to_application_migration.rb',
-      'db/migrate/add_owner_to_application.rb'
+      'add_owner_to_application_migration.rb.erb',
+      'db/migrate/add_owner_to_application.rb',


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-08-24T16:17:32Z

lib/generators/doorkeeper/application_owner_generator.rb

@@ -7,8 +7,9 @@ class Doorkeeper::ApplicationOwnerGenerator < Rails::Generators::Base

  def application_owner
    migration_template(
-      'add_owner_to_application_migration.rb',
-      'db/migrate/add_owner_to_application.rb'
+      'add_owner_to_application_migration.rb.erb',


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

…to_application_migration.rb.erb

houndci-bot · 2017-08-24T16:18:21Z

lib/generators/doorkeeper/templates/add_previous_refresh_token_to_access_tokens.rb

@@ -1,4 +1,4 @@
-class AddPreviousRefreshTokenToAccessTokens < ActiveRecord::Migration
+class AddPreviousRefreshTokenToAccessTokens < ActiveRecord::Migration<%= migration_version %>


unterminated string meets end of file

houndci-bot · 2017-08-24T16:19:34Z

lib/generators/doorkeeper/previous_refresh_token_generator.rb

-        'add_previous_refresh_token_to_access_tokens.rb',
-        'db/migrate/add_previous_refresh_token_to_access_tokens.rb'
+        'add_previous_refresh_token_to_access_tokens.rb.erb',
+        'db/migrate/add_previous_refresh_token_to_access_tokens.rb',


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2017-08-24T16:19:35Z

lib/generators/doorkeeper/previous_refresh_token_generator.rb

@@ -12,8 +12,9 @@ def self.next_migration_number(path)
  def previous_refresh_token
    if no_previous_refresh_token_column?
      migration_template(
-        'add_previous_refresh_token_to_access_tokens.rb',
-        'db/migrate/add_previous_refresh_token_to_access_tokens.rb'
+        'add_previous_refresh_token_to_access_tokens.rb.erb',


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

…_refresh_token_to_access_tokens.rb.erb

houndci-bot · 2017-08-24T18:45:57Z

lib/generators/doorkeeper/application_owner_generator.rb

    )
  end

  def self.next_migration_number(dirname)
    ActiveRecord::Generators::Base.next_migration_number(dirname)
  end
+


Trailing whitespace detected.

houndci-bot · 2017-08-24T18:46:13Z

lib/generators/doorkeeper/previous_refresh_token_generator.rb

@@ -8,12 +8,19 @@ class Doorkeeper::PreviousRefreshTokenGenerator < Rails::Generators::Base
  def self.next_migration_number(path)
    ActiveRecord::Generators::Base.next_migration_number(path)
  end
+


Trailing whitespace detected.

nbulaj · 2017-08-27T10:47:41Z

@jonhue take a look at the #981 - already in master.

jonhue · 2017-08-27T11:13:59Z

@nbulaj

It has not been done for the other migration like adding applications & stuff.

jcamejo · 2017-09-06T14:11:14Z

Hi
i'm having some troubles with this fix, the migration is being generated without the rails version, even with the master fix. If i download the gem into my local environment it creates the migration with the version but not with the remote one. Is there anything that i'm doing wrong?

i've updated the gem to the last version.

Thanks!

jonhue · 2017-09-06T19:43:57Z

@jcamejo

This branch works fine. Master does not.
For now, I suggest you just add it manually.

nbulaj · 2017-10-17T14:02:35Z

Hi @maclover7 . I think we can merge it with the master branch.

@jonhue one thing that I would change: Rails version check. Why not to compare integers, not strings? I mean why not Rails::VERSION::MAJOR >= 5 instead of Rails.version >= "5.0.0" ? It works, yes, but looks a little bit hacky

jonhue · 2017-10-17T14:12:39Z

@nbulaj Here you go ...

I want to point out though, that the other generators in this repository are sing a string comparison:
https://github.com/doorkeeper-gem/doorkeeper/blob/master/lib/generators/doorkeeper/migration_generator.rb#L21

nbulaj · 2017-10-17T14:50:09Z

@jonhue take a look at the TravisCI log - there are some errors :)

jonhue · 2017-10-17T16:39:58Z

@nbulaj Seems like poor little Travis didn't like that change too much :)

nbulaj · 2018-01-26T20:45:41Z

Hi @jonhue . Some comments:

1. Could you please check Rails version as:

if Rails::VERSION::MAJOR >= 5

As I said before, I don't like the way of string comparison. Let's check integers :)

2. Could you add some specs for this feature (check Rails version in the specs, and if it more than 5, then check if migration template has a version specification) ? You can take a look at some already existing examples

After that please rebase your commits and squash them into one, and add an entry to NEWS.md. Thanks for your work and patience!

Add migration versioning for migration templates if ActiveRecord greater than 5.0. Add specs to ensure valid generator implementation.

nbulaj · 2018-01-31T14:05:30Z

Thanks @jonhue for your work! I already fixed this issue with some corrections and specs to prepare a new release of the gem.

fix migration file for rails 5

4e47770