Merge branch 'master' of github.com:budibase/budibase into budi-8579-issue-with-google-sheets

.github/workflows/pr-labeler.yml

@@ -21,6 +21,7 @@ jobs:
           l_max_size: "1000"
           fail_if_xl: "false"
           files_to_ignore: "yarn.lock"
+          message_if_xl: ""
 
   team-labeler:
     runs-on: ubuntu-latest

README.md

@@ -65,7 +65,7 @@ Budibase is open-source - licensed as GPL v3. This should fill you with confiden
 <br /><br />
 
 ### Load data or start from scratch
-Budibase pulls data from multiple sources, including MongoDB, CouchDB, PostgreSQL, MySQL, Airtable, S3, DynamoDB, or a REST API. And unlike other platforms, with Budibase you can start from scratch and create business apps with no data sources. [Request new datasources](https://github.com/Budibase/budibase/discussions?discussions_q=category%3AIdeas).
+Budibase pulls data from multiple sources, including MongoDB, CouchDB, PostgreSQL, MariaDB, MySQL, Airtable, S3, DynamoDB, or a REST API. And unlike other platforms, with Budibase you can start from scratch and create business apps with no data sources. [Request new datasources](https://github.com/Budibase/budibase/discussions?discussions_q=category%3AIdeas).
 
 <p align="center">
   <img alt="Budibase data" src="https://res.cloudinary.com/daog6scxm/image/upload/v1680281798/ui/data_klbuna.png">

lerna.json

@@ -1,6 +1,6 @@
 {
   "$schema": "node_modules/lerna/schemas/lerna-schema.json",
-  "version": "2.31.5",
+  "version": "2.31.7",
   "npmClient": "yarn",
   "packages": [
     "packages/*",

packages/builder/src/pages/builder/portal/_components/LockedFeature.svelte

@@ -1,10 +1,12 @@
 <script>
   import {
+    AbsTooltip,
     Layout,
     Heading,
     Body,
     Button,
     Divider,
+    Icon,
     Tags,
     Tag,
   } from "@budibase/bbui"
@@ -15,6 +17,8 @@
   export let description
   export let enabled
   export let upgradeButtonClick
+
+  $: upgradeDisabled = !$auth.accountPortalAccess && $admin.cloud
 </script>
 
 <Layout noPadding>
@@ -36,8 +40,9 @@
   {:else}
     <div class="buttons">
       <Button
-        primary
+        primary={!upgradeDisabled}
-        disabled={!$auth.accountPortalAccess && $admin.cloud}
+        secondary={upgradeDisabled}
+        disabled={upgradeDisabled}
         on:click={async () => upgradeButtonClick()}
       >
         Upgrade
@@ -51,6 +56,16 @@
       >
         View Plans
       </Button>
+      {#if upgradeDisabled}
+        <AbsTooltip
+          text={"Please contact the account holder to upgrade"}
+          position={"right"}
+        >
+          <div class="icon" on:focus>
+            <Icon name="InfoOutline" size="L" disabled hoverable />
+          </div>
+        </AbsTooltip>
+      {/if}
     </div>
   {/if}
 </Layout>
@@ -67,7 +82,11 @@
     justify-content: flex-start;
     gap: var(--spacing-m);
   }
-
+  .icon {
+    position: relative;
+    display: flex;
+    justify-content: center;
+  }
   .buttons {
     display: flex;
     flex-direction: row;

packages/builder/src/pages/builder/portal/users/users/index.svelte

@@ -127,7 +127,10 @@
         name: user.firstName ? user.firstName + " " + user.lastName : "",
         userGroups,
         __selectable:
-          role.value === Constants.BudibaseRoles.Owner ? false : undefined,
+          role.value === Constants.BudibaseRoles.Owner ||
+          $auth.user?.email === user.email
+            ? false
+            : true,
         apps: [...new Set(Object.keys(user.roles))],
         access: role.sortOrder,
       }
@@ -392,7 +395,7 @@
     allowSelectRows={!readonly}
     {customRenderers}
     loading={!$fetch.loaded || !groupsLoaded}
-    defaultSortColumn={"access"}
+    defaultSortColumn={"__selectable"}
   />
 
   <div class="pagination">

packages/client/src/components/Component.svelte

@@ -153,7 +153,7 @@
   $: builderInteractive =
     $builderStore.inBuilder && insideScreenslot && !isBlock && !instance.static
   $: devToolsInteractive = $devToolsStore.allowSelection && !isBlock
-  $: interactive = !isRoot && (builderInteractive || devToolsInteractive)
+  $: interactive = builderInteractive || devToolsInteractive
   $: editing = editable && selected && $builderStore.editMode
   $: draggable =
     !inDragPath &&
@@ -189,12 +189,6 @@
   // Scroll the selected element into view
   $: selected && scrollIntoView()
 
-  // When dragging and dropping, pad components to allow dropping between
-  // nested layers. Only reset this when dragging stops.
-  let pad = false
-  $: pad = pad || (interactive && hasChildren && inDndPath)
-  $: $dndIsDragging, (pad = false)
-
   // Themes
   $: currentTheme = $context?.device?.theme
   $: darkMode = !currentTheme?.includes("light")
@@ -206,8 +200,10 @@
   }
 
   // Metadata to pass into grid action to apply CSS
-  const insideGrid =
+  const checkGrid = x =>
-    parent?._component.endsWith("/container") && parent?.layout === "grid"
+    x?._component?.endsWith("/container") && x?.layout === "grid"
+  $: insideGrid = checkGrid(parent)
+  $: isGrid = checkGrid(instance)
   $: gridMetadata = {
     insideGrid,
     ignoresLayout: definition?.ignoresLayout === true,
@@ -219,6 +215,12 @@
     errored: errorState,
   }
 
+  // When dragging and dropping, pad components to allow dropping between
+  // nested layers. Only reset this when dragging stops.
+  let pad = false
+  $: pad = pad || (!isGrid && interactive && hasChildren && inDndPath)
+  $: $dndIsDragging, (pad = false)
+
   // Update component context
   $: store.set({
     id,
@@ -231,12 +233,14 @@
       empty: emptyState,
       selected,
       interactive,
+      isRoot,
       draggable,
       editable,
       isBlock,
     },
     empty: emptyState,
     selected,
+    isRoot,
     inSelectedPath,
     name,
     editing,
@@ -672,6 +676,7 @@
     class:parent={hasChildren}
     class:block={isBlock}
     class:error={errorState}
+    class:root={isRoot}
     data-id={id}
     data-name={name}
     data-icon={icon}

packages/client/src/components/Screen.svelte

@@ -14,7 +14,6 @@
 
   // Get the screen definition for the current route
   $: screen = $screenStore.activeScreen
-  $: screenDefinition = { ...screen?.props, addEmptyRows: true }
   $: onLoadActions.set(screen?.onLoad)
   $: runOnLoadActions($onLoadActions, params)
 
@@ -42,10 +41,10 @@
 </script>
 
 <!-- Ensure to fully remount when screen changes -->
-{#if $routeStore.routerLoaded}
+{#if $routeStore.routerLoaded && screen?.props}
-  {#key screenDefinition?._id}
+  {#key screen.props._id}
     <Provider key="url" data={params}>
-      <Component isRoot instance={screenDefinition} />
+      <Component isRoot instance={screen.props} />
     </Provider>
   {/key}
 {/if}

packages/client/src/components/app/EmptyPlaceholder.svelte

@@ -18,9 +18,11 @@
     display: flex;
     flex-direction: row;
     justify-content: flex-start;
-    align-items: center;
+    align-items: flex-start;
     color: var(--spectrum-global-color-gray-600);
     font-size: var(--font-size-s);
     gap: var(--spacing-s);
+    grid-column: 1 / -1;
+    grid-row: 1 / -1;
   }
 </style>

packages/client/src/components/app/ScreenPlaceholder.svelte

@@ -23,6 +23,8 @@
     align-items: center;
     gap: var(--spacing-s);
     flex: 1 1 auto;
+    grid-column: 1 / -1;
+    grid-row: 1 / -1;
   }
   .placeholder :global(.spectrum-Button) {
     margin-top: var(--spacing-m);

packages/client/src/components/app/container/GridContainer.svelte

@@ -4,8 +4,6 @@
   import { GridRowHeight, GridColumns } from "constants"
   import { memo } from "@budibase/frontend-core"
 
-  export let addEmptyRows = false
-
   const component = getContext("component")
   const { styleable, builderStore } = getContext("sdk")
   const context = getContext("context")
@@ -18,16 +16,12 @@
   let styles = memo({})
 
   $: inBuilder = $builderStore.inBuilder
-  $: requiredRows = calculateRequiredRows(
+  $: addEmptyRows = $component.isRoot && inBuilder
-    $children,
+  $: requiredRows = calculateRequiredRows($children, mobile, addEmptyRows)
-    mobile,
-    addEmptyRows && inBuilder
-  )
   $: requiredHeight = requiredRows * GridRowHeight
   $: availableRows = Math.floor(height / GridRowHeight)
   $: rows = Math.max(requiredRows, availableRows)
   $: mobile = $context.device.mobile
-  $: empty = $component.empty
   $: colSize = width / GridColumns
   $: styles.set({
     ...$component.styles,
@@ -40,7 +34,6 @@
       "--col-size": colSize,
       "--row-size": GridRowHeight,
     },
-    empty: false,
   })
 
   // Calculates the minimum number of rows required to render all child
@@ -145,9 +138,7 @@
       {/each}
     </div>
   {/if}
-
+  {#if mounted}
-  <!-- Only render the slot if not empty, as we don't want the placeholder -->
-  {#if !empty && mounted}
     <slot />
   {/if}
 </div>

packages/client/src/components/preview/DNDPlaceholderOverlay.svelte

@@ -6,8 +6,11 @@
   let left, top, height, width
 
   const updatePosition = () => {
-    const node =
+    let node = document.getElementsByClassName(DNDPlaceholderID)[0]
-      document.getElementsByClassName(DNDPlaceholderID)[0]?.childNodes[0]
+    const insideGrid = node?.dataset.insideGrid === "true"
+    if (!insideGrid) {
+      node = document.getElementsByClassName(`${DNDPlaceholderID}-dom`)[0]
+    }
     if (!node) {
       height = 0
       width = 0

packages/client/src/components/preview/HoverIndicator.svelte

@@ -19,7 +19,7 @@
       newId = e.target.dataset.id
     } else {
       // Handle normal components
-      const element = e.target.closest(".interactive.component")
+      const element = e.target.closest(".interactive.component:not(.root)")
       newId = element?.dataset?.id
     }
 

packages/client/src/stores/dnd.js

@@ -1,5 +1,5 @@
 import { writable } from "svelte/store"
-import { computed } from "../utils/computed.js"
+import { derivedMemo } from "@budibase/frontend-core"
 
 const createDndStore = () => {
   const initialState = {
@@ -78,11 +78,11 @@ export const dndStore = createDndStore()
 // performance by deriving any state that needs to be externally observed.
 // By doing this and using primitives, we can avoid invalidating other stores
 // or components which depend on DND state unless values actually change.
-export const dndParent = computed(dndStore, x => x.drop?.parent)
+export const dndParent = derivedMemo(dndStore, x => x.drop?.parent)
-export const dndIndex = computed(dndStore, x => x.drop?.index)
+export const dndIndex = derivedMemo(dndStore, x => x.drop?.index)
-export const dndBounds = computed(dndStore, x => x.source?.bounds)
+export const dndBounds = derivedMemo(dndStore, x => x.source?.bounds)
-export const dndIsDragging = computed(dndStore, x => !!x.source)
+export const dndIsDragging = derivedMemo(dndStore, x => !!x.source)
-export const dndIsNewComponent = computed(
+export const dndIsNewComponent = derivedMemo(
   dndStore,
   x => x.source?.newComponentType != null
 )

packages/client/src/stores/screens.js

@@ -92,6 +92,8 @@ const createScreenStore = () => {
               width: `${$dndBounds?.width || 400}px`,
               height: `${$dndBounds?.height || 200}px`,
               opacity: 0,
+              "--default-width": $dndBounds?.width || 400,
+              "--default-height": $dndBounds?.height || 200,
             },
           },
           static: true,

packages/client/src/utils/computed.js

@@ -1,38 +0,0 @@
-import { writable } from "svelte/store"
-
-/**
- * Extension of Svelte's built in "derived" stores, which the addition of deep
- * comparison of non-primitives. Falls back to using shallow comparison for
- * primitive types to avoid performance penalties.
- * Useful for instances where a deep comparison is cheaper than an additional
- * store invalidation.
- * @param store the store to observer
- * @param deriveValue the derivation function
- * @returns {Writable<*>} a derived svelte store containing just the derived value
- */
-export const computed = (store, deriveValue) => {
-  const initialValue = deriveValue(store)
-  const computedStore = writable(initialValue)
-  let lastKey = getKey(initialValue)
-
-  store.subscribe(state => {
-    const value = deriveValue(state)
-    const key = getKey(value)
-    if (key !== lastKey) {
-      lastKey = key
-      computedStore.set(value)
-    }
-  })
-
-  return computedStore
-}
-
-// Helper function to serialise any value into a primitive which can be cheaply
-// and shallowly compared
-const getKey = value => {
-  if (value == null || typeof value !== "object") {
-    return value
-  } else {
-    return JSON.stringify(value)
-  }
-}

packages/client/src/utils/grid.js

@@ -92,8 +92,12 @@ export const gridLayout = (node, metadata) => {
     }
 
     // Determine default width and height of component
-    let width = errored ? 500 : definition?.size?.width || 200
+    let width = styles["--default-width"] ?? definition?.size?.width ?? 200
-    let height = errored ? 60 : definition?.size?.height || 200
+    let height = styles["--default-height"] ?? definition?.size?.height ?? 200
+    if (errored) {
+      width = 500
+      height = 60
+    }
     width += 2 * GridSpacing
     height += 2 * GridSpacing
     let vars = {

packages/client/src/utils/styleable.js

@@ -93,7 +93,7 @@ export const styleable = (node, styles = {}) => {
     node.addEventListener("mouseout", applyNormalStyles)
 
     // Add builder preview click listener
-    if (newStyles.interactive) {
+    if (newStyles.interactive && !newStyles.isRoot) {
       node.addEventListener("click", selectComponent, false)
       node.addEventListener("dblclick", editComponent, false)
     }

packages/server/src/api/routes/tests/automation.spec.ts

@@ -15,6 +15,7 @@ import { Automation, FieldType, Table } from "@budibase/types"
 import { mocks } from "@budibase/backend-core/tests"
 import { FilterConditions } from "../../../automations/steps/filter"
 import { removeDeprecated } from "../../../automations/utils"
+import { createAutomationBuilder } from "../../../automations/tests/utilities/AutomationTestBuilder"
 
 const MAX_RETRIES = 4
 let {
@@ -121,6 +122,104 @@ describe("/automations", () => {
       expect(events.automation.stepCreated).toHaveBeenCalledTimes(2)
     })
 
+    it("Should ensure you can't have a branch as not a last step", async () => {
+      const automation = createAutomationBuilder({
+        name: "String Equality Branching",
+        appId: config.getAppId(),
+      })
+        .appAction({ fields: { status: "active" } })
+        .branch({
+          activeBranch: {
+            steps: stepBuilder =>
+              stepBuilder.serverLog({ text: "Active user" }),
+            condition: {
+              equal: { "trigger.fields.status": "active" },
+            },
+          },
+        })
+        .serverLog({ text: "Inactive user" })
+        .build()
+
+      await config.api.automation.post(automation, {
+        status: 400,
+        body: {
+          message:
+            "Invalid body - Branch steps are only allowed as the last step",
+        },
+      })
+    })
+
+    it("Should check validation on an automation that has a branch step with no children", async () => {
+      const automation = createAutomationBuilder({
+        name: "String Equality Branching",
+        appId: config.getAppId(),
+      })
+        .appAction({ fields: { status: "active" } })
+        .branch({})
+        .serverLog({ text: "Inactive user" })
+        .build()
+
+      await config.api.automation.post(automation, {
+        status: 400,
+        body: {
+          message:
+            'Invalid body - "definition.steps[0].inputs.branches" must contain at least 1 items',
+        },
+      })
+    })
+
+    it("Should check validation on a branch step with empty conditions", async () => {
+      const automation = createAutomationBuilder({
+        name: "String Equality Branching",
+        appId: config.getAppId(),
+      })
+        .appAction({ fields: { status: "active" } })
+        .branch({
+          activeBranch: {
+            steps: stepBuilder =>
+              stepBuilder.serverLog({ text: "Active user" }),
+            condition: {},
+          },
+        })
+        .build()
+
+      await config.api.automation.post(automation, {
+        status: 400,
+        body: {
+          message:
+            'Invalid body - "definition.steps[0].inputs.branches[0].condition" must have at least 1 key',
+        },
+      })
+    })
+
+    it("Should check validation on an branch that has a condition that is not valid", async () => {
+      const automation = createAutomationBuilder({
+        name: "String Equality Branching",
+        appId: config.getAppId(),
+      })
+        .appAction({ fields: { status: "active" } })
+        .branch({
+          activeBranch: {
+            steps: stepBuilder =>
+              stepBuilder.serverLog({ text: "Active user" }),
+            condition: {
+              //@ts-ignore
+              INCORRECT: { "trigger.fields.status": "active" },
+            },
+          },
+        })
+        .serverLog({ text: "Inactive user" })
+        .build()
+
+      await config.api.automation.post(automation, {
+        status: 400,
+        body: {
+          message:
+            'Invalid body - "definition.steps[0].inputs.branches[0].condition.INCORRECT" is not allowed',
+        },
+      })
+    })
+
     it("should apply authorization to endpoint", async () => {
       const automation = newAutomation()
       await checkBuilderEndpoint({

packages/server/src/api/routes/tests/rowAction.spec.ts

@@ -5,11 +5,10 @@ import {
   CreateRowActionRequest,
   DocumentType,
   PermissionLevel,
-  Row,
   RowActionResponse,
 } from "@budibase/types"
 import * as setup from "./utilities"
-import { generator } from "@budibase/backend-core/tests"
+import { generator, mocks } from "@budibase/backend-core/tests"
 import { Expectations } from "../../../tests/utilities/api/base"
 import { roles } from "@budibase/backend-core"
 import { automations } from "@budibase/pro"
@@ -651,13 +650,27 @@ describe("/rowsActions", () => {
   })
 
   describe("trigger", () => {
-    let row: Row
+    let viewId: string
+    let rowId: string
     let rowAction: RowActionResponse
 
     beforeEach(async () => {
-      row = await config.api.row.save(tableId, {})
+      const row = await config.api.row.save(tableId, {})
+      rowId = row._id!
       rowAction = await createRowAction(tableId, createRowActionRequest())
 
+      viewId = (
+        await config.api.viewV2.create(
+          setup.structures.viewV2.createRequest(tableId)
+        )
+      ).id
+
+      await config.api.rowAction.setViewPermission(
+        tableId,
+        viewId,
+        rowAction.id
+      )
+
       await config.publish()
       tk.travel(Date.now() + 100)
     })
@@ -673,9 +686,7 @@ describe("/rowsActions", () => {
 
     it("can trigger an automation given valid data", async () => {
       expect(await getAutomationLogs()).toBeEmpty()
-      await config.api.rowAction.trigger(tableId, rowAction.id, {
+      await config.api.rowAction.trigger(viewId, rowAction.id, { rowId })
-        rowId: row._id!,
-      })
 
       const automationLogs = await getAutomationLogs()
       expect(automationLogs).toEqual([
@@ -687,8 +698,11 @@ describe("/rowsActions", () => {
             inputs: null,
             outputs: {
               fields: {},
-              row: await config.api.row.get(tableId, row._id!),
+              row: await config.api.row.get(tableId, rowId),
-              table: await config.api.table.get(tableId),
+              table: {
+                ...(await config.api.table.get(tableId)),
+                views: expect.anything(),
+              },
               automation: expect.objectContaining({
                 _id: rowAction.automationId,
               }),
@@ -709,9 +723,7 @@ describe("/rowsActions", () => {
       await config.api.rowAction.trigger(
         viewId,
         rowAction.id,
-        {
+        { rowId },
-          rowId: row._id!,
-        },
         {
           status: 403,
           body: {
@@ -738,10 +750,9 @@ describe("/rowsActions", () => {
       )
 
       await config.publish()
+
       expect(await getAutomationLogs()).toBeEmpty()
-      await config.api.rowAction.trigger(viewId, rowAction.id, {
+      await config.api.rowAction.trigger(viewId, rowAction.id, { rowId })
-        rowId: row._id!,
-      })
 
       const automationLogs = await getAutomationLogs()
       expect(automationLogs).toEqual([
@@ -750,5 +761,170 @@ describe("/rowsActions", () => {
         }),
       ])
     })
+
+    describe("role permission checks", () => {
+      beforeAll(() => {
+        mocks.licenses.useViewPermissions()
+      })
+
+      afterAll(() => {
+        mocks.licenses.useCloudFree()
+      })
+
+      function createUser(role: string) {
+        return config.createUser({
+          admin: { global: false },
+          builder: {},
+          roles: { [config.getProdAppId()]: role },
+        })
+      }
+
+      const allowedRoleConfig = (() => {
+        function getRolesLowerThan(role: string) {
+          const result = Object.values(roles.BUILTIN_ROLE_IDS).filter(
+            r => r !== role && roles.lowerBuiltinRoleID(r, role) === r
+          )
+          return result
+        }
+        return Object.values(roles.BUILTIN_ROLE_IDS).flatMap(r =>
+          [r, ...getRolesLowerThan(r)].map(p => [r, p])
+        )
+      })()
+
+      const disallowedRoleConfig = (() => {
+        function getRolesHigherThan(role: string) {
+          const result = Object.values(roles.BUILTIN_ROLE_IDS).filter(
+            r => r !== role && roles.lowerBuiltinRoleID(r, role) === role
+          )
+          return result
+        }
+        return Object.values(roles.BUILTIN_ROLE_IDS).flatMap(r =>
+          getRolesHigherThan(r).map(p => [r, p])
+        )
+      })()
+
+      describe.each([
+        [
+          "view (with implicit views)",
+          async () => {
+            const viewId = (
+              await config.api.viewV2.create(
+                setup.structures.viewV2.createRequest(tableId)
+              )
+            ).id
+
+            await config.api.rowAction.setViewPermission(
+              tableId,
+              viewId,
+              rowAction.id
+            )
+            return { permissionResource: viewId, triggerResouce: viewId }
+          },
+        ],
+        [
+          "view (without implicit views)",
+          async () => {
+            const viewId = (
+              await config.api.viewV2.create(
+                setup.structures.viewV2.createRequest(tableId)
+              )
+            ).id
+
+            await config.api.rowAction.setViewPermission(
+              tableId,
+              viewId,
+              rowAction.id
+            )
+            return { permissionResource: tableId, triggerResouce: viewId }
+          },
+        ],
+      ])("checks for %s", (_, getResources) => {
+        it.each(allowedRoleConfig)(
+          "allows triggering if the user has read permission (user %s, table %s)",
+          async (userRole, resourcePermission) => {
+            const { permissionResource, triggerResouce } = await getResources()
+
+            await config.api.permission.add({
+              level: PermissionLevel.READ,
+              resourceId: permissionResource,
+              roleId: resourcePermission,
+            })
+
+            const normalUser = await createUser(userRole)
+
+            await config.withUser(normalUser, async () => {
+              await config.publish()
+              await config.api.rowAction.trigger(
+                triggerResouce,
+                rowAction.id,
+                { rowId },
+                { status: 200 }
+              )
+            })
+          }
+        )
+
+        it.each(disallowedRoleConfig)(
+          "rejects if the user does not have table read permission (user %s, table %s)",
+          async (userRole, resourcePermission) => {
+            const { permissionResource, triggerResouce } = await getResources()
+            await config.api.permission.add({
+              level: PermissionLevel.READ,
+              resourceId: permissionResource,
+              roleId: resourcePermission,
+            })
+
+            const normalUser = await createUser(userRole)
+
+            await config.withUser(normalUser, async () => {
+              await config.publish()
+              await config.api.rowAction.trigger(
+                triggerResouce,
+                rowAction.id,
+                { rowId },
+                {
+                  status: 403,
+                  body: { message: "User does not have permission" },
+                }
+              )
+
+              const automationLogs = await getAutomationLogs()
+              expect(automationLogs).toBeEmpty()
+            })
+          }
+        )
+      })
+
+      it.each(allowedRoleConfig)(
+        "does not allow running row actions for tables by default even",
+        async (userRole, resourcePermission) => {
+          await config.api.permission.add({
+            level: PermissionLevel.READ,
+            resourceId: tableId,
+            roleId: resourcePermission,
+          })
+
+          const normalUser = await createUser(userRole)
+
+          await config.withUser(normalUser, async () => {
+            await config.publish()
+            await config.api.rowAction.trigger(
+              tableId,
+              rowAction.id,
+              { rowId },
+              {
+                status: 403,
+                body: {
+                  message: `Row action '${rowAction.id}' is not enabled for table '${tableId}'`,
+                },
+              }
+            )
+
+            const automationLogs = await getAutomationLogs()
+            expect(automationLogs).toBeEmpty()
+          })
+        }
+      )
+    })
   })
 })

packages/server/src/api/routes/utils/validators.ts

@@ -1,6 +1,9 @@
 import { auth, permissions } from "@budibase/backend-core"
 import { DataSourceOperation } from "../../../constants"
 import {
+  AutomationActionStepId,
+  AutomationStep,
+  AutomationStepType,
   EmptyFilterOption,
   SearchFilters,
   Table,
@@ -88,7 +91,8 @@ export function datasourceValidator() {
   )
 }
 
-function filterObject() {
+function filterObject(opts?: { unknown: boolean }) {
+  const { unknown = true } = opts || {}
   const conditionalFilteringObject = () =>
     Joi.object({
       conditions: Joi.array().items(Joi.link("#schema")).required(),
@@ -115,7 +119,7 @@ function filterObject() {
     fuzzyOr: Joi.forbidden(),
     documentType: Joi.forbidden(),
   }
-  return Joi.object(filtersValidators).unknown(true).id("schema")
+  return Joi.object(filtersValidators).unknown(unknown).id("schema")
 }
 
 export function internalSearchValidator() {
@@ -259,6 +263,11 @@ export function screenValidator() {
 }
 
 function generateStepSchema(allowStepTypes: string[]) {
+  const branchSchema = Joi.object({
+    name: Joi.string().required(),
+    condition: filterObject({ unknown: false }).required().min(1),
+  })
+
   return Joi.object({
     stepId: Joi.string().required(),
     id: Joi.string().required(),
@@ -267,11 +276,35 @@ function generateStepSchema(allowStepTypes: string[]) {
     tagline: Joi.string().required(),
     icon: Joi.string().required(),
     params: Joi.object(),
+    inputs: Joi.when("stepId", {
+      is: AutomationActionStepId.BRANCH,
+      then: Joi.object({
+        branches: Joi.array().items(branchSchema).min(1).required(),
+        children: Joi.object()
+          .pattern(Joi.string(), Joi.array().items(Joi.link("#step")))
+          .required(),
+      }).required(),
+      otherwise: Joi.object(),
+    }),
+
     args: Joi.object(),
     type: Joi.string()
       .required()
       .valid(...allowStepTypes),
-  }).unknown(true)
+  })
+    .unknown(true)
+    .id("step")
+}
+
+const validateStepsArray = (
+  steps: AutomationStep[],
+  helpers: Joi.CustomHelpers
+) => {
+  for (const step of steps.slice(0, -1)) {
+    if (step.stepId === AutomationActionStepId.BRANCH) {
+      return helpers.error("branchStepPosition")
+    }
+  }
 }
 
 export function automationValidator(existing = false) {
@@ -284,9 +317,20 @@ export function automationValidator(existing = false) {
       definition: Joi.object({
         steps: Joi.array()
           .required()
-          .items(generateStepSchema(["ACTION", "LOGIC"])),
+          .items(
-        trigger: generateStepSchema(["TRIGGER"]).allow(null),
+            generateStepSchema([
+              AutomationStepType.ACTION,
+              AutomationStepType.LOGIC,
+            ])
+          )
+          .custom(validateStepsArray)
+          .messages({
+            branchStepPosition:
+              "Branch steps are only allowed as the last step",
+          }),
+        trigger: generateStepSchema([AutomationStepType.TRIGGER]).allow(null),
       })
+
         .required()
         .unknown(true),
     }).unknown(true)

packages/server/src/automations/tests/scenarios/scenarios.spec.ts

@@ -63,8 +63,8 @@ describe("Automation Scenarios", () => {
           },
         })
         .run()
-
+      expect(results.steps[3].outputs.status).toContain("branch1 branch taken")
-      expect(results.steps[2].outputs.message).toContain("Branch 1.1")
+      expect(results.steps[4].outputs.message).toContain("Branch 1.1")
     })
 
     it("should execute correct branch based on string equality", async () => {
@@ -91,8 +91,10 @@ describe("Automation Scenarios", () => {
           },
         })
         .run()
-
+      expect(results.steps[0].outputs.status).toContain(
-      expect(results.steps[0].outputs.message).toContain("Active user")
+        "activeBranch branch taken"
+      )
+      expect(results.steps[1].outputs.message).toContain("Active user")
     })
 
     it("should handle multiple conditions with AND operator", async () => {
@@ -124,7 +126,7 @@ describe("Automation Scenarios", () => {
         })
         .run()
 
-      expect(results.steps[0].outputs.message).toContain("Active admin user")
+      expect(results.steps[1].outputs.message).toContain("Active admin user")
     })
 
     it("should handle multiple conditions with OR operator", async () => {
@@ -162,7 +164,7 @@ describe("Automation Scenarios", () => {
         })
         .run()
 
-      expect(results.steps[0].outputs.message).toContain("Special user")
+      expect(results.steps[1].outputs.message).toContain("Special user")
     })
   })
 
@@ -362,6 +364,32 @@ describe("Automation Scenarios", () => {
         }
       )
     })
+
+    it("should run an automation where a loop is used twice to ensure context correctness further down the tree", async () => {
+      const builder = createAutomationBuilder({
+        name: "Test Trigger with Loop and Create Row",
+      })
+
+      const results = await builder
+        .appAction({ fields: {} })
+        .loop({
+          option: LoopStepType.ARRAY,
+          binding: [1, 2, 3],
+        })
+        .serverLog({ text: "Message {{loop.currentItem}}" })
+        .serverLog({ text: "{{steps.1.iterations}}" })
+        .loop({
+          option: LoopStepType.ARRAY,
+          binding: [1, 2, 3],
+        })
+        .serverLog({ text: "{{loop.currentItem}}" })
+        .serverLog({ text: "{{steps.3.iterations}}" })
+        .run()
+
+      // We want to ensure that bindings are corr
+      expect(results.steps[1].outputs.message).toContain("- 3")
+      expect(results.steps[3].outputs.message).toContain("- 3")
+    })
   })
 
   describe("Row Automations", () => {

packages/server/src/automations/tests/utilities/AutomationTestBuilder.ts

@@ -179,7 +179,7 @@ class AutomationBuilder extends BaseStepBuilder {
   private triggerOutputs: any
   private triggerSet: boolean = false
 
-  constructor(options: { name?: string } = {}) {
+  constructor(options: { name?: string; appId?: string } = {}) {
     super()
     this.automationConfig = {
       name: options.name || `Test Automation ${uuidv4()}`,
@@ -188,7 +188,7 @@ class AutomationBuilder extends BaseStepBuilder {
         trigger: {} as AutomationTrigger,
       },
       type: "automation",
-      appId: setup.getConfig().getAppId(),
+      appId: options.appId ?? setup.getConfig().getAppId(),
     }
     this.config = setup.getConfig()
   }
@@ -261,13 +261,14 @@ class AutomationBuilder extends BaseStepBuilder {
     return this
   }
 
-  branch(branchConfig: BranchConfig): {
+  branch(branchConfig: BranchConfig): this {
-    run: () => Promise<AutomationResults>
-  } {
     this.addBranchStep(branchConfig)
-    return {
+    return this
-      run: () => this.run(),
   }
+
+  build(): Automation {
+    this.automationConfig.definition.steps = this.steps
+    return this.automationConfig
   }
 
   async run() {
@@ -275,7 +276,7 @@ class AutomationBuilder extends BaseStepBuilder {
       throw new Error("Please add a trigger to this automation test")
     }
     this.automationConfig.definition.steps = this.steps
-    const automation = await this.config.createAutomation(this.automationConfig)
+    const automation = await this.config.createAutomation(this.build())
     const results = await testAutomation(
       this.config,
       automation,
@@ -295,6 +296,9 @@ class AutomationBuilder extends BaseStepBuilder {
   }
 }
 
-export function createAutomationBuilder(options?: { name?: string }) {
+export function createAutomationBuilder(options?: {
+  name?: string
+  appId?: string
+}) {
   return new AutomationBuilder(options)
 }

packages/server/src/middleware/authorized.ts

@@ -88,7 +88,7 @@ const authorized =
     opts = { schema: false },
     resourcePath?: string
   ) =>
-  async (ctx: any, next: any) => {
+  async (ctx: UserCtx, next: any) => {
     // webhooks don't need authentication, each webhook unique
     // also internal requests (between services) don't need authorized
     if (isWebhookEndpoint(ctx) || ctx.internal) {

packages/server/src/middleware/triggerRowActionAuthorised.ts

@@ -1,18 +1,18 @@
 import { Next } from "koa"
-import { Ctx } from "@budibase/types"
+import { PermissionLevel, PermissionType, UserCtx } from "@budibase/types"
-import { paramSubResource } from "./resourceId"
 import { docIds } from "@budibase/backend-core"
 import * as utils from "../db/utils"
 import sdk from "../sdk"
+import { authorizedResource } from "./authorized"
 
 export function triggerRowActionAuthorised(
   sourcePath: string,
   actionPath: string
 ) {
-  return async (ctx: Ctx, next: Next) => {
+  return async (ctx: UserCtx, next: Next) => {
-    // Reusing the existing middleware to extract the value
+    async function getResourceIds() {
-    paramSubResource(sourcePath, actionPath)(ctx, () => {})
+      const sourceId: string = ctx.params[sourcePath]
-    const { resourceId: sourceId, subResourceId: rowActionId } = ctx
+      const rowActionId: string = ctx.params[actionPath]
 
       const isTableId = docIds.isTableId(sourceId)
       const isViewId = utils.isViewID(sourceId)
@@ -23,18 +23,30 @@ export function triggerRowActionAuthorised(
       const tableId = isTableId
         ? sourceId
         : utils.extractViewInfoFromID(sourceId).tableId
+      const viewId = isTableId ? undefined : sourceId
+      return { tableId, viewId, rowActionId }
+    }
 
+    const { tableId, viewId, rowActionId } = await getResourceIds()
+
+    // Check if the user has permissions to the table/view
+    await authorizedResource(
+      !viewId ? PermissionType.TABLE : PermissionType.VIEW,
+      PermissionLevel.READ,
+      sourcePath
+    )(ctx, () => {})
+
+    // Check is the row action can run for the given view/table
     const rowAction = await sdk.rowActions.get(tableId, rowActionId)
-
+    if (!viewId && !rowAction.permissions.table.runAllowed) {
-    if (isTableId && !rowAction.permissions.table.runAllowed) {
       ctx.throw(
         403,
-        `Row action '${rowActionId}' is not enabled for table '${sourceId}'`
+        `Row action '${rowActionId}' is not enabled for table '${tableId}'`
       )
-    } else if (isViewId && !rowAction.permissions.views[sourceId]?.runAllowed) {
+    } else if (viewId && !rowAction.permissions.views[viewId]?.runAllowed) {
       ctx.throw(
         403,
-        `Row action '${rowActionId}' is not enabled for view '${sourceId}'`
+        `Row action '${rowActionId}' is not enabled for view '${viewId}'`
       )
     }
 

packages/server/src/sdk/app/rowActions.ts

@@ -75,7 +75,7 @@ export async function create(tableId: string, rowAction: { name: string }) {
     name: action.name,
     automationId: automation._id!,
     permissions: {
-      table: { runAllowed: true },
+      table: { runAllowed: false },
       views: {},
     },
   }

packages/server/src/tests/utilities/api/automation.ts

@@ -14,4 +14,14 @@ export class AutomationAPI extends TestAPI {
     )
     return result
   }
+  post = async (
+    body: Automation,
+    expectations?: Expectations
+  ): Promise<Automation> => {
+    const result = await this._post<Automation>(`/api/automations`, {
+      body,
+      expectations,
+    })
+    return result
+  }
 }

packages/server/src/threads/automation.ts

@@ -449,7 +449,11 @@ class Orchestrator {
         outputs: tempOutput,
         inputs: steps[stepToLoopIndex].inputs,
       })
-      this.context.steps[currentIndex + 1] = tempOutput
+      this.context.steps[this.context.steps.length] = tempOutput
+      this.context.steps = this.context.steps.filter(
+        item => !item.hasOwnProperty.call(item, "currentItem")
+      )
+
       this.loopStepOutputs = []
     }
 
@@ -461,6 +465,19 @@ class Orchestrator {
     for (const branch of branches) {
       const condition = await this.evaluateBranchCondition(branch.condition)
       if (condition) {
+        let branchStatus = {
+          status: `${branch.name} branch taken`,
+          success: true,
+        }
+
+        this.updateExecutionOutput(
+          branchStep.id,
+          branchStep.stepId,
+          branchStep.inputs,
+          branchStatus
+        )
+        this.context.steps[this.context.steps.length] = branchStatus
+
         const branchSteps = children?.[branch.name] || []
         await this.executeSteps(branchSteps)
         break
@@ -569,9 +586,9 @@ class Orchestrator {
       this.loopStepOutputs.push(outputs)
     } else {
       this.updateExecutionOutput(step.id, step.stepId, step.inputs, outputs)
-    }
       this.context.steps[this.context.steps.length] = outputs
     }
+  }
 }
 
 export function execute(job: Job<AutomationData>, callback: WorkerCallback) {