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

Feat/job application show #35

Merged
merged 9 commits into from
Dec 12, 2024
146 changes: 90 additions & 56 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,31 +48,31 @@ Successful Response:
```
Status: 201 Created
Body: {
"data": {
"id": "4",
"type": "user",
"attributes": {
"name": "John Doe",
"email": "[email protected]"
}
"data": {
"id": "4",
"type": "user",
"attributes": {
"name": "John Doe",
"email": "[email protected]"
}
}
}
```

Error Responses:
```
Status: 400 Bad Request
Body: {
"message": "Email has already been taken",
"status": 400
"message": "Email has already been taken",
"status": 400
}
```

```
Status: 400 Bad Request
Body: {
"message": "Password confirmation doesn't match Password",
"status": 400
"message": "Password confirmation doesn't match Password",
"status": 400
}
```

Expand All @@ -86,25 +86,25 @@ Successful Response:
```
Status: 200 OK
Body: {
"data": [
{
"id": "1",
"type": "user",
"attributes": {
"name": "Danny DeVito",
"email": "danny_de_v"
}
},
...
{
"id": "4",
"type": "user",
"attributes": {
"name": "John Doe",
"email": "[email protected]"
}
}
]
"data": [
{
"id": "1",
"type": "user",
"attributes": {
"name": "Danny DeVito",
"email": "danny_de_v"
}
},
...
{
"id": "4",
"type": "user",
"attributes": {
"name": "John Doe",
"email": "[email protected]"
}
}
]
}
```

Expand All @@ -118,14 +118,14 @@ Successful Response:
```
Status: 200 OK
Body: {
"data": {
"id": "3",
"type": "user",
"attributes": {
"name": "Lionel Messi",
"email": "futbol_geek"
}
"data": {
"id": "3",
"type": "user",
"attributes": {
"name": "Lionel Messi",
"email": "futbol_geek"
}
}
}
```

Expand All @@ -147,30 +147,30 @@ Successful Response:
```
Status: 200 OK
Body: {
"data": {
"id": "4",
"type": "user",
"attributes": {
"name": "Nathan Fillon",
"email": "firefly_captian"
}
"data": {
"id": "4",
"type": "user",
"attributes": {
"name": "Nathan Fillon",
"email": "firefly_captian"
}
}
}
```
Error Responses:
```
Status: 400 Bad Request
Body: {
"message": "Email has already been taken",
"status": 400
"message": "Email has already been taken",
"status": 400
}
```

```
Status: 400 Bad Request
Body: {
"message": "Password confirmation doesn't match Password",
"status": 400
"message": "Password confirmation doesn't match Password",
"status": 400
}
```

