Skip to content
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

Added json support for tokens #276

Merged
merged 1 commit into from
Jul 10, 2015
Merged

Added json support for tokens #276

merged 1 commit into from
Jul 10, 2015

Conversation

shicholas
Copy link

In the installation generator script, the migration template checks to see if the app is using postgres by seeing what sub-class of ActiveRecord::ConnectionAdapter the app is using. If it is postgres, the app will use a token column instead of a text one.

@shicholas
Copy link
Author

Since the tests run with SQLite, what do you think the best way to add tests are? I just did it manually.

@shicholas
Copy link
Author

hmm sorry it looks like my branch is out of sync. I'll update that soon

edit: updated

@nicolas-besnard
Copy link
Contributor

Could be nice to change column too ;)

@shicholas
Copy link
Author

fwiw, I got the inspiration from this comment: #121 (comment)

@booleanbetrayal
Copy link
Collaborator

@shicholas - I think we need an additional check on pg_types as json was not supported until 9.3, and just checking the adapter is insufficient. see also: http://www.postgresql.org/docs/current/interactive/catalog-pg-type.html

Likewise, MySQL is now rolling json support, so we should account for that possibility as well. Not sure about SQLite at this point ...

@shicholas
Copy link
Author

okay sounds good, I will do a check for mysql and postgres with the right versions. I'll push up the some code soon. How should I test this though? Should we run the tests w/ those database versions?

@shicholas
Copy link
Author

pushed, let me know what you think.

@booleanbetrayal
Copy link
Collaborator

@shicholas - LGTM ... would you mind rebasing and squashing your commits?

In the installation generator script, the migration template checks to
see if the app is using postgres by seeing what sub-class of
``ActiveRecord::ConnectionAdapter`` the app is using. If it is postgres, or mysql with the correct version, the app will use a token column instead of a text one

Postgres has JSON types as of 9.3
Mysql has JSON types as of 5.7.7
http://mysqlserverteam.com/json-labs-release-native-json-data-type-and-binary-format/

teeny refactoring
@shicholas
Copy link
Author

np, rebased and squashed.

@booleanbetrayal
Copy link
Collaborator

thanks @shicholas !

booleanbetrayal added a commit that referenced this pull request Jul 10, 2015
@booleanbetrayal booleanbetrayal merged commit b8254e7 into lynndylanhurley:master Jul 10, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants