Add salary range #31
Add salary range #31
Conversation
|
Looks good! Any chance you could update the migration to set the lower/upper salary value to the same values based on the salary_range before dropping the column ? The original salary_range was to have a enum of ranges, as well, which may work. |
|
Totally. Before I do that, should I make it an enum of ranges or keep it as is? |
|
Ranges are fine as long as we have the migration for existing data. |
|
I ended up switching to an enum so that it follows the same pattern as experience range which I think is important for consistency. I also added a seed for test data. |
server/controllers/job.js
Outdated
| @@ -99,8 +98,7 @@ exports.update = async function(req, res, next) { | |||
| req | |||
| .assert("experience_range", "Please enter a valid experience_range") | |||
| .isInt(); | |||
| req.assert("lower_salary", "Please enter a valid lower_salary").isInt(); | |||
| req.assert("upper_salary", "Please enter a valid upper_salary").isInt(); | |||
| req.assert("salary_range", "Please enter a valid lower_salary").isInt(); | |||
There was a problem hiding this comment.
Should this read "Please enter a valid salary range."?
|
Great! Thanks. We may need to convert the existing jobs salary data (migration?) to use the new ENUM values. Any ideas? |
|
I think that is covered in the migration. The seeder that I added has a salary saved as a dollar value so you could run the seeder, then run the migrations to test. |
|
Argh, you are right! So sorry, I overlooked that file. Thank you for doing this! |
.env.exampleknexfile.js