-
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
2e2721a
32020d4
5f9707d
a7355ca
d3ac929
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 | ||
| mFormItems[itemData->id()]->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?
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. +1 to remove this - let's keep the logic as it was |
||
| { | ||
| QVariant rememberedValue; | ||
| bool shouldUseRememberedValue = mRememberAttributesController->rememberedValue( | ||
|
|
@@ -622,8 +623,36 @@ void AttributeController::updateOnFeatureChange() | |
| ); | ||
| if ( shouldUseRememberedValue ) | ||
| { | ||
| mFeatureLayerPair.featureRef().setAttribute( fieldIndex, rememberedValue ); | ||
| itemData->setRawValue( rememberedValue ); | ||
| QVariant valueToUse = rememberedValue; | ||
|
|
||
| if ( itemData->editorWidgetType() == QStringLiteral( "ExternalResource" ) && !rememberedValue.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. Check if |
||
| { | ||
| 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 ), true ); | ||
|
Comment on lines
+637
to
+642
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. Set file name as the default timestamp, store it in the correct folder |
||
|
|
||
| if ( InputUtils::copyFile( src, dst ) ) | ||
| { | ||
| valueToUse = InputUtils::getRelativePath( dst, prefix ); | ||
| itemData->setReusedCopyPath( dst ); | ||
| } | ||
| } | ||
|
|
||
| mFeatureLayerPair.featureRef().setAttribute( fieldIndex, valueToUse ); | ||
|
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. Watch out, this code is run even if |
||
| itemData->setRawValue( valueToUse ); | ||
| itemData->setOriginalValue( valueToUse ); | ||
| // an empty value means there's nothing to reuse, so don't mark it as such | ||
| itemData->setReusedValue( !valueToUse.toString().isEmpty() ); | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -789,9 +818,11 @@ void AttributeController::recalculateDefaultValues( | |
| const QgsField field = item->field(); | ||
| const QgsDefaultValue defaultDefinition = field.defaultValueDefinition(); | ||
|
|
||
| // don't let a Default Value expression overwrite a value we just reused | ||
| bool shouldApplyDefaultValue = | ||
| !defaultDefinition.expression().isEmpty() && | ||
| ( isFirstUpdateOfNewFeature || ( isFormValueChange && defaultDefinition.applyOnUpdate() ) ); | ||
| ( isFirstUpdateOfNewFeature || ( isFormValueChange && defaultDefinition.applyOnUpdate() ) ) && | ||
| !item->isReusedValue(); | ||
|
xkello marked this conversation as resolved.
|
||
|
|
||
| if ( shouldApplyDefaultValue ) | ||
| { | ||
|
|
@@ -1211,6 +1242,8 @@ bool AttributeController::deleteFeature() | |
|
|
||
| bool AttributeController::rollback() | ||
| { | ||
| discardReusedPhotoCopies( true ); | ||
|
|
||
| if ( !mFeatureLayerPair.layer() ) | ||
| return false; | ||
|
|
||
|
|
@@ -1293,6 +1326,12 @@ bool AttributeController::save() | |
| disconnect( mFeatureLayerPair.layer(), &QgsVectorLayer::featureAdded, this, &AttributeController::onFeatureAdded ); | ||
| } | ||
|
|
||
| if ( rv ) | ||
| { | ||
| // catches a reused copy that got deleted/replaced before save | ||
| discardReusedPhotoCopies( false ); | ||
| } | ||
|
xkello marked this conversation as resolved.
|
||
|
|
||
| // Store the feature attributes for future use | ||
| if ( featureIsNew && mRememberAttributesController ) | ||
| { | ||
|
|
@@ -1497,6 +1536,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 +1616,27 @@ void AttributeController::onFeatureAdded( QgsFeatureId newFeatureId ) | |
| emit featureIdChanged(); | ||
| } | ||
|
|
||
| void AttributeController::discardReusedPhotoCopies( bool force ) | ||
|
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. drop |
||
| { | ||
| 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() ) | ||
| { | ||
| // isReusedValue() is cleared as soon as the field changes, so it already tells | ||
| // whether the copy is still the field's current value | ||
| if ( force || !item->isReusedValue() ) | ||
| { | ||
| 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 +1669,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 ); | ||
|
|
@@ -1649,13 +1710,18 @@ void AttributeController::renamePhotos() | |
| InputUtils::sanitizePath( newName ); | ||
|
|
||
| const QFileInfo fi( src ); | ||
| newName = QStringLiteral( "%1.%2" ).arg( newName, fi.completeSuffix() ); | ||
| newName = QStringLiteral( "%1.%2" ).arg( newName, fi.suffix() ); | ||
|
|
||
| const QString dst = CoreUtils::findUniquePath( InputUtils::getAbsolutePath( newName, targetDir ) ); | ||
| const QString dst = CoreUtils::findUniquePath( InputUtils::getAbsolutePath( newName, targetDir ), true ); | ||
| if ( InputUtils::renameFile( src, dst ) ) | ||
| { | ||
| const QString newValue = InputUtils::getRelativePath( dst, prefix ); | ||
| setFormValue( item->id(), newValue ); | ||
| // keep originalValue() in sync so this doesn't get renamed again on the next save | ||
| item->setOriginalValue( newValue ); | ||
| // only clear if we actually renamed the tracked clone, not a replacement photo | ||
| if ( src == item->reusedCopyPath() ) | ||
| item->setReusedCopyPath( QString() ); | ||
| expressionContext.setFeature( featureLayerPair().featureRef() ); | ||
| } | ||
| else | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -222,6 +222,7 @@ class AttributeController : public QObject | |
| */ | ||
| bool allowTabs( QgsAttributeEditorContainer *container ); | ||
|
|
||
| void discardReusedPhotoCopies( bool force ); | ||
|
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. Let's drop this |
||
| //! renames photos if necessary | ||
| void renamePhotos(); | ||
| //! save temporary sketched image to original image | ||
|
|
||
| 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 ); | ||||||||||||
|
xkello marked this conversation as resolved.
Comment on lines
+155
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. This part of the app should not be affected
Suggested change
|
||||||||||||
|
|
||||||||||||
| 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 | ||||||||||||
| }; | ||||||||||||
|
|
||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,6 +94,8 @@ - ( void )picker:( PHPickerViewController * )picker didFinishPicking:( NSArray<P | |
| BOOL writeSuccess = NO; | ||
| if ( url && !error ) | ||
| { | ||
| [[NSFileManager defaultManager] createDirectoryAtPath:targetDir withIntermediateDirectories:YES attributes:nil error:nil]; | ||
|
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. If needed, please create a separate PR |
||
|
|
||
| NSError *copyError = nil; | ||
| [[NSFileManager defaultManager] copyItemAtURL:url toURL:[NSURL fileURLWithPath:imagePath] error:©Error]; | ||
| writeSuccess = ( copyError == nil ); | ||
|
|
||
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.
If needed, please create a separate PR