Skip to content

Commit

Permalink
Add Confirmed boolean to DeleteConversation RPC (keybase#9621)
Browse files Browse the repository at this point in the history
This is so that the GUI can handle the confirmation dialog itself, and
then just call the RPC as a final step.
  • Loading branch information
akalin-keybase authored Nov 20, 2017
1 parent 885b775 commit 8ae01ea
Show file tree
Hide file tree
Showing 8 changed files with 116 additions and 9 deletions.
21 changes: 15 additions & 6 deletions go/chat/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2222,13 +2222,22 @@ func (h *Server) DeleteConversationLocal(ctx context.Context, arg chat1.DeleteCo
return res, err
}

return h.deleteConversationLocal(ctx, arg)
}

// deleteConversationLocal contains the functionality of
// DeleteConversationLocal split off for easier testing.
func (h *Server) deleteConversationLocal(ctx context.Context, arg chat1.DeleteConversationLocalArg) (res chat1.DeleteConversationLocalRes, err error) {
ui := h.getChatUI(arg.SessionID)
confirmed, err := ui.ChatConfirmChannelDelete(ctx, chat1.ChatConfirmChannelDeleteArg{
SessionID: arg.SessionID,
Channel: arg.ChannelName,
})
if err != nil {
return res, err
confirmed := arg.Confirmed
if !confirmed {
confirmed, err = ui.ChatConfirmChannelDelete(ctx, chat1.ChatConfirmChannelDeleteArg{
SessionID: arg.SessionID,
Channel: arg.ChannelName,
})
if err != nil {
return res, err
}
}
if !confirmed {
return res, errors.New("channel delete unconfirmed")
Expand Down
90 changes: 90 additions & 0 deletions go/chat/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package chat

import (
"encoding/hex"
"errors"
"fmt"
"os"
"sort"
Expand Down Expand Up @@ -3227,6 +3228,95 @@ func TestChatSrvDeleteConversation(t *testing.T) {
})
}

type fakeInboxSource struct {
types.InboxSource
}

func (is fakeInboxSource) IsOffline(context.Context) bool {
return false
}

type fakeChatUI struct {
confirmChannelDelete bool
libkb.ChatUI
}

func (fc fakeChatUI) ChatConfirmChannelDelete(ctx context.Context, arg chat1.ChatConfirmChannelDeleteArg) (bool, error) {
return fc.confirmChannelDelete, nil
}

type fakeUISource struct {
UISource
chatUI libkb.ChatUI
}

func (ui *fakeUISource) GetChatUI(sessionID int) libkb.ChatUI {
return ui.chatUI
}

type fakeRemoteInterface struct {
chat1.RemoteInterface
deleteConversationCalled bool
}

func (ri *fakeRemoteInterface) DeleteConversation(context.Context, chat1.ConversationID) (chat1.DeleteConversationRemoteRes, error) {
ri.deleteConversationCalled = true
return chat1.DeleteConversationRemoteRes{}, nil
}

func TestChatSrvDeleteConversationConfirmed(t *testing.T) {
gc := libkb.NewGlobalContext()
gc.Init()
var is fakeInboxSource
cc := globals.ChatContext{
InboxSource: is,
}
g := globals.NewContext(gc, &cc)

ui := fakeUISource{}
h := NewServer(g, nil, nil, &ui)

var ri fakeRemoteInterface
h.setTestRemoteClient(&ri)

_, err := h.deleteConversationLocal(context.Background(), chat1.DeleteConversationLocalArg{
Confirmed: true,
})
require.NoError(t, err)
require.True(t, ri.deleteConversationCalled)
}

func TestChatSrvDeleteConversationUnconfirmed(t *testing.T) {
gc := libkb.NewGlobalContext()
gc.Init()
var is fakeInboxSource
cc := globals.ChatContext{
InboxSource: is,
}
g := globals.NewContext(gc, &cc)

chatUI := fakeChatUI{confirmChannelDelete: false}
ui := fakeUISource{
chatUI: chatUI,
}
h := NewServer(g, nil, nil, &ui)

var ri fakeRemoteInterface
h.setTestRemoteClient(&ri)

ctx := context.Background()
var arg chat1.DeleteConversationLocalArg

_, err := h.deleteConversationLocal(ctx, arg)
require.Equal(t, errors.New("channel delete unconfirmed"), err)
require.False(t, ri.deleteConversationCalled)

ui.chatUI = fakeChatUI{confirmChannelDelete: true}
_, err = h.deleteConversationLocal(ctx, arg)
require.NoError(t, err)
require.True(t, ri.deleteConversationCalled)
}

func TestChatSrvUserReset(t *testing.T) {
runWithMemberTypes(t, func(mt chat1.ConversationMembersType) {
ctc := makeChatTestContext(t, "TestChatSrvUserReset", 2)
Expand Down
1 change: 1 addition & 0 deletions go/protocol/chat1/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -3752,6 +3752,7 @@ type DeleteConversationLocalArg struct {
SessionID int `codec:"sessionID" json:"sessionID"`
ConvID ConversationID `codec:"convID" json:"convID"`
ChannelName string `codec:"channelName" json:"channelName"`
Confirmed bool `codec:"confirmed" json:"confirmed"`
}

type GetTLFConversationsLocalArg struct {
Expand Down
2 changes: 1 addition & 1 deletion protocol/avdl/chat1/local.avdl
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ protocol local {
boolean offline;
array<RateLimit> rateLimits;
}
DeleteConversationLocalRes deleteConversationLocal(int sessionID, ConversationID convID, string channelName);
DeleteConversationLocalRes deleteConversationLocal(int sessionID, ConversationID convID, string channelName, boolean confirmed);

record GetTLFConversationsLocalRes {
array<InboxUIItem> convs;
Expand Down
3 changes: 2 additions & 1 deletion protocol/js/flow-types-chat.js
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,8 @@ export type JoinLeaveConversationRemoteRes = {|rateLimit?: ?RateLimit,|}
export type LocalCancelPostRpcParam = {| outboxID: OutboxID|}

export type LocalDeleteConversationLocalRpcParam = {| convID: ConversationID,
channelName: String|}
channelName: String,
confirmed: Boolean|}

export type LocalDownloadAttachmentLocalRpcParam = {| conversationID: ConversationID,
messageID: MessageID,
Expand Down
4 changes: 4 additions & 0 deletions protocol/json/chat1/local.json
Original file line number Diff line number Diff line change
Expand Up @@ -2941,6 +2941,10 @@
{
"name": "channelName",
"type": "string"
},
{
"name": "confirmed",
"type": "boolean"
}
],
"response": "DeleteConversationLocalRes"
Expand Down
1 change: 1 addition & 0 deletions shared/actions/teams/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,7 @@ function* _deleteChannel({payload: {conversationIDKey}}): Saga.SagaGenerator<any
const param = {
convID: ChatConstants.keyToConversationID(conversationIDKey),
channelName,
confirmed: false,
}

yield Saga.call(ChatTypes.localDeleteConversationLocalRpcPromise, {param})
Expand Down
3 changes: 2 additions & 1 deletion shared/constants/types/flow-types-chat.js
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,8 @@ export type JoinLeaveConversationRemoteRes = {|rateLimit?: ?RateLimit,|}
export type LocalCancelPostRpcParam = {| outboxID: OutboxID|}

export type LocalDeleteConversationLocalRpcParam = {| convID: ConversationID,
channelName: String|}
channelName: String,
confirmed: Boolean|}

export type LocalDownloadAttachmentLocalRpcParam = {| conversationID: ConversationID,
messageID: MessageID,
Expand Down

0 comments on commit 8ae01ea

Please sign in to comment.