-
Notifications
You must be signed in to change notification settings - Fork 3
346 add independent jobs #347
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
base: develop
Are you sure you want to change the base?
Conversation
|
Your Render PR Server URL is https://nijobs-backend-pr-347.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-cn5up7vsc6pc739doo60. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #347 +/- ##
===========================================
+ Coverage 90.34% 90.37% +0.03%
===========================================
Files 54 54
Lines 1470 1475 +5
Branches 246 248 +2
===========================================
+ Hits 1328 1333 +5
Misses 137 137
Partials 5 5 ☔ View full report in Codecov by Sentry. |
|
Your Render PR Server URL is https://nijobs-backend-pr-347.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-cnaae6mct0pc73d9v080. |
6c46143 to
f6240d0
Compare
7ecca88 to
b3c7f8d
Compare
b3c7f8d to
2577513
Compare
e0d1712 to
55413c3
Compare
55413c3 to
02c7145
Compare
581408a to
525ba4d
Compare
dsantosferreira
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything you did seems really good. Left a small recommendation and I'm left with a doubt. To my understanding of the issue, shouldn't the backend raise an error when freelance offers are defined with a non-null jobMinDuration? I am open to discuss that.
src/models/Offer.js
Outdated
| if (this.jobType === "FREELANCE" && this.jobMinDuration === undefined) return true; | ||
| return value >= this.jobMinDuration; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition isn't consistent with the values used in the tests for the jobMinDuration field, as here the value "undefined" is used and "null" is used in the tests. Either change the value "undefined" to "null" or vice versa in the tests. I would recommend using "null". The tests only pass because of numeric type coersion (null is cast to 0 when using ">=").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing that out, I followed your suggestion and changed "undefined" to "null"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you suggested and as was discussed in the meeting I decided to make it impossible for freelance offers to have a min duration
0709570 to
0e62eb4
Compare
5f18af1 to
700be2a
Compare
I added the new type of jobs (Freelance) to the JobTypes I also made some params optional for instance the min and max duration might not make a lot of sense for these types of jobs that are goal oriented