Work period automation #345

eisbilir · 2021-06-14T00:08:21Z

No description provided.

maxceem

@eisbilir here are some issues I found so far:

If we have fisrt/last WP with daysWorked < 5, and we extend RB time so it adds one more week, we have to recalculate daysWorked for the first/last WP which is not last anymore to 5.

Example: We have RB:
```
 "startDate": "2021-02-03",
 "endDate": "2021-02-17"     
```
The last WP is:
```
"startDate": "2021-02-14",
"endDate": "2021-02-20",
"daysWorked": 3,
```
Now we extend end of RB to "2021-02-23"

Now the the WP which was last before "startDate": "2021-02-14", "endDate": "2021-02-20", hast to have "daysWorked": 5
You've added new WP.paymentStatus noDays. It's good, but let's name it as no-days because we use dash in all other statuses for consistency.
You've implemented logic to recalculate fields in WP when we update WPP in file src/eventHandlers/WorkPeriodPaymentEventHandler.js. But this logic also depends on WP.daysWorked. So we have to execute the same logic if WP.daysWorked is changed.

⏳ I'm still reviewing the PR, and would update this review if find something else...

src/eventHandlers/ResourceBookingEventHandler.js

src/services/WorkPeriodPaymentService.js

eisbilir · 2021-06-14T18:03:36Z

@maxceem

If we have fisrt/last WP with daysWorked < 5, and we extend RB time so it adds one more week, we have to recalculate daysWorked for the first/last WP which is not last anymore to 5.

I tried several times and it works fine for me. Could you please retry. The first week of the example you show has also 3 daysWorked, maybe you thought that it was the last week.

You've implemented logic to recalculate fields in WP when we update WPP in file src/eventHandlers/WorkPeriodPaymentEventHandler.js. But this logic also depends on WP.daysWorked. So we have to execute the same logic if WP.daysWorked is changed.

Yes. There are 3 areas that causes WP changes:

RB changes: we handle that in ResourceBookingEventHandler via WP services.
WPP changes: we handle that in WorkPeriodPaymentEventHandler via direct DB and ES (post event) calls.
by calling WP update service.

1 and 3 resolves in WP services so I execute the update logic in there https://github.com/topcoder-platform/taas-apis/pull/345/files#diff-f5f86437db6a63effab82db4749996d32166a41a096fc487c3fc805ad823ab4eR282-R290
2 has different logic than 3, because 2 evaluates WPP.status, WP.daysWorked, WP.daysPaid after a WPP change. but 3 deals with the subject in terms of the change of WP.daysWorked.

maxceem · 2021-06-15T08:54:23Z

I tried several times and it works fine for me. Could you please retry. The first week of the example you show has also 3 daysWorked, maybe you thought that it was the last week.

Sorry, looks like I overlooked something. It works good for me to now, below my verification steps. I keep them just for history, not need to go through them.

Create RB:

  "startDate": "2021-02-09",
  "endDate": "2021-02-23",

Work Periods:

"workPeriods": [
            {
                "endDate": "2021-02-13",
                "daysWorked": 4,
                "id": "62849337-ac4e-42be-9dcf-4e317b1059ec",
                "startDate": "2021-02-07"
            },
            {
                "endDate": "2021-02-20",
                "daysWorked": 5,
                "id": "0d560869-c8ac-49bc-88f6-fe7e2cc329fa",
                "startDate": "2021-02-14"
            },
            {
                "endDate": "2021-02-27",
                "daysWorked": 2,
                "id": "921099bf-78a3-4b59-9fe0-9a106f355cc4",
                "startDate": "2021-02-21"
            }
        ],

Update WP 0d560869-c8ac-49bc-88f6-fe7e2cc329fa to have daysWorked: 3

"workPeriods": [
            {
                "endDate": "2021-02-13",
                "daysWorked": 4,
                "id": "62849337-ac4e-42be-9dcf-4e317b1059ec",
                "startDate": "2021-02-07"
            },
            {
                "endDate": "2021-02-20",
                "daysWorked": 3,
                "id": "0d560869-c8ac-49bc-88f6-fe7e2cc329fa",
                "startDate": "2021-02-14"
            },
            {
                "endDate": "2021-02-27",
                "daysWorked": 2,
                "id": "921099bf-78a3-4b59-9fe0-9a106f355cc4",
                "startDate": "2021-02-21"
            }
        ],

Update RB endDate to 2021-03-01:

