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.

Sign up for GitHub

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

Jump to bottom

346 add independent jobs #347

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

346 add independent jobs #347

wants to merge 10 commits into from

Conversation

jose-carlos-sousa
Copy link

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

@jose-carlos-sousa jose-carlos-sousa linked an issue Feb 13, 2024 that may be closed by this pull request
Add independent jobs #346
Open
@render Render
Copy link

render bot commented Feb 13, 2024

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 Codecov
Copy link

codecov bot commented Feb 14, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.37%. Comparing base (82886d0) to head (525ba4d).

❗ Current head 525ba4d differs from pull request most recent head 700be2a. Consider uploading reports for the commit 700be2a to get more accurate results

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.
📢 Have feedback on the report? Share it here.

@render Render
Copy link

render bot commented Feb 20, 2024

Your Render PR Server URL is https://nijobs-backend-pr-347.onrender.com.

Follow its progress at https://dashboard.render.com/web/srv-cnaae6mct0pc73d9v080.

@jose-carlos-sousa jose-carlos-sousa self-assigned this Feb 25, 2024
@jose-carlos-sousa jose-carlos-sousa force-pushed the 346-add-independent-jobs branch 2 times, most recently from 6c46143 to f6240d0 Compare February 28, 2024 19:01
@jose-carlos-sousa jose-carlos-sousa force-pushed the 346-add-independent-jobs branch 3 times, most recently from 7ecca88 to b3c7f8d Compare March 5, 2024 22:26
dsantosferreira
dsantosferreira requested changes Apr 8, 2024
View reviewed changes
Copy link
Contributor

@dsantosferreira dsantosferreira left a 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
Comment on lines 141 to 142
if (this.jobType === "FREELANCE" && this.jobMinDuration === undefined) return true;
return value >= this.jobMinDuration;
Copy link
Contributor

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 ">=").

Copy link
Author

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"

Copy link
Author

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

@jose-carlos-sousa jose-carlos-sousa requested review from dsantosferreira and removed request for FranciscoCardoso913 April 9, 2024 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dsantosferreira dsantosferreira Awaiting requested review from dsantosferreira

Requested changes must be addressed to merge this pull request.

Assignees

@jose-carlos-sousa jose-carlos-sousa

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add independent jobs
2 participants
@jose-carlos-sousa @dsantosferreira