feature group managed ola

[SebSept]

Group managed OLA

Checklist before requesting a review

Please delete options that are not relevant.

• I have read the CONTRIBUTING document.
• I have performed a self-review of my code.
• I have added tests that prove my fix is effective or that my feature works.
• This change requires a documentation update.

Description

• It fixes # (issue number, if applicable)
• Here is a brief description of what this PR does

Screenshots (if appropriate):

[cconard96]

You shouldn't need a new item instance in most cases. getFromDB replaces all of the fields. If some old data remains, it may be an indication of a missing post_getFromDB.
$item->getFromDB($item->getID);

[SebSept]

I was not sure, to I decided to create a new Object, this is to be sure that anything hidden remains.

[SebSept]

Remaining values inside the object is the problem, as you noticed.
In tests we can use the same object for convenience. But in source code, the object saving and loading don't happens at the same time. I don't think I introduced this kind of problem (no cache or static data implemented). But getting a fresh object is a way to be closer of real usage and avoid special cases.
It's more safe and we avoid having a test to pass because of remaining data (which is not the case at runtime, not in tests).

On the other hand, the code tend to use the same variable object to do different things. But that's a bad practice imo.

What do you think about it @cedric-anne ?

[PierreTeclib]

@SebSept :

Small issue. This field doesn’t seem relevant here:

Moreover, when a change is made and a date is displayed, the calendar icon on the right does nothing when clicked.

[SebSept]

this is a readonly datetime input, not the best option.

I just removed this field from display.