Skip to content

Commit 59a73d2

Browse files
committed
fix: log a warning for missing mandatory variables in BrickCreate and update test cases
1 parent 650906d commit 59a73d2

File tree

2 files changed

+19
-6
lines changed

2 files changed

+19
-6
lines changed

internal/orchestrator/bricks/bricks.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,9 @@ func (s *Service) BrickCreate(
290290
for _, brickVar := range brick.Variables {
291291
if brickVar.DefaultValue == "" {
292292
if _, exist := req.Variables[brickVar.Name]; !exist {
293-
return fmt.Errorf("required variable %q is mandatory", brickVar.Name)
293+
// PATCH: to allow the AppLab to add a brick to a app created from scratch becasue currently
294+
// the FE does not send the required variables in the request.
295+
slog.Warn("[Skip] variable has no default value and it is not set by user", "variable", brickVar.Name)
294296
}
295297
}
296298
}

internal/orchestrator/bricks/bricks_test.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package bricks
1717

1818
import (
1919
"testing"
20+
"time"
2021

2122
"github.com/arduino/go-paths-helper"
2223
"github.com/stretchr/testify/require"
@@ -46,6 +47,7 @@ func TestBrickCreateFromAppExample(t *testing.T) {
4647
require.Equal(t, "variable \"NON_EXISTING_VARIABLE\" does not exist on brick \"arduino:arduino_cloud\"", err.Error())
4748
})
4849

50+
//TODO: allow a variable to have empty string
4951
t.Run("fails if a required variable is set empty", func(t *testing.T) {
5052
req := BrickCreateUpdateRequest{ID: "arduino:arduino_cloud", Variables: map[string]string{
5153
"ARDUINO_DEVICE_ID": "",
@@ -56,13 +58,22 @@ func TestBrickCreateFromAppExample(t *testing.T) {
5658
require.Equal(t, "variable \"ARDUINO_DEVICE_ID\" cannot be empty", err.Error())
5759
})
5860

59-
t.Run("fails if a mandatory variable is not present in the request", func(t *testing.T) {
61+
t.Run("log a warning if a mandatory variable is not present in the request", func(t *testing.T) {
62+
tempApp, cleanUp := copyToTempApp(t, paths.New("testdata/AppFromExample"))
63+
defer cleanUp()
64+
6065
req := BrickCreateUpdateRequest{ID: "arduino:arduino_cloud", Variables: map[string]string{
6166
"ARDUINO_SECRET": "a-secret-a",
6267
}}
63-
err = brickService.BrickCreate(req, f.Must(app.Load("testdata/AppFromExample")))
64-
require.Error(t, err)
65-
require.Equal(t, "required variable \"ARDUINO_DEVICE_ID\" is mandatory", err.Error())
68+
err = brickService.BrickCreate(req, f.Must(app.Load(tempApp.String())))
69+
require.Nil(t, err)
70+
71+
after, err := app.Load(tempApp.String())
72+
require.Nil(t, err)
73+
require.Len(t, after.Descriptor.Bricks, 1)
74+
require.Equal(t, "arduino:arduino_cloud", after.Descriptor.Bricks[0].ID)
75+
require.Equal(t, "", after.Descriptor.Bricks[0].Variables["ARDUINO_DEVICE_ID"]) // <-- the DEVICE_ID is set to empty
76+
require.Equal(t, "a-secret-a", after.Descriptor.Bricks[0].Variables["ARDUINO_SECRET"])
6677
})
6778

6879
t.Run("the brick is added if it does not exist in the app", func(t *testing.T) {
@@ -108,7 +119,7 @@ func TestBrickCreateFromAppExample(t *testing.T) {
108119
}
109120

110121
func copyToTempApp(t *testing.T, srcApp *paths.Path) (tmpApp *paths.Path, cleanUp func()) {
111-
tmpAppPath := paths.New(srcApp.String() + ".temp")
122+
tmpAppPath := paths.New(srcApp.String() + time.Now().Format(time.RFC3339) + ".temp")
112123
require.Nil(t, srcApp.CopyDirTo(tmpAppPath))
113124
return tmpAppPath, func() {
114125
require.Nil(t, tmpAppPath.RemoveAll())

0 commit comments

Comments
 (0)