Skip to content

Commit d95fe82

Browse files
committed
fix deleted check (keybase#23774)
1 parent f847c09 commit d95fe82

File tree

3 files changed

+22
-18
lines changed

3 files changed

+22
-18
lines changed

go/chat/sender.go

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -276,14 +276,15 @@ func (s *BlockingSender) getAllDeletedEdits(ctx context.Context, uid gregor1.UID
276276
}
277277

278278
// Get the one message to be deleted by ID.
279-
deleteTarget, isValidFull, err := s.getMessage(ctx, uid, convID, deleteTargetID, false /* resolveSupersedes */)
279+
deleteTarget, err := s.getMessage(ctx, uid, convID, deleteTargetID, false /* resolveSupersedes */)
280280
if err != nil {
281281
return msg, nil, nil, err
282-
} else if !isValidFull {
283-
// If the message is already deleted just get out of here.
284-
return msg, nil, nil, nil
285282
}
286-
switch deleteTarget.ClientHeader.MessageType {
283+
bodyTyp, err := deleteTarget.MessageBody.MessageType()
284+
if err != nil {
285+
return msg, nil, nil, err
286+
}
287+
switch bodyTyp {
287288
case chat1.MessageType_REACTION:
288289
// Don't do anything here for reactions/unfurls, they can't be edited
289290
return msg, nil, nil, nil
@@ -379,22 +380,22 @@ func (s *BlockingSender) getAllDeletedEdits(ctx context.Context, uid gregor1.UID
379380
}
380381

381382
func (s *BlockingSender) getMessage(ctx context.Context, uid gregor1.UID,
382-
convID chat1.ConversationID, msgID chat1.MessageID, resolveSupersedes bool) (mvalid chat1.MessageUnboxedValid, isValidFull bool, err error) {
383+
convID chat1.ConversationID, msgID chat1.MessageID, resolveSupersedes bool) (mvalid chat1.MessageUnboxedValid, err error) {
383384
reason := chat1.GetThreadReason_PREPARE
384385
messages, err := s.G().ConvSource.GetMessages(ctx, convID, uid, []chat1.MessageID{msgID},
385386
&reason, nil, resolveSupersedes)
386387
if err != nil {
387-
return mvalid, false, err
388+
return mvalid, err
388389
}
389390
if len(messages) == 0 {
390-
return mvalid, false, fmt.Errorf("getMessage: message not found")
391+
return mvalid, fmt.Errorf("getMessage: message not found")
391392
}
392393
if !messages[0].IsValid() {
393394
st, err := messages[0].State()
394-
return mvalid, false, fmt.Errorf("getMessage returned invalid message: msgID: %v st: %v: err %v",
395+
return mvalid, fmt.Errorf("getMessage returned invalid message: msgID: %v st: %v: err %v",
395396
msgID, st, err)
396397
}
397-
return messages[0].Valid(), messages[0].IsValidFull(), nil
398+
return messages[0].Valid(), nil
398399
}
399400

400401
// If we are superseding an ephemeral message, we have to set the
@@ -410,7 +411,7 @@ func (s *BlockingSender) getSupersederEphemeralMetadata(ctx context.Context, uid
410411
return nil, nil
411412
}
412413

413-
supersededMsg, _, err := s.getMessage(ctx, uid, convID, msg.ClientHeader.Supersedes, false /* resolveSupersedes */)
414+
supersededMsg, err := s.getMessage(ctx, uid, convID, msg.ClientHeader.Supersedes, false /* resolveSupersedes */)
414415
if err != nil {
415416
return nil, err
416417
}
@@ -432,7 +433,7 @@ func (s *BlockingSender) processReactionMessage(ctx context.Context, uid gregor1
432433
}
433434

434435
// We could either be posting a reaction or removing one that we already posted.
435-
supersededMsg, _, err := s.getMessage(ctx, uid, convID, msg.ClientHeader.Supersedes,
436+
supersededMsg, err := s.getMessage(ctx, uid, convID, msg.ClientHeader.Supersedes,
436437
true /* resolveSupersedes */)
437438
if err != nil {
438439
return clientHeader, body, err
@@ -1029,7 +1030,7 @@ func (s *BlockingSender) applyTeamBotSettings(ctx context.Context, uid gregor1.U
10291030
// Check if we are superseding a bot message. If so, just take what the
10301031
// superseded has. Don't automatically key for replies, run the normal checks.
10311032
if msg.ClientHeader.Supersedes > 0 && opts.ReplyTo == nil && convID != nil {
1032-
target, _, err := s.getMessage(ctx, uid, *convID, msg.ClientHeader.Supersedes, false /*resolveSupersedes */)
1033+
target, err := s.getMessage(ctx, uid, *convID, msg.ClientHeader.Supersedes, false /*resolveSupersedes */)
10331034
if err != nil {
10341035
return nil, err
10351036
}

go/chat/utils/utils.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1927,13 +1927,9 @@ func PresentMessageUnboxed(ctx context.Context, g *globals.Context, rawMsg chat1
19271927
case chat1.MessageUnboxedState_VALID:
19281928
valid := rawMsg.Valid()
19291929
if !rawMsg.IsValidFull() {
1930-
showErr := true
19311930
// If we have an expired ephemeral message, don't show an error
19321931
// message.
1933-
if valid.IsEphemeral() && valid.IsEphemeralExpired(time.Now()) {
1934-
showErr = false
1935-
}
1936-
if showErr {
1932+
if !(valid.IsEphemeral() && valid.IsEphemeralExpired(time.Now())) {
19371933
return miscErr(fmt.Errorf("unexpected deleted %v message",
19381934
strings.ToLower(rawMsg.GetMessageType().String())))
19391935
}

go/protocol/chat1/extras.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,6 +1046,13 @@ func (m UIMessage) IsValid() bool {
10461046
return false
10471047
}
10481048

1049+
func (m UIMessage) IsError() bool {
1050+
if state, err := m.State(); err == nil {
1051+
return state == MessageUnboxedState_ERROR
1052+
}
1053+
return false
1054+
}
1055+
10491056
func (m UIMessage) IsOutbox() bool {
10501057
if state, err := m.State(); err == nil {
10511058
return state == MessageUnboxedState_OUTBOX

0 commit comments

Comments
 (0)