Bugfix/fix reuse last entered value

app/attributes/attributecontroller.cpp

@@ -18,6 +18,7 @@
 #include "attributetabmodel.h"
 #include "fieldvalidator.h"
 
+#include <QDateTime>
 #include <QDebug>
 #include <QSet>
 
@@ -622,8 +623,40 @@ 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();
+            QString targetDir, prefix;
+            resolveExternalResourcePaths( config, targetDir, prefix );
+            const QString src = InputUtils::getAbsolutePath( rememberedValue.toString(), prefix );
+            const QFileInfo fi( src );
+
+            if ( !fi.isFile() )
+            {
+              ++formItemsIterator;
+              continue;
+            }
+
+            // temporary name; renamePhotos() applies the custom naming expression at save
+            QString newName = QDateTime::currentDateTime().toString( QStringLiteral( "yyyyMMdd_HHmmsszzz" ) );
+            if ( !fi.suffix().isEmpty() )
+              newName += QStringLiteral( "." ) + fi.suffix();
+
+            const QString dst = CoreUtils::findUniquePath( InputUtils::getAbsolutePath( newName, targetDir ) );
+
+            if ( !InputUtils::copyFile( src, dst ) )
+            {
+              ++formItemsIterator;
+              continue;
+            }
+
+            valueToUse = InputUtils::getRelativePath( dst, prefix );
+          }
+
+          mFeatureLayerPair.featureRef().setAttribute( fieldIndex, valueToUse );
+          itemData->setRawValue( valueToUse );
         }
       }
     }
@@ -1576,6 +1609,13 @@ void AttributeController::onFeatureAdded( QgsFeatureId newFeatureId )
   emit featureIdChanged();
 }
 
+void AttributeController::resolveExternalResourcePaths( const QVariantMap &config, QString &targetDir, QString &prefix ) const
+{
+  const FeatureLayerPair parentPair = mParentController ? mParentController->featureLayerPair() : FeatureLayerPair();
+  targetDir = InputUtils::resolveTargetDir( QgsProject::instance()->homePath(), config, mFeatureLayerPair, parentPair, QgsProject::instance() );
+  prefix = InputUtils::resolvePrefixForRelativePath( config[ QStringLiteral( "RelativeStorage" ) ].toInt(), QgsProject::instance()->homePath(), targetDir );
+}
+
 void AttributeController::renamePhotos()
 {
   const QStringList photoNameFormat = QgsProject::instance()->entryList( QStringLiteral( "Mergin" ), QStringLiteral( "PhotoNaming/%1" ).arg( mFeatureLayerPair.layer()->id() ) );
@@ -1636,9 +1676,8 @@ void AttributeController::renamePhotos()
           continue;
         }
 
-        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 );
+        QString targetDir, prefix;
+        resolveExternalResourcePaths( config, targetDir, prefix );
         const QString src = InputUtils::getAbsolutePath( mFeatureLayerPair.feature().attribute( item->fieldIndex() ).toString(), prefix );
         QString newName = val.toString();
 
@@ -1649,13 +1688,15 @@ 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 ) );
         if ( InputUtils::renameFile( src, dst ) )
         {
           const QString newValue = InputUtils::getRelativePath( dst, prefix );
           setFormValue( item->id(), newValue );
+          // avoids renaming it again on the next save
+          item->setOriginalValue( newValue );
           expressionContext.setFeature( featureLayerPair().featureRef() );
         }
         else
@@ -1680,9 +1721,8 @@ void AttributeController::saveSketches()
       if ( item->rawValue().isValid() )
       {
         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 );
+        QString targetDir, prefix;
+        resolveExternalResourcePaths( config, targetDir, prefix );
         const QString src = InputUtils::getAbsolutePath( mFeatureLayerPair.feature().attribute( item->fieldIndex() ).toString(), prefix );
 
         const QString tempFilePath = QString( "%1/%2/%3" ).arg( QDir::temp().absolutePath(), QUrl::fromLocalFile( QgsProject::instance()->homePath() ).fileName(), src.section( "/", -1 ) );

app/attributes/attributecontroller.h

@@ -225,6 +225,8 @@ class  AttributeController : public QObject
      */
     bool allowTabs( QgsAttributeEditorContainer *container );
 
