Skip to content

Feature3#3

Open
Smokashi23 wants to merge 5 commits into
feature2from
feature3
Open

Feature3#3
Smokashi23 wants to merge 5 commits into
feature2from
feature3

Conversation

@Smokashi23

Copy link
Copy Markdown
Owner

No description provided.

Comment thread .byebug_history
params
n
c
n

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@Smokashi23 Unnecessary file pushed

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

updated changes

begin
JWT.decode(token, Rails.application.credentials[:secret_key_base] )
rescue JWT::DecodeError
nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why using nil here ?

if current_user.nil?
def authorized
if current_user.nil?
render json: { message: 'Please log in' }, status: :unauthorized

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Display message through I18n.t

Comment thread app/controllers/appts_controller.rb Outdated
end

def show
@appt = Appt.find(params[:id])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Error handling is missing

Comment thread app/controllers/appts_controller.rb Outdated
end

def appt_params
params.permit(:id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Instead of permitting, you can directly access id through query params as well

appt = create(:appt)
put :update, params: { id: appt.id, status: "cancelled" }
appt.reload
expect(appt.status).to eq("cancelled")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Add more expects to check database changes as well

slot.update(booked: true)
post :booked, params: { id: slot.id }
expect(response).to have_http_status(:unprocessable_entity)
expect(JSON.parse(response.body)["error"]).to eq("This slot is already booked")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pleas use I18n to display or check any type of messages

Comment thread spec/factories/users.rb
role { create(:role, role_name: 'Doctor') }
end

trait :patient do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What is trait here used for ?

Comment thread spec/factories/users.rb Outdated
end
end


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unnecessary space detected

@@ -0,0 +1,7 @@
require "test_helper"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please do not push unnecessary files

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.

2 participants