Add support for reading time phased planned work from MPP for manual scheduled tasks.

[Fabian-Schmidt]

Add support for reading time phased planned work from MPP for manual scheduled tasks. Manual scheduled tasks in Project permit starting an assignment outside of regular work calendar.

This PR addresses the issue reported in #733 .

I added a number of test cases to ensure the support is working correctly.

For manual scheduled tasks I identified the following rules:

• Only the first day of an assignment can have work outside of the work calendar.
• If assignment start is on a non working day then the generic work calendar, without any exceptions, is applied. Project starts looking for working hours used by searching work calendar on Tuesday.
• If assignment starts before work hours then work full until regular work range start and then use range.
• If assignment starts after work hours range then work full until end of day.
• If assignment starts on range end then work start on next regular work day.
• If assignment starts within work range then work normal.

On purpose I tried to gate the changes behind the flag taskIsManualScheduled to minimize the likelihood for regression.

Also I only implemented planned work as the size of the PR is quite large and I would rather get feedback as is.

[joniles]

Hello! Thank you for putting all the work in to create this pull request. As you indicate, this pull request is quite large and in particular also makes some significant changes in ProjectCalendar.

My suggestion to proceed would be to break the work down into a series of more "bite sized" changes. I think the key to this problem is how we handle the calendar calculations. My ultimate aim would be to ensure that we Task.getEffectiveCalendar() or ResourceAssignment.getEffectiveCalendar() is called for a manually scheduled task, we return an instance of a ManuallyScheduledTaskCalendar class, which performs the various calculations appropriately for a manually scheduled task. ManuallyScheduledTaskCalendar would wrap the underlying calendar, overriding the methods necessary to implement the calculations correctly for a manually scheduled task. This also has the advantage that any code using calendar calculations for a manually schedule task will see the correct results, not just the timephased code.

So... what to do first. Before we jump in and try and implement ManuallyScheduledTaskCalendar fully, my suggestion would be to create a class which just contains the methods we're interested in:

package net.sf.mpxj;

import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;

public class ManuallyScheduledTaskCalendar
{
   public ManuallyScheduledTaskCalendar(ProjectCalendar calendar)
   {
      m_calendar = calendar;
   }

   public LocalDateTime getDate(LocalDateTime date, Duration duration)
   {
      throw new UnsupportedOperationException();
   }

   public Duration getWork(LocalDateTime startDate, LocalDateTime endDate, TimeUnit format)
   {
      throw new UnsupportedOperationException();
   }

   public LocalTime getFinishTime(LocalDate date)
   {
      throw new UnsupportedOperationException();
   }

   public LocalDateTime getNextWorkStart(LocalDateTime date)
   {
      throw new UnsupportedOperationException();
   }

   private final ProjectCalendar m_calendar;
}

We can then work on implementing these methods, and writing tests for them based on the test data you've already produced.

The approach to the implementation should call the underlying calendar as much as possible to implement the calculation, rather than duplicating code. For example: we want to call getWork for a manually scheduled 7 day task, for which we know the start day and end day need special handling (as the times can be outside of normal working time). We should split the problem into three parts: start day, end day, remaining time range. For the remaining time range we can just ask the underlying calendar to perform the calculation, then we need to handle the start day and end day ourselves in ManuallyScheduledTaskCalendar and return the combined result to the caller.

I hope my description of the approach makes sense. If we build up the functionality we need in small pieces like this we can test and merge our changes easily before we move on to the next part of the problem.

[Fabian-Schmidt]

Hi thanks for the feedback. Took me a moment to think about the next steps.

I understand the approach to have getEffectiveCalendar return the correct calendar and encapsulate the logic.

There are two problems with this approach:

1. Normally ProjectCalendar class is returned. So the ManuallyScheduledTaskCalendar must extend ProjectCalendar to work. But then this calendar must overwrite all ProjectCalendar methods and redirect them to the base calendar. This adds a lot of boilerplate code. Also the overwritten class cannot access the private methods of the base calendar to maximize code reuse.
2. For Manual scheduled task the behavior of the calendar depends on the Assignment. The current ProjectCalendar architecture does not support this concept. It assumes the Calendar will behave the same regardless of the Assignment. For a manual scheduled task it depends when the assignment start what the calendar data will returned.

Thanks

[joniles]

Thanks for your notes.

Normally ProjectCalendar class is returned. So the ManuallyScheduledTaskCalendar must extend ProjectCalendar to work. But then this calendar must overwrite all ProjectCalendar methods and redirect them to the base calendar. This adds a lot of boilerplate code.

My main concern at the this stage is getting the functionality of the four main methods in the calendar class working and tested in isolation, I'm less concerned about how the final class looks. I am happy to manage the boilerplate code, or provide a WrapperCalendar class which delegates calls for all methods to a wrapped calendar class, which can then be subclassed to implement just the methods we are interested in.

Also the overwritten class cannot access the private methods of the base calendar to maximize code reuse.

There are potentially a couple of approaches to this depending on how the final implementation looks, including provision of protected methods or factoring out common functionality into separate helper classes to facilitate reuse. We can deal with that as we move forward.

For Manual scheduled task the behavior of the calendar depends on the Assignment. The current ProjectCalendar architecture does not support this concept. It assumes the Calendar will behave the same regardless of the Assignment. For a manual scheduled task it depends when the assignment start what the calendar data will returned.

That shouldn't be a problem. It sounds like the only method which would return our new calendar class would be ResourceAssignment.getEffectiveCalendar(). The new class might then look something like this, including the resource assignment as an argument to the constructor.

public class ManuallyScheduledTaskCalendar
{
   public ManuallyScheduledTaskCalendar(ProjectCalendar calendar, ResourceAssignment assignment)
   {
      m_calendar = calendar;
      m_assignment = assignment;
   }

   // ... implementation detail ...

   private final ProjectCalendar m_calendar;
   private final ResourceAssignment m_assignment;
}

[Fabian-Schmidt]

Okay. My understanding of the next step is as follow:

I will create a new branch with the proposed stub class and add the unit test as failing tests. For this new branch I will open a new PR and we will use this to slowly flush out the implementation? I will mark this PR as Draft and later close it.

Is my understanding correct?

[joniles]

Is my understanding correct?

Sounds great - excited to see this take shape!