"workPeriods": [
            {
                "endDate": "2021-02-13",
                "daysWorked": 4,
                "id": "62849337-ac4e-42be-9dcf-4e317b1059ec",
                "startDate": "2021-02-07"
            },
            {
                "endDate": "2021-02-20",
                "daysWorked": 3,
                "id": "0d560869-c8ac-49bc-88f6-fe7e2cc329fa",
                "startDate": "2021-02-14"
            },
            {
                "endDate": "2021-02-27",
                "daysWorked": 5,
                "id": "921099bf-78a3-4b59-9fe0-9a106f355cc4",
                "startDate": "2021-02-21"
            },
            {
                "endDate": "2021-03-06",
                "daysWorked": 1,
                "id": "2c151d25-097b-42c2-a26a-11dbe1f23c69",
                "startDate": "2021-02-28"
            }
        ],

Update RB startDate to 2021-01-25:

"workPeriods": [
            {
                "endDate": "2021-01-30",
                "daysWorked": 5,
                "id": "f9a180f9-1a6c-412d-bd2c-c737eceb64d1",
                "startDate": "2021-01-24"
            },
            {
                "endDate": "2021-02-06",
                "daysWorked": 5,
                "id": "02c40bc8-6ce7-4277-9664-d99ad2161ed8",
                "startDate": "2021-01-31"
            },
            {
                "endDate": "2021-02-13",
                "daysWorked": 5,
                "id": "62849337-ac4e-42be-9dcf-4e317b1059ec",
                "startDate": "2021-02-07"
            },
            {
                "endDate": "2021-02-20",
                "daysWorked": 3,
                "id": "0d560869-c8ac-49bc-88f6-fe7e2cc329fa",
                "startDate": "2021-02-14"
            },
            {
                "endDate": "2021-02-27",
                "daysWorked": 5,
                "id": "921099bf-78a3-4b59-9fe0-9a106f355cc4",
                "startDate": "2021-02-21"
            },
            {
                "endDate": "2021-03-06",
                "daysWorked": 1,
                "id": "2c151d25-097b-42c2-a26a-11dbe1f23c69",
                "startDate": "2021-02-28"
            }
        ]

Update RB endDate to 2021-03-04:

"workPeriods": [
            {
                "endDate": "2021-01-30",
                "daysWorked": 5,
                "id": "f9a180f9-1a6c-412d-bd2c-c737eceb64d1",
                "startDate": "2021-01-24"
            },
            {
                "endDate": "2021-02-06",
                "daysWorked": 5,
                "id": "02c40bc8-6ce7-4277-9664-d99ad2161ed8",
                "startDate": "2021-01-31"
            },
            {
                "endDate": "2021-02-13",
                "daysWorked": 5,
                "id": "62849337-ac4e-42be-9dcf-4e317b1059ec",
                "startDate": "2021-02-07"
            },
            {
                "endDate": "2021-02-20",
                "daysWorked": 3,
                "id": "0d560869-c8ac-49bc-88f6-fe7e2cc329fa",
                "startDate": "2021-02-14"
            },
            {
                "endDate": "2021-02-27",
                "daysWorked": 5,
                "id": "921099bf-78a3-4b59-9fe0-9a106f355cc4",
                "startDate": "2021-02-21"
            },
            {
                "endDate": "2021-03-06",
                "daysWorked": 4,
                "id": "2c151d25-097b-42c2-a26a-11dbe1f23c69",
                "startDate": "2021-02-28"
            }
        ]

Update RB endDate to 2021-03-02:

"workPeriods": [
            {
                "endDate": "2021-01-30",
                "daysWorked": 5,
                "id": "f9a180f9-1a6c-412d-bd2c-c737eceb64d1",
                "startDate": "2021-01-24"
            },
            {
                "endDate": "2021-02-06",
                "daysWorked": 5,
                "id": "02c40bc8-6ce7-4277-9664-d99ad2161ed8",
                "startDate": "2021-01-31"
            },
            {
                "endDate": "2021-02-13",
                "daysWorked": 5,
                "id": "62849337-ac4e-42be-9dcf-4e317b1059ec",
                "startDate": "2021-02-07"
            },
            {
                "endDate": "2021-02-20",
                "daysWorked": 3,
                "id": "0d560869-c8ac-49bc-88f6-fe7e2cc329fa",
                "startDate": "2021-02-14"
            },
            {
                "endDate": "2021-02-27",
                "daysWorked": 5,
                "id": "921099bf-78a3-4b59-9fe0-9a106f355cc4",
                "startDate": "2021-02-21"
            },
            {
                "endDate": "2021-03-06",
                "daysWorked": 2,
                "id": "2c151d25-097b-42c2-a26a-11dbe1f23c69",
                "startDate": "2021-02-28"
            }
        ]

Update RB endDate to 2021-02-18

