Use more complex logic for layer/group ordering

Create a simplified, one-level deep, representation of the
QGIS project structure and then use this to determine the desired
ordering of the Felt project. Always specify ordering keys
for layers and groups.

felt/core/api_client.py

@@ -472,7 +472,8 @@ def update_layer_details(self,
 
     def create_layer_groups(self,
                             map_id: str,
-                            layer_group_names: List[str]) \
+                            layer_group_names: List[str],
+                            ordering_keys: Optional[Dict[str, int]] = None) \
             -> QgsNetworkReplyContent:
         """
         Creates layer groups for a map
@@ -483,10 +484,17 @@ def create_layer_groups(self,
             version=2
         )
 
-        group_post_data = [
-            {'name': g,
-             'ordering_key': i} for i, g in enumerate(layer_group_names)
-        ]
+        if not ordering_keys:
+            group_post_data = [
+                {'name': g,
+                 'ordering_key': i} for i, g in enumerate(layer_group_names)
+            ]
+        else:
+            group_post_data = [
+                {'name': g,
+                 'ordering_key': ordering_keys[g] or 0} for g in
+                layer_group_names
+            ]
 
         return QgsNetworkAccessManager.instance().blockingPost(
             request,

felt/core/layer_exporter.py

@@ -87,6 +87,7 @@ class ZippedExportResult:
     qgis_style_xml: str
     style: Optional[LayerStyle] = None
     group_name: Optional[str] = None
+    ordering_key: Optional[int] = None
 
 
 class LayerExporter(QObject):

felt/core/map_uploader.py

@@ -37,6 +37,7 @@
     QgsReferencedRectangle,
     QgsRasterLayer,
     QgsLayerTree,
+    QgsLayerTreeLayer,
     QgsLayerTreeGroup
 )
 from qgis.utils import iface
@@ -60,6 +61,94 @@ class LayerDetails:
     """
     layer: QgsMapLayer
     destination_group_name: Optional[str] = None
+    ordering_key: Optional[int] = None
+
+
+@dataclass
+class ProjectComponent:
+    """
+    Encapsulates details of a project component
+    """
+    object_index: Optional[int] = None
+    group_name: Optional[str] = None
+    layer: Optional[LayerDetails] = None
+
+
+class SimplifiedProjectStructure:
+    """
+    Represents a simplified structure of a QGIS project
+    """
+    def __init__(self):
+        self.components: List[ProjectComponent] = []
+        self._current_index: int = 0
+
+    def append_component(self, component: ProjectComponent):
+        """
+        Adds a component to the project structure
+        """
+        component.object_index = self._current_index
+        self.components.append(component)
+        self._current_index += 1
+
+    def group_ordering_key(self, group_name: str) -> Optional[int]:
+        """
+        Returns the ordering key for a group name
+        """
+        for i, component in enumerate(self.components):
+            if component.group_name == group_name:
+                return len(self.components) - i
+        return None
+
+    def find_layer(self, layer: QgsMapLayer) -> Optional[ProjectComponent]:
+        """
+        Finds a layer in the project structure
+        """
+        for component in self.components:
+            if component.layer and component.layer.layer == layer:
+                return component
+        return None
+
+    @staticmethod
+    def from_project(project: QgsProject) -> 'SimplifiedProjectStructure':
+        """
+        Creates a simplified structure of a QGIS project
+        """
+        tree = project.layerTreeRoot()
+        res = SimplifiedProjectStructure()
+
+        def traverse_group(group: QgsLayerTreeGroup,
+                           top_level_group_name: str):
+            for _child in group.children():
+                if isinstance(_child, QgsLayerTreeGroup):
+                    traverse_group(_child, top_level_group_name)
+                elif isinstance(_child, QgsLayerTreeLayer):
+                    res.append_component(
+                        ProjectComponent(
+                            layer=LayerDetails(
+                                layer=_child.layer(),
+                                destination_group_name=top_level_group_name
+                            )
+                        )
+                    )
+
+        for child in tree.children():
+            if isinstance(child, QgsLayerTreeGroup):
+                res.append_component(
+                    ProjectComponent(
+                        group_name=child.name()
+                    )
+                )
+                traverse_group(child, child.name())
+            elif isinstance(child, QgsLayerTreeLayer):
+                res.append_component(
+                    ProjectComponent(
+                        layer=LayerDetails(
+                            layer=child.layer(),
+                        )
+                    )
+                )
+
+        return res
 
 
 class MapUploaderTask(QgsTask):
