Fix: filler goal test - Githubissues

moaz-mokhtar commented 9 months ago

[x] Develop a function to get parent_goal for a child
[x] Passing parent_goal to functions responsible for adjusting start and deadline for generated activities which considered child for that parent_goal
[x] Develop a function to adjust activities for parent goals to make sure children deducting from min_duration. It is function adjust_parent_activities. Developed test case.
[x] Calling new function adjust_parent_activities from test templates.

moaz-mokhtar commented 8 months ago

This test is not passing - so I can't accept this PR. Also, this PR breaks the default_budgets test case.

Please notice there's an error in the expected. 'Plan a party' should be of duration 2h (4h -1h Buy stuff -1h Invite friends).

Test case filler_goals not passing because I didn't edit expected.json. Check difference between expected (left) and observed (right) it is about duration of Plan a party duration and remaining hours!

For the goal 'Plan a party', logically it is acceptable that duration is 4 hours based on below assumptions

minDuration is 4 hours
total duration is 6 hours 
total child duration: 2 hours
system deduct from total duration

I did test the filler_goal based on input said:

  {
    "startDate": "2022-01-01T00:00:00",
    "endDate": "2022-01-02T00:00:00",
    "goals": [
      {
        "id": "1",
        "title": "Plan a party",
        "minDuration": 2,
        "start": "2022-01-01T10:00:00",
        "deadline": "2022-01-01T18:00:00",
        "children": [
          "2",
          "3"
        ]
      },
...

Output is expected (left) and observed (right):

So does current behavior meets the requirements or we need to change this behavior to deduct from Goal.minDuration instead of Goal.total_duration?

tijlleenders commented 8 months ago

Do not change thr input. Only expected needs to be corrected as indicated in https://github.com/tijlleenders/ZinZen-scheduler/pull/449#pullrequestreview-1912670200

The total is not 6 - but always 4.

moaz-mokhtar commented 8 months ago

For default_budgets, I expected that it will fail and as mentioned in https://github.com/tijlleenders/ZinZen-scheduler/pull/434#issuecomment-1969268825 that default-budgets.expected.json is adjusted and not accurate which:

No Daily habits activities which should be
Linked childs in the Goal Daily habits is not accurate and will not have a childs

So what is your advise

moaz-mokhtar commented 8 months ago

Do not change thr input. Only expected needs to be corrected as indicated in #449 (review)

The total is not 6 - but always 4.

I didn't commit a change in input.json! I'm just clarifying. I will fix behavior based on this. Thanks

tijlleenders commented 8 months ago

For default_budgets, I expected that it will fail and as mentioned in https://github.com/tijlleenders/ZinZen-scheduler/pull/434#issuecomment-1969268825 that default-budgets.expected.json is adjusted and not accurate which:

No Daily habits activities which should be

Linked childs in the Goal Daily habits is not accurate and will not have a childs

So what is your advise

The PR can only be merged if all tests are passing. If you think default_budgets input and/or expected is not correct please make a commit correcting it - so that I know what you mean in code - not only in text.

moaz-mokhtar commented 8 months ago

For default_budgets, I expected that it will fail and as mentioned in #434 (comment) that default-budgets.expected.json is adjusted and not accurate which:

No Daily habits activities which should be

Linked childs in the Goal Daily habits is not accurate and will not have a childs

So what is your advise

The PR can only be merged if all tests are passing. If you think default_budgets input and/or expected is not correct please make a commit correcting it - so that I know what you mean in code - not only in text.

Noted, I will after fixing filler-goal

moaz-mokhtar commented 8 months ago

Now filler_goal fixed.

For the case default_budgets, we have a problem which children for goal Daily Habits is not valid, that is why it panics. That is the input.json

 "goals": [
    {
      "id": "44faf46c-73ad-4140-a5cb-d9a69f51859b",
      "title": "Daily habits 🔁",
      "createdAt": "2024-01-08T10:59:47.134Z",
      "children": [
        "breakfast",
        "lunch",
        "dinner",
        "meTime",
        "walk"
      ]
    },

For the children, you can find that no goal id called breakfast, lunch, dinner, meTme, or walk as below:

{
  "startDate": "2024-01-08T00:00:00",
  "endDate": "2024-01-15T00:00:00",
  "goals": [
    ...
    {
      "id": "77e1f762-3a4f-44a3-8f24-1560641a3548",
      "title": "Walk 🚶🏽",
      ...
    },
    {
      "id": "0eac2855-7fc3-4947-a560-81b5ebe88572",
      "title": "Hobby project 🚂🚋",
      ...
    },
    {
      "id": "a700170a-eb34-4162-a59a-3ce763f62205",
      "title": "House chores 🏡🧹🛠️",
      ...
    },
    {
      "id": "f9a02a6a-8ba9-43d0-b4aa-c6503f77306f",
      "title": "Family time 🥰",
      ...
    },
    {
      "id": "445f787b-d742-4441-9744-e81c286aa3c8",
      "title": "Me time 🧘🏽😌",
      ...
    },
    {
      "id": "678eab49-960e-4519-ad0b-031a2f22aaba",
      "title": "Work 💪🏽",
      ...
    },
    {
      "id": "49b05463-56a0-4af5-9034-83822abf24f6",
      "title": "Sleep 😴🌙",
      ...
    },
    {
      "id": "40842a7d-c282-406f-9cdf-3d1fbd8e4f61",
      "title": "Breakfast 🥐🥣",
      ...
    },
    {
      "id": "18be6978-ffac-46db-8b70-d321c311428a",
      "title": "Lunch 🥪",
      ...
    },
    {
      "id": "103b2eff-6ba5-47b5-ad4f-81d8e8ef5998",
      "title": "Dinner 🍽️",
      ...
    }
  ]
}

So in this case, we should change children into accurate goal_id

tijlleenders commented 8 months ago

Now filler_goal fixed.

For the case default_budgets, we have a problem which children for goal Daily Habits is not valid, that is why it panics. That is the input.json:

    {
      "id": "44faf46c-73ad-4140-a5cb-d9a69f51859b",
      "title": "Daily habits 🔁",
      "createdAt": "2024-01-08T10:59:47.134Z",
      "children": [
        "445f787b-d742-4441-9744-e81c286aa3c8",
        "f9a02a6a-8ba9-43d0-b4aa-c6503f77306f",
        "a700170a-eb34-4162-a59a-3ce763f62205",
        "0eac2855-7fc3-4947-a560-81b5ebe88572",
        "77e1f762-3a4f-44a3-8f24-1560641a3548",
        "49b05463-56a0-4af5-9034-83822abf24f6",
        "40842a7d-c282-406f-9cdf-3d1fbd8e4f61",
        "18be6978-ffac-46db-8b70-d321c311428a",
        "103b2eff-6ba5-47b5-ad4f-81d8e8ef5998"
      ]

Note I didn't put 'Work' in the 'Daily habits' - it's a personal choice.

Yes, I see that is wrong.
Please correct it in the default_budgets input.json and replace the current names with the correct UUIDs.

On the other hand, this should not change anything as the container goal 'Daily habits' wil not result in any Activities as it is not a Budget and also not a Simple Goal (it has no duration).

tijlleenders commented 8 months ago

Please also note the comment here : https://github.com/tijlleenders/ZinZen-scheduler/pull/447#pullrequestreview-1874663087

Input goals are/should be immutable.

moaz-mokhtar commented 8 months ago

Now filler_goal fixed. For the case default_budgets, we have a problem which children for goal Daily Habits is not valid, that is why it panics. That is the input.json:
    {
      "id": "44faf46c-73ad-4140-a5cb-d9a69f51859b",
      "title": "Daily habits 🔁",
      "createdAt": "2024-01-08T10:59:47.134Z",
      "children": [
        "445f787b-d742-4441-9744-e81c286aa3c8",
        "f9a02a6a-8ba9-43d0-b4aa-c6503f77306f",
        "a700170a-eb34-4162-a59a-3ce763f62205",
        "0eac2855-7fc3-4947-a560-81b5ebe88572",
        "77e1f762-3a4f-44a3-8f24-1560641a3548",
        "49b05463-56a0-4af5-9034-83822abf24f6",
        "40842a7d-c282-406f-9cdf-3d1fbd8e4f61",
        "18be6978-ffac-46db-8b70-d321c311428a",
        "103b2eff-6ba5-47b5-ad4f-81d8e8ef5998"
      ]
Note I didn't put 'Work' in the 'Daily habits' - it's a personal choice.

Yes, I see that is wrong. Please correct it in the default_budgets input.json and replace the current names with the correct UUIDs.

Should children contains all other 10 goals, or just the current list but with accurate UUID? Current goals are:
"children": [
"breakfast",
"lunch",
"dinner",
"meTime",
"walk"
]

moaz-mokhtar commented 8 months ago

Please also note the comment here : #447 (review)

Input goals are/should be immutable.

I think I'm following this. For function adjust_parent_activities , it is only pass reference to input.goals as below snippet:

let simple_goal_activities = adjust_parent_activities(&simple_goal_activities, &input.goals);

If you mean something different, kindly advise.

tijlleenders commented 8 months ago

Should children contains all other 10 goals, or just the current list but with accurate UUID? Current goals are:
"children": [
        "breakfast",
        "lunch",
        "dinner",
        "meTime",
        "walk"
      ]

You are right to question my code snippet. We can follow what is currently on the default goals list of the dev version:

So add Sleep too:

tijlleenders / ZinZen-scheduler

Fix: filler goal test #449