Merge pull request #1919 from anyproto/go-4079-fix-bool-unset

GO-4079 Fix bool unset in ObjectListModifyDetailValues

core/block/detailservice/service.go

@@ -137,7 +137,7 @@ func (s *service) ModifyDetailsList(req *pb.RpcObjectListModifyDetailValuesReque
 		}
 		err := modifyDetailsFunc(objectId, func(current *types.Struct) (*types.Struct, error) {
 			for _, op := range req.Operations {
-				if !pbtypes.IsEmptyValue(op.Set) {
+				if !pbtypes.IsNullValue(op.Set) {
 					// Set operation has higher priority than Add and Remove, because it modifies full value
 					current.Fields[op.RelationKey] = op.Set
 					continue

core/block/detailservice/service_test.go

@@ -217,6 +217,31 @@ func TestService_ModifyDetailsList(t *testing.T) {
 		// then
 		assert.Error(t, err)
 	})
+
+	t.Run("set false value", func(t *testing.T) {
+		// given
+		fx := newFixture(t)
+		object := smarttest.New("obj1")
+		err := object.SetDetails(nil, []*model.Detail{{Key: bundle.RelationKeyDone.String(), Value: pbtypes.Bool(true)}}, false)
+		require.NoError(t, err)
+		fx.getter.EXPECT().GetObject(mock.Anything, mock.Anything).RunAndReturn(func(_ context.Context, objectId string) (smartblock.SmartBlock, error) {
+			return object, nil
+		})
+		doneSet := []*pb.RpcObjectListModifyDetailValuesRequestOperation{{
+			RelationKey: bundle.RelationKeyDone.String(),
+			Set:         pbtypes.Bool(false),
+		}}
+
+		// when
+		err = fx.ModifyDetailsList(&pb.RpcObjectListModifyDetailValuesRequest{
+			ObjectIds:  []string{"obj1"},
+			Operations: doneSet,
+		})
+
+		// then
+		assert.NoError(t, err)
+		assert.False(t, pbtypes.GetBool(object.Details(), bundle.RelationKeyDone.String()))
+	})
 }
 
 func TestService_SetSpaceInfo(t *testing.T) {

core/block/editor/smartblock/smarttest/smarttest.go

@@ -301,6 +301,8 @@ func (st *SmartTest) UpdateDetailsAndLastUsed(update func(current *types.Struct)
 		return nil
 	}
 
+	st.Doc.(*state.State).SetDetails(newDetails)
+
 	for key := range diff.Fields {
 		st.Results.LastUsedUpdates = append(st.Results.LastUsedUpdates, key)
 	}

util/pbtypes/pbtypes.go

@@ -131,8 +131,9 @@ func IsEmptyValueOrAbsent(s *types.Struct, name string) bool {
 	return IsEmptyValue(value)
 }
 
+// IsEmptyValue returns true for nil, null value, unknown kind of value, empty strings and empty lists
 func IsEmptyValue(value *types.Value) bool {
-	if value == nil {
+	if IsNullValue(value) {
 		return true
 	}
 
@@ -140,22 +141,18 @@ func IsEmptyValue(value *types.Value) bool {
 		return len(v.StringValue) == 0
 	}
 
-	if v, ok := value.Kind.(*types.Value_NumberValue); ok {
-		return v.NumberValue == 0
+	if _, ok := value.Kind.(*types.Value_NumberValue); ok {
+		return false
 	}
 
-	if v, ok := value.Kind.(*types.Value_BoolValue); ok {
-		return !v.BoolValue
+	if _, ok := value.Kind.(*types.Value_BoolValue); ok {
+		return false
 	}
 
 	if _, ok := value.Kind.(*types.Value_ListValue); ok {
 		return len(GetStringListValue(value)) == 0
 	}
 
-	if _, ok := value.Kind.(*types.Value_NullValue); ok {
-		return true
-	}
-
 	if _, ok := value.Kind.(*types.Value_StructValue); ok {
 		return false
 	}
@@ -163,6 +160,16 @@ func IsEmptyValue(value *types.Value) bool {
 	return true
 }
 
+func IsNullValue(value *types.Value) bool {
+	if value == nil {
+		return true
+	}
+	if _, ok := value.Kind.(*types.Value_NullValue); ok {
+		return true
+	}
+	return false
+}
+
 func GetStruct(s *types.Struct, name string) *types.Struct {
 	if s == nil || s.Fields == nil {
 		return nil

util/pbtypes/pbtypes_test.go

@@ -163,9 +163,9 @@ func TestIsEmptyValueOrAbsent(t *testing.T) {
 		{"AbsentField", data, "nonExistentField", true},
 		{"EmptyStringValue", data, "emptyStringValue", true},
 		{"NonEmptyStringValue", data, "stringValue", false},
-		{"ZeroNumberValue", data, "emptyNumberValue", true},
+		{"ZeroNumberValue", data, "emptyNumberValue", false},
 		{"NonZeroNumberValue", data, "numberValue", false},
-		{"FalseBoolValue", data, "emptyBoolValue", true},
+		{"FalseBoolValue", data, "emptyBoolValue", false},
 		{"TrueBoolValue", data, "boolValue", false},
 		{"EmptyListValue", data, "emptyListValue", true},
 		{"NullValue", data, "nullValue", true},