"workPeriods": [
            {
                "endDate": "2021-01-30",
                "daysWorked": 5,
                "id": "f9a180f9-1a6c-412d-bd2c-c737eceb64d1",
                "startDate": "2021-01-24"
            },
            {
                "endDate": "2021-02-06",
                "daysWorked": 5,
                "id": "02c40bc8-6ce7-4277-9664-d99ad2161ed8",
                "startDate": "2021-01-31"
            },
            {
                "endDate": "2021-02-13",
                "daysWorked": 5,
                "id": "62849337-ac4e-42be-9dcf-4e317b1059ec",
                "startDate": "2021-02-07"
            },
            {
                "endDate": "2021-02-20",
                "daysWorked": 3,
                "id": "0d560869-c8ac-49bc-88f6-fe7e2cc329fa",
                "startDate": "2021-02-14"
            }
        ]

Update WP days Worked to 3 for "WP.id": "02c40bc8-6ce7-4277-9664-d99ad2161ed8":

"workPeriods": [
            {
                "endDate": "2021-01-30",
                "daysWorked": 5,
                "id": "f9a180f9-1a6c-412d-bd2c-c737eceb64d1",
                "startDate": "2021-01-24"
            },
            {
                "endDate": "2021-02-06",
                "daysWorked": 3,
                "id": "02c40bc8-6ce7-4277-9664-d99ad2161ed8",
                "startDate": "2021-01-31"
            },
            {
                "endDate": "2021-02-13",
                "daysWorked": 5,
                "id": "62849337-ac4e-42be-9dcf-4e317b1059ec",
                "startDate": "2021-02-07"
            },
            {
                "endDate": "2021-02-20",
                "daysWorked": 3,
                "id": "0d560869-c8ac-49bc-88f6-fe7e2cc329fa",
                "startDate": "2021-02-14"
            }
        ]

Update RB endDate to 2021-02-02

"workPeriods": [
            {
                "endDate": "2021-01-30",
                "daysWorked": 5,
                "id": "f9a180f9-1a6c-412d-bd2c-c737eceb64d1",
                "startDate": "2021-01-24"
            },
            {
                "endDate": "2021-02-06",
                "daysWorked": 2,
                "id": "02c40bc8-6ce7-4277-9664-d99ad2161ed8",
                "startDate": "2021-01-31"
            }
        ]

Update RB "startDate": "2021-02-01":

"workPeriods": [
            {
                "endDate": "2021-02-06",
                "daysWorked": 2,
                "id": "02c40bc8-6ce7-4277-9664-d99ad2161ed8",
                "startDate": "2021-01-31"
            }
        ],

maxceem · 2021-06-15T14:07:04Z

in general there is one thing which I'd really like to improve - the logic to calculate paymentStatus
at the moment it's hard to understand the logic while reading the code
also, there is 2 palaces with different logic:

inside event handler
inside WP update endpoint

What would be great to have is some kind of method:

calculateWorkPeriodPaymentStatus(WP) which would get all payments and calculate status, and it would be reused in both places
it's fine if we load payments in the WP endpoint, it would be better to loose a bit of performance but make the code robust

and the second thing, logic with if is not super clear too, it's hard to understand it when reading:

if (workPeriod.daysWorked === 0) {
    data.paymentStatus = PaymentStatus.NO_DAYS
  } else if (paymentStatuses[WorkPeriodPaymentStatus.SCHEDULED] || paymentStatuses[WorkPeriodPaymentStatus.IN_PROGRESS]) {
    data.paymentStatus = PaymentStatus.IN_PROGRESS
  } else if (workPeriod.daysWorked === data.daysPaid) {
    data.paymentStatus = PaymentStatus.COMPLETED
  } else if (paymentStatuses[WorkPeriodPaymentStatus.FAILED]) {
    data.paymentStatus = PaymentStatus.FAILED
  } else if (paymentStatuses[WorkPeriodPaymentStatus.COMPLETED]) {
    data.paymentStatus = PaymentStatus.PARTIALLY_COMPLETED
  } else {
    data.paymentStatus = PaymentStatus.PENDING
  }

I'm thinking about some kind of declarative way, something like this:

// define rules for the payment status as constant
const PAYMENT_STATUS_RULES = [
    { paymentStatus: PaymentStatus.NO_DAYS, condition: { daysWorked: 0 } },
    { paymentStatus: PaymentStatus.IN_PROGRESS, condition: { hasWorkPeriodPaymentStatus: [WorkPeriodPaymentStatus.SCHEDULED, WorkPeriodPaymentStatus.IN_PROGRESS] } },
    { paymentStatus: PaymentStatus.COMPLETED, condition: { hasWorkPeriodPaymentStatus: [WorkPeriodPaymentStatus.COMPLETED], hasDueDays: false } },
    { paymentStatus: PaymentStatus.PARTIALLY_COMPLETED, condition: { hasWorkPeriodPaymentStatus: [WorkPeriodPaymentStatus.COMPLETED], hasDueDays: true } },
    { paymentStatus: PaymentStatus.FAILED, condition: { hasWorkPeriodPaymentStatus: [PaymentStatus.FAILED], hasDueDays: true } },
    { paymentStatus: PaymentStatus.PENDING, condition: { hasDueDays: true } }
  ]
