fix(follow): materialize ext rows for default-followed groups (#151)

Closes #151.

Materializes user_conversation_ext rows for default-followed groups (those
in a category but never explicitly touched) so /v1/follow/sort drag works
and OnThreadCreated fans out new threads.  Materialization is gated by a
two-stage guard (group_setting × group_category live join + per-group
membership/Space/Disband check via threadAuthChecker), and the move-out
path cleans up auto_follow_threads in lock-step with the read predicate
to keep buildFollowItems and selectEligibleForFanoutTx consistent.

Schema-app invariant tightened: QueryCategorySettingsByGroupNos now selects
gc.category_id so a soft-deleted category yields CategoryID=nil and is
correctly excluded from the follow tab + materialization.

Author: an9xyz

modules/category/api.go

@@ -638,12 +638,49 @@ func (c *Category) moveGroupToCategory(ctx *wkhttp.Context) {
 		}
 	}
 
+	// Project-wide lock order is group_setting → user_follow_version →
+	// user_conversation_ext (matches UpdateSort, FollowChannel, UnfollowGroupsTx,
+	// deleteCategory).  Bump version BEFORE writing ext: otherwise a concurrent
+	// /v1/follow/sort holding the version FOR UPDATE while waiting on ext
+	// would AB-BA-deadlock against a move-out holding ext while waiting on
+	// version (issue #151 review #4 by an9xyz).
 	if _, err := convext.BumpFollowVersionTx(tx, loginUID, groupSpaceID); err != nil {
 		c.Error("更新 follow_version 失败", zap.Error(err))
 		ctx.ResponseError(errors.New("更新群设置失败"))
 		return
 	}
 
