PS : i really don't understand all this code … https://github.com/LimeSurvey/LimeSurvey/blob/78a2949a70c0f98bb53882ccfd21a7faa27501ca/application/models/SurveysGroupsettings.php#L311

We end by survey : https://github.com/LimeSurvey/LimeSurvey/blob/78a2949a70c0f98bb53882ccfd21a7faa27501ca/application/models/SurveysGroupsettings.php#L362

But we need to start by survey, and not end by …
Why we don't have such issue before ?

@tibor.pacalat : my solution here is a big rewrite of the system.

I'm still unsure on the desired behavior …

Opinion:

Survey->datestamp must return Y/N or I
Survey->otions->datestamp must return only Y/N

?

@gabrieljenik : do you have an opinion here ?

Plugin can need to read if settings are inherited or not

In Survey settings part OK for

• Survey->datestamp must return (Y|N|I)
• Survey->options->datestamp must return only (Y|N) (only get parent if Survey->datestamp == "I")
• Survey->getIsDateStamp must use options->datestamp

In Survey and all other controller : Survey->datestamp = Survey->options->datestamp

Survey->datestamp must return (Y|N|I)

Agree. That is the survey data field.

Survey->options->datestamp must return only (Y|N) (only get parent if Survey->datestamp == "I")

I guess you talk about oOptions.
Agree. This is the value from the survey merged with the one inherited from the survey group hirearchy.
The value inheritance decision is executed here I think (from quick search) SurveysGroupsettings::shouldInherit()

Survey->getIsDateStamp must use options->datestamp

Agree.

In Survey and all other controller : Survey->datestamp = Survey->options->datestamp

Not sure I follow that statement.
datestamp and options->datestamp shouldn't be accessed much from outside Survey model.

Still, if the datestamp is Inherited, could end up Survey->datestamp <> Survey->options->datestamp

In Survey and all other controller : Survey->datestamp = Survey->options->datestamp

Not sure I follow that statement.
datestamp and options->datestamp shouldn't be accessed much from outside Survey model.

It's the current situation : when you check Survey->datestamp in beforeSurveyPage or newExport for example : you get Survey->oOptions->datestamp . Updating this : we broke a lot of plugins !
Then : i's for don't update this situation.

Only update for beforeSurveySettings and newSurveySettings (seems the only situation where we have this issue) or when controller are SurveyAdministrationController

The upadte was here : https://github.com/LimeSurvey/LimeSurvey/blob/ec5825c1ce2c5d953b1f634e2e57e837c9df2c12/application/controllers/SurveyAdministrationController.php#L1923

From what I read, I think the issue could be in 2 places maybe:

1) Inheritance Mechanism in SurveysGroupsettings::getInstance

2) The line you commented (https://github.com/LimeSurvey/LimeSurvey/blob/ec5825c1ce2c5d953b1f634e2e57e837c9df2c12/application/controllers/SurveyAdministrationController.php#L1923) maybe fecthing the instance from the cache and then setting the options for the instance in the cache, whereas in other places options should be set from inheritance.

Will debug and be back

Inheritance Mechanism in SurveysGroupsettings::getInstance

This one is very strange ....

I see several problems.

1) One of the problems is that the survey is cached.
When calling setOptionsFromDB from the actionRendersidemenulink, the survey model acquires a special configuration (used to present inherited values). Then, since the retrieval of the model was performed with findByPk, that instance is cached. Once cached, other scripts use it without expecting this special configuration.

2) Another problem is that, when using setOptionsFromDatabase(), you are using the same attribute and mixing inherited values ​​with final values.

setOptionsFromDatabase, which has a very confusing name, sets the value that would correspond to each option if inherited in oOptions (and oOptionsLabels and aOptions). This is basically done to be able to display in the UI which option would be inherited.

I think things should be separate: The final value should always be in oOptions, and values ​​that would be inherited if an attribute were "inherited" should have separate properties. Something like "parentOptions" and "parentOptionLabels."

Then, should review all the places where oOptions (or others) are currently used, as we need to be sure whether we expect to use the inherited value or the final value.

At last, we should also remove setOptionsFromDB() and review other places where that method is used, such as the bShowRealValues ​​attribute. This includes APIs or tests.

This is an initial branch, where we are starting to separate the use of oOptions:
https://github.com/LimeSurvey/LimeSurvey/tree/bug/19778--getIsDateStamp-return-false-with-date-stamped-survey

I look forward to your comments.

Wow ! You make it really complex !!!

I really don't like parentOptions : parent is group (and grand parent Global), then more clear name can be dbvalue ?

Another idea is (for datestamp), where group is Y and survey is I

• Survey->datestamp : Y
• Survey->options->datestamp : Y
• Survey->getIsDateStamp : true
• Survey->dbvalue->datestamp : I

I think it's a really bad idea to touch to Survey->datestamp, Survey->options->datestamp and Survey->getIsDateStamp in all other page (for example in Survey Runtime)

At last, we should also remove setOptionsFromDB() and review other places where that method is used, such as the bShowRealValues ​​attribute. This includes APIs or tests.

Yep, I think we already have some report about remote control : no way to get DB value OR do not have current values.

If the issue are no way to get DB value : it's great, because we fix issue without breaking API version.

PS : when assign it to yoiu, it's not to be done by you, but to have advice. Since you start : i don't take it again

I really don't like parentOptions : parent is group (and grand parent Global), then more clear name can be dbvalue ?

I agree the name we set parentEffectiveOptions needs reviewal. Maybe inheritedOptions?

Another idea is (for datestamp), where group is Y and survey is I

I am not sure this will solve the real problem.
The issue is that on some situations we are setting inherited values in some attributes whereas the final values are expected.
Creating new attributes for the model value could be usefull for some cases, but not sure it will solve the situations.

I think it's a really bad idea to touch to Survey->datestamp, Survey->options->datestamp and Survey->getIsDateStamp in all other page (for example in Survey Runtime)

Not aiming to do so. The idea is to review those pieces of code and make sure they are using the expected value.
On runtime, as setOptionsFromDB is not called, I don't think there is an issue.

Yep, I think we already have some report about remote control : no way to get DB value OR do not have current values.

In those cases, a modelValues attribute could be usefull.

@c_schmitz or someone else... any comment?
How are these values and setOptions handled from the API?

Would like some input there before moving forward.

Thanks