+    //! resolves the storage folder and relative-path prefix for an ExternalResource field
+    void resolveExternalResourcePaths( const QVariantMap &config, QString &targetDir, QString &prefix ) const;
     //! renames photos if necessary
     void renamePhotos();
     //! save temporary sketched image to original image

app/inpututils.cpp

@@ -1018,6 +1018,8 @@ QString InputUtils::resolveTargetDir( const QString &homePath, const QVariantMap
   {
     QString result = evaluateExpression( pair, parentPair, activeProject, expression );
     sanitizePath( result );
+    if ( !result.isEmpty() && !QDir::isAbsolutePath( result ) )
+      result = QDir( homePath ).absoluteFilePath( result );
     return result;
   }
   else
@@ -1029,6 +1031,8 @@ QString InputUtils::resolveTargetDir( const QString &homePath, const QVariantMap
     }
     else
     {
+      if ( !QDir::isAbsolutePath( defaultRoot ) )
+        defaultRoot = QDir( homePath ).absoluteFilePath( defaultRoot );
       return defaultRoot;
     }
   }

app/test/testattributecontroller.cpp

@@ -894,6 +894,183 @@ void TestAttributeController::testPhotoRenaming()
   }
 }
 
+void TestAttributeController::testPhotoReuseRenamesWithFreshExpressionValue()
+{
+  QString projectName = QStringLiteral( "testPhotoReuseRenamesWithFreshExpressionValue" );
+  QString projectDir = QDir::tempPath() + "/MM_test_projects/" + projectName;
+
+  QDir tempDir( projectDir );
+  QVERIFY( tempDir.removeRecursively() );
+
+  QVERIFY( InputUtils::cpDir( TestUtils::testDataDir() + "/test_photo_rename", projectDir ) );
+  QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image1.jpg" ) ) );
+
+  QVERIFY( QgsProject::instance()->read( projectDir + QStringLiteral( "/test_photo_rename.qgz" ) ) );
+
+  QgsMapLayer *layer = QgsProject::instance()->mapLayersByName( QStringLiteral( "Survey" ) ).at( 0 );
+  QgsVectorLayer *surveyLayer = static_cast<QgsVectorLayer *>( layer );
+  QVERIFY( surveyLayer && surveyLayer->isValid() );
+
+  RememberAttributesController remController;
+  remController.reset();
+  remController.setRememberValuesAllowed( true );
+
+  // feature 1: notes = "first" -> naming expression 'image_' + "notes" saves image_first.jpg
+  QgsFeature feat1( surveyLayer->fields() );
+  FeatureLayerPair pair1( feat1, surveyLayer );
+
+  AttributeController controller1;
+  controller1.setRememberAttributesController( &remController );
+  controller1.setFeatureLayerPair( pair1 );
+
+  const TabItem *tab1 = controller1.tabItem( 0 );
+  const QVector<QUuid> items1 = tab1->formItems();
+
+  // mark the photo field to be reused on the next new feature
+  remController.setShouldRememberValue( surveyLayer, 3, true );
+
+  controller1.setFormValue( items1.at( 2 ), QStringLiteral( "first" ) );
+  controller1.setFormValue( items1.at( 3 ), QStringLiteral( "image1.jpg" ) );
+  controller1.save();
+
+  QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_first.jpg" ) ) );
+
+  // feature 2 reuses the photo but has notes = "second" - expression must re-evaluate fresh
+  QgsFeature feat2( surveyLayer->fields() );
+  FeatureLayerPair pair2( feat2, surveyLayer );
+
+  AttributeController controller2;
+  controller2.setRememberAttributesController( &remController );
+  controller2.setFeatureLayerPair( pair2 );
+
+  const TabItem *tab2 = controller2.tabItem( 0 );
+  const QVector<QUuid> items2 = tab2->formItems();
+
+  controller2.setFormValue( items2.at( 2 ), QStringLiteral( "second" ) );
+  controller2.save();
+
+  const QgsFeature f2 = controller2.featureLayerPair().feature();
+
+  QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_first.jpg" ) ) ); // untouched
+  QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_second.jpg" ) ) ); // fresh name
+  QCOMPARE( f2.attribute( 3 ), QStringLiteral( "image_second.jpg" ) );
+}
+
+void TestAttributeController::testPhotoRenamingNotRepeatedOnResave()
+{
+  QString projectName = QStringLiteral( "testPhotoRenamingNotRepeatedOnResave" );
+  QString projectDir = QDir::tempPath() + "/MM_test_projects/" + projectName;
+
+  QDir tempDir( projectDir );
+  QVERIFY( tempDir.removeRecursively() );
+
+  QVERIFY( InputUtils::cpDir( TestUtils::testDataDir() + "/test_photo_rename", projectDir ) );
+  QVERIFY( QgsProject::instance()->read( projectDir + QStringLiteral( "/test_photo_rename.qgz" ) ) );
+
+  QgsMapLayer *layer = QgsProject::instance()->mapLayersByName( QStringLiteral( "Survey" ) ).at( 0 );
+  QgsVectorLayer *surveyLayer = static_cast<QgsVectorLayer *>( layer );
+  QVERIFY( surveyLayer && surveyLayer->isValid() );
+
+  RememberAttributesController remController;
+  remController.reset();
+  remController.setRememberValuesAllowed( true );
+
+  QgsFeature feat1( surveyLayer->fields() );
+  FeatureLayerPair pair1( feat1, surveyLayer );
+
+  AttributeController controller1;
+  controller1.setRememberAttributesController( &remController );
+  controller1.setFeatureLayerPair( pair1 );
+
+  const TabItem *tab1 = controller1.tabItem( 0 );
+  const QVector<QUuid> items1 = tab1->formItems();
+
+  remController.setShouldRememberValue( surveyLayer, 3, true );
+
+  controller1.setFormValue( items1.at( 2 ), QStringLiteral( "first" ) );
+  controller1.setFormValue( items1.at( 3 ), QStringLiteral( "image1.jpg" ) );
+  controller1.save();
+
+  QgsFeature feat2( surveyLayer->fields() );
+  FeatureLayerPair pair2( feat2, surveyLayer );
+
+  AttributeController controller2;
+  controller2.setRememberAttributesController( &remController );
+  controller2.setFeatureLayerPair( pair2 );
+
+  const TabItem *tab2 = controller2.tabItem( 0 );
+  const QVector<QUuid> items2 = tab2->formItems();
+
+  controller2.setFormValue( items2.at( 2 ), QStringLiteral( "second" ) );
+  controller2.save();
+
+  QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_second.jpg" ) ) );
+
+  // save the very same feature again without changing anything - must not re-trigger the rename
+  controller2.save();
+
+  QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_second.jpg" ) ) );
+  QVERIFY( !QFile::exists( projectDir + QStringLiteral( "/image_second (1).jpg" ) ) );
+}
+
+void TestAttributeController::testReusedPhotoIsIndependentFile()
+{
+  QString projectName = QStringLiteral( "testReusedPhotoIsIndependentFile" );
+  QString projectDir = QDir::tempPath() + "/MM_test_projects/" + projectName;
+
+  QDir tempDir( projectDir );
+  QVERIFY( tempDir.removeRecursively() );
+
+  QVERIFY( InputUtils::cpDir( TestUtils::testDataDir() + "/test_photo_rename", projectDir ) );
+  QVERIFY( QgsProject::instance()->read( projectDir + QStringLiteral( "/test_photo_rename.qgz" ) ) );
+
+  QgsMapLayer *layer = QgsProject::instance()->mapLayersByName( QStringLiteral( "Survey" ) ).at( 0 );
+  QgsVectorLayer *surveyLayer = static_cast<QgsVectorLayer *>( layer );
+  QVERIFY( surveyLayer && surveyLayer->isValid() );
+
+  RememberAttributesController remController;
+  remController.reset();
+  remController.setRememberValuesAllowed( true );
+
+  QgsFeature feat1( surveyLayer->fields() );
+  FeatureLayerPair pair1( feat1, surveyLayer );
+
+  AttributeController controller1;
+  controller1.setRememberAttributesController( &remController );
+  controller1.setFeatureLayerPair( pair1 );
+
+  const TabItem *tab1 = controller1.tabItem( 0 );
+  const QVector<QUuid> items1 = tab1->formItems();
+
+  remController.setShouldRememberValue( surveyLayer, 3, true );
+
+  controller1.setFormValue( items1.at( 2 ), QStringLiteral( "first" ) );
+  controller1.setFormValue( items1.at( 3 ), QStringLiteral( "image1.jpg" ) );
+  controller1.save();
+
+  QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_first.jpg" ) ) );
+
+  QgsFeature feat2( surveyLayer->fields() );
+  FeatureLayerPair pair2( feat2, surveyLayer );
+
+  AttributeController controller2;
+  controller2.setRememberAttributesController( &remController );
+  controller2.setFeatureLayerPair( pair2 );
+
+  const QString reusedRelativePath = controller2.featureLayerPair().feature().attribute( 3 ).toString();
+  QVERIFY( !reusedRelativePath.isEmpty() );
+  // must be its own file, not literally feature 1's path
+  QVERIFY( reusedRelativePath != QStringLiteral( "image_first.jpg" ) );
+
+  const QString reusedAbsolutePath = projectDir + "/" + reusedRelativePath;
+  QVERIFY( QFile::exists( reusedAbsolutePath ) );
+  QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_first.jpg" ) ) );
+
+  // removing feature 2's file directly must not affect feature 1's, since they're independent
+  QVERIFY( QFile::remove( reusedAbsolutePath ) );
+  QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_first.jpg" ) ) );
+}
+
 void TestAttributeController::testHtmlAndTextWidgets()
 {
   QString projectDir = TestUtils::testDataDir() + "/expressions";

app/test/testattributecontroller.h

@@ -28,6 +28,16 @@ class TestAttributeController: public QObject
     void testRawValue();
     void testFieldsOutsideForm();
     void testPhotoRenaming();
+
+    //! A reused photo must be renamed with the new feature's own expression value, not the old one
+    void testPhotoReuseRenamesWithFreshExpressionValue();
+
+    //! Saving the same feature twice must not rename an already-renamed photo again
+    void testPhotoRenamingNotRepeatedOnResave();
+
+    //! Reusing a photo must create an independent file, not just copy the path string
+    void testReusedPhotoIsIndependentFile();
+
     void testHtmlAndTextWidgets();
     void testVirtualFields();
 

app/test/testutilsfunctions.cpp

@@ -331,6 +331,7 @@ void TestUtilsFunctions::resolveTargetDir()
 {
   QString homePath = TestUtils::testDataDir();
   QString DEFAULT_ROOT( "DEFAULT/ROOT/PATH" ); // can be not existing path
+  QString ABSOLUTE_DEFAULT_ROOT( "/absolute/default/root/path" ); // can be not existing path
 
   QgsProject *activeProject = nullptr;
   QVariantMap config;
@@ -344,10 +345,16 @@ void TestUtilsFunctions::resolveTargetDir()
   QString resultDir = mUtils->resolveTargetDir( homePath, config, pair, FeatureLayerPair(), activeProject );
   QCOMPARE( resultDir, homePath );
 
-  // case 2: defined default root config, no expression
+  // case 2: defined default root config as a relative path, no expression - resolved against homePath
   config.insert( QStringLiteral( "DefaultRoot" ), DEFAULT_ROOT );
   QString resultDir2 = mUtils->resolveTargetDir( homePath, config, pair, FeatureLayerPair(), activeProject );
-  QCOMPARE( resultDir2, DEFAULT_ROOT );
+  QCOMPARE( resultDir2, QStringLiteral( "%1/%2" ).arg( homePath, DEFAULT_ROOT ) );
+  config.clear();
+
+  // case 2b: defined default root config as an already-absolute path, no expression - stays unchanged
+  config.insert( QStringLiteral( "DefaultRoot" ), ABSOLUTE_DEFAULT_ROOT );
+  QString resultDir2b = mUtils->resolveTargetDir( homePath, config, pair, FeatureLayerPair(), activeProject );
+  QCOMPARE( resultDir2b, ABSOLUTE_DEFAULT_ROOT );
   config.clear();
 
   // case 3: defined expression in config->"PropertyCollection" -> "properties" -> "propertyRootPath" -> "expression"