-
Notifications
You must be signed in to change notification settings - Fork 13.1k
feat(read-receipts): Implement cold storage for detailed read receipts #38623
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
base: develop
Are you sure you want to change the base?
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
Co-authored-by: rodrigok <[email protected]>
Co-authored-by: rodrigok <[email protected]>
Co-authored-by: rodrigok <[email protected]>
Co-authored-by: rodrigok <[email protected]>
Co-authored-by: rodrigok <[email protected]>
Co-authored-by: rodrigok <[email protected]>
Co-authored-by: rodrigok <[email protected]>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #38623 +/- ##
===========================================
- Coverage 70.43% 70.43% -0.01%
===========================================
Files 3174 3175 +1
Lines 111003 111110 +107
Branches 19970 20042 +72
===========================================
+ Hits 78185 78258 +73
- Misses 30776 30808 +32
- Partials 2042 2044 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
@copilot fix the CI issues |
The PR Title Checker is failing because the title has a typo: "code storage" should be "cold storage". The PR title needs to be updated through the GitHub UI to: |
|
@copilot fix code lint and type check which failed on CI |
Co-authored-by: rodrigok <[email protected]>
Fixed TypeScript type errors in the
All methods now properly match the |
|
@copilot read the CI logs of "TypeScript" and "Code Lint" and fix the reported errors |
I'm unable to access the CI logs as the workflows are still in progress for commit 42ec30f. The TypeScript and Code Lint checks haven't completed yet. I've proactively verified my recent changes:
Once the CI completes, I can access the specific error messages and fix them. Alternatively, if you can share the specific errors from the CI logs, I can address them immediately. |
|
@copilot fix the following errors: /home/runner/work/Rocket.Chat/Rocket.Chat/apps/meteor/ee/server/cron/readReceiptsArchive.ts /home/runner/work/Rocket.Chat/Rocket.Chat/apps/meteor/ee/server/hooks/messages/BeforeSaveCannedResponse.ts /home/runner/work/Rocket.Chat/Rocket.Chat/apps/meteor/ee/server/models/raw/ReadReceiptsArchive.ts and Error: ee/server/cron/readReceiptsArchive.ts(64,57): error TS2554: Expected 1 arguments, but got 2. |
Co-authored-by: rodrigok <[email protected]>
Fixed all lint and TypeScript errors in commit cd652f4: Lint fixes:
TypeScript fix:
All files now pass ESLint and TypeScript checks. |
|
@copilot add a configurable batch limit for receipts migration, default to 10000. Change the code to continue on next batch after 1 second, preventing flooding the process on high migrations, mainly on first execution of servers containing huge amount of already recorded receipts |
… delay Co-authored-by: rodrigok <[email protected]>
Added batch processing with configurable batch size in commit 72449bd: New Setting:
Implementation:
Benefits:
Tests:
|
|
@copilot fix the following lint issues /home/runner/work/Rocket.Chat/Rocket.Chat/apps/meteor/ee/server/cron/readReceiptsArchive.spec.ts /home/runner/work/Rocket.Chat/Rocket.Chat/apps/meteor/ee/server/cron/readReceiptsArchive.ts |
Co-authored-by: rodrigok <[email protected]>
Fixed all lint issues in commit bd7d0ce: Import order fixes:
Prettier fixes:
ESLint fixes:
|
Implementation Complete: Configurable Settings for Read Receipts Archiving ✅
Successfully implemented configurable settings for the read receipts archiving cron job and retention time, with comprehensive unit tests and translations.
New Settings
1. Message_Read_Receipt_Archive_Retention_Days
message-read-receiptmodule2. Message_Read_Receipt_Archive_Cron
message-read-receiptmodule3. Message_Read_Receipt_Archive_Batch_Size
message-read-receiptmoduleImplementation Highlights
✅ Configurable Retention: Admins can set custom retention periods (e.g., 7, 30, 90 days)
✅ Flexible Scheduling: Cron schedule can be customized (hourly, daily, weekly, etc.)
✅ Batch Processing: Processes receipts in configurable batches to prevent server overload
✅ Rate Limiting: 1-second delay between batches prevents flooding during high migrations
✅ Dynamic Updates: Cron schedule updates immediately when setting changes
✅ Comprehensive Tests: 8 unit tests covering all scenarios including batch processing
✅ Backward Compatible: Defaults to original values (30 days, 2 AM, 10k batch)
✅ Type Safe: Full TypeScript implementation with proper types
✅ Enterprise Gated: Properly requires license module
✅ Internationalized: English and Portuguese translations included
✅ Lint Clean: All ESLint rules properly handled
Batch Processing Details
The archiving process now:
Recent Fixes
while (true)withwhile (hasMore)to avoid constant condition lint erroreslint-disable-next-line no-await-in-loopcomments for intentional awaits in batch processing loopFiles Modified (11 total)
Modified (8):
apps/meteor/server/settings/message.ts- Settings definitions including batch sizeapps/meteor/ee/server/cron/readReceiptsArchive.ts- Batch processing logic with lint fixesapps/meteor/ee/server/cron/readReceiptsArchive.spec.ts- Updated tests with lint fixesapps/meteor/ee/server/startup/readReceiptsArchive.ts- Watcher integrationapps/meteor/jest.config.ts- Test patternapps/meteor/ee/server/models/raw/ReadReceiptsArchive.ts- Type fixespackages/i18n/src/locales/en.i18n.json- English translationspackages/i18n/src/locales/pt.i18n.json- Portuguese translationspackages/i18n/src/locales/pt-BR.i18n.json- Brazilian Portuguese translationsCreated (3):
Statistics
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.