Fix activity feed

Preserve actions after user/repo name change

Author: ethantkoenig

models/action.go

@@ -74,11 +74,11 @@ type Action struct {
 	UserID       int64 `xorm:"INDEX"` // Receiver user id.
 	OpType       ActionType
 	ActUserID    int64  `xorm:"INDEX"` // Action user id.
-	ActUserName  string // Action user name.
+	ActUserName  string `xorm:"-"`
 	ActAvatar    string `xorm:"-"`
 	RepoID       int64  `xorm:"INDEX"`
-	RepoUserName string
-	RepoName     string
+	RepoUserName string `xorm:"-"`
+	RepoName     string `xorm:"-"`
 	RefName      string
 	IsPrivate    bool      `xorm:"INDEX NOT NULL DEFAULT false"`
 	Content      string    `xorm:"TEXT"`

models/consistency.go

@@ -162,11 +162,5 @@ func (team *Team) checkForConsistency(t *testing.T) {
 
 func (action *Action) checkForConsistency(t *testing.T) {
 	repo := AssertExistsAndLoadBean(t, &Repository{ID: action.RepoID}).(*Repository)
-	owner := AssertExistsAndLoadBean(t, &User{ID: repo.OwnerID}).(*User)
-	actor := AssertExistsAndLoadBean(t, &User{ID: action.ActUserID}).(*User)
-
-	assert.Equal(t, repo.Name, action.RepoName, "action: %+v", action)
 	assert.Equal(t, repo.IsPrivate, action.IsPrivate, "action: %+v", action)
-	assert.Equal(t, owner.Name, action.RepoUserName, "action: %+v", action)
-	assert.Equal(t, actor.Name, action.ActUserName, "action: %+v", action)
 }

models/fixtures/action.yml

@@ -3,21 +3,15 @@
   user_id: 2
   op_type: 12 # close issue
   act_user_id: 2
-  act_user_name: user2
   repo_id: 2
-  repo_user_name: user2
-  repo_name: repo2
   is_private: true
 
 -
   id: 2
   user_id: 3
   op_type: 2 # rename repo
   act_user_id: 3
-  act_user_name: user3
   repo_id: 3
-  repo_user_name: user3
-  repo_name: repo3
   is_private: true
   content: oldRepoName
 
@@ -26,8 +20,5 @@
   user_id: 11
   op_type: 1 # create repo
   act_user_id: 11
-  act_user_name: user11
   repo_id: 9
-  repo_user_name: user11
-  repo_name: repo9
   is_private: false

models/migrations/migrations.go

@@ -114,6 +114,8 @@ var migrations = []Migration{
 	NewMigration("add field for login source synchronization", addLoginSourceSyncEnabledColumn),
 	// v32 -> v33
 	NewMigration("add units for team", addUnitsToRepoTeam),
+	// v33 -> v34
+	NewMigration("remove columns from action", removeActionColumns),
 }
 
 // Migrate database to current version

models/migrations/v34.go

@@ -0,0 +1,29 @@
+// Copyright 2017 Gitea. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package migrations
+
+import (
+	"fmt"
+
+	"github.com/go-xorm/xorm"
+)
+
+type ActionV34 struct {
+	ActUserName  string `xorm:"-"`
+	RepoUserName string `xorm:"-"`
+	RepoName     string `xorm:"-"`
+}
+
+// TableName will be invoked by XORM to customize the table name
+func (*ActionV34) TableName() string {
+	return "action"
+}
+
+func removeActionColumns(x *xorm.Engine) (err error) {
+	if err = x.Sync(new(ActionV34)); err != nil {
+		return fmt.Errorf("Sync: %v", err)
+	}
+	return nil
+}

routers/user/home.go

@@ -65,25 +65,51 @@ func retrieveFeeds(ctx *context.Context, ctxUser *models.User, userID, offset in
 
 	// Check access of private repositories.
 	feeds := make([]*models.Action, 0, len(actions))
-	unameAvatars := map[string]string{
-		ctxUser.Name: ctxUser.RelAvatarLink(),
-	}
+	userCache := map[int64]*models.User{ctxUser.ID: ctxUser}
+	repoCache := map[int64]*models.Repository{}
 	for _, act := range actions {
 		// Cache results to reduce queries.
-		_, ok := unameAvatars[act.ActUserName]
+		u, ok := userCache[act.ActUserID]
 		if !ok {
-			u, err := models.GetUserByName(act.ActUserName)
+			u, err = models.GetUserByID(act.ActUserID)
 			if err != nil {
 				if models.IsErrUserNotExist(err) {
 					continue
 				}
-				ctx.Handle(500, "GetUserByName", err)
+				ctx.Handle(500, "GetUserByID", err)
+				return
+			}
+			userCache[act.ActUserID] = u
+		}
+		act.ActUserName = u.Name
+		act.ActAvatar = u.RelAvatarLink()
+
+		repo, ok := repoCache[act.RepoID]
+		if !ok {
+			repo, err = models.GetRepositoryByID(act.RepoID)
+			if err != nil {
+				if models.IsErrRepoNotExist(err) {
+					continue
+				}
+				ctx.Handle(500, "GetRepositoryByID", err)
+				return
+			}
+		}
+		act.RepoName = repo.Name
+
+		repoOwner, ok := userCache[repo.OwnerID]
+		if !ok {
+			repoOwner, err = models.GetUserByID(repo.OwnerID)
+			if err != nil {
+				if models.IsErrUserNotExist(err) {
+					continue
+				}
+				ctx.Handle(500, "GetUserByID", err)
 				return
 			}
-			unameAvatars[act.ActUserName] = u.RelAvatarLink()
 		}
+		act.RepoUserName = repoOwner.Name
 
-		act.ActAvatar = unameAvatars[act.ActUserName]
 		feeds = append(feeds, act)
 	}
 	ctx.Data["Feeds"] = feeds