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

feat: add a way to undelete a deleted messages #1237

Merged
merged 7 commits into from
Mar 8, 2024
Merged

feat: add a way to undelete a deleted messages #1237

merged 7 commits into from
Mar 8, 2024

Conversation

akupila
Copy link
Contributor

@akupila akupila commented Feb 22, 2024

CLA

  • I have signed the Stream CLA (required).
  • Code changes are tested

Description of the changes, What, Why and How?

This adds support for calling a new undelete endpoint, which can be used for undeleting previously soft deleted messages. The API requires this to be called from serverside clients and will return a 403 if called from the browser.

The API will also publish a new message.undeleted event when the message has been undelete. I'm not sure if more changes are required here beyond adding it to the event map.

Changelog

  • Add undeleteMessage for bringing back soft deleting messages

Copy link
Contributor

github-actions bot commented Feb 22, 2024

Size Change: +851 B (0%)

Total Size: 353 kB

Filename Size Change
dist/browser.es.js 76.2 kB +201 B (0%)
dist/browser.full-bundle.min.js 46.4 kB +40 B (0%)
dist/browser.js 77.1 kB +204 B (0%)
dist/index.es.js 76.2 kB +202 B (0%)
dist/index.js 77.2 kB +204 B (0%)

compressed-size-action

@akupila akupila changed the title feat: Add a way to undelete a deleted messages feat: add a way to undelete a deleted messages Feb 22, 2024
@akupila
Copy link
Contributor Author

akupila commented Feb 23, 2024

The tests are failing here, but i'm not entirely sure why. Happy to fix if somebody can provide some guidance on what needs to be done 🙂

oliverlaz
oliverlaz previously approved these changes Feb 23, 2024
@oliverlaz
Copy link
Member

The tests are failing here, but i'm not entirely sure why. Happy to fix if somebody can provide some guidance on what needs to be done 🙂

I see threads there. Perhaps @vishalnarkhede knows more about this one :)

oliverlaz
oliverlaz previously approved these changes Feb 23, 2024
@arnautov-anton
Copy link
Contributor

We're missing a WS event handler here - is that expected?

@akupila
Copy link
Contributor Author

akupila commented Feb 23, 2024

We're missing a WS event handler here - is that expected?

Sorry, I'm not very familiar with this codebase. I didn't deliberately leave it out if it's required. Could you point me in the right direction, and I'll try to fix it?

@arnautov-anton
Copy link
Contributor

We're missing a WS event handler here - is that expected?

Sorry, I'm not very familiar with this codebase. I didn't deliberately leave it out if it's required. Could you point me in the right direction, and I'll try to fix it?

Oh, no worries! I'm not sure what the specs are here but it should look something like this:

stream-chat-js/src/channel.ts

Lines 1272 to 1284 in cf27583

case 'message.deleted':
if (event.message) {
this._extendEventWithOwnReactions(event);
if (event.hard_delete) channelState.removeMessage(event.message);
else channelState.addMessageSorted(event.message, false, false);
channelState.removeQuotedMessageReferences(event.message);
if (event.message.pinned) {
channelState.removePinnedMessage(event.message);
}
}
break;

@akupila
Copy link
Contributor Author

akupila commented Feb 23, 2024

@arnautov-anton thanks, that makes a lot of sense. AFAIK, the handler should be basically the same as for when a message is updated. I did some manual testing with this event sent from the API instead, and messages re-appeared. I added this ebb589f

arnautov-anton
arnautov-anton previously approved these changes Feb 23, 2024
Copy link
Contributor

@vishalnarkhede vishalnarkhede left a comment

Choose a reason for hiding this comment

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

Ignore the failing type test. I will fix those separately. You can even disable them for now.

@akupila akupila merged commit d2193a2 into master Mar 8, 2024
5 of 6 checks passed
@akupila akupila deleted the undelete branch March 8, 2024 12:04
@github-actions github-actions bot mentioned this pull request Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants