PMM-14655 Add listeners for settings change and service added

[matejkubinec]

PMM-14655

FB: SUBMODULES-4167

Related PRs:

• PMM-14655 Send events when service is added & settings are changed grafana#864

[dmitri-saricev-3pillargloball]

The main thing I noticed - listener cleanup, the listeners are being added, but I don't see them getting removed:

messenger.addListener({
type: 'SETTINGS_CHANGED',
onMessage: () => refetchSettings(),
});

This could cause a memory leak if the component remounts (like during navigation) - you'd keep stacking up listeners. The fix is pretty simple, though, just need to return a cleanup function:

useEffect(() => {
const settingsListener = messenger.addListener({
type: 'SETTINGS_CHANGED',
onMessage: () => refetchSettings(),
});

const serviceListener = messenger.addListener({
  type: 'SERVICE_ADDED',
  onMessage: () => refetchServiceTypes(),
});

return () => {
  messenger.removeListener(settingsListener);
  messenger.removeListener(serviceListener);
};

}, [refetchSettings, refetchServiceTypes]);

[matejkubinec]

Good catch, I've updated the unregister method in messenger to clear the listeners.

[dmitri-saricev-3pillargloball]

Quick question about initial data loading

I see you're creating these queries with enabled: false:

const { refetch: refetchSettings } = useSettings({
enabled: false,
});

Just want to make sure I understand - are these queries being run somewhere else on initial load? If not, the UI might not have this data until the first event comes in. Might be worth a quick double-check, or if it's intentional, maybe add a comment so future devs know what's up ))))

[matejkubinec]

The initial fetch of settings is in SettingsProvider. This is just used for refetching when triggered from the Grafana side.

[dmitri-saricev-3pillargloball]

A few other things to consider (not blockers, just ideas):

Error handling on refetch:
Right now if the refetch fails, it'll be silent. Maybe worth catching errors and showing a toast or logging them? Totally up to you though:

onMessage: async () => {
try {
await refetchSettings();
} catch (error) {
console.error('Failed to refetch settings:', error);
}
}

[matejkubinec]

I don't think there's a need to notify the user about it failing.