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

Release - 0.3.0 #50

Merged
merged 222 commits into from Jun 28, 2021
Merged

Release - 0.3.0 #50

merged 222 commits into from Jun 28, 2021

Conversation

@malparty malparty self-assigned this Jun 25, 2021
@malparty malparty added this to In development in Product backlog via automation Jun 25, 2021
# frozen_string_literal: true

module API
module V1

Choose a reason for hiding this comment

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

⚠️ Has the name 'v1'

private

# helper method to access the current user from the token
def current_user

Choose a reason for hiding this comment

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

⚠️ Remove unused methods (api::v1::applicationcontroller#current_user)

# frozen_string_literal: true

module API
module V1

Choose a reason for hiding this comment

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

⚠️ Has the name 'v1'

include ErrorHandlerConcern

# Overridden from doorkeeper as the doorkeeper revoke action does not return response according to json-api spec
def revoke

Choose a reason for hiding this comment

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

⚠️ Remove unused methods (api::v1::tokenscontroller#revoke)

# The authorization server responds with HTTP status code 200 if the client
# submitted an invalid token or the token has been revoked successfully.
if token.blank?
render json: token_revoke_response, status: :ok

Choose a reason for hiding this comment

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

⚠️ Calls 'render json: token_revoke_response, status: :ok' 2 times

# of the error by the authorization server as described below.
elsif authorized?
revoke_token
render json: token_revoke_response, status: :ok

Choose a reason for hiding this comment

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

⚠️ Calls 'render json: token_revoke_response, status: :ok' 2 times

}
end

def token_revoke_response

Choose a reason for hiding this comment

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

⚠️ Doesn't depend on instance state (maybe move it to another class?)

# frozen_string_literal: true

module API
module V1

Choose a reason for hiding this comment

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

⚠️ Has the name 'v1'

# frozen_string_literal: true

module API
module V1

Choose a reason for hiding this comment

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

⚠️ Has the name 'v1'

render json: { errors: jsonapi_errors }, status: status
end

def build_error(detail:, source: nil, code: nil)

Choose a reason for hiding this comment

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

⚠️ Doesn't depend on instance state (maybe move it to another class?)

protected

def update_allowed_parameters
devise_parameter_sanitizer.permit(:sign_up) do |u|

Choose a reason for hiding this comment

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

⚠️ Has the variable name 'u'

devise_parameter_sanitizer.permit(:sign_up) do |u|
u.permit(:first_name, :last_name, :email, :password, :password_confirmation)
end
devise_parameter_sanitizer.permit(:account_update) do |u|

Choose a reason for hiding this comment

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

⚠️ Has the variable name 'u'

@@ -11,4 +11,25 @@ def self.authenticate(email, password)
user = User.find_for_authentication(email: email)
user&.valid_password?(password) ? user : nil

Choose a reason for hiding this comment

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

⚠️ Is controlled by argument 'password'

end

def as_json(_options = nil)
{ errors: @errors.errors.map { |e| { detail: e.full_message, source: e.attribute } } }

Choose a reason for hiding this comment

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

⚠️ Refers to 'e' more than self (maybe move it to another class?)

end

def as_json(_options = nil)
{ errors: @errors.errors.map { |e| { detail: e.full_message, source: e.attribute } } }

Choose a reason for hiding this comment

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

⚠️ Has the variable name 'e'

@uri = URI("#{BASE_SEARCH_URL}?q=#{@escaped_keyword}&hl=#{lang}&gl=#{lang}")
end

def call

Choose a reason for hiding this comment

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

⚠️ Has approx 6 statements

return false unless valid_result? result

result
rescue HTTParty::Error, Timeout::Error, SocketError => e

Choose a reason for hiding this comment

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

⚠️ Has the variable name 'e'

</a>
</div>
<div class="collapse" id="newPassword">
<% if @minimum_password_length %>

Choose a reason for hiding this comment

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

⚠️ Replace instance variable with local variable

</div>
<div class="collapse" id="newPassword">
<% if @minimum_password_length %>
<em>(<%= "#{@minimum_password_length} #{t('min_char')}" %>)</em>

Choose a reason for hiding this comment

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

⚠️ Replace instance variable with local variable

</div>
<div class="form-group">
<%= f.label :password %><br/>
<% if @minimum_password_length %>

Choose a reason for hiding this comment

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

⚠️ Replace instance variable with local variable

@github-actions
Copy link

6 Warnings
⚠️ spec/support/authentication_helper.rb#L27 - Has approx 8 statements
⚠️ spec/support/authentication_helper.rb#L36 - Is controlled by argument 'password_confirm'
⚠️ spec/support/vcr.rb#L42 - Has approx 8 statements
⚠️ spec/support/vcr.rb#L43 - Refers to 'vcr_options' more than self (maybe move it to another class?)
⚠️ spec/support/vcr.rb#L44 - Refers to 'vcr_options' more than self (maybe move it to another class?)
⚠️ spec/support/vcr.rb#L45 - Refers to 'vcr_options' more than self (maybe move it to another class?)

Generated by 🚫 Danger

<div class="form-group">
<%= f.label :password %><br/>
<% if @minimum_password_length %>
<em>(<%= "#{@minimum_password_length} #{t('min_char')}" %>)</em>

Choose a reason for hiding this comment

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

⚠️ Replace instance variable with local variable

Product backlog automation moved this from In development to Ready for QA Jun 25, 2021
@malparty malparty merged commit d575ab4 into main Jun 28, 2021
@malparty malparty moved this from Ready for QA to Completed in Product backlog Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants