Skip to content

Fix resume uploads, improve job application / resume upload experience. WIP-327 #171

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

Merged
merged 2 commits into from
Aug 13, 2014

Conversation

Mileshosky
Copy link
Contributor

Addresses issue in: https://assembly.com/coderwall/wips/327

Fixed and improved experience for uploading resumes and applying to a job.

Before Applying

team_teampro___coderwall_com

Upload your resume

apply-upload

Upload in progress

apply-uploading

Uploaded successfully

apply-uploaded

Application submitted - flash message

apply-success

Resume already submitted

apply-prev

@@ -415,6 +415,8 @@
get "/#{provider}/:username" => 'users#show', :provider => provider
end

resources :resume_uploads, only: [:create]
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need a whole resource for that. please user a member action under users .

@Mileshosky
Copy link
Contributor Author

I added the odt and txt formats for Resume uploads, adjusted the user controller update to not raise an exception, and created a basic routing spec for the /resume_uploads action.

Feel free to merge this morning as you are comfortable, to get the fix live. I don't have time for a controller spec this am, but will be happy to add it this eve / when I work on the other cleanup and refactors.

My preference is to keep the separate ResumeUploadsController. We don't need to overload the Users controller with custom actions every time we want to do something like this. This is a single action purpose driven controller.

@seuros
Copy link
Contributor

seuros commented Aug 13, 2014

Thank you.
We can use a controller concern in this case.

The link will be post('users/resume')

@Mileshosky
Copy link
Contributor Author

@seuros, I still wouldn't consider using concerns in place of having a dedicated controller and action. Uploading a resume is not a concern (like session management helpers), it's a first class action and concept that should be given its own controller (and conceptual) space.

@seuros
Copy link
Contributor

seuros commented Aug 13, 2014

I don't think it need a whole controller for now since you using just 1 action anyway , an action will be fine.

If you want to use a controller, then nest it under Users module, and have /users/resume respond to (get, post, delete) => (show, create, destroy)

@Mileshosky
Copy link
Contributor Author

I'm open to going that route in the next round of changes / refactors. This works for the moment, and was intended to get a critical fix live - I don't see anything wrong with this structure for our immediate needs.

I appreciate the direction and review seuros

@just3ws
Copy link
Contributor

just3ws commented Aug 13, 2014

I like the latter approach. Hybrid, better routing with less crowded controller.

@seuros
Copy link
Contributor

seuros commented Aug 13, 2014

@Mileshosky
Thank you for your work.
I'm merging this for now, and we will test it in prod. Waiting for the others tests so we can close the bounty

seuros added a commit that referenced this pull request Aug 13, 2014
Fix resume uploads, improve job application / resume upload experience. WIP-327
@seuros seuros merged commit 872f17f into coderwall:master Aug 13, 2014
@seuros
Copy link
Contributor

seuros commented Aug 13, 2014

While i'm thinking about it, please remove the image from the uploads as well . Nobody should give an image as CV. We need to be able to analyze the text from it and possibly in the future convert it to different format.
Also recruiters are expert in ctrl+f to find their keywords and image don't provide that.

@Mileshosky
Copy link
Contributor Author

Thanks guys.

@just3ws - can you clarify 'latter' approach. I think you're meaning the controller nested under users module that seuros mentioned. Just want to clarify.

@seuros, I'll pull out the image formats from ResumeUploader next.

Thanks guys!

@just3ws
Copy link
Contributor

just3ws commented Aug 14, 2014

Yes

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.

3 participants