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

fix: Added check if count is truthy on cleanRoomHistory #28081

Merged
merged 15 commits into from
May 25, 2023

Conversation

LucianoPierdona
Copy link
Contributor

@LucianoPierdona LucianoPierdona commented Feb 16, 2023

Proposed changes (including videos or screenshots)

This PR adds a check when the room is updated inside the function removeByIdPinnedTimestampLimitAndUsers which is called on cleanRoomHistory; It has been added because even when the room wasn't being updated, the _updatedAt was changed, and caused move chats to the top of the list even when nothing has changed there.

Issue(s)

#27956

Steps to test or reproduce

  • Enable Retention Policy.
  • For clarity, set "Use Advanced Retention Policy Cron" to every 5 minutes
    */5 * * * *
  • After 5 minutes, query the database to verify that the _updatedAt value has changed in each room:
    db.rocketchat_room.find({ }, {"_updatedAt": 1 }).pretty()

Further comments

TC-488

@codecov
Copy link

codecov bot commented Feb 16, 2023

Codecov Report

Merging #28081 (3733d6b) into develop (c75272b) will decrease coverage by 5.89%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #28081      +/-   ##
===========================================
- Coverage    46.86%   40.98%   -5.89%     
===========================================
  Files          707      688      -19     
  Lines        13241    13022     -219     
  Branches      2221     2182      -39     
===========================================
- Hits          6206     5337     -869     
- Misses        6720     7398     +678     
+ Partials       315      287      -28     
Flag Coverage Δ
e2e 40.96% <ø> (-5.88%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@LucianoPierdona LucianoPierdona marked this pull request as ready for review February 28, 2023 17:15
@sampaiodiego
Copy link
Member

while I don't have to time this properly, what do you think about creating some tests?

it doesn't need to use the scheduled "history clean" that you need to wait some minutes for it to run, you can trigger the "history clean" manually and assert the room data has not changed.

@LucianoPierdona
Copy link
Contributor Author

@sampaiodiego sure, I can do that

@LucianoPierdona LucianoPierdona requested a review from a team as a code owner March 6, 2023 16:51
@LucianoPierdona LucianoPierdona changed the title Chore: Added check if count is truthy on cleanRoomHistory chore: Added check if count is truthy on cleanRoomHistory Mar 14, 2023
@matheusbsilva137 matheusbsilva137 requested a review from a team as a code owner March 27, 2023 12:12
@scuciatto scuciatto added this to the 6.3.0 milestone May 8, 2023
@changeset-bot
Copy link

changeset-bot bot commented May 22, 2023

🦋 Changeset detected

Latest commit: 3733d6b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 24 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/api-client Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/ddp-client Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/models Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/instance-status Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels May 22, 2023
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels May 22, 2023
@kodiakhq kodiakhq bot requested a review from a team as a code owner May 25, 2023 21:15
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels May 25, 2023
@matheusbsilva137 matheusbsilva137 changed the title chore: Added check if count is truthy on cleanRoomHistory fix: Added check if count is truthy on cleanRoomHistory May 25, 2023
@kodiakhq kodiakhq bot merged commit 3f58495 into develop May 25, 2023
@kodiakhq kodiakhq bot deleted the fix/room-updating-on-prune-messages branch May 25, 2023 21:52
gabriellsh added a commit that referenced this pull request May 26, 2023
…ove/mentions

* 'develop' of github.com:RocketChat/Rocket.Chat: (48 commits)
  fix: Changed contact form async validations to onSubmit (#29250)
  refactor: Omnichannel Department re-write (#28948)
  feat: Added attachments to contact history message list (#29336)
  fix: Clicking uploaded file title replaces current tab (#29174)
  fix: broken error messages on room.saveInfo & missing CF validations on omni/contact api (#28367)
  regression: Missing loading indicator (#29374)
  fix: Added check if count is truthy on `cleanRoomHistory` (#28081)
  chore: small tricks with keys and invalidations with marketplace page (#29369)
  fix: Missing await on agent leave action (#29358)
  refactor: useQuery for Marketplace Lists (#29348)
  test: add missing omnichannel contact-center tests (#28989)
  ci: omit vite log (#29360)
  chore: update `status-warning-2` color (#29321)
  fix: File upload in Safari, IOS devices (#27121)
  chore: update status-bullet colors (#29316)
  chore: `Contextualbar` empty state consistency (#29341)
  chore(deps-dev): Bump @storybook/source-loader from 6.5.15 to 6.5.16 (#27866)
  feat(apps): Disabling apps on trial license expiration (#29037)
  chore(deps-dev): Bump @storybook/manager-webpack4 from 6.5.15 to 6.5.16 (#27865)
  chore(deps-dev): Bump @storybook/addon-actions from 6.5.15 to 6.5.16 (#27917)
  ...
gabriellsh added a commit that referenced this pull request May 26, 2023
…memberList

* 'develop' of github.com:RocketChat/Rocket.Chat: (30 commits)
  fix: Changed contact form async validations to onSubmit (#29250)
  refactor: Omnichannel Department re-write (#28948)
  feat: Added attachments to contact history message list (#29336)
  fix: Clicking uploaded file title replaces current tab (#29174)
  fix: broken error messages on room.saveInfo & missing CF validations on omni/contact api (#28367)
  regression: Missing loading indicator (#29374)
  fix: Added check if count is truthy on `cleanRoomHistory` (#28081)
  chore: small tricks with keys and invalidations with marketplace page (#29369)
  fix: Missing await on agent leave action (#29358)
  refactor: useQuery for Marketplace Lists (#29348)
  test: add missing omnichannel contact-center tests (#28989)
  ci: omit vite log (#29360)
  chore: update `status-warning-2` color (#29321)
  fix: File upload in Safari, IOS devices (#27121)
  chore: update status-bullet colors (#29316)
  chore: `Contextualbar` empty state consistency (#29341)
  chore(deps-dev): Bump @storybook/source-loader from 6.5.15 to 6.5.16 (#27866)
  feat(apps): Disabling apps on trial license expiration (#29037)
  chore(deps-dev): Bump @storybook/manager-webpack4 from 6.5.15 to 6.5.16 (#27865)
  chore(deps-dev): Bump @storybook/addon-actions from 6.5.15 to 6.5.16 (#27917)
  ...
hugocostadev pushed a commit that referenced this pull request May 30, 2023
Co-authored-by: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants