Rocket.Chat.ReactNative
Rocket.Chat.ReactNative copied to clipboard
feat: automatically send failed messages on app reconnect
Proposed changes
The feature to automatically send failed messages when app reconnects
Issue(s)
closes #4471
How to test or reproduce
Screenshots
After
https://github.com/RocketChat/Rocket.Chat.ReactNative/assets/61021881/05a08fb1-021b-4715-85c0-50dcfc538250
Types of changes
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] Improvement (non-breaking change which improves a current function)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Documentation update (if none of the other choices apply)
Checklist
- [x] I have read the CONTRIBUTING doc
- [x] I have signed the CLA
- [x] Lint and unit tests pass locally with my changes
- [ ] I have added tests that prove my fix is effective or that my feature works (if applicable)
- [ ] I have added necessary documentation (if applicable)
- [ ] Any dependent changes have been merged and published in downstream modules
Further comments
@diegolmello I have implemented the feature. Kindly review the code at your convenience sir, Thank you.
@GleidsonDaniel Can you check my pull request when you have time? Thank you sir
@diegolmello Should I also consider resending attachments?
@diegolmello Should I also consider resending attachments?
I don't think so, it's better to deal with it in another PR and another function. Looking at the method, I don't think there's anything wrong, but @diegolmello has more knowledge of all the APP's flows, so I leave this double check to him.
@diegolmello Should I also consider resending attachments?
@diegolmello Should I also consider resending attachments?
I don't think so, it's better to deal with it in another PR and another function. Looking at the method, I don't think there's anything wrong, but @diegolmello has more knowledge of all the APP's flows, so I leave this double check to him.
@jsathu07 I'd do it on the same function, because of where it's called and timestamps. I think it's better to do it on this PR as well, because It's going to look weird if it resends only half of failing messages.
@diegolmello Should I also consider resending attachments?
@diegolmello Should I also consider resending attachments?
I don't think so, it's better to deal with it in another PR and another function. Looking at the method, I don't think there's anything wrong, but @diegolmello has more knowledge of all the APP's flows, so I leave this double check to him.
@jsathu07 I'd do it on the same function, because of where it's called and timestamps. I think it's better to do it on this PR as well, because It's going to look weird if it resends only half of failing messages.
Sure sir I'll implement that too
@diegolmello How the app should behave if it needs to resend msgs and attachments (doesn't have ts)
@jsathu07 Can you elaborate your question?
@diegolmello Sir, if someone sends text messages and uploads images, the current code doesn't provide information about when the uploads were started. To address this, when resending messages, should I initiate resending of all uploads first and then the messages or do it the other way around. Best option is to include a "created at" field in the uploads database, sort all messages based on timestamps, and then initiate the resending process. Please share your thoughts whether I can modify the db
@diegolmello Sir, if someone sends text messages and uploads images, the current code doesn't provide information about when the uploads were started. To address this, when resending messages, should I initiate resending of all uploads first and then the messages or do it the other way around. Best option is to include a "created at" field in the uploads database, sort all messages based on timestamps, and then initiate the resending process. Please share your thoughts whether I can modify the db
@jsathu07 If what's missing is a date column to sort, I say we create Uploads.ts
column to do it. I didn't find any created_at
, so I think ts
is better just for the sake of consistency.
Don't forget to add a migration.
@diegolmello Sir, if user has any attachments which needs to be resend, which were initiated before migration how should I handle it? Should I leave it without resending so that user can initiate resending if they want
@diegolmello Sir, if user has any attachments which needs to be resend, which were initiated before migration how should I handle it? Should I leave it without resending so that user can initiate resending if they want
@jsathu07 Yes, you can do it like that.
@diegolmello, I've completed the feature sir. Could you review it when you have time, Thanks
@diegolmello tell me if there is anything I can do to improve or change in this pr sir. Thanks
@jsathu07 In a few days. We have other priorities rn.