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] Settings listeners not receiving overwritten values from env vars #25448

Merged
merged 10 commits into from
May 12, 2022

Conversation

sampaiodiego
Copy link
Member

@sampaiodiego sampaiodiego commented May 9, 2022

Proposed changes (including videos or screenshots)

I've completely changed the order on how things are done during startup so listeners can receive the correct values when they're updated via env vars.

it is done by setting up oplog listener before doing the updates to settings, so if an update happens the correct flow will still happen:

graph LR;
    A([update setting on db])-->B([oplog event triggered]);
    B-->C([received by listener]);
    C-->D([update cache]);
    D-->E([trigger watchers]);
Loading

Issue(s)

Steps to test or reproduce

Further comments

@sampaiodiego sampaiodiego requested review from a team and geekgonecrazy May 9, 2022 21:35
@sampaiodiego sampaiodiego changed the title [FIX] Overwritten settings not getting cached [FIX] Settings listeners not receiving overwritten values from env vars May 11, 2022
@sampaiodiego
Copy link
Member Author

sampaiodiego commented May 11, 2022

this is good for review again =) I've completely changed the initial changes, I'm not forcing cache to be updated anymore when a setting value is overwritten, it is now setting up oplog listener before doing the updates, so if an update happens the correct flow will still happen:

graph LR;
    A([update setting on db])-->B([oplog event triggered]);
    B-->C([received by listener]);
    C-->D([update cache]);
    D-->E([trigger watchers]);
Loading

@sampaiodiego sampaiodiego merged commit 20180a5 into develop May 12, 2022
@sampaiodiego sampaiodiego deleted the fix-settings-not-cached branch May 12, 2022 16:32
gabriellsh added a commit that referenced this pull request May 12, 2022
…lists

* 'develop' of github.com:RocketChat/Rocket.Chat:
  Chore: Move markdown message parser to a `callback` (#25413)
  [FIX] Settings listeners not receiving overwritten values from env vars (#25448)
  Chore: Move ddp-streamer micro service to its own sub-repo (#25246)
  [NEW] Use setting to determine if initial general channel is needed (#25441)
  [IMPROVE] New admin settings Page (#25439)
  [FIX] Failure to update Integration History index (#25473)
  Chore: Rewrite 2fa to typescript (#25285)
  Chore: solve yarn issues from env var (#25468)
  Chore: REST query and body params validation (#25446)
  Chore: Tests with Playwright (task: ROC-66, Intermittent resolution in tests) (#25416)
  Chore: Convert email inbox feature to TypeScript (#25298)
gabriellsh added a commit that referenced this pull request May 16, 2022
* 'develop' of github.com:RocketChat/Rocket.Chat:
  Chore: Migrate NotFoundPage to TS (#25509)
  [FIX] Unable to see channel member list by authorized channel roles (#25412)
  Regression: Fix services-image-build-check (#25519)
  Chore: Migrate spotify to ts (#25507)
  [Chore] Reorder unreleased migrations (#25508)
  [FIX] Spotlight results showing usernames instead of real names (#25471)
  [FIX] LDAP sync removing users from channels when multiple groups are mapped to it (#25434)
  Chore: Move markdown message parser to a `callback` (#25413)
  [FIX] Settings listeners not receiving overwritten values from env vars (#25448)
  Chore: Move ddp-streamer micro service to its own sub-repo (#25246)
  [NEW] Use setting to determine if initial general channel is needed (#25441)
  [IMPROVE] New admin settings Page (#25439)
  [FIX] Failure to update Integration History index (#25473)
  Chore: Rewrite 2fa to typescript (#25285)
  Chore: solve yarn issues from env var (#25468)
  Chore: REST query and body params validation (#25446)
  Chore: Tests with Playwright (task: ROC-66, Intermittent resolution in tests) (#25416)
  Chore: Convert email inbox feature to TypeScript (#25298)
gabriellsh added a commit that referenced this pull request May 17, 2022
…threadTemplate

* 'develop' of github.com:RocketChat/Rocket.Chat: (29 commits)
  Chore: migrate from cypress to pw 14-setting-permission (#25523)
  Chore: Tests with Playwright (task: ROC-31, 12-settings) (#25253)
  Chore: Migrate 15-message-popup from cypress to playwright (#25462)
  Chore: Convert apps/meteor/client/views/admin/settings/inputs folder (#25427)
  [FIX] UI/UX issues on Live Chat widget (#25407)
  Chore: Convert Admin -> Rooms to TS (#25348)
  Chore: Migrate NotFoundPage to TS (#25509)
  [FIX] Unable to see channel member list by authorized channel roles (#25412)
  Regression: Fix services-image-build-check (#25519)
  Chore: Migrate spotify to ts (#25507)
  [Chore] Reorder unreleased migrations (#25508)
  [FIX] Spotlight results showing usernames instead of real names (#25471)
  [FIX] LDAP sync removing users from channels when multiple groups are mapped to it (#25434)
  Chore: Move markdown message parser to a `callback` (#25413)
  [FIX] Settings listeners not receiving overwritten values from env vars (#25448)
  Chore: Move ddp-streamer micro service to its own sub-repo (#25246)
  [NEW] Use setting to determine if initial general channel is needed (#25441)
  [IMPROVE] New admin settings Page (#25439)
  [FIX] Failure to update Integration History index (#25473)
  Chore: Rewrite 2fa to typescript (#25285)
  ...
@d-gubert d-gubert mentioned this pull request May 31, 2022
@murtaza98 murtaza98 mentioned this pull request Jul 21, 2022
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.

2 participants