fix: login service configuration notifier event#33410
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 |
|
|
d173b7a to
1cb7dc9
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #33410 +/- ##
===========================================
- Coverage 58.58% 58.55% -0.03%
===========================================
Files 2737 2737
Lines 65757 65781 +24
Branches 14825 14831 +6
===========================================
- Hits 38521 38520 -1
- Misses 24457 24482 +25
Partials 2779 2779
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
matheusbsilva137
left a comment
There was a problem hiding this comment.
Please add a changeset to this PR
pierre-lehnen-rc
left a comment
There was a problem hiding this comment.
commented under matheus' previous comment.
| const item = await LoginServiceConfiguration.findOneById<Omit<LoginServiceConfigurationData, 'secret'>>(_id, { | ||
| projection: { secret: 0 }, | ||
| }); | ||
| if (!item) { |
There was a problem hiding this comment.
this check will prevent this function from working for removed items
| if (oAuthServices.status === 'fulfilled') { | ||
| await Promise.all( | ||
| oAuthServices.value.map((service) => | ||
| notifyOnLoginServiceConfigurationChangedById(service._id, service.deleted ? 'removed' : 'inserted'), |
There was a problem hiding this comment.
previously we always sent updated instead of inserted.. not sure which one is the correct one though
As per OPI-39, we've identified scenarios where a "no broker set to broadcast" message appears, particularly in the "watch.loginServiceConfiguration" event. After reviewing the root cause, we've determined that it's unnecessary to stream changes in real-time through the socket when the server checks for updates to OAuth provider configurations.