Skip to content

Conversation

@eisbilir
Copy link
Member

No description provided.

Copy link
Contributor

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

@eisbilir it works good.

Though I'm not sure about changes for Resource Bookings logic, could you please check the comments below?

if (thisWeek.daysWorked < data.daysWorked) {
throw new errors.BadRequestError(`Maximum allowed daysWorked is (${thisWeek.daysWorked})`)
}
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

As we allow up to 10 and don't care about RB.endDate anymore, let's remove this code.

throw new errors.ConflictError(`Cannot make maximum daysWorked (${newWP.daysWorked}) to the value less than daysPaid (${wp.daysPaid}) for WorkPeriod: ${wp.id}`)
}
})
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why this check is not valid anymore?
It looks like if we allow maximum 10 working days, we still should not allow this value to be less than daysPaid.

Copy link
Member Author

Choose a reason for hiding this comment

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

this checks if daysPaid exceeds maximum possible daysWorked.
maximum possible daysWorked is calculated regarding the startDay and endDate.
this check won't let us update any RB which has a WP has more paid daysWorked than expected (1-5).
So I moved the part of the logic we need to eventHandler, I don't allow to lower daysWorked less then daysPaid regardless of dates here:
https://github.com/eisbilir/taas-apis/blob/cb194d63722be0d8b6e1b040172c3236cfeadcd0/src/eventHandlers/ResourceBookingEventHandler.js#L192
https://github.com/eisbilir/taas-apis/blob/cb194d63722be0d8b6e1b040172c3236cfeadcd0/src/eventHandlers/ResourceBookingEventHandler.js#L197
https://github.com/eisbilir/taas-apis/blob/cb194d63722be0d8b6e1b040172c3236cfeadcd0/src/eventHandlers/ResourceBookingEventHandler.js#L207
https://github.com/eisbilir/taas-apis/blob/cb194d63722be0d8b6e1b040172c3236cfeadcd0/src/eventHandlers/ResourceBookingEventHandler.js#L212

Example 1
WP-1 has 4 daysWorked as maximum.
daysWorked was increased to 7.
payment was processed and daysPaid increased to 2.
RB dates updated and it forced WP-1 to have 3 daysWorked.
I will allow both RB dates and WP-1 daysWorked (3) to be updated.

Example 2
WP-1 has 4 daysWorked as maximum.
daysWorked was increased to 7.
payment was processed and daysPaid increased to 6.
RB dates updated and it forced WP-1 to have 3 daysWorked.
I will allow RB dates to be updated but WP-1 will keep it daysWorked at 7, same as before update.

WP-1 refers to 1st week.
we use recalculating of daysWorked logic for only the first and the last WP.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, got it.

Copy link
Contributor

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

@eisbilir works good as per local testing after updating ES.

@maxceem maxceem merged commit 0174cf7 into topcoder-platform:dev Jul 31, 2021
@eisbilir eisbilir deleted the update/10-working-days branch July 31, 2021 21:12
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.

2 participants