Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MM-380] Fix issue of todo permalink opening in new tab #251

Merged
merged 5 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions server/bot.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"fmt"
"strings"

"github.com/mattermost/mattermost/server/public/model"
Expand All @@ -16,16 +17,17 @@ func (p *Plugin) PostBotDM(userID string, message string) {
}

// PostBotCustomDM posts a DM as the cloud bot user using custom post with action buttons.
func (p *Plugin) PostBotCustomDM(userID string, message string, todo string, issueID string) {
func (p *Plugin) PostBotCustomDM(userID, message, todo, postPermalink, issueID string) {
p.createBotPostDM(&model.Post{
UserId: p.BotUserID,
Message: message + ": " + todo,
Type: "custom_todo",
Props: map[string]interface{}{
"type": "custom_todo",
"message": message,
"todo": todo,
"issueId": issueID,
"type": "custom_todo",
"message": message,
"todo": todo,
"postPermalink": postPermalink,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens for posts that do not have this prop?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't have this prop, we won't have a permalink in the message in the todo sent to other user in the post

"issueId": issueID,
},
}, userID)
}
Expand All @@ -51,7 +53,7 @@ func (p *Plugin) createBotPostDM(post *model.Post, userID string) {
}

// ReplyPostBot post a message and a todo in the same thread as the post postID
func (p *Plugin) ReplyPostBot(postID, message, todo string) error {
func (p *Plugin) ReplyPostBot(postID, message, todo, postPermalink string) error {
if postID == "" {
return errors.New("post ID not defined")
}
Expand All @@ -65,7 +67,8 @@ func (p *Plugin) ReplyPostBot(postID, message, todo string) error {
rootID = post.RootId
}

quotedTodo := "\n> " + strings.Join(strings.Split(todo, "\n"), "\n> ")
postPermalink = fmt.Sprintf("[Permalink](%s)", postPermalink)
quotedTodo := "\n> " + strings.Join([]string{todo, postPermalink}, "\n> ")
_, appErr = p.API.CreatePost(&model.Post{
UserId: p.BotUserID,
ChannelId: post.ChannelId,
Expand Down
8 changes: 4 additions & 4 deletions server/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func (p *Plugin) runSendCommand(args []string, extra *model.CommandArgs) (bool,

message := strings.Join(args[1:], " ")

receiverIssueID, err := p.listManager.SendIssue(extra.UserId, receiver.Id, message, "", "")
receiverIssueID, err := p.listManager.SendIssue(extra.UserId, receiver.Id, message, "", "", "")
if err != nil {
return false, err
}
Expand All @@ -201,7 +201,7 @@ func (p *Plugin) runSendCommand(args []string, extra *model.CommandArgs) (bool,

receiverMessage := fmt.Sprintf("You have received a new Todo from @%s", senderName)

p.PostBotCustomDM(receiver.Id, receiverMessage, message, receiverIssueID)
p.PostBotCustomDM(receiver.Id, receiverMessage, message, "", receiverIssueID)
ayusht2810 marked this conversation as resolved.
Show resolved Hide resolved
p.postCommandResponse(extra, responseMessage)
return false, nil
}
Expand All @@ -214,7 +214,7 @@ func (p *Plugin) runAddCommand(args []string, extra *model.CommandArgs) (bool, e
return false, nil
}

newIssue, err := p.listManager.AddIssue(extra.UserId, message, "", "")
newIssue, err := p.listManager.AddIssue(extra.UserId, message, "", "", "")
if err != nil {
return false, err
}
Expand Down Expand Up @@ -311,7 +311,7 @@ func (p *Plugin) runPopCommand(_ []string, extra *model.CommandArgs) (bool, erro
responseMessage := "Removed top Todo."

replyMessage := fmt.Sprintf("@%s popped a todo attached to this thread", userName)
p.postReplyIfNeeded(issue.PostID, replyMessage, issue.Message)
p.postReplyIfNeeded(issue.PostID, replyMessage, issue.Message, issue.PostPermalink)

issues, err := p.listManager.GetIssueList(extra.UserId, MyListKey)
if err != nil {
Expand Down
24 changes: 13 additions & 11 deletions server/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ import (

// Issue represents a Todo issue
type Issue struct {
ID string `json:"id"`
Message string `json:"message"`
Description string `json:"description,omitempty"`
CreateAt int64 `json:"create_at"`
PostID string `json:"post_id"`
ID string `json:"id"`
Message string `json:"message"`
PostPermalink string `json:"postPermalink"`
Description string `json:"description,omitempty"`
CreateAt int64 `json:"create_at"`
PostID string `json:"post_id"`
}

// ExtendedIssue extends the information on Issue to be used on the front-end
Expand All @@ -24,13 +25,14 @@ type ExtendedIssue struct {
ForeignPosition int `json:"position"`
}

func newIssue(message string, description, postID string) *Issue {
func newIssue(message, postPermalink, description, postID string) *Issue {
return &Issue{
ID: model.NewId(),
CreateAt: model.GetMillis(),
Message: message,
Description: description,
PostID: postID,
ID: model.NewId(),
CreateAt: model.GetMillis(),
Message: message,
PostPermalink: postPermalink,
Description: description,
PostID: postID,
}
}

Expand Down
58 changes: 31 additions & 27 deletions server/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ func NewListManager(api plugin.API) ListManager {
}
}

func (l *listManager) AddIssue(userID, message, description, postID string) (*Issue, error) {
issue := newIssue(message, description, postID)
func (l *listManager) AddIssue(userID, message, postPermalink, description, postID string) (*Issue, error) {
issue := newIssue(message, postPermalink, description, postID)

if err := l.store.SaveIssue(issue); err != nil {
return nil, err
Expand All @@ -75,13 +75,13 @@ func (l *listManager) AddIssue(userID, message, description, postID string) (*Is
return issue, nil
}

func (l *listManager) SendIssue(senderID, receiverID, message, description, postID string) (string, error) {
senderIssue := newIssue(message, description, postID)
func (l *listManager) SendIssue(senderID, receiverID, message, postPermalink, description, postID string) (string, error) {
senderIssue := newIssue(message, postPermalink, description, postID)
if err := l.store.SaveIssue(senderIssue); err != nil {
return "", err
}

receiverIssue := newIssue(message, description, postID)
receiverIssue := newIssue(message, postPermalink, description, postID)
if err := l.store.SaveIssue(receiverIssue); err != nil {
if rollbackError := l.store.RemoveIssue(senderIssue.ID); rollbackError != nil {
l.api.LogError("cannot rollback sender issue after send error, Err=", err.Error())
Expand Down Expand Up @@ -201,30 +201,30 @@ func (l *listManager) EditIssue(userID, issueID, newMessage, newDescription stri
return ir.ForeignUserID, list, oldMessage, nil
}

func (l *listManager) ChangeAssignment(issueID string, userID string, sendTo string) (issueMessage, oldOwner string, err error) {
issue, err := l.store.GetIssue(issueID)
func (l *listManager) ChangeAssignment(issueID string, userID string, sendTo string) (issue *Issue, oldOwner string, err error) {
issue, err = l.store.GetIssue(issueID)
if err != nil {
return "", "", err
return nil, "", err
}

list, ir, _ := l.store.GetIssueListAndReference(userID, issueID)
if ir == nil {
return "", "", errors.New("reference not found")
return nil, "", errors.New("reference not found")
}

if (list == InListKey) || (ir.ForeignIssueID != "" && list == MyListKey) {
return "", "", errors.New("trying to change the assignment of a todo not owned")
return nil, "", errors.New("trying to change the assignment of a todo not owned")
}

if ir.ForeignUserID != "" {
// Remove reference from foreign user
foreignList, foreignIR, _ := l.store.GetIssueListAndReference(ir.ForeignUserID, ir.ForeignIssueID)
if foreignIR == nil {
return "", "", errors.New("reference not found")
return nil, "", errors.New("reference not found")
}

if err := l.store.RemoveReference(ir.ForeignUserID, ir.ForeignIssueID, foreignList); err != nil {
return "", "", err
return nil, "", err
}

_, err := l.store.GetAndRemoveIssue(ir.ForeignIssueID)
Expand All @@ -235,34 +235,34 @@ func (l *listManager) ChangeAssignment(issueID string, userID string, sendTo str

if userID == sendTo && list == OutListKey {
if err := l.store.RemoveReference(userID, issueID, OutListKey); err != nil {
return "", "", err
return nil, "", err
}

if err := l.store.AddReference(userID, issueID, MyListKey, "", ""); err != nil {
return "", "", err
return nil, "", err
}

return issue.Message, ir.ForeignUserID, nil
return issue, ir.ForeignUserID, nil
}

if err := l.store.RemoveReference(userID, issueID, list); err != nil {
return "", "", err
return nil, "", err
}

receiverIssue := newIssue(issue.Message, issue.Description, issue.PostID)
receiverIssue := newIssue(issue.Message, issue.PostPermalink, issue.Description, issue.PostID)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this could just accept a issue instead of these 4 fields derived from the issue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if err := l.store.SaveIssue(receiverIssue); err != nil {
return "", "", err
return nil, "", err
}

if err := l.store.AddReference(userID, issueID, OutListKey, sendTo, receiverIssue.ID); err != nil {
return "", "", err
return nil, "", err
}

if err := l.store.AddReference(sendTo, receiverIssue.ID, InListKey, userID, issue.ID); err != nil {
return "", "", err
return nil, "", err
}

return issue.Message, ir.ForeignUserID, nil
return issue, ir.ForeignUserID, nil
}

func (l *listManager) AcceptIssue(userID, issueID string) (todoMessage string, foreignUserID string, outErr error) {
Expand Down Expand Up @@ -292,6 +292,10 @@ func (l *listManager) AcceptIssue(userID, issueID string) (todoMessage string, f
return "", "", err
}

if issue.PostPermalink != "" {
issue.Message = fmt.Sprintf("%s\n[Permalink](%s)", issue.Message, issue.PostPermalink)
}

return issue.Message, ir.ForeignUserID, nil
}

Expand Down Expand Up @@ -360,28 +364,28 @@ func (l *listManager) PopIssue(userID string) (issue *Issue, foreignID string, e
return issue, ir.ForeignUserID, nil
}

func (l *listManager) BumpIssue(userID, issueID string) (todoMessage string, receiver string, foreignIssueID string, outErr error) {
func (l *listManager) BumpIssue(userID, issueID string) (todo *Issue, receiver string, foreignIssueID string, outErr error) {
ir, _, err := l.store.GetIssueReference(userID, issueID, OutListKey)
if err != nil {
return "", "", "", err
return nil, "", "", err
}

if ir == nil {
return "", "", "", fmt.Errorf("cannot find sender issue")
return nil, "", "", fmt.Errorf("cannot find sender issue")
}

err = l.store.BumpReference(ir.ForeignUserID, ir.ForeignIssueID, InListKey)
if err != nil {
return "", "", "", err
return nil, "", "", err
}

issue, err := l.store.GetIssue(ir.ForeignIssueID)
if err != nil {
l.api.LogError("cannot find foreigner issue after bump, Err=", err.Error())
return "", "", "", nil
return nil, "", "", nil
}

return issue.Message, ir.ForeignUserID, ir.ForeignIssueID, nil
return issue, ir.ForeignUserID, ir.ForeignIssueID, nil
}

func (l *listManager) GetUserName(userID string) string {
Expand Down
Loading
Loading