[PATCH] Syncronize thumbnails ids with SfxDocumentTemplates when mov...

Hi,

I have submitted a patch for review:

    https://gerrit.libreoffice.org/3054

To pull it, you can do:

    git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/54/3054/1

Syncronize thumbnails ids with SfxDocumentTemplates when moving.

Change-Id: I9ee17e00f769ca16f566b1397d09b76a09273912
---
M sfx2/inc/sfx2/templatelocalview.hxx
M sfx2/source/control/templatelocalview.cxx
M sfx2/source/doc/templatedlg.cxx
3 files changed, 62 insertions(+), 47 deletions(-)



diff --git a/sfx2/inc/sfx2/templatelocalview.hxx b/sfx2/inc/sfx2/templatelocalview.hxx
index cf12ddc..7ea226c 100644
--- a/sfx2/inc/sfx2/templatelocalview.hxx
+++ b/sfx2/inc/sfx2/templatelocalview.hxx
@@ -56,9 +56,9 @@
     bool removeTemplate (const sal_uInt16 nItemId, const sal_uInt16 nSrcItemId);
 
     bool moveTemplate (const ThumbnailViewItem* pItem, const sal_uInt16 nSrcItem,
-                       const sal_uInt16 nTargetItem, bool bCopy);
+                       const sal_uInt16 nTargetItem);
 
-    bool moveTemplates (const std::set<const ThumbnailViewItem*,selection_cmp_fn> &rItems, const 
sal_uInt16 nTargetItem, bool bCopy);
+    bool moveTemplates (const std::set<const ThumbnailViewItem*,selection_cmp_fn> &rItems, const 
sal_uInt16 nTargetItem);
 
     bool copyFrom (const sal_uInt16 nRegionItemId, const BitmapEx &rThumbnail, const OUString 
&rPath);
 
diff --git a/sfx2/source/control/templatelocalview.cxx b/sfx2/source/control/templatelocalview.cxx
index 9bb94c3..ddab413 100644
--- a/sfx2/source/control/templatelocalview.cxx
+++ b/sfx2/source/control/templatelocalview.cxx
@@ -289,11 +289,8 @@
 }
 
 bool TemplateLocalView::moveTemplate (const ThumbnailViewItem *pItem, const sal_uInt16 nSrcItem,
-                                       const sal_uInt16 nTargetItem, bool bCopy)
+                                       const sal_uInt16 nTargetItem)
 {
-    bool bRet = true;
-    bool bRefresh = false;
-
     TemplateContainerItem *pTarget = NULL;
     TemplateContainerItem *pSrc = NULL;
 
@@ -313,16 +310,13 @@
 
         const TemplateViewItem *pViewItem = static_cast<const TemplateViewItem*>(pItem);
 
-        bool bOK;
+        bool bCopy = 
!mpDocTemplates->Move(nTargetRegion,nTargetIdx,nSrcRegionId,pViewItem->mnDocId);
 
         if (bCopy)
-            bOK = mpDocTemplates->Copy(nTargetRegion,nTargetIdx,nSrcRegionId,pViewItem->mnId-1);
-        else
-            bOK = mpDocTemplates->Move(nTargetRegion,nTargetIdx,nSrcRegionId,pViewItem->mnId-1);
-
-        if (!bOK)
-            return false;
-
+        {
+            if (!mpDocTemplates->Copy(nTargetRegion,nTargetIdx,nSrcRegionId,pViewItem->mnDocId))
+                return false;
+        }
         // move template to destination
 
         TemplateItemProperties aTemplateItem;
@@ -340,37 +334,45 @@
             // remove template from region cached data
 
             std::vector<TemplateItemProperties>::iterator aIter;
-            for (aIter = pSrc->maTemplates.begin(); aIter != pSrc->maTemplates.end(); ++aIter)
+            for (aIter = pSrc->maTemplates.begin(); aIter != pSrc->maTemplates.end();)
             {
-                if (aIter->nId == pViewItem->mnId)
+                if (aIter->nDocId == pViewItem->mnDocId)
                 {
-                    pSrc->maTemplates.erase(aIter);
-
-                    RemoveItem(pViewItem->mnId);
-                    break;
+                    aIter = pSrc->maTemplates.erase(aIter);
                 }
+                else
+                {
+                    // Keep region document id syncronized with SfxDocumentTemplates
+                    if (aIter->nDocId > pViewItem->mnDocId)
+                        --aIter->nDocId;
+
+                    ++aIter;
+                }
+            }
+
+            // Keep view document id syncronized with SfxDocumentTemplates
+            std::vector<ThumbnailViewItem*>::iterator pItemIter = mItemList.begin();
+            for (; pItemIter != mItemList.end(); ++pItemIter)
+            {
+                if (static_cast<TemplateViewItem*>(*pItemIter)->mnDocId > pViewItem->mnDocId)
+                    --static_cast<TemplateViewItem*>(*pItemIter)->mnDocId;
             }
         }
 
-        bRefresh = true;
-    }
-    else
-        bRet = false;
-
-    if (bRefresh)
-    {
         lcl_updateThumbnails(pSrc);
         lcl_updateThumbnails(pTarget);
 
         CalculateItemPositions();
         Invalidate();
+
+        return true;
     }
 