@@ -82,6 +171,10 @@ def __init__(self,
 
         self.unsupported_layers: List[Tuple[str, str]] = []
         self.unsupported_styles: List[Tuple[str, List[str]]] = []
+        self.project_structure = SimplifiedProjectStructure.from_project(
+                project
+            )
+
         if layers:
             self.current_map_crs = QgsCoordinateReferenceSystem('EPSG:4326')
             self.current_map_extent = QgsMapLayerUtils.combinedExtent(
@@ -171,19 +264,18 @@ def __init__(self,
         self.error_string: Optional[str] = None
         self.feedback: Optional[QgsFeedback] = None
         self.was_canceled = False
-        self.ordered_top_level_groups: List[str] = []
-        if project:
-            for child in project.layerTreeRoot().children():
-                if isinstance(child, QgsLayerTreeGroup):
-                    self.ordered_top_level_groups.append(child.name())
-            self.ordered_top_level_groups.reverse()
 
     @staticmethod
-    def layer_and_group(project: QgsProject, layer: QgsMapLayer) \
+    def layer_and_group(
+            project: QgsProject,
+            layer: QgsMapLayer) \
             -> LayerDetails:
         """
         Clones a layer, and returns the layer details
         """
+        simplified_project = SimplifiedProjectStructure.from_project(project)
+        simplified_project_layer = simplified_project.find_layer(layer)
+
         res = MapUploaderTask.clone_layer(layer)
 
         layer_tree_root = project.layerTreeRoot()
@@ -200,9 +292,16 @@ def layer_and_group(project: QgsProject, layer: QgsMapLayer) \
         if parent != layer_tree_root and QgsLayerTree.isGroup(parent):
             group_name = parent.name()
 
+        ordering_key = None
+        if (simplified_project_layer and
+                simplified_project_layer.object_index is not None):
+            object_index = simplified_project_layer.object_index
+            ordering_key = len(simplified_project.components) - object_index
+
         return LayerDetails(
             layer=res,
-            destination_group_name=group_name
+            destination_group_name=group_name,
+            ordering_key=ordering_key
         )
 
     @staticmethod
@@ -467,6 +566,7 @@ def run(self):
                     return False
 
                 result.group_name = group_name
+                result.ordering_key = layer_details.ordering_key
                 layer.moveToThread(None)
                 to_upload[layer] = result
 
@@ -487,13 +587,14 @@ def run(self):
 
         if all_group_names:
             # ensure group names match their order in the QGIS project
-            all_group_names = sorted(
-                all_group_names,
-                key=lambda x: self.ordered_top_level_groups.index(x)
-            )
+            ordering_keys = {
+                group: self.project_structure.group_ordering_key(group)
+                for group in all_group_names
+            }
             reply = API_CLIENT.create_layer_groups(
                 map_id=self.associated_map.id,
-                layer_group_names=all_group_names
+                layer_group_names=all_group_names,
+                ordering_keys=ordering_keys
             )
             group_details = json.loads(reply.content().data().decode())
             group_ids = {
@@ -638,23 +739,32 @@ def _upload_progress(sent, total):
                     reply.finished.connect(loop.exit)
                     loop.exec()
 
+            reply = None
             if details.group_name:
                 group_id = group_ids[details.group_name]
                 reply = API_CLIENT.update_layer_details(
                     map_id=self.associated_map.id,
                     layer_id=layer_id,
-                    layer_group_id=group_id
+                    layer_group_id=group_id,
+                    ordering_key=details.ordering_key,
                 )
-                if reply.error() != QNetworkReply.NoError:
-                    self.error_string = reply.errorString()
-                    Logger.instance().log_error_json(
-                        {
-                            'type': Logger.MAP_EXPORT,
-                            'error': 'Error updating layer group: {}'.format(
-                                self.error_string)
-                        }
-                    )
-                    return False
+            elif details.ordering_key is not None:
+                reply = API_CLIENT.update_layer_details(
+                    map_id=self.associated_map.id,
+                    layer_id=layer_id,
+                    ordering_key=details.ordering_key,
+                )
+
+            if reply and reply.error() != QNetworkReply.NoError:
+                self.error_string = reply.errorString()
+                Logger.instance().log_error_json(
+                    {
+                        'type': Logger.MAP_EXPORT,
+                        'error': 'Error updating layer details: {}'.format(
+                            self.error_string)
+                    }
+                )
+                return False
 
             multi_step_feedback.step_finished()
 

felt/test/test_map_uploader.py

@@ -16,7 +16,10 @@
 )
 
 from .utilities import get_qgis_app
-from ..core.map_uploader import MapUploaderTask
+from ..core.map_uploader import (
+    MapUploaderTask,
+    SimplifiedProjectStructure
+)
 
 QGIS_APP = get_qgis_app()
 TEST_DATA_PATH = Path(__file__).parent
@@ -51,6 +54,65 @@ def test_map_name(self):
         self.assertEqual(uploader.default_map_title(),
                          'My project title')
 
+    def test_simplified_project_structure(self):
+        """
+        Test generating simplified project structures
+        """
+        project = QgsProject()
+
+        file = str(TEST_DATA_PATH / 'points.gpkg')
+        layer1 = QgsVectorLayer(file, 'layer1')
+        layer2 = QgsVectorLayer(file, 'layer2')
+        layer3 = QgsVectorLayer(file, 'layer3')
+        layer4 = QgsVectorLayer(file, 'layer4')
+
+        project.addMapLayer(layer1, addToLegend=False)
+        project.addMapLayer(layer2, addToLegend=False)
+        project.addMapLayer(layer3, addToLegend=False)
+        project.addMapLayer(layer4, addToLegend=False)
+
+        transport_group: QgsLayerTreeGroup = (
+            project.layerTreeRoot().addGroup('Transport'))
+        transport_group.addLayer(layer1)
+        lines_group = transport_group.addGroup('Lines')
+        lines_group.addLayer(layer2)
+
+        environment_group = project.layerTreeRoot().addGroup('Environment')
+        environment_group.addLayer(layer3)
+
+        project.layerTreeRoot().addLayer(layer4)
+
+        structure = SimplifiedProjectStructure.from_project(project)
+        self.assertEqual(len(structure.components), 6)
+        self.assertEqual(structure.components[0].object_index, 0)
+        self.assertEqual(structure.components[0].group_name, 'Transport')
+        self.assertEqual(structure.components[1].object_index, 1)
+        self.assertEqual(structure.components[1].layer.layer, layer1)
+        self.assertEqual(structure.components[1].layer.destination_group_name,
+                         'Transport')
+        self.assertEqual(structure.components[2].object_index, 2)
+        self.assertEqual(structure.components[2].layer.layer, layer2)
+        self.assertEqual(structure.components[2].layer.destination_group_name,
+                         'Transport')
+        self.assertEqual(structure.components[3].object_index, 3)
+        self.assertEqual(structure.components[3].group_name, 'Environment')
+        self.assertEqual(structure.components[4].object_index, 4)
+        self.assertEqual(structure.components[4].layer.layer, layer3)
+        self.assertEqual(structure.components[4].layer.destination_group_name,
+                         'Environment')
+        self.assertEqual(structure.components[5].object_index, 5)
+        self.assertEqual(structure.components[5].layer.layer, layer4)
+        self.assertFalse(structure.components[5].layer.destination_group_name)
+
+        self.assertEqual(structure.group_ordering_key('Transport'), 6)
+        self.assertEqual(structure.group_ordering_key('Environment'), 3)
+        self.assertIsNone(structure.group_ordering_key('x'))
+
+        self.assertEqual(structure.find_layer(layer1).object_index, 1)
+        self.assertEqual(structure.find_layer(layer2).object_index, 2)
+        self.assertEqual(structure.find_layer(layer3).object_index, 4)
+        self.assertEqual(structure.find_layer(layer4).object_index, 5)
+
     def test_group_names(self):
         """
         Test destination group name logic
@@ -83,16 +145,20 @@ def test_group_names(self):
         details1 = MapUploaderTask.layer_and_group(project, layer1)
         self.assertEqual(details1.layer.name(), 'layer1')
         self.assertEqual(details1.destination_group_name, 'Transport')
+        self.assertEqual(details1.ordering_key, 5)
         details2 = MapUploaderTask.layer_and_group(project, layer2)
         self.assertEqual(details2.layer.name(), 'layer2')
+        self.assertEqual(details2.ordering_key, 4)
         # this must be the top level group name, not "Lines"
         self.assertEqual(details2.destination_group_name, 'Transport')
         details3 = MapUploaderTask.layer_and_group(project, layer3)
         self.assertEqual(details3.layer.name(), 'layer3')
         self.assertEqual(details3.destination_group_name, 'Environment')
+        self.assertEqual(details3.ordering_key, 2)
         details4 = MapUploaderTask.layer_and_group(project, layer4)
         self.assertEqual(details4.layer.name(), 'layer4')
         self.assertIsNone(details4.destination_group_name)
+        self.assertEqual(details4.ordering_key, 1)
 
 
 if __name__ == "__main__":