Skip to content

Commit

Permalink
The source of a Dropdown should be fetched before its value is set wh…
Browse files Browse the repository at this point in the history
…en reading a JASP file

Fixes jasp-stats/jasp-test-release#1780
This solves 2 problems:
. a dropdown with a source is not set properly when a JASP file is read: if the dropdown was set with a value coming from the source, when reading the JASP file, if the values of the source are not yet read, setting the dropdown generates an error. This is the case in the ANOVA analysis: the 'Reference model for weight ratios' dropdown in the Order Restriction, has for source the titles of the tabs of the Order Restriction models. If the dropdown is set with one of these values, reading a JASP file with this setting will lead to an error.
. if an error is set to a control, removing this error lets the analysis is an error state, and the analysis cannot run anymore.

In AnalysisForm, the terms of an available model (which is the case for a dropdown) should be fetched before the value of the dropdown is set.
Also remove the _getControlErrorMessageOfControl method which is not used anymore.
In ControlErrorMessage, when the message is closed, its association with its control should be removed: in this way the AnalysisForm::hasError becomes false.
  • Loading branch information
boutinb committed Apr 13, 2022
1 parent f4470cc commit 51437d0
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 32 deletions.
49 changes: 18 additions & 31 deletions Desktop/analysis/analysisform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,24 @@ void AnalysisForm::bindTo()
BoundControl* boundControl = control->boundControl();
JASPListControl* listControl = dynamic_cast<JASPListControl *>(control);

if (listControl && listControl->hasSource())
{
ListModelAvailableInterface* availableModel = qobject_cast<ListModelAvailableInterface*>(listControl->model());
// The availableList control are not bound with options, but they have to be updated from their sources when the form is initialized.
// The availableList cannot signal their assigned models now because they are not yet bound (the controls are ordered by dependency)
// When the options come from a JASP file, an assigned model needs sometimes the available model (eg. to determine the kind of terms they have).
// So in this case resetTermsFromSourceModels has to be called now but with updateAssigned argument set to false.
// When the options come from default options (from source models), the availableList needs sometimes to signal their assigned models (e.g. when addAvailableVariablesToAssigned if true).
// As their assigned models are not yet bound, resetTermsFromSourceModels (with updateAssigned argument set to true) must be called afterwards.
if (availableModel)
{
if (defaultOptions != Json::nullValue || _analysis->isDuplicate())
availableModel->resetTermsFromSources(false);
else
availableModelsToBeReset.push_back(availableModel);
}
}

if (boundControl)
{
std::string name = control->name().toStdString();
Expand All @@ -382,24 +400,6 @@ void AnalysisForm::bindTo()
boundControl->bindTo(optionValue);
}

if (listControl && listControl->hasSource())
{
ListModelAvailableInterface* availableModel = qobject_cast<ListModelAvailableInterface*>(listControl->model());
// The availableList control are not bound with options, but they have to be updated from their sources when the form is initialized.
// The availableList cannot signal their assigned models now because they are not yet bound (the controls are ordered by dependency)
// When the options come from a JASP file, an assigned model needs sometimes the available model (eg. to determine the kind of terms they have).
// So in this case resetTermsFromSourceModels has to be called now but with updateAssigned argument set to false.
// When the options come from default options (from source models), the availableList needs sometimes to signal their assigned models (e.g. when addAvailableVariablesToAssigned if true).
// As their assigned models are not yet bound, resetTermsFromSourceModels (with updateAssigned argument set to true) must be called afterwards.
if (availableModel)
{
if (defaultOptions != Json::nullValue || _analysis->isDuplicate())
availableModel->resetTermsFromSources(false);
else
availableModelsToBeReset.push_back(availableModel);
}
}

control->setInitialized(hasOption);
}

Expand Down Expand Up @@ -437,19 +437,6 @@ void AnalysisForm::addFormError(const QString &error)
emit errorsChanged();
}

QQuickItem* AnalysisForm::_getControlErrorMessageOfControl(JASPControl* jaspControl)
{
QQuickItem* result = nullptr;

for (QQuickItem* item : _controlErrorMessageCache)
if (item->parentItem() == jaspControl)
{
result = item;
break;
}

return result;
}

//This should be moved to JASPControl maybe?
//Maybe even to full QML? Why don't we just use a loader...
Expand Down
1 change: 0 additions & 1 deletion Desktop/analysis/analysisform.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ public slots:
void setControlMustContain( QString controlName, QStringList containThis);
void setControlIsDependency( std::string controlName, bool isDependency) { setControlIsDependency(tq(controlName), isDependency); }
void setControlMustContain( std::string controlName, std::set<std::string> containThis) { setControlMustContain(tq(controlName), tql(containThis)); }
QQuickItem* _getControlErrorMessageOfControl(JASPControl* jaspControl);
void setAnalysisUp();
std::vector<std::vector<std::string> > _getValuesFromJson(const Json::Value& jsonValues, const QStringList& searchPath);

Expand Down
1 change: 1 addition & 0 deletions Desktop/components/JASP/Widgets/ControlErrorMessage.qml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ Rectangle
{
control.hasError = false;
control.hasWarning = false;
control = null;
parent = null;
}

Expand Down

0 comments on commit 51437d0

Please sign in to comment.