-
Notifications
You must be signed in to change notification settings - Fork 87
Bugfix/fix reuse last entered value #4668
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -612,7 +612,8 @@ void AttributeController::updateOnFeatureChange() | |
| const QVariant newVal = feature.attribute( fieldIndex ); | ||
| mFormItems[itemData->id()]->setOriginalValue( newVal ); | ||
| mFormItems[itemData->id()]->setRawValue( newVal ); // we need to set raw value as well, as we use it in form now | ||
| if ( mRememberAttributesController && isNewFeature() ) // this is a new feature | ||
| itemData->setReusedValue( false ); | ||
| if ( mRememberAttributesController && isNewFeature() && newVal.toString().isEmpty() ) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. won't the new condition somehow mess up other fieldtypes? or was it just expected until now? |
||
| { | ||
| QVariant rememberedValue; | ||
| bool shouldUseRememberedValue = mRememberAttributesController->rememberedValue( | ||
|
|
@@ -622,8 +623,35 @@ void AttributeController::updateOnFeatureChange() | |
| ); | ||
| if ( shouldUseRememberedValue ) | ||
| { | ||
| mFeatureLayerPair.featureRef().setAttribute( fieldIndex, rememberedValue ); | ||
| itemData->setRawValue( rememberedValue ); | ||
| QVariant valueToUse = rememberedValue; | ||
|
|
||
| if ( itemData->editorWidgetType() == QStringLiteral( "ExternalResource" ) && !rememberedValue.toString().isEmpty() ) | ||
| { | ||
| const QVariantMap config = itemData->editorWidgetConfig(); | ||
| const FeatureLayerPair parentPair = mParentController ? mParentController->featureLayerPair() : FeatureLayerPair(); | ||
| const QString targetDir = InputUtils::resolveTargetDir( QgsProject::instance()->homePath(), config, mFeatureLayerPair, parentPair, QgsProject::instance() ); | ||
| const QString prefix = InputUtils::resolvePrefixForRelativePath( config[ QStringLiteral( "RelativeStorage" ) ].toInt(), QgsProject::instance()->homePath(), targetDir ); | ||
| const QString src = InputUtils::getAbsolutePath( rememberedValue.toString(), prefix ); | ||
| const QFileInfo fi( src ); | ||
|
|
||
| static const QRegularExpression trailingCounter( QStringLiteral( "\\s\\(\\d+\\)$" ) ); | ||
| QString baseName = fi.completeBaseName(); | ||
| baseName.remove( trailingCounter ); | ||
| const QString canonicalName = fi.suffix().isEmpty() ? baseName : QStringLiteral( "%1.%2" ).arg( baseName, fi.suffix() ); | ||
|
|
||
| const QString dst = CoreUtils::findUniquePath( InputUtils::getAbsolutePath( canonicalName, targetDir ) ); | ||
|
|
||
| if ( InputUtils::copyFile( src, dst ) ) | ||
| { | ||
| valueToUse = InputUtils::getRelativePath( dst, prefix ); | ||
| itemData->setReusedCopyPath( dst ); | ||
| } | ||
| } | ||
|
|
||
| mFeatureLayerPair.featureRef().setAttribute( fieldIndex, valueToUse ); | ||
| itemData->setRawValue( valueToUse ); | ||
| itemData->setOriginalValue( valueToUse ); | ||
| itemData->setReusedValue( true ); | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -791,7 +819,8 @@ void AttributeController::recalculateDefaultValues( | |
|
|
||
| bool shouldApplyDefaultValue = | ||
| !defaultDefinition.expression().isEmpty() && | ||
| ( isFirstUpdateOfNewFeature || ( isFormValueChange && defaultDefinition.applyOnUpdate() ) ); | ||
| ( isFirstUpdateOfNewFeature || ( isFormValueChange && defaultDefinition.applyOnUpdate() ) ) && | ||
| !item->isReusedValue(); | ||
|
Comment on lines
+822
to
+823
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this necessary, what does it solve? |
||
|
|
||
| if ( shouldApplyDefaultValue ) | ||
| { | ||
|
|
@@ -1211,6 +1240,8 @@ bool AttributeController::deleteFeature() | |
|
|
||
| bool AttributeController::rollback() | ||
| { | ||
| discardReusedPhotoCopies( true ); | ||
|
|
||
| if ( !mFeatureLayerPair.layer() ) | ||
| return false; | ||
|
|
||
|
|
@@ -1293,6 +1324,11 @@ bool AttributeController::save() | |
| disconnect( mFeatureLayerPair.layer(), &QgsVectorLayer::featureAdded, this, &AttributeController::onFeatureAdded ); | ||
| } | ||
|
|
||
| if ( rv ) | ||
| { | ||
| discardReusedPhotoCopies( false ); | ||
| } | ||
|
Comment on lines
+1327
to
+1330
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what's the case we want to remove photocopies on successful save? |
||
|
|
||
| // Store the feature attributes for future use | ||
| if ( featureIsNew && mRememberAttributesController ) | ||
| { | ||
|
|
@@ -1497,6 +1533,7 @@ bool AttributeController::setFormValue( const QUuid &id, QVariant value ) | |
| QgsField field = item->field(); | ||
| QVariant val( value ); | ||
|
|
||
| item->setReusedValue( false ); | ||
| item->setRawValue( val ); | ||
| emit formDataChanged( item->id(), { AttributeFormModel::RawValue } ); | ||
|
|
||
|
|
@@ -1576,6 +1613,36 @@ void AttributeController::onFeatureAdded( QgsFeatureId newFeatureId ) | |
| emit featureIdChanged(); | ||
| } | ||
|
|
||
| void AttributeController::discardReusedPhotoCopies( bool force ) | ||
| { | ||
| QMap<QUuid, std::shared_ptr<FormItem>>::const_iterator formItemsIterator = mFormItems.constBegin(); | ||
| while ( formItemsIterator != mFormItems.constEnd() ) | ||
| { | ||
| std::shared_ptr<FormItem> item = formItemsIterator.value(); | ||
| const QString copyPath = item->reusedCopyPath(); | ||
| if ( !copyPath.isEmpty() ) | ||
| { | ||
| bool stillReferenced = false; | ||
| if ( !force ) | ||
| { | ||
| const QVariantMap config = item->editorWidgetConfig(); | ||
| const FeatureLayerPair parentPair = mParentController ? mParentController->featureLayerPair() : FeatureLayerPair(); | ||
| const QString targetDir = InputUtils::resolveTargetDir( QgsProject::instance()->homePath(), config, mFeatureLayerPair, parentPair, QgsProject::instance() ); | ||
| const QString prefix = InputUtils::resolvePrefixForRelativePath( config[ QStringLiteral( "RelativeStorage" ) ].toInt(), QgsProject::instance()->homePath(), targetDir ); | ||
| const QString currentPath = InputUtils::getAbsolutePath( mFeatureLayerPair.feature().attribute( item->fieldIndex() ).toString(), prefix ); | ||
| stillReferenced = ( currentPath == copyPath ); | ||
| } | ||
|
|
||
| if ( force || !stillReferenced ) | ||
| { | ||
| InputUtils::removeFile( copyPath ); | ||
| } | ||
| item->setReusedCopyPath( QString() ); | ||
| } | ||
| ++formItemsIterator; | ||
| } | ||
| } | ||
|
|
||
| void AttributeController::renamePhotos() | ||
| { | ||
| const QStringList photoNameFormat = QgsProject::instance()->entryList( QStringLiteral( "Mergin" ), QStringLiteral( "PhotoNaming/%1" ).arg( mFeatureLayerPair.layer()->id() ) ); | ||
|
|
@@ -1608,7 +1675,7 @@ void AttributeController::renamePhotos() | |
| continue; | ||
| } | ||
|
|
||
| if ( item->originalValue() != mFeatureLayerPair.feature().attribute( item->fieldIndex() ) ) | ||
| if ( item->isReusedValue() || item->originalValue() != mFeatureLayerPair.feature().attribute( item->fieldIndex() ) ) | ||
| { | ||
| const QString expString = QgsProject::instance()->readEntry( QStringLiteral( "Mergin" ), QStringLiteral( "PhotoNaming/%1/%2" ).arg( mFeatureLayerPair.layer()->id(), field.name() ) ); | ||
| QgsExpression exp( expString ); | ||
|
|
@@ -1656,6 +1723,7 @@ void AttributeController::renamePhotos() | |
| { | ||
| const QString newValue = InputUtils::getRelativePath( dst, prefix ); | ||
| setFormValue( item->id(), newValue ); | ||
| item->setReusedCopyPath( QString() ); | ||
| expressionContext.setFeature( featureLayerPair().featureRef() ); | ||
| } | ||
| else | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -152,6 +152,12 @@ class FormItem | |
| QVariant rawValue() const; | ||
| void setRawValue( const QVariant &rawValue ); | ||
|
|
||
| bool isReusedValue() const; | ||
| void setReusedValue( bool reused ); | ||
|
|
||
| QString reusedCopyPath() const; | ||
| void setReusedCopyPath( const QString &path ); | ||
|
Comment on lines
+158
to
+159
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can't we instead of this field reuse |
||
|
|
||
| QgsRelation relation() const; | ||
| QString fieldError() const; | ||
|
|
||
|
|
@@ -178,6 +184,8 @@ class FormItem | |
| bool mVisible = false; | ||
| QVariant mOriginalValue; // original unmodified value | ||
| QVariant mRawValue; | ||
| bool mIsReusedValue = false; | ||
| QString mReusedCopyPath; | ||
|
|
||
| const QgsRelation mRelation; // Only used for FormItemType::Relation | ||
| }; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would reuse the access approach above