Expand Down Expand Up @@ -245,21 +245,55 @@ Status: 200
:type=>"job_application",
:attributes=>
{:position_title=>"Jr. CTO",
:date_applied=>"2024-10-31",
:status=>1,
:notes=>"Fingers crossed!",
:job_description=>"Looking for Turing grad/jr dev to be CTO",
:application_url=>"www.example.com",
:contact_information=>"[email protected]",
:company_id=>35}}
}
:date_applied=>"2024-10-31",
:status=>1,
:notes=>"Fingers crossed!",
:job_description=>"Looking for Turing grad/jr dev to be CTO",
:application_url=>"www.example.com",
:contact_information=>"[email protected]",
:company_id=>35}}
}
```

Unsuccessful Response:
```
{:message=>"Company must exist and Position title can't be blank", :status=>400}
```

#### Get a Job Application
Request:
```
GET /api/v1/users/:user_id/job_applications/:job_application_id
```

Successful Response:
```
Status: 200 OK
{:data=>
{:id=>"4",
:type=>"job_application",
:attributes=>
{:position_title=>"Jr. CTO",
:date_applied=>"2024-10-31",
:status=>1,
:notes=>"Fingers crossed!",
:job_description=>"Looking for Turing grad/jr dev to be CTO",
:application_url=>"www.example.com",
:contact_information=>"[email protected]",
:company_id=>35}}
}
```

Unsuccessful Response(job application does not exist OR belongs to another user):
```
{:message=>"Job application not found", :status=>404}
```

Unsuccessful Response(missing job application ID param):
```
{:message=>"Job application ID is missing", :status=>400}
```

### Companies

Get login credentials
Expand Down
15 changes: 15 additions & 0 deletions app/controllers/api/v1/job_applications_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,21 @@ def create
end
end

def show
if params[:id].blank?
render json: ErrorSerializer.format_error(ErrorMessage.new("Job application ID is missing", 400)), status: :bad_request
return
end
Comment on lines +18 to +21
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a nice addition to the show action. I noticed that you are grabbing the ID from the params, and not from the authentication token. I'm curious about this decision, as both the companies and contacts are collecting it from the token.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I noticed that the JobApplications controller is not utilizing the authentication tokens or the authenticate_user method that has been defined in the ApplicationController. Other controllers in the application are using these for authentication, so I wanted to flag this for consistency and security purposes.

Could you clarify if this was intentional or an oversight? If it's an oversight, it might be worth adding the necessary authentication logic to ensure this controller aligns with the others. I'm curious to hear your thoughts!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At first I felt the need to address this refactor in this ticket. However, since the integration of gems related to authorization and authentication there has been a new ticket created (ticket #42) that was made for the express purpose of handling these types of refactors, that being said. I think we should merge the PR as it currently stands so it does not serve as a blocker for the person who will pick up the controller refactor ticket.

user = User.find(params[:user_id])
job_application = JobApplication.find_by(id: params[:id])

if job_application.nil? || job_application.user_id != user.id
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would we want a different error if the user doesn't match?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Kinda torn on this, Im not sure if the effort is worth the result, but i'm willing to hear out justifications. Feel free to post some in this thread if anyone is interested.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not 100% one way or the other, but clearer messages to the front is helpful and since a lack of job application is a slightly different error than an incorrect user id. They both result in the job application not working as intended, so I also see why one error for each does the job.

render json: ErrorSerializer.format_error(ErrorMessage.new("Job application not found", 404)), status: :not_found
else
render json: JobApplicationSerializer.new(job_application), status: :ok
end
end

private

def job_application_params
Expand Down
3 changes: 2 additions & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
namespace :api do
namespace :v1 do
resources :users, only: [:create, :index, :show, :update] do
resources :job_applications, only: :create
resources :job_applications, only: [:create, :show]
get '/job_applications(/:id)', to: 'job_applications#show'
end
resources :companies, only: [:create, :index]

Expand Down
10 changes: 6 additions & 4 deletions db/seeds.rb
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like that you added a seed of the job application. That way you can use it as like default values for the front end and do some testing in the development and testing stages. And you didn't create a bunch of seeds either which would populate even the production database with a bunch of dummy data.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you, I think once we have company seeds this will be very useful for viewing responses with tools like postman.

Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
# ["Action", "Comedy", "Drama", "Horror"].each do |genre_name|
# MovieGenre.find_or_create_by!(name: genre_name)
# end
User.create!(name: "Danny DeVito", email: "[email protected]", password: "jerseyMikesRox7")
User.create!(name: "Dolly Parton", email: "[email protected]", password: "Jolene123")
User.create!(name: "Lionel Messi", email: "[email protected]", password: "test123")
user_1 = User.create!(name: "Danny DeVito", email: "[email protected]", password: "jerseyMikesRox7")
user_2 = User.create!(name: "Dolly Parton", email: "[email protected]", password: "Jolene123")
user_3 = User.create!(name: "Lionel Messi", email: "[email protected]", password: "test123")

JobApplication.create!(
position_title: "Jr. CTO",
Expand All @@ -19,6 +19,8 @@
job_description: "Looking for Turing grad/jr dev to be CTO",
application_url: "www.example.com",
contact_information: "[email protected]",
company_id: 1
company_id: 1,
user_id: user_1.id
)


126 changes: 126 additions & 0 deletions spec/requests/api/v1/job_applications/job_application_show_spec.rb
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another potential edge case would be a lack of parameters being sent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is worth doing. I will incorporate this edge case in the testing, if it requires reworking the method, I will incorporate that as well. Thank you :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Edge case has been addressed and read me has been updated accordingly, If the change is suitable, feel free to resolve conversation and/or resolve conflict to merge :) Reach out if there Is anything I can help clarify

Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
require "rails_helper"

RSpec.describe "Job Application #create", type: :request do
describe "Create Job Application" do
before(:each) do
@user = User.create!(name: "Dolly Parton", email: "[email protected]", password: "Jolene123")

@google = Company.create!(user_id: @user.id, name: "Google", website: "google.com", street_address: "1600 Amphitheatre Parkway", city: "Mountain View", state: "CA", zip_code: "94043", notes: "Search engine")

@facebook = Company.create!(user_id: @user.id, name: "Facebook", website: "facebook.com", street_address: "1 Hacker Way", city: "Menlo Park", state: "CA", zip_code: "94025", notes: "Social media")

@amazon = Company.create!(user_id: @user.id, name: "Amazon", website: "amazon.com", street_address: "410 Terry Ave N", city: "Seattle", state: "WA", zip_code: "98109", notes: "E-commerce")

@google_application = JobApplication.create!(
position_title: "Crank Operator",
date_applied: "2024-10-31",
status: 1,
notes: "Not sure im familiar with the tech-stack",
job_description: "You turn the big crank that powers google",
application_url: "www.example.com",
contact_information: "[email protected]",
company_id: @google.id,
user_id: @user.id
)

@facebook_application = JobApplication.create!(
position_title: "Smiler",
date_applied: "2024-10-31",
status: 1,
notes: "Visit dentist before interview!!!",
job_description: "Raise the facility smile rates.",
application_url: "www.example.com/secret_param",
contact_information: "[email protected]",
company_id: @facebook.id,
user_id: @user.id
)

@google_application = JobApplication.create!(
position_title: "Hallway Enrichment",
date_applied: "2024-10-31",
status: 1,
job_description: "Make our hallways look fresh and clean",
application_url: "www.example.com/super_secret_param",
contact_information: "[email protected]",
company_id: @amazon.id,
user_id: @user.id
)
end

context "happy path" do
it "Returns the specified instance of a user's job applications" do

get "/api/v1/users/#{@user.id}/job_applications/#{@facebook_application.id}"

expect(response).to be_successful
expect(response.status).to eq(200)

jobApp = JSON.parse(response.body, symbolize_names: true)

expect(jobApp[:data][:type]).to eq("job_application")
expect(jobApp[:data][:id]).to eq(@facebook_application.id.to_s)
expect(jobApp[:data][:attributes][:position_title]).to eq(@facebook_application[:position_title])
expect(jobApp[:data][:attributes][:date_applied]).to eq(@facebook_application[:date_applied].to_s)
expect(jobApp[:data][:attributes][:status]).to eq(@facebook_application[:status])
expect(jobApp[:data][:attributes][:notes]).to eq(@facebook_application[:notes])
expect(jobApp[:data][:attributes][:job_description]).to eq(@facebook_application[:job_description])
expect(jobApp[:data][:attributes][:application_url]).to eq(@facebook_application[:application_url])
expect(jobApp[:data][:attributes][:contact_information]).to eq(@facebook_application[:contact_information])
expect(jobApp[:data][:attributes][:company_id]).to eq(@facebook_application[:company_id])
end
end

context "sad path" do
it "returns error serializer if job application id does not exist" do
get "/api/v1/users/#{@user.id}/job_applications/0"

expect(response).to_not be_successful
expect(response.status).to eq(404)

json = JSON.parse(response.body, symbolize_names: true)


expect(json[:message]).to eq("Job application not found")
expect(json[:status]).to eq(404)
end

it "returns error serializer if job application id belongs to another user" do
user_2 = User.create!(name: "Daniel Averdaniel", email: "[email protected]", password: "nuggetonnabiscut")

user_2_application = JobApplication.create!(
position_title: "Crank Operator",
date_applied: "2024-10-31",
status: 1,
notes: "Not sure im familiar with the tech-stack",
job_description: "You turn the big crank that powers google",
application_url: "www.example.com",
contact_information: "[email protected]",
company_id: @google.id,
user_id: user_2.id
)

get "/api/v1/users/#{@user.id}/job_applications/#{user_2_application.id}"

expect(response).to_not be_successful
expect(response.status).to eq(404)

json = JSON.parse(response.body, symbolize_names: true)

expect(json[:message]).to eq("Job application not found")
expect(json[:status]).to eq(404)
end

it "returns error serializer if no params are passed in URL for job application id" do
get "/api/v1/users/#{@user.id}/job_applications/"

expect(response).to_not be_successful
expect(response.status).to eq(400)

json = JSON.parse(response.body, symbolize_names: true)

expect(json[:message]).to eq("Job application ID is missing")
expect(json[:status]).to eq(400)
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for adding this test and being very through with your work. I like your error message here, as I feel it is very informative and user-friendly.

end
end
end