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

Update gems #338

Merged
merged 11 commits into from Jan 4, 2017
Merged

Update gems #338

merged 11 commits into from Jan 4, 2017

Conversation

tsujigiri
Copy link
Collaborator

@tsujigiri tsujigiri commented Dec 26, 2016

  • Passwords now need to be 8 characters long (new default in authlogic).
  • To login to the Sidekiq web interface, log in to opensnp.org, if you
    have the admin flag set, you will find it at the usual place.
  • I finally found out why Rubocop didn't complain about the frozen string literal pragma missing and added all of them.
  • Rubocop complained about keyword arguments, which is only relevant for Rails 5, so I disabled the cop.
  • I removed the inherit_from: .rubocop_todo.yml from .rubocop.yml, so Hound does complain about all the issues. I also added a few tasks for running Rubocop.

@@ -0,0 +1,5 @@
class AddAdminFlagToUsers < ActiveRecord::Migration
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing frozen string literal comment.

@@ -40,8 +40,8 @@ class UsersControllerTest < ActionController::TestCase
should "be able to create accounts" do
UsersController.any_instance.expects(:verify_recaptcha).returns(true)
assert_difference 'User.count' do
put :create, user: { name: "Fubert Barfuß", password: 'jeheim',
password_confirmation: 'jeheim', email: 'fubert@example.com'}, read: 1
put :create, user: { name: "Fubert Barfuß", password: 'strengjeheim',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use keyword arguments instead of positional arguments for http call: put.
Prefer single-quoted strings when you don't need string interpolation or special symbols.

@@ -30,8 +30,8 @@ class UsersControllerTest < ActionController::TestCase
should "not be able to create accounts when failing reCAPTCHA" do
UsersController.any_instance.expects(:verify_recaptcha).returns(false)
assert_no_difference 'User.count' do
put :create, user: { name: "Fubert Barfuß", password: 'jeheim',
password_confirmation: 'jeheim', email: 'fubert@example.com'}, read: 1
put :create, user: { name: "Fubert Barfuß", password: 'strengjeheim',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use keyword arguments instead of positional arguments for http call: put.
Prefer single-quoted strings when you don't need string interpolation or special symbols.

@@ -1,7 +1,7 @@
module Authlogic
module RequestSpecHelper
def sign_in(user)
post user_session_path, user_session: { email: user.email, password: 'jeheim' }
post user_session_path, user_session: { email: user.email, password: 'strengjeheim' }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use keyword arguments instead of positional arguments for http call: post.

require 'codeclimate-test-reporter'
CodeClimate::TestReporter.start
end
require 'simplecov'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing frozen string literal comment.

class AdminConstraint
def matches?(request)
return false unless request.cookie_jar['user_credentials'].present?
user = User.find_by_persistence_token(request.cookie_jar['user_credentials'].split(':')[0])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use find_by instead of dynamic find_by_persistence_token.

@@ -0,0 +1,7 @@
class AdminConstraint
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing frozen string literal comment.

* Passwords now need to be 8 characters long (new default in authlogic).
* To login to the Sidekiq web interface, log in to opensnp.org, if you
  have the `admin` flag set, you will find it at the usual place.
assert_difference 'User.count' do
put :create, user: { name: "Fubert Barfuß", password: 'jeheim',
password_confirmation: 'jeheim', email: 'fubert@example.com'}, read: 1
put :create, user: { name: "Fubert Barfuß", password: 'strengjeheim',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use keyword arguments instead of positional arguments for http call: put.
Prefer single-quoted strings when you don't need string interpolation or special symbols.

@@ -1,4 +1,4 @@
ENV["RAILS_ENV"] = "test"
ENV['RAILS_ENV'] = 'test'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing frozen string literal comment.

@@ -0,0 +1,21 @@
namespace :rubocop do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing frozen string literal comment.

On the CI two tests failed because the ReCaptcha site key wasn't set,
even though ReCaptcha should be completely ignored in the tests.
@tsujigiri
Copy link
Collaborator Author

This should be good to go, now.

@gedankenstuecke
Copy link
Member

Looks good to me. @philippbayer any complains from your side? 😺

Copy link
Member

@gedankenstuecke gedankenstuecke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(trying this new review system for the first time) Looks good to me. Thanks for making the 🐶 happy!

@tsujigiri
Copy link
Collaborator Author

Sooo... should I merge or do we wait for @philippbayer to respond? 🙂

@gedankenstuecke
Copy link
Member

Fire away, I think @philippbayer just arrived in 🇩🇪 this morning from 🇯🇵 , so it would be a few days before he's able to read straight again. 😂

@tsujigiri tsujigiri merged commit 906eef5 into openSNP:master Jan 4, 2017
@tsujigiri tsujigiri deleted the update-gems branch January 4, 2017 14:30
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.

None yet

3 participants