-    return bRet;
+    return false;
 }
 
 bool TemplateLocalView::moveTemplates(const std::set<const ThumbnailViewItem*, selection_cmp_fn> 
&rItems,
-                                      const sal_uInt16 nTargetItem, bool bCopy)
+                                      const sal_uInt16 nTargetItem)
 {
     bool ret = true;
     bool refresh = false;
@@ -400,17 +402,15 @@
         {
             const TemplateViewItem *pViewItem = static_cast<const TemplateViewItem*>(*aSelIter);
 
-            bool bOK;
+            bool bCopy = 
!mpDocTemplates->Move(nTargetRegion,nTargetIdx,nSrcRegionId,pViewItem->mnDocId);
 
             if (bCopy)
-                bOK = 
mpDocTemplates->Copy(nTargetRegion,nTargetIdx,nSrcRegionId,pViewItem->mnId-1);
-            else
-                bOK = 
mpDocTemplates->Move(nTargetRegion,nTargetIdx,nSrcRegionId,pViewItem->mnId-1);
-
-            if (!bOK)
             {
-                ret = false;
-                continue;
+                if 
(!mpDocTemplates->Copy(nTargetRegion,nTargetIdx,nSrcRegionId,pViewItem->mnDocId))
+                {
+                    ret = false;
+                    continue;
+                }
             }
 
             // move template to destination
@@ -429,15 +429,30 @@
             {
                 // remove template from region cached data
 
-                std::vector<TemplateItemProperties>::iterator pIter;
-                for (pIter = pSrc->maTemplates.begin(); pIter != pSrc->maTemplates.end(); ++pIter)
+                std::vector<TemplateItemProperties>::iterator pPropIter;
+                for (pPropIter = pSrc->maTemplates.begin(); pPropIter != pSrc->maTemplates.end();)
                 {
-                    if (pIter->nId == pViewItem->mnId)
+                    if (pPropIter->nDocId == pViewItem->mnDocId)
                     {
-                        pSrc->maTemplates.erase(pIter);
+                        pPropIter = pSrc->maTemplates.erase(pPropIter);
                         aItemIds.push_back(pViewItem->mnId);
-                        break;
                     }
+                    else
+                    {
+                        // Keep region document id syncronized with SfxDocumentTemplates
+                        if (pPropIter->nDocId > pViewItem->mnDocId)
+                            --pPropIter->nDocId;
+
+                        ++pPropIter;
+                    }
+                }
+
+                // Keep view document id syncronized with SfxDocumentTemplates
+                std::vector<ThumbnailViewItem*>::iterator pItemIter = mItemList.begin();
+                for (; pItemIter != mItemList.end(); ++pItemIter)
+                {
+                    if (static_cast<TemplateViewItem*>(*pItemIter)->mnDocId > pViewItem->mnDocId)
+                        --static_cast<TemplateViewItem*>(*pItemIter)->mnDocId;
                 }
             }
 
diff --git a/sfx2/source/doc/templatedlg.cxx b/sfx2/source/doc/templatedlg.cxx
index c86704d..6899cf6 100644
--- a/sfx2/source/doc/templatedlg.cxx
+++ b/sfx2/source/doc/templatedlg.cxx
@@ -1366,8 +1366,7 @@
     {
         // Move templates to desired folder if for some reason move fails
         // try copying them.
-        if (!maView->moveTemplates(maSelTemplates,nItemId,false) &&
-                !maView->moveTemplates(maSelTemplates,nItemId,true))
+        if (!maView->moveTemplates(maSelTemplates,nItemId))
         {
             OUString aTemplateList;
 
@@ -1472,13 +1471,14 @@
         // Move templates to desired folder if for some reason move fails
         // try copying them.
         std::set<const ThumbnailViewItem*>::const_iterator aIter;
-        for (aIter = maSelTemplates.begin(); aIter != maSelTemplates.end(); ++aIter)
+        std::set<const ThumbnailViewItem*,selection_cmp_fn> aSelTemplates = maSelTemplates; //Copy 
to avoid invalidating an iterator
+
+        for (aIter = aSelTemplates.begin(); aIter != aSelTemplates.end(); ++aIter)
         {
             const TemplateSearchViewItem *pItem =
                     static_cast<const TemplateSearchViewItem*>(*aIter);
 
-            if(!maView->moveTemplate(pItem,pItem->mnRegionId+1,nItemId,false)
-                    && !maView->moveTemplate(pItem,pItem->mnRegionId+1,nItemId,true))
+            if(!maView->moveTemplate(pItem,pItem->mnRegionId+1,nItemId))
             {
                 if (aTemplateList.isEmpty())
                     aTemplateList = (*aIter)->maTitle;

-- 
To view, visit https://gerrit.libreoffice.org/3054
To unsubscribe, visit https://gerrit.libreoffice.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I9ee17e00f769ca16f566b1397d09b76a09273912
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: master
Gerrit-Owner: Rafael Dominguez <venccsralph@gmail.com>