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

[POC] WS browser events #1895

Closed

Conversation

Hyperkid123
Copy link
Contributor

@Hyperkid123 Hyperkid123 commented May 10, 2022

Description

This is a POC of the upcoming dynamic browser events service (web sockets in short)

It adds two main features

  1. Notification drawer. That is something we will have to support.
  2. Allow application WS listeners registration. This is something that can provide a huge potential

The WS service is not published anywhere yet but it does these things

  1. Emit events based on Kafka messages. Currently, it listens on chrome-events channel. Tapping into existing channels might be an issue due to frequency and due to the fact that the messages probably do not follow strict structures.
  2. Emit events based on a POST request. Just passes along the payload.
  3. Emit events based on a client event send from the browser connection.

The WS service has 3 types of granularity setup

  1. Broadcast: send message to every client connected
  2. Tennant based (org): send message to every connected client in a tenant
  3. User ID based: only a user that matches the ID will receive a message

Each message can have multiple targets. Meaning that we can send a message to more than tenants or users or combinations.

Do not take any code seriously it will change. The Notification drawer has a high priority as it is a future requirement.

Let me know your thoughts. You can check this RBAC POC to see advanced caching and data invalidation options: RedHatInsights/insights-rbac-ui#1139.

@Hyperkid123 Hyperkid123 requested a review from a team May 10, 2022 14:48
@Hyperkid123 Hyperkid123 changed the title Browser events poc [POC] WS browser events May 10, 2022
@@ -6,7 +6,7 @@
<title>
console.redhat.com
</title>
<meta http-equiv="Content-Security-Policy" content="default-src 'self' https: wss: data: 'unsafe-inline' 'unsafe-eval' https://*.redhat.com/ https://www.redhat.com https://*.openshift.com/ https://api.stage.openshift.com/ https://identity.api.openshift.com/ https://www.youtube.com/ https://redhat.sc.omtrdc.net/ https://assets.adobedtm.com https://www.redhat.com https://*.storage.googleapis.com/ https://*.hotjar.com:* https://*.hotjar.io wss://*.hotjar.com;">
<meta http-equiv="Content-Security-Policy" content="default-src 'self' https: wss: data: 'unsafe-inline' 'unsafe-eval' ws://localhost:8080 https://*.redhat.com/ https://www.redhat.com https://*.openshift.com/ https://api.stage.openshift.com/ https://identity.api.openshift.com/ https://www.youtube.com/ https://redhat.sc.omtrdc.net/ https://assets.adobedtm.com https://www.redhat.com https://*.storage.googleapis.com/ https://*.hotjar.com:* https://*.hotjar.io wss://*.hotjar.com;">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about wss:? Also, shouldn't there be a wss:///.redhat.com?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't take these seriously we I don't even know where the service will live

Comment on lines +85 to +86
'toggle-notifications-drawer': notificationsDrawerReducer,
'add-notification': addNewNotificationReducer,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make these into constants?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely. This is just a draft to demo the features. Do not take anything seriously, only the concept.

* Establish connection
*/
useEffect(() => {
const x = new WebSocket('ws://localhost:8080/ws');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather see a proxy config in webpack for websockets.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to do some research on this. I am not sure how the webpack proxy works with WS. Not sure if our implementation will work properly.

Object.values(events.current)
.flat()
.forEach((cb) => {
cb(payload);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd check if the callback is defined in here

Suggested change
cb(payload);
cb?.(payload);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's checked when a callback is registered. It will not register listener if a callback is not a function

@Hyperkid123
Copy link
Contributor Author

Obsolete

@Hyperkid123 Hyperkid123 deleted the browser-events-poc branch September 11, 2023 09:59
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