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.

Sign up for GitHub

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

Jump to bottom

Api docs (and rubocop) #5

Closed
wants to merge 3 commits into from
Closed

Api docs (and rubocop) #5

wants to merge 3 commits into from

Conversation

mikker
Copy link
Owner

@mikker mikker commented Dec 14, 2017

No description provided.

houndci-bot
houndci-bot reviewed Dec 14, 2017
View reviewed changes
test/test_helper.rb Outdated
ActionDispatch::IntegrationTest.fixture_path = ActiveSupport::TestCase.fixture_path
ActiveSupport::TestCase.file_fixture_path = ActiveSupport::TestCase.fixture_path + "/files"
ActiveSupport::TestCase.file_fixture_path = ActiveSupport::TestCase.fixture_path + '/files'
Copy link

Choose a reason for hiding this comment

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

Line is too long. [93/80]

test/test_helper.rb Outdated
# Load fixtures from the engine
if ActiveSupport::TestCase.respond_to?(:fixture_path=)
ActiveSupport::TestCase.fixture_path = File.expand_path("../fixtures", __FILE__)
ActiveSupport::TestCase.fixture_path = File.expand_path('../fixtures', __FILE__)
Copy link

Choose a reason for hiding this comment

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

Line is too long. [82/80]

test/test_helper.rb Outdated
# frozen_string_literal: true

require File.expand_path('../../test/dummy/config/environment.rb', __FILE__)
ActiveRecord::Migrator.migrations_paths = [File.expand_path('../../test/dummy/db/migrate', __FILE__)]
Copy link

Choose a reason for hiding this comment

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

Line is too long. [101/80]

test/passwordless_for_test.rb Outdated
{ method: :get, path: "/users/sign_in/abc123" },
{ controller: 'passwordless/sessions', action: 'show', params: { token: 'abc123' }, authenticatable: 'user' }
{ method: :get, path: '/users/sign_in/abc123' },
controller: 'passwordless/sessions', action: 'show', params: { token: 'abc123' }, authenticatable: 'user'
Copy link

Choose a reason for hiding this comment

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

Line is too long. [115/80]

test/passwordless_for_test.rb Outdated
{ method: :delete, path: "/users/sign_out" },
{ controller: 'passwordless/sessions', action: 'destroy', authenticatable: 'user' }
{ method: :delete, path: '/users/sign_out' },
controller: 'passwordless/sessions', action: 'destroy', authenticatable: 'user'
Copy link

Choose a reason for hiding this comment

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

Line is too long. [87/80]

test/passwordless_for_test.rb Outdated
{ method: :post, path: "/users/sign_in", params: { passwordless: { email: 'a@a' } } },
{ controller: 'passwordless/sessions', action: 'create', authenticatable: 'user' }
{ method: :post, path: '/users/sign_in', params: { passwordless: { email: 'a@a' } } },
controller: 'passwordless/sessions', action: 'create', authenticatable: 'user'
Copy link

Choose a reason for hiding this comment

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

Line is too long. [86/80]

test/passwordless_for_test.rb Outdated
)
assert_routes(
{ method: :post, path: "/users/sign_in", params: { passwordless: { email: 'a@a' } } },
{ controller: 'passwordless/sessions', action: 'create', authenticatable: 'user' }
{ method: :post, path: '/users/sign_in', params: { passwordless: { email: 'a@a' } } },
Copy link

Choose a reason for hiding this comment

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

Line is too long. [94/80]

test/passwordless_for_test.rb Outdated
{ method: :get, path: "/users/sign_in" },
{ controller: 'passwordless/sessions', action: 'new', authenticatable: 'user' }
{ method: :get, path: '/users/sign_in' },
controller: 'passwordless/sessions', action: 'new', authenticatable: 'user'
Copy link

Choose a reason for hiding this comment

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

Line is too long. [83/80]

test/models/passwordless/session_test.rb Outdated
authenticatable: User.create(email: 'session_test_valid@a')
remote_addr: '0.0.0.0',
user_agent: 'wooden box',
authenticatable: User.create(email: 'session_test_valid@a')
Copy link

Choose a reason for hiding this comment

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

Line is too long. [83/80]

app/mailers/passwordless/mailer.rb Outdated
@@ -11,7 +13,7 @@ def magic_link(session)
send(authenticatable_resource_name).token_sign_in_url(session.token)

email_field = @session.authenticatable.class.passwordless_email_field
mail to: @session.authenticatable.send(email_field), subject: "Your magic link ✨"
mail to: @session.authenticatable.send(email_field), subject: 'Your magic link ✨'
Copy link

Choose a reason for hiding this comment

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

Line is too long. [87/80]

@mftaff
Copy link
Contributor

mftaff commented Dec 17, 2017 •
edited
Loading

I have started working on adding documentation. I just want to make sure that the style is aligned with what you are looking for :-)

Here is a link to the api-docs branch on my fork. If/When you have a minute to look it over, your feedback would be amazing.
I want my contributions to be helpful to the project, so if you don't like any/part/all of it, please let me know, so I can learn better what the end goal is!

Again Thanks for letting me contribute!

@mftaff
Copy link
Contributor

mftaff commented Dec 18, 2017

What does this method return? I want to make sure I understand what authenticatable_class refers to.

def authenticatable
  params.fetch(:authenticatable)
end

Thanks!

@mikker
Copy link
Owner Author

mikker commented Dec 18, 2017 •
edited
Loading

@mftaff It comes from this: https://github.com/mikker/passwordless/blob/master/lib/passwordless/router_helpers.rb#L8

It gets defined when the user does passwordless_for :users in her routes file. Eg. :users => "user".
We're using .fetch to make sure that we raise an exception if it isn't found because we strictly need it to know what kind of Model to look for 😄

@mftaff
Copy link
Contributor

mftaff commented Dec 18, 2017

@mikker A bit of a technical question...
How do I push my changes to add them to this pull request? I have never done this before, and I couldn't find a clear answer on google 😛
Do I push straight to this branch from my fork? Or do I create a PR from my fork? Or something else entirely?

@mikker
Copy link
Owner Author

mikker commented Dec 18, 2017

The easiest way is probably to create a pull request from your fork. It's based off of this branch, right? Then we can just merge that and delete this PR.

@mikker
Copy link
Owner Author

mikker commented Dec 27, 2017

Merged in #7

@mikker mikker closed this Dec 27, 2017
@mikker mikker deleted the api-docs branch December 27, 2017 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mikker @mftaff @houndci-bot