+	// Issue #151 review #3/#4 — synchronize user_conversation_ext.auto_follow_threads
+	// with the new category membership state.  buildFollowItems decides follow-tab
+	// membership by `cs.CategoryID != nil`; selectEligibleForFanoutTx decides
+	// OnThreadCreated fan-out by `auto_follow_threads=1 AND group_unfollowed=0`.
+	// These two reads share no column, so the writer (this handler) must keep
+	// them in lockstep:
+	//
+	//   - Move-out (categoryIDPtr == nil): clear auto_follow_threads to match
+	//     the new "not in follow tab" state.  Without this, fan-out continues
+	//     to target a user whose follow tab no longer shows the group.
+	//   - Move-in (categoryIDPtr != nil): restore auto_follow_threads=1 if
+	//     an ext row already exists with =0 (left over from a prior move-out).
+	//     Sidebar materialization (api_sidebar.go) skips its INSERT IGNORE
+	//     when the ext row is already present, so without this restore the
+	//     group re-appears in the follow tab but fan-out stays disabled.
+	//     For first-time categorize (no ext row) the call is a no-op —
+	//     sidebar materialization later creates the row with =1.
+	if categoryIDPtr == nil {
+		if err := convext.ClearAutoFollowThreadsTx(tx, loginUID, groupSpaceID, []string{groupNo}); err != nil {
+			c.Error("清理 auto_follow_threads 失败", zap.Error(err))
+			ctx.ResponseError(errors.New("更新群设置失败"))
+			return
+		}
+	} else {
+		if err := convext.RestoreAutoFollowThreadsTx(tx, loginUID, groupSpaceID, []string{groupNo}); err != nil {
+			c.Error("恢复 auto_follow_threads 失败", zap.Error(err))
+			ctx.ResponseError(errors.New("更新群设置失败"))
+			return
+		}
+	}
+
 	if err := tx.Commit(); err != nil {
 		c.Error("提交事务失败", zap.Error(err))
 		ctx.ResponseError(errors.New("更新群设置失败"))

modules/category/api_test.go

@@ -9,13 +9,38 @@ import (
 	"sync"
 	"testing"
 
+	"github.com/Mininglamp-OSS/octo-lib/config"
 	"github.com/Mininglamp-OSS/octo-lib/pkg/util"
 	"github.com/Mininglamp-OSS/octo-lib/pkg/wkhttp"
 	"github.com/Mininglamp-OSS/octo-lib/testutil"
 	convext "github.com/Mininglamp-OSS/octo-server/modules/conversation_ext"
+	redis "github.com/go-redis/redis"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
+// resetUIDRateLimit clears the per-uid token-bucket keys
+// (ratelimit:uid:{uid}) so subsequent HTTP calls in this test start from a
+// full bucket.  Without this, tests that came earlier in the same go test
+// binary will have consumed tokens, and a later high-burst test (e.g.
+// TestCategory_CreateLimit, which makes 20+ category POSTs back-to-back)
+// fails with HTTP 429 even though the per-test logic is correct.  See
+// pkg/wkhttp/ratelimit_helper.go SharedUIDRateLimiter for the bucket key
+// scheme.  Pattern mirrors modules/space/api_email_invite_public_test.go's
+// resetSpaceInviteRateLimit.
+func resetUIDRateLimit(t *testing.T, ctx *config.Context) {
+	t.Helper()
+	rdsClient := redis.NewClient(&redis.Options{
+		Addr:     ctx.GetConfig().DB.RedisAddr,
+		Password: ctx.GetConfig().DB.RedisPass,
+	})
+	defer rdsClient.Close()
+	keys, err := rdsClient.Keys("ratelimit:uid:*").Result()
+	if err == nil && len(keys) > 0 {
+		_ = rdsClient.Del(keys...).Err()
+	}
+}
+
 // ---------- helpers ----------
 
 func resetDefaultCategoryName() {
@@ -427,6 +452,263 @@ func TestCategory_MoveGroupToCategory(t *testing.T) {
 	assert.Nil(t, setting2.CategoryID)
 }
 
+// TestCategory_MoveGroupOutOfCategory_ClearsAutoFollowThreads is the
+// regression test for issue #151 review #3 (yujiawei).  When a user moves a
+// group out of any category, the auto_follow_threads flag on the
+// user_conversation_ext row (which the new sidebar materialization may have
+// set to 1) must be cleared in the same transaction.  Without this cleanup,
+// selectEligibleForFanoutTx would keep this user eligible for OnThreadCreated
+// fan-out — the read side (buildFollowItems) drops the group because
+// CategoryID is now nil, but the write side only checks auto_follow_threads.
+//
+// Repro before fix:
+//  1. group g placed in category c, no ext row yet (default-followed).
+//  2. /v1/sidebar/sync follow tab materializes (uid, space, g) with
+//     auto_follow_threads=1, group_unfollowed=0.
+//  3. User moves g out of category via PUT /v1/groups/g/category {"":""}.
+//  4. ext row still has auto_follow_threads=1 — fan-out continues.
+//
+// After fix the move-out branch in api.go calls ClearAutoFollowThreadsTx
+// in the same tx, restoring the read/write contract.
+func TestCategory_MoveGroupOutOfCategory_ClearsAutoFollowThreads(t *testing.T) {
+	s, ctx := testutil.NewTestServer()
+	f := New(ctx)
+
+	err := testutil.CleanAllTables(ctx)
+	assert.NoError(t, err)
+	resetUIDRateLimit(t, ctx)
+
+	spaceID := "space-move-clr-001"
+	seedSpaceAndMember(t, f, spaceID, 0)
+	route := s.GetRoute()
+
+	// 1. Set up: category + group + group-in-category.
+	wc := createCategory(t, route, spaceID, "工作")
+	require.Equal(t, http.StatusOK, wc.Code)
+	cat := parseJSON(t, wc)
+	catID := cat["category_id"].(string)
+
+	groupNo := "group-move-clr-001"
+	seedGroup(t, f, groupNo, spaceID)
+
+	wm := doRequest(t, route, "PUT", "/v1/groups/"+groupNo+"/category", map[string]string{
+		"category_id": catID,
+	})
+	require.Equal(t, http.StatusOK, wm.Code)
+
+	// 2. Simulate the sidebar materialization step (would normally fire on
+	//    /v1/sidebar/sync).  Insert ext row with auto_follow_threads=1,
+	//    matching what MaterializeDefaultFollowedGroups writes.
+	_, err = f.db.session.InsertBySql(
+		"INSERT INTO user_conversation_ext (uid, space_id, target_type, target_id, group_unfollowed, auto_follow_threads) "+
+			"VALUES (?, ?, 2, ?, 0, 1)",
+		testutil.UID, spaceID, groupNo,
+	).Exec()
+	require.NoError(t, err, "seed materialized ext row")
+
+	// Precondition sanity check.
+	var preAutoFollow int
+	_, err = f.db.session.SelectBySql(
+		"SELECT auto_follow_threads FROM user_conversation_ext"+
+			" WHERE uid=? AND space_id=? AND target_type=2 AND target_id=?",
+		testutil.UID, spaceID, groupNo,
+	).Load(&preAutoFollow)
+	require.NoError(t, err)
+	require.Equal(t, 1, preAutoFollow, "precondition: row is materialized auto_follow_threads=1")
+
+	// 3. Move group OUT of category.
+	wm2 := doRequest(t, route, "PUT", "/v1/groups/"+groupNo+"/category", map[string]string{
+		"category_id": "",
+	})
+	require.Equal(t, http.StatusOK, wm2.Code)
+
+	// 4. Assert auto_follow_threads is now 0 — the actual regression fix.
+	var postAutoFollow int
+	_, err = f.db.session.SelectBySql(
+		"SELECT auto_follow_threads FROM user_conversation_ext"+
+			" WHERE uid=? AND space_id=? AND target_type=2 AND target_id=?",
+		testutil.UID, spaceID, groupNo,
+	).Load(&postAutoFollow)
+	require.NoError(t, err)
+	assert.Equal(t, 0, postAutoFollow,
+		"auto_follow_threads must be cleared after move-out (issue #151 review #3); "+
+			"otherwise selectEligibleForFanoutTx would still target this user")
+
+	// 5. Other flags MUST be preserved — uncategorize is NOT a full unfollow.
+	var groupUnfollowed int
+	_, err = f.db.session.SelectBySql(
+		"SELECT group_unfollowed FROM user_conversation_ext"+
+			" WHERE uid=? AND space_id=? AND target_type=2 AND target_id=?",
+		testutil.UID, spaceID, groupNo,
+	).Load(&groupUnfollowed)
+	require.NoError(t, err)
+	assert.Equal(t, 0, groupUnfollowed,
+		"group_unfollowed must NOT be set — uncategorize ≠ explicit unfollow; "+
+			"the cleanup only revokes auto-subscribe to NEW threads, not all subscriptions")
+}
+
+// TestCategory_MoveGroupBetweenCategories_PreservesAutoFollowThreads pins the
+// non-regression: moving a group from category A to category B preserves the
+// implicit follow, so auto_follow_threads stays 1.  Without this guard, a
+// future change might over-eagerly clear in every move and break the
+// "default-followed across category-to-category move" contract.
+func TestCategory_MoveGroupBetweenCategories_PreservesAutoFollowThreads(t *testing.T) {
+	s, ctx := testutil.NewTestServer()
+	f := New(ctx)
+
+	err := testutil.CleanAllTables(ctx)
+	assert.NoError(t, err)
+	resetUIDRateLimit(t, ctx)
+
+	spaceID := "space-move-keep-001"
+	seedSpaceAndMember(t, f, spaceID, 0)
+	route := s.GetRoute()
+
+	// Two categories, one group, materialized ext row.
+	wcA := createCategory(t, route, spaceID, "工作A")
+	require.Equal(t, http.StatusOK, wcA.Code)
+	catA := parseJSON(t, wcA)["category_id"].(string)
+	wcB := createCategory(t, route, spaceID, "工作B")
+	require.Equal(t, http.StatusOK, wcB.Code)
+	catB := parseJSON(t, wcB)["category_id"].(string)
+
+	groupNo := "group-move-keep-001"
+	seedGroup(t, f, groupNo, spaceID)
+	require.Equal(t, http.StatusOK, doRequest(t, route, "PUT", "/v1/groups/"+groupNo+"/category", map[string]string{"category_id": catA}).Code)
+	_, err = f.db.session.InsertBySql(
+		"INSERT INTO user_conversation_ext (uid, space_id, target_type, target_id, group_unfollowed, auto_follow_threads) "+
+			"VALUES (?, ?, 2, ?, 0, 1)",
+		testutil.UID, spaceID, groupNo,
+	).Exec()
+	require.NoError(t, err)
+
+	// Move A → B.
+	w := doRequest(t, route, "PUT", "/v1/groups/"+groupNo+"/category", map[string]string{"category_id": catB})
+	require.Equal(t, http.StatusOK, w.Code)
+
+	var autoFollow int
+	_, err = f.db.session.SelectBySql(
+		"SELECT auto_follow_threads FROM user_conversation_ext"+
+			" WHERE uid=? AND space_id=? AND target_type=2 AND target_id=?",
+		testutil.UID, spaceID, groupNo,
+	).Load(&autoFollow)
+	require.NoError(t, err)
+	assert.Equal(t, 1, autoFollow,
+		"category A→B move must NOT clear auto_follow_threads — the group is "+
+			"still in the follow tab, just under a different category")
+}
+
+// TestCategory_MoveGroupBackIntoCategory_RestoresAutoFollowThreads pins
+// issue #151 review #4 (an9xyz) symptom #1: a default-followed group that
+// has been materialized and then moved out must have auto_follow_threads
+// restored to 1 when it is moved back into any category.  Otherwise the
+// sidebar materialization branch skips the existing groupExts entry,
+// the group reappears in the follow tab via buildFollowItems, but
+// selectEligibleForFanoutTx still excludes the user (=0) — phantom missing
+// fan-out.
+func TestCategory_MoveGroupBackIntoCategory_RestoresAutoFollowThreads(t *testing.T) {
+	s, ctx := testutil.NewTestServer()
+	f := New(ctx)
+
+	err := testutil.CleanAllTables(ctx)
+	assert.NoError(t, err)
+	resetUIDRateLimit(t, ctx)
+
+	spaceID := "space-move-back-001"
+	seedSpaceAndMember(t, f, spaceID, 0)
+	route := s.GetRoute()
+
+	wc := createCategory(t, route, spaceID, "工作")
+	require.Equal(t, http.StatusOK, wc.Code)
+	catID := parseJSON(t, wc)["category_id"].(string)
+
+	groupNo := "group-move-back-001"
+	seedGroup(t, f, groupNo, spaceID)
+
+	// Cycle: move into category → simulate sidebar materialization → move
+	// out (clears auto_follow_threads) → move back into the SAME category.
+	require.Equal(t, http.StatusOK, doRequest(t, route, "PUT", "/v1/groups/"+groupNo+"/category", map[string]string{"category_id": catID}).Code)
+	_, err = f.db.session.InsertBySql(
+		"INSERT INTO user_conversation_ext (uid, space_id, target_type, target_id, group_unfollowed, auto_follow_threads) "+
+			"VALUES (?, ?, 2, ?, 0, 1)",
+		testutil.UID, spaceID, groupNo,
+	).Exec()
+	require.NoError(t, err, "simulate sidebar materialization")
+	require.Equal(t, http.StatusOK, doRequest(t, route, "PUT", "/v1/groups/"+groupNo+"/category", map[string]string{"category_id": ""}).Code,
+		"move out — must clear auto_follow_threads")
+
+	var afterOut int
+	_, err = f.db.session.SelectBySql(
+		"SELECT auto_follow_threads FROM user_conversation_ext"+
+			" WHERE uid=? AND space_id=? AND target_type=2 AND target_id=?",
+		testutil.UID, spaceID, groupNo,
+	).Load(&afterOut)
+	require.NoError(t, err)
+	require.Equal(t, 0, afterOut, "precondition: move-out cleared auto_follow_threads")
+
+	// Move BACK into the same category.  Sidebar materialization would skip
+	// this row (groupExts hit), so the move-in path itself must restore =1.
+	require.Equal(t, http.StatusOK, doRequest(t, route, "PUT", "/v1/groups/"+groupNo+"/category", map[string]string{"category_id": catID}).Code)
+
+	var afterIn int
+	_, err = f.db.session.SelectBySql(
+		"SELECT auto_follow_threads FROM user_conversation_ext"+
+			" WHERE uid=? AND space_id=? AND target_type=2 AND target_id=?",
+		testutil.UID, spaceID, groupNo,
+	).Load(&afterIn)
+	require.NoError(t, err)
+	assert.Equal(t, 1, afterIn,
+		"move-in must restore auto_follow_threads=1 on an existing ext row "+
+			"(issue #151 review #4 symptom #1); sidebar materialization would "+
+			"otherwise skip the existing row, leaving OnThreadCreated fan-out "+
+			"disabled even though the group is back in the follow tab")
+}
+
+// TestCategory_MoveFirstTimeIntoCategory_NoOpRestore ensures the move-in
+// restore call is a safe no-op when no ext row has been materialized yet —
+// sidebar materialization at the next /v1/sidebar/sync creates the row with
+// auto_follow_threads=1 anyway, and the move-in handler must not
+// short-circuit any subsequent paths or write inappropriate rows.
+func TestCategory_MoveFirstTimeIntoCategory_NoOpRestore(t *testing.T) {
+	s, ctx := testutil.NewTestServer()
+	f := New(ctx)
+
+	err := testutil.CleanAllTables(ctx)
+	assert.NoError(t, err)
+	resetUIDRateLimit(t, ctx)
+
+	spaceID := "space-move-first-001"
+	seedSpaceAndMember(t, f, spaceID, 0)
+	route := s.GetRoute()
+
+	wc := createCategory(t, route, spaceID, "工作")
+	require.Equal(t, http.StatusOK, wc.Code)
+	catID := parseJSON(t, wc)["category_id"].(string)
+
+	groupNo := "group-move-first-001"
+	seedGroup(t, f, groupNo, spaceID)
+
+	// Move into category for the first time — no ext row exists yet.
+	require.Equal(t, http.StatusOK, doRequest(t, route, "PUT", "/v1/groups/"+groupNo+"/category", map[string]string{"category_id": catID}).Code)
+
+	// No row should have been written by the move-in path — sidebar's
+	// MaterializeDefaultFollowedGroups is the canonical materialization site
+	// and stays solely responsible for creating ext rows.  Letting the
+	// move-in path INSERT here would race with the unique key and silently
+	// pick whichever flag set ends up committing first.
+	var count int
+	_, err = f.db.session.SelectBySql(
+		"SELECT COUNT(*) FROM user_conversation_ext"+
+			" WHERE uid=? AND space_id=? AND target_type=2 AND target_id=?",
+		testutil.UID, spaceID, groupNo,
+	).Load(&count)
+	require.NoError(t, err)
+	assert.Equal(t, 0, count,
+		"first-time move-in must NOT create an ext row — sidebar materialization "+
+			"is the single materialization site; RestoreAutoFollowThreadsTx is "+
+			"strictly UPDATE")
+}
+
 // ---------- Validation / Error Tests ----------
 
 func TestCategory_CreateLimit(t *testing.T) {

modules/category/db_group_setting.go

@@ -26,6 +26,27 @@ func (d *categoryDB) updateGroupSettingCategory(id int64, categoryID *string, ca
 	return err
 }
 
+// queryUserGroupsInSpace returns the user's groups in the given Space, each
+// annotated with the category_id the user has assigned (NULL if uncategorized).
+//
+// KNOWN ISSUE (issue #151 follow-up, NOT addressed in this PR): the SELECT
+// returns gs.category_id (the persisted field) without joining group_category.
+// If the user soft-deleted the assigned category before category_cleanup ran,
+// or a TOCTOU race produced a dangling reference (see
+// MoveGroupToCategory_TOCTOU_DanglingReference test in this module), this
+// query returns the stale category_id to the caller — the /category list API
+// then shows the group nested under a category that no longer exists, and the
+// client may use that id in a follow-up call which silently fails.
+//
+// Fix is the same shape as modules/message/db_group_category.go
+// QueryCategorySettingsByGroupNos: INNER/LEFT JOIN group_category gc ON
+// (gs.category_id, gs.uid) AND gc.status != 2, then SELECT gc.category_id so
+// dangling refs surface as NULL.  Out of scope here because:
+//   (a) issue #151 is scoped to the follow tab / sidebar materialization;
+//   (b) the API consumer of this function may rely on stale ids to render the
+//       "uncategorize-after-delete" affordance — needs PM input before
+//       changing user-visible behaviour.
+// Track this as a follow-up; see PR description for rationale.
 func (d *categoryDB) queryUserGroupsInSpace(uid, spaceID string) ([]*userGroupInfo, error) {
 	var results []*userGroupInfo
 	_, err := d.session.SelectBySql(`