IDE-69 End of enclosing bricks (loops, conditionals etc) can't be moved

catroid/src/main/java/org/catrobat/catroid/ui/dragndrop/BrickListView.kt

@@ -39,6 +39,8 @@ import android.widget.ListAdapter
 import android.widget.ListView
 import androidx.annotation.VisibleForTesting
 import org.catrobat.catroid.content.bricks.Brick
+import org.catrobat.catroid.content.bricks.CompositeBrick
+import org.catrobat.catroid.content.bricks.EndBrick
 import java.util.ArrayList
 
 private const val SMOOTH_SCROLL_BY = 15
@@ -104,11 +106,15 @@ class BrickListView : ListView {
         cancelMove()
         val flatList: MutableList<Brick> = ArrayList()
         brickToMove?.addToFlatList(flatList)
-        if (brickToMove !== flatList[0]) {
+        if (brickToMove?.parent is CompositeBrick && brickToMove is EndBrick) {
+            this.brickToMove = brickToMove
+            flatList.clear()
+        } else if (brickToMove !== flatList[0]) {
             return
+        } else {
+            this.brickToMove = flatList[0]
+            flatList.removeAt(0)
         }
-        this.brickToMove = flatList[0]
-        flatList.removeAt(0)
 
         upperScrollBound = height / UPPER_SCROLL_BOUND_DIVISOR
         lowerScrollBound = height / LOWER_SCROLL_BOUND_DIVISOR

catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/adapter/BrickAdapter.kt

@@ -34,7 +34,9 @@ import androidx.annotation.IntDef
 import org.catrobat.catroid.content.Script
 import org.catrobat.catroid.content.Sprite
 import org.catrobat.catroid.content.bricks.Brick
+import org.catrobat.catroid.content.bricks.CompositeBrick
 import org.catrobat.catroid.content.bricks.EmptyEventBrick
+import org.catrobat.catroid.content.bricks.EndBrick
 import org.catrobat.catroid.content.bricks.FormulaBrick
 import org.catrobat.catroid.content.bricks.ListSelectorBrick
 import org.catrobat.catroid.content.bricks.ScriptBrick
@@ -365,18 +367,120 @@ class BrickAdapter(private val sprite: Sprite) :
         if (source !is ScriptBrick && targetPosition == 0) {
             return false
         }
-        if (source.allParts.contains(items[targetPosition])) {
+        if (source !is EndBrick && source.allParts.contains(items[targetPosition])) {
             return false
         }
         Collections.swap(items, sourcePosition, targetPosition)
         return true
     }
 
+    private fun getParentBrickInDragAndDropList(
+        brickAboveTarget: Brick,
+        enclosureBrick: Brick
+    ): Pair<Brick, Int>? {
+
+        if (brickAboveTarget == enclosureBrick) {
+            return brickAboveTarget to 0
+        }
+
+        var brickInEnclosure = brickAboveTarget
+        while (brickInEnclosure.parent !== null &&
+            brickInEnclosure.parent !in enclosureBrick.allParts &&
+            brickInEnclosure !in enclosureBrick.dragAndDropTargetList
+        ) {
+
+            brickInEnclosure = brickInEnclosure.parent
+        }
+
+        if (brickInEnclosure.parent !== enclosureBrick &&
+            brickInEnclosure !in enclosureBrick.dragAndDropTargetList
+        ) {
+            return null
+        }
+
+        return brickInEnclosure to
+            enclosureBrick.dragAndDropTargetList.indexOf(brickInEnclosure) + 1
+    }
+
+    private fun moveEndIntoExtendedSection(
+        position: Int,
+        endBrick: Brick,
+        brickAboveTargetPosition: Brick
+    ): Boolean {
+        var tmpParent = brickAboveTargetPosition
+        val firstPart = endBrick.parent
+        while (tmpParent.parent != null) {
+            if (tmpParent is CompositeBrick) {
+                moveItemTo(position, firstPart)
+                return true
+            }
+            tmpParent = tmpParent.parent
+            if (tmpParent !is CompositeBrick || tmpParent == firstPart.parent) {
+                break
+            }
+        }
+        return false
+    }
+
+    private fun moveEndTo(position: Int, endBrick: Brick, brickAboveTargetPosition: Brick) {
+        if (endBrick.script !== brickAboveTargetPosition.script) {
+            return
+        }
+
+        var startBrick = endBrick.parent
+        var enclosure = (startBrick as CompositeBrick).nestedBricks
+        if (startBrick.hasSecondaryList()) {
+            enclosure = startBrick.secondaryNestedBricks
+            startBrick = startBrick.allParts[1]
+        }
+
+        val (parentOfBrickAboveTargetPosition, destinationPosition) =
+            getParentBrickInDragAndDropList(brickAboveTargetPosition, startBrick)
+                ?: (null to 0)
+
+        val indexStartBrick = getPosition(startBrick)
+        if (getPosition(brickAboveTargetPosition) + 1 < indexStartBrick) {
+            return
+        }
+
+        if (parentOfBrickAboveTargetPosition == null) {
+            if (moveEndIntoExtendedSection(position, endBrick, brickAboveTargetPosition)) {
+                return
+            }
+
+            var (outsideParent, outPosition) =
+                getParentBrickInDragAndDropList(
+                    brickAboveTargetPosition,
+                    endBrick.parent.parent
+                )
+                    ?: return
+            outsideParent = outsideParent.parent
+
+            val startIndex =
+                outsideParent.dragAndDropTargetList.indexOf(endBrick.parent) + 1
+            for (i in startIndex until outPosition) {
+                val brick = outsideParent.dragAndDropTargetList.removeAt(startIndex)
+                brick.parent = startBrick
+                enclosure.add(brick)
+            }
+        } else {
+            val parentOfCompBrick = endBrick.parent.parent
+            val positionInList = parentOfCompBrick.dragAndDropTargetList.indexOf(endBrick.parent)
+            for (index in (destinationPosition until enclosure.size).withIndex()) {
+                val brick = enclosure.removeAt(destinationPosition)
+                brick.parent = parentOfCompBrick
+                parentOfCompBrick.dragAndDropTargetList.add(positionInList + index.index + 1, brick)
+            }
+        }
+    }
+
     override fun moveItemTo(position: Int, itemToMove: Brick?) {
         val brickAboveTargetPosition = getBrickAbovePosition(position)
 
         if (itemToMove is ScriptBrick) {
             moveScript(itemToMove, brickAboveTargetPosition)
+        } else if (itemToMove is EndBrick) {
+            moveEndTo(position, itemToMove, brickAboveTargetPosition)
         } else {
             for (script in scripts) {
                 script.removeBrick(itemToMove)

catroid/src/test/java/org/catrobat/catroid/test/ui/brickadapter/EndBrickDragAndDropTest.kt

@@ -0,0 +1,234 @@
+/*
+ * Catroid: An on-device visual programming system for Android devices
+ * Copyright (C) 2010-2023 The Catrobat Team
+ * (<http://developer.catrobat.org/credits>)
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * An additional term exception under section 7 of the GNU Affero
+ * General Public License, version 3, is available at
+ * http://developer.catrobat.org/license_additional_term
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package org.catrobat.catroid.test.ui.brickadapter
+
+import org.catrobat.catroid.content.Script
+import org.catrobat.catroid.content.Sprite
+import org.catrobat.catroid.content.StartScript
+import org.catrobat.catroid.content.bricks.ChangeSizeByNBrick
+import org.catrobat.catroid.content.bricks.ForeverBrick
+import org.catrobat.catroid.content.bricks.IfLogicBeginBrick
+import org.catrobat.catroid.content.bricks.RepeatBrick
+import org.catrobat.catroid.content.bricks.SetXBrick
+import org.catrobat.catroid.ui.recyclerview.adapter.BrickAdapter
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.mockito.Mockito.mock
+import org.mockito.Mockito.`when`
+
+@RunWith(JUnit4::class)
+class EndBrickDragAndDropTest {
+    lateinit var adapter: BrickAdapter
+    lateinit var script: Script
+    var ifLogicBeginBrick = IfLogicBeginBrick()
+    var foreverBrick = ForeverBrick()
+    var repeatBrick = RepeatBrick()
+    var changeSizeByNBrick = ChangeSizeByNBrick()
+    var setXBrick = SetXBrick()
+    lateinit var sprite: Sprite
+    val scripts: MutableList<Script> = ArrayList()
+
+    @Before
+    fun setUp() {
+        script = StartScript()
+
+        scripts.add(script)
+
+        sprite = mock(Sprite::class.java)
+        `when`(sprite.scriptList).thenReturn(scripts)
+    }
+
+    @Test
+    fun testDragDownEndBrick() {
+        script.addBrick(ifLogicBeginBrick)
+        script.addBrick(changeSizeByNBrick)
+        script.addBrick(setXBrick)
+        adapter = BrickAdapter(sprite)
+
+        val endBrick = adapter.items[3]
+
+        assertTrue(adapter.onItemMove(3, 4))
+        assertTrue(adapter.onItemMove(4, 5))
+
+        adapter.moveItemTo(5, endBrick)
+
+        assertTrue(ifLogicBeginBrick.secondaryNestedBricks.contains(changeSizeByNBrick))
+        assertEquals(0, ifLogicBeginBrick.secondaryNestedBricks.indexOf(changeSizeByNBrick))
+
+        assertTrue(ifLogicBeginBrick.secondaryNestedBricks.contains(setXBrick))
+        assertEquals(1, ifLogicBeginBrick.secondaryNestedBricks.indexOf(setXBrick))
+
+        assertFalse(script.brickList.contains(changeSizeByNBrick))
+        assertFalse(script.brickList.contains(setXBrick))
+    }
+
+    @Test
+    fun testDragUpEndBrick() {
+        ifLogicBeginBrick.addBrickToElseBranch(changeSizeByNBrick)
+        ifLogicBeginBrick.addBrickToElseBranch(setXBrick)
+        script.addBrick(ifLogicBeginBrick)
+        adapter = BrickAdapter(sprite)
+
+        val endBrick = adapter.items[5]
+
+        assertTrue(adapter.onItemMove(5, 4))
+        assertTrue(adapter.onItemMove(4, 3))
+
+        adapter.moveItemTo(3, endBrick)
+
+        assertTrue(script.brickList.contains(changeSizeByNBrick))
+        assertEquals(1, script.brickList.indexOf(changeSizeByNBrick))
+
+        assertTrue(script.brickList.contains(setXBrick))
+        assertEquals(2, script.brickList.indexOf(setXBrick))
+
+        assertFalse(ifLogicBeginBrick.secondaryNestedBricks.contains(changeSizeByNBrick))
+        assertFalse(ifLogicBeginBrick.secondaryNestedBricks.contains(setXBrick))
+    }
+
+    @Test
+    fun testDragEndBrickDownIntoAnotherEnclosure() {
+        ifLogicBeginBrick.addBrickToElseBranch(changeSizeByNBrick)
+        ifLogicBeginBrick.addBrickToElseBranch(setXBrick)
+        script.addBrick(ifLogicBeginBrick)
+
+        script.addBrick(foreverBrick)
+        adapter = BrickAdapter(sprite)
+
+        val endBrick = adapter.items[5]
+
+        assertTrue(adapter.onItemMove(5, 6))
+
+        adapter.moveItemTo(6, endBrick)
+
+        assertTrue(foreverBrick.dragAndDropTargetList.contains(ifLogicBeginBrick))
+        assertFalse(script.brickList.contains(ifLogicBeginBrick))
+
+        assertEquals(0, ifLogicBeginBrick.secondaryNestedBricks.indexOf(changeSizeByNBrick))
+        assertEquals(1, ifLogicBeginBrick.secondaryNestedBricks.indexOf(setXBrick))
+    }
+
+    @Test
+    fun testDragEndBrickDownIntoAnotherEnclosureInsideEnclosure() {
+        repeatBrick.addBrick(setXBrick)
+        script.addBrick(repeatBrick)
+
+        foreverBrick.addBrick(ifLogicBeginBrick)
+        script.addBrick(foreverBrick)
+
+        adapter = BrickAdapter(sprite)
+
+        val endBrick = adapter.items[3]
+
+        assertTrue(adapter.onItemMove(3, 4))
+        assertTrue(adapter.onItemMove(4, 5))
+        assertTrue(adapter.onItemMove(5, 6))
+
+        adapter.moveItemTo(6, endBrick)
+
+        assertFalse(script.brickList.contains(repeatBrick))
+
+        assertTrue(ifLogicBeginBrick.secondaryNestedBricks.contains(repeatBrick))
+        assertTrue(repeatBrick.dragAndDropTargetList.contains(setXBrick))
+    }
+
+    @Test
+    fun testDragEndBrickAboveElseBrick() {
+        ifLogicBeginBrick.addBrickToElseBranch(changeSizeByNBrick)
+        ifLogicBeginBrick.addBrickToElseBranch(setXBrick)
+        script.addBrick(ifLogicBeginBrick)
+        adapter = BrickAdapter(sprite)
+
+        val endBrick = adapter.items[5]
+
+        assertTrue(adapter.onItemMove(5, 4))
+        assertTrue(adapter.onItemMove(4, 3))
+        assertTrue(adapter.onItemMove(3, 2))
+
+        adapter.moveItemTo(2, endBrick)
+        assertEquals(endBrick, adapter.items[5])
+
+        assertFalse(script.brickList.contains(changeSizeByNBrick))
+        assertFalse(script.brickList.contains(setXBrick))
+
+        assertTrue(ifLogicBeginBrick.secondaryNestedBricks.contains(changeSizeByNBrick))
+        assertEquals(0, ifLogicBeginBrick.secondaryNestedBricks.indexOf(changeSizeByNBrick))
+
+        assertTrue(ifLogicBeginBrick.secondaryNestedBricks.contains(setXBrick))
+        assertEquals(1, ifLogicBeginBrick.secondaryNestedBricks.indexOf(setXBrick))
+    }
+
+    @Test
+    fun testDragEndBrickAboveHead() {
+        foreverBrick.addBrick(setXBrick)
+        script.addBrick(foreverBrick)
+        adapter = BrickAdapter(sprite)
+
+        val endBrick = adapter.items[3]
+
+        assertTrue(adapter.onItemMove(3, 2))
+        assertTrue(adapter.onItemMove(2, 1))
+
+        adapter.moveItemTo(1, endBrick)
+
+        assertEquals(foreverBrick, adapter.items[1])
+
+        assertTrue(foreverBrick.dragAndDropTargetList.contains(setXBrick))
+        assertFalse(script.brickList.contains(setXBrick))
+
+        assertEquals(endBrick, adapter.items[3])
+    }
+
+    @Test
+    fun testDragEndBrickIntoOtherScript() {
+        val scriptOther: Script
+        scriptOther = StartScript()
+        scripts.add(scriptOther)
+        scriptOther.addBrick(setXBrick)
+
+        ifLogicBeginBrick.addBrickToElseBranch(changeSizeByNBrick)
+        script.addBrick(ifLogicBeginBrick)
+
+        adapter = BrickAdapter(sprite)
+
+        val endBrick = adapter.items[4]
+
+        assertTrue(adapter.onItemMove(4, 5))
+
+        adapter.moveItemTo(5, endBrick)
+        assertEquals(endBrick, adapter.items[4])
+
+        assertFalse(scriptOther.brickList.contains(endBrick))
+        assertFalse(scriptOther.brickList.contains(ifLogicBeginBrick))
+
+        assertTrue(script.brickList.contains(ifLogicBeginBrick))
+
+        assertEquals(changeSizeByNBrick, adapter.items[3])
+    }
+}