[office/tellico/4.2] src: Avoid calling userTemplates() twice

Git commit 419b1cb225442eda809019ce04c190bcea34cbf9 by Robby Stephenson.
Committed on 19/08/2026 at 12:25.
Pushed by rstephenson into branch '4.2'.

Avoid calling userTemplates() twice

M  +7    -6    src/configdialog.cpp
M  +21   -13   src/newstuff/manager.cpp
M  +1    -1    src/newstuff/manager.h
M  +0    -1    src/newstuff/tellico-template.knsrc

https://invent.kde.org/office/tellico/-/commit/419b1cb225442eda809019ce04c190bcea34cbf9

diff --git a/src/configdialog.cpp b/src/configdialog.cpp
index c6eba5c78..d115daa61 100644
--- a/src/configdialog.cpp
+++ b/src/configdialog.cpp
@@ -1183,13 +1183,14 @@ void ConfigDialog::slotUpdateTemplates(const QList<KNSCore::Entry>& list_) {
 
 void ConfigDialog::slotDeleteTemplate() {
   bool ok;
-  QString name = QInputDialog::getItem(this,
-                                       i18n("Delete Template"),
-                                       i18n("Select template to delete:"),
-                                       Tellico::NewStuff::Manager::self()->userTemplates().keys(),
-                                       0, false, &ok);
+  const auto userTemplates = Tellico::NewStuff::Manager::self()->userTemplates();
+  const QString name = QInputDialog::getItem(this,
+                                             i18n("Delete Template"),
+                                             i18n("Select template to delete:"),
+                                             userTemplates.keys(),
+                                             0, false, &ok);
   if(ok && !name.isEmpty()) {
-    Tellico::NewStuff::Manager::self()->removeTemplateByName(name);
+    Tellico::NewStuff::Manager::self()->removeTemplateByName(name, userTemplates.value(name));
     loadTemplateList();
   }
 }
diff --git a/src/newstuff/manager.cpp b/src/newstuff/manager.cpp
index 9457a9bc6..ec569f187 100644
--- a/src/newstuff/manager.cpp
+++ b/src/newstuff/manager.cpp
@@ -97,13 +97,17 @@ bool Manager::installTemplate(const QString& file_) {
 
   bool success = true;
 
+  myLog() << "Installing template from" << file_;
   static const QRegularExpression digitsDashRx(QStringLiteral("^\\d+-"));
   // is there a better way to figure out if the url points to a XSL file or a tar archive
   // than just trying to open it?
   KTar archive(file_);
   if(archive.open(QIODevice::ReadOnly)) {
     const KArchiveDirectory* archiveDir = archive.directory();
-    archiveDir->copyTo(Tellico::saveLocation(QStringLiteral("entry-templates/")));
+    success = archiveDir->copyTo(Tellico::saveLocation(QStringLiteral("entry-templates/")));
+    if(!success) {
+      myLog() << "Failed to copy archive to" << Tellico::saveLocation(QStringLiteral("entry-templates/"));
+    }
 
     allFiles = archiveFiles(archiveDir);
     // remember files installed for template
@@ -117,6 +121,7 @@ bool Manager::installTemplate(const QString& file_) {
     name = Tellico::saveLocation(QStringLiteral("entry-templates/")) + name;
     // Should overwrite since we might be upgrading
     if(QFile::exists(name)) {
+      myLog() << "Removing existing file:" << name;
       QFile::remove(name);
     }
     KIO::JobFlags flags = KIO::DefaultFlags;
@@ -124,7 +129,7 @@ bool Manager::installTemplate(const QString& file_) {
       flags |= KIO::HideProgressInfo;
     }
     auto job = KIO::file_copy(QUrl::fromLocalFile(file_), QUrl::fromLocalFile(name), -1, flags);
-    if(job->exec()) {
+    if((success = job->exec())) {
       xslFile = QFileInfo(name).fileName();
       allFiles << xslFile;
     }
@@ -132,12 +137,14 @@ bool Manager::installTemplate(const QString& file_) {
 
   if(xslFile.isEmpty()) {
     success = false;
-  } else {
+  } else if(success) {
     KConfigGroup config(KSharedConfig::openConfig(), QStringLiteral("KNewStuffFiles"));
+    // the only thing the 'file_' value does is tie the xsl file name to the full list of installed files
     config.writeEntry(file_, allFiles);
     config.writeEntry(xslFile, file_);
   }
   Tellico::checkCommonXSLFile();
+  myLog() << "Template installation success:" << success;
   return success;
 }
 
@@ -156,15 +163,17 @@ QMap<QString, QString> Manager::userTemplates() {
   return nameFileMap;
 }
 
-bool Manager::removeTemplateByName(const QString& name_) {
+bool Manager::removeTemplateByName(const QString& name_, const QString& xslFile_) {
   if(name_.isEmpty()) {
     return false;
   }
 
-  QString xslFile = userTemplates().value(name_);
+  const QString xslFile = xslFile_.isEmpty() ?
+                            userTemplates().value(name_) :
+                            xslFile_;
   if(!xslFile.isEmpty()) {
     KConfigGroup config(KSharedConfig::openConfig(), QStringLiteral("KNewStuffFiles"));
-    QString file = config.readEntry(xslFile, QString());
+    const QString file = config.readEntry(xslFile, QString());
     if(!file.isEmpty()) {
       return removeTemplate(file);
     }
@@ -190,24 +199,23 @@ bool Manager::removeTemplate(const QString& file_) {
   }
 
   bool success = true;
-  QString path = Tellico::saveLocation(QStringLiteral("entry-templates/"));
+  const QString path = Tellico::saveLocation(QStringLiteral("entry-templates/"));
   foreach(const QString& file, files) {
     if(file.endsWith(QDir::separator())) {
       // ok to not delete all directories
-      QDir().rmdir(path + file);
+      const bool dirRemoved = QDir().rmdir(path + file);
+      myLog() << "Removed directory:" << dirRemoved << (path + file);
     } else {
       success = QFile::remove(path + file) && success;
-      if(!success) {
-        myDebug() << "Failed to remove" << (path+file);
-      }
+      myLog() << "Removed file:" << success << (path + file);
     }
   }
 
   // remove config entries even if unsuccessful
   fileGroup.deleteEntry(file_);
-  QString key = fileGroup.entryMap().key(file_);
-  fileGroup.deleteEntry(key);
+  fileGroup.deleteEntry(fileGroup.entryMap().key(file_));
   KSharedConfig::openConfig()->sync();
+  myLog() << "Template removal success:" << success << file_;
   return success;
 }
 
diff --git a/src/newstuff/manager.h b/src/newstuff/manager.h
index 9694ac291..abd5d1e5c 100644
--- a/src/newstuff/manager.h
+++ b/src/newstuff/manager.h
@@ -40,7 +40,7 @@ public:
   static Manager* self();
   QMap<QString, QString> userTemplates();
   bool installTemplate(const QString& file);
-  bool removeTemplateByName(const QString& name);
+  bool removeTemplateByName(const QString& name, const QString& xslFile=QString());
   bool removeTemplate(const QString& file);
 
   bool installScript(const QString& file);
diff --git a/src/newstuff/tellico-template.knsrc b/src/newstuff/tellico-template.knsrc
index c1f364204..51aa87a77 100644
--- a/src/newstuff/tellico-template.knsrc
+++ b/src/newstuff/tellico-template.knsrc
@@ -5,4 +5,3 @@ TargetDir=tellico/tmp
 Uncompress=never
 InstallationCommand=dbus-send --type=method_call --dest=org.kde.tellico /NewStuff org.kde.tellico.newstuff.installTemplate string:%f
 UninstallCommand=dbus-send --type=method_call --dest=org.kde.tellico /NewStuff org.kde.tellico.newstuff.removeTemplate string:%f
-