// find the first rule which is matched by the Work Period
 for (let rule of PAYMENT_STATUS_RULES) {
    if (matchRule(workPeriod, rule)) {
      data.paymentStatus = rule.paymentStatus
    }
  }
// so have to implement method `matchRule` which would check if rule is matched or no
// possible conditions:
// - daysWorked - how many days worked
// - hasWorkPeriodPaymentStatus - check if any of the payments has any of these statuses
// - hasDueDays - true if daysWorked > daysPaid

Also, I see in _ensurePaidWorkPeriodsNotDeleted we are checking if we can delete WP or no. In this and other places instead of checking paymentStatus, let's simplify this logic and check daysPaid so daysPiad > 0 then we cannot delete such WP, and no need to check exact status

maxceem · 2021-06-15T15:20:47Z

@eisbilir I think everything good. Just would like to make code improvements mentioned above #345 (comment).

src/eventHandlers/ResourceBookingEventHandler.js

maxceem · 2021-06-16T07:09:41Z

src/services/WorkPeriodService.js

+  data.paymentStatus = helper.calculateWorkPeriodPaymentStatus(_.assign({}, oldValue, data))
+  if (oldValue.paymentStatus === data.paymentStatus) {
+    return oldValue
+  }


@eisbilir if paymentStatus is not changed this logic would not update WP at all. But we can also update other values, like daysWorked, daysPaid and so on. Would not this cause issues?

looks like it is not updated if I change daysWorked

@eisbilir can we avoid all such places?

It feels like it would be better just update the records even though data is not changed. Would it break anything if I just remove such places?

maxceem · 2021-06-16T07:37:17Z

If I process a payment and then update WP, then there is error in ES processor:

eisbilir added 5 commits June 10, 2021 21:31

set RB dates to null logic

4064e5e

calculate daysWorked

30c627a

Merge remote-tracking branch 'upstream/dev' into dev

90d0a00

daysWorked constraint

01c6595

wp-wpp update and automation

8bb17e5

eisbilir requested a review from maxceem June 14, 2021 00:08

maxceem suggested changes Jun 14, 2021

View reviewed changes

maxceem reviewed Jun 14, 2021

View reviewed changes

src/eventHandlers/ResourceBookingEventHandler.js Outdated Show resolved Hide resolved

maxceem reviewed Jun 14, 2021

View reviewed changes

src/services/WorkPeriodPaymentService.js Show resolved Hide resolved

use paymentStatus constats

9db3798

eisbilir force-pushed the work-period-automation branch from 233ae68 to 9db3798 Compare June 14, 2021 20:59

Merge branch 'dev' into work-period-automation

8096c4d

eisbilir requested a review from maxceem June 14, 2021 21:11

update postman

d8bcc78

maxceem reviewed Jun 15, 2021

View reviewed changes

src/eventHandlers/ResourceBookingEventHandler.js Outdated Show resolved Hide resolved

eisbilir and others added 2 commits June 16, 2021 01:32

payment status calculation

04af7f1

Merge branch 'dev' into work-period-automation

b82f8c3

maxceem reviewed Jun 16, 2021

View reviewed changes

fix: workPeriod update

e68e4fc

maxceem changed the base branch from dev to feature/work-period-automation June 16, 2021 07:58

maxceem merged commit 47d188e into topcoder-platform:feature/work-period-automation Jun 16, 2021

eisbilir deleted the work-period-automation branch July 31, 2021 21:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Work period automation #345

Work period automation #345

Uh oh!

eisbilir commented Jun 14, 2021

Uh oh!

maxceem left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

eisbilir commented Jun 14, 2021 •

edited

Loading

Uh oh!

maxceem commented Jun 15, 2021

Uh oh!

maxceem commented Jun 15, 2021

Uh oh!

maxceem commented Jun 15, 2021

Uh oh!

Uh oh!

maxceem Jun 16, 2021

Uh oh!

maxceem Jun 16, 2021

Uh oh!

maxceem Jun 16, 2021

Uh oh!

maxceem commented Jun 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Work period automation #345

Work period automation #345

Uh oh!

Conversation

eisbilir commented Jun 14, 2021

Uh oh!

maxceem left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

eisbilir commented Jun 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maxceem commented Jun 15, 2021

Uh oh!

maxceem commented Jun 15, 2021

Uh oh!

maxceem commented Jun 15, 2021

Uh oh!

Uh oh!

maxceem Jun 16, 2021

Choose a reason for hiding this comment

Uh oh!

maxceem Jun 16, 2021

Choose a reason for hiding this comment

Uh oh!

maxceem Jun 16, 2021

Choose a reason for hiding this comment

Uh oh!

maxceem commented Jun 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

maxceem left a comment •

edited

Loading

eisbilir commented Jun 14, 2021 •

edited

Loading