From d182c4f81f52bce48f62016f0606b53ffd37cf99 Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Fri, 18 Jul 2025 15:50:56 +0200 Subject: [PATCH] fix select type being ignored when it's null (#33399) --- .../store/sqlstore/attributes_store.go | 2 +- .../store/storetest/attributes_store.go | 149 +++++++++++++++++- 2 files changed, 148 insertions(+), 3 deletions(-) diff --git a/server/channels/store/sqlstore/attributes_store.go b/server/channels/store/sqlstore/attributes_store.go index f4deb802edc..def3a95f187 100644 --- a/server/channels/store/sqlstore/attributes_store.go +++ b/server/channels/store/sqlstore/attributes_store.go @@ -186,7 +186,7 @@ func (s *SqlAttributesStore) GetChannelMembersToRemove(rctx request.CTX, channel OrderBy("ChannelMembers.UserId ASC") if opts.Query != "" { - query = query.Where(sq.Expr(fmt.Sprintf("(NOT (%s) OR AttributeView.TargetID IS NULL)", opts.Query), opts.Args...)) + query = query.Where(sq.Expr(fmt.Sprintf("(NOT COALESCE((%s), FALSE) OR AttributeView.TargetID IS NULL)", opts.Query), opts.Args...)) } argCount := len(opts.Args) diff --git a/server/channels/store/storetest/attributes_store.go b/server/channels/store/storetest/attributes_store.go index 477c7be2184..1459a5259dc 100644 --- a/server/channels/store/storetest/attributes_store.go +++ b/server/channels/store/storetest/attributes_store.go @@ -18,9 +18,12 @@ const ( testPropertyGroupName = "test_property_group" testPropertyA = "test_property_a" testPropertyB = "test_property_b" + testPropertyC = "test_property_c" testPropertyValueA1 = "value_a1" testPropertyValueA2 = "value_a2" testPropertyValueB1 = "value_b1" + testPropertyValueC1 = "option_1" + testPropertyValueC2 = "option_2" ) var ( @@ -30,8 +33,22 @@ var ( func TestAttributesStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) { t.Run("RefreshAndGet", func(t *testing.T) { testAttributesStoreRefresh(t, rctx, ss) }) t.Run("SearchUsers", func(t *testing.T) { testAttributesStoreSearchUsers(t, rctx, ss, s) }) + t.Run("GetChannelMembersToRemove", func(t *testing.T) { testAttributesStoreGetChannelMembersToRemove(t, rctx, ss, s) }) } +// To help mental model of the test users created by this function: +// - user[0] : { +// "test_property_a":"value_a1", +// "test_property_b":"value_b1" +// } +// - user[1] : { +// "test_property_a":"value_a2" +// } +// - user[2] : { +// "test_property_a": "value_a1" +// "test_property_c": "option_2" // this is select type +// } +// - user[3] : {} func createTestUsers(t *testing.T, rctx request.CTX, ss store.Store) ([]*model.User, string, func()) { maxUsersPerTeam := 50 @@ -92,6 +109,19 @@ func createTestUsers(t *testing.T, rctx request.CTX, ss store.Store) ([]*model.U Type: model.PropertyFieldTypeText, }) require.NoError(t, err) + attrs := map[string]any{ + "options": []any{ + map[string]any{"id": model.NewId(), "name": testPropertyValueC1, "color": ""}, + map[string]any{"id": model.NewId(), "name": testPropertyValueC2, "color": ""}, + }, + } + fieldC, err := ss.PropertyField().Create(&model.PropertyField{ + GroupID: groupID, + Name: "test_property_c", + Type: model.PropertyFieldTypeSelect, + Attrs: attrs, + }) + require.NoError(t, err) vala1, err := json.Marshal(testPropertyValueA1) require.NoError(t, err) @@ -99,6 +129,8 @@ func createTestUsers(t *testing.T, rctx request.CTX, ss store.Store) ([]*model.U require.NoError(t, err) valab1, err := json.Marshal(testPropertyValueB1) require.NoError(t, err) + valc2, err := json.Marshal(attrs["options"].([]any)[0].(map[string]any)["id"]) + require.NoError(t, err) pva1, err := ss.PropertyValue().Create(&model.PropertyValue{ TargetID: u1.Id, @@ -136,12 +168,21 @@ func createTestUsers(t *testing.T, rctx request.CTX, ss store.Store) ([]*model.U }) require.NoError(t, err) + pva4, err := ss.PropertyValue().Create(&model.PropertyValue{ + TargetID: u3.Id, + TargetType: "user", + GroupID: groupID, + FieldID: fieldC.ID, + Value: valc2, + }) + require.NoError(t, err) + return []*model.User{&u1, &u2, &u3}, groupID, func() { - for _, pv := range []*model.PropertyValue{pva1, pvb1, pva2, pva3} { + for _, pv := range []*model.PropertyValue{pva1, pvb1, pva2, pva3, pva4} { dErr := ss.PropertyValue().Delete(groupID, pv.ID) require.NoError(t, dErr, "couldn't delete property value") } - for _, field := range []*model.PropertyField{fieldA, fieldB} { + for _, field := range []*model.PropertyField{fieldA, fieldB, fieldC} { dErr := ss.PropertyField().Delete(groupID, field.ID) require.NoError(t, dErr, "couldn't delete property field") } @@ -280,3 +321,107 @@ func testAttributesStoreSearchUsers(t *testing.T, rctx request.CTX, ss store.Sto } }) } + +func testAttributesStoreGetChannelMembersToRemove(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) { + users, _, cleanup := createTestUsers(t, rctx, ss) + t.Cleanup(cleanup) + require.Len(t, users, 3, "expected 3 users") + + err := ss.Attributes().RefreshAttributes() + require.NoError(t, err, "couldn't refresh attributes") + + ch, err := ss.Channel().Save(rctx, &model.Channel{ + Name: "test-channel", + TeamId: testTeamID, + Type: model.ChannelTypePrivate, + CreateAt: model.GetMillis(), + }, 1000) + require.NoError(t, err, "couldn't save channel") + + defaultNotifyProps := model.GetDefaultChannelNotifyProps() + + cm1, err := ss.Channel().SaveMember(rctx, &model.ChannelMember{ + ChannelId: ch.Id, + UserId: users[0].Id, + NotifyProps: defaultNotifyProps, + }) + require.NoError(t, err, "couldn't save channel member for user 1") + cm2, err := ss.Channel().SaveMember(rctx, &model.ChannelMember{ + ChannelId: ch.Id, + UserId: users[1].Id, + NotifyProps: defaultNotifyProps, + }) + require.NoError(t, err, "couldn't save channel member for user 2") + cm3, err := ss.Channel().SaveMember(rctx, &model.ChannelMember{ + ChannelId: ch.Id, + UserId: users[2].Id, + NotifyProps: defaultNotifyProps, + }) + require.NoError(t, err, "couldn't save channel member for user 3") + t.Cleanup(func() { + dErr := ss.Channel().RemoveMember(rctx, cm1.ChannelId, cm1.UserId) + require.NoError(t, dErr, "couldn't delete channel member for user 1") + dErr = ss.Channel().RemoveMember(rctx, cm2.ChannelId, cm2.UserId) + require.NoError(t, dErr, "couldn't delete channel member for user 2") + dErr = ss.Channel().RemoveMember(rctx, cm3.ChannelId, cm3.UserId) + require.NoError(t, dErr, "couldn't delete channel member for user 3") + dErr = ss.Channel().Delete(ch.Id, model.GetMillis()) + require.NoError(t, dErr, "couldn't delete channel") + }) + + t.Run("Get channel members to remove single attribute", func(t *testing.T) { + var query string + switch s.DriverName() { + case model.DatabaseDriverMysql: + query = "Attributes ->> '$." + testPropertyA + "' = ?" // Attributes ->> '$.Clearance' >= ? + case model.DatabaseDriverPostgres: + query = "Attributes ->> '" + testPropertyA + "' = $1::text" // Attributes ->> '$.Clearance' >= $1::text + } + members, err := ss.Attributes().GetChannelMembersToRemove(rctx, ch.Id, model.SubjectSearchOptions{ + Query: query, + Args: []any{testPropertyValueA1}, + }) + require.NoError(t, err, "couldn't get channel members to remove") + require.Len(t, members, 1, "expected 1 channel members to remove") + }) + + t.Run("Get channel members to remove multiple attribute", func(t *testing.T) { + var query string + switch s.DriverName() { + case model.DatabaseDriverMysql: + query = "Attributes ->> '$." + testPropertyA + "' = ? AND Attributes ->> '$." + testPropertyB + "' = ?" + case model.DatabaseDriverPostgres: + query = "Attributes ->> '" + testPropertyA + "' = $1::text AND Attributes ->> '" + testPropertyB + "' = $2::text" + } + members, err := ss.Attributes().GetChannelMembersToRemove(rctx, ch.Id, model.SubjectSearchOptions{ + Query: query, + Args: []any{testPropertyValueA1, testPropertyValueB1}, + }) + require.NoError(t, err, "couldn't get channel members to remove") + require.Len(t, members, 2, "expected 2 channel members to remove") + }) + + t.Run("Get channel members to remove with empty query", func(t *testing.T) { + members, err := ss.Attributes().GetChannelMembersToRemove(rctx, ch.Id, model.SubjectSearchOptions{ + Query: "", + }) + require.NoError(t, err, "couldn't get channel members to remove") + require.Len(t, members, 3, "expected 3 channel members to remove") + }) + + t.Run("Get channel members for select type attribute", func(t *testing.T) { + var query string + switch s.DriverName() { + case model.DatabaseDriverMysql: + t.Skip("select type attributes are not supported in MySQL") + case model.DatabaseDriverPostgres: + query = "Attributes ->> '" + testPropertyC + "' = $1::text" + } + members, err := ss.Attributes().GetChannelMembersToRemove(rctx, ch.Id, model.SubjectSearchOptions{ + Query: query, + Args: []any{testPropertyValueC1}, + }) + require.NoError(t, err, "couldn't get channel members to remove") + require.Len(t, members, 2, "expected 2 channel member to remove") + }) +}