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

PubNub chat iteration 1 #520

Merged
merged 7 commits into from Apr 24, 2024
Merged

PubNub chat iteration 1 #520

merged 7 commits into from Apr 24, 2024

Conversation

markus-kohler
Copy link
Contributor

@markus-kohler markus-kohler commented Mar 26, 2024

Description

PubNub integration utilizing the PubNub ChatSDK for the demo with-pubnub demo. After adding your PubNub publish, subscribe and secret key to the .env file you should have the ability to:

View History
Send Chat Messages
As admin: Mute users
As admin: Ban users
As an admin: View flagged messages
As an admin: View Reported Users
As admin: View Admin Dashboard
As an admin: Delete message
As an admin: Restore message
As a user: Report Message
As user: Report User
View members in the live
As User: Sign in to Chat

Additional Information

@markus-kohler markus-kohler requested a review from a team as a code owner March 26, 2024 19:54
Copy link

vercel bot commented Mar 26, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
coinbase-lvpr-tv ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 24, 2024 9:16pm
ui-kit-docs-embed ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 24, 2024 9:16pm
ui-kit-next-pages ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 24, 2024 9:16pm
ui-kit-with-pubnub ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 24, 2024 9:16pm

Copy link

vercel bot commented Mar 26, 2024

@markus-kohler is attempting to deploy a commit to the Livepeer Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Member

@0xcadams 0xcadams left a comment

Choose a reason for hiding this comment

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

LGTM, a few nits here

}
config.plugins.push(
new NormalModuleReplacementPlugin(
/^hexoid$/,
Copy link
Member

Choose a reason for hiding this comment

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

Is this necessary? I'd prefer to keep the next config as simple as possible so devs don't have to make these changes when they build with this example

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 reported it as a bug to our engineering team to fix inside the SDK. I will check the progress on that. Hopefully, (not likely) we will have it removed before the Webinar. It is not a breaking change it just throws an error in the console if it is not in there which I did not like the look of.

let userId: string | null;
if (window) {
userId =
window.location.pathname === "/"
Copy link
Member

Choose a reason for hiding this comment

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

can we use useRouter here instead of window.location?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

useRouter wasn't working for me so I used usePathname from next/navigation hope thats ok

@@ -39,6 +39,8 @@ export const getPlaybackInfo = unstable_cache(
);

export const createStreamClip = async (opts: ClipPayload) => {
console.log(process.env.STUDIO_API_KEY);
Copy link
Member

Choose a reason for hiding this comment

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

let's remove these console logs

Copy link
Member

@0xcadams 0xcadams left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +29 to +31
pathname === "/"
? "admin"
: `user_${Math.floor(Math.random() * 1000)}_${Date.now()}`;

Check failure

Code scanning / CodeQL

Insecure randomness High

This uses a cryptographically insecure random number generated at
Math.random()
in a security context.
<span className="text-xs font-medium">{username}</span>
{isBroadcaster ? (
<div className="flex flex-row">
{isBroadcaster && flagCount > 0 && (

Check warning

Code scanning / CodeQL

Useless conditional Warning

This use of variable 'isBroadcaster' always evaluates to true.
@@ -0,0 +1,45 @@
import { faBan, faUser, faVolumeMute } from "@fortawesome/free-solid-svg-icons";

Check notice

Code scanning / CodeQL

Unused variable, import, function or class Note

Unused import faUser.
@@ -1,134 +1,664 @@
"use client";

import { cn } from "@/lib/utils";
import { faCog, faUser } from "@fortawesome/free-solid-svg-icons";
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
import { type Channel, Membership, type Message, User } from "@pubnub/chat";

Check notice

Code scanning / CodeQL

Unused variable, import, function or class Note

Unused import Membership.
const [chatMessages, setChatMessages] = useState<Message[]>([]);
const [storedUsers, setStoredUsers] = useState<Map<string, User>>(new Map());
const [loading, setLoading] = useState(false);
const [disconnectMessageStream, setDisconnectMessageStream] = useState<

Check notice

Code scanning / CodeQL

Unused variable, import, function or class Note

Unused variable setDisconnectMessageStream.
@@ -0,0 +1,119 @@
"use client";

Check warning

Code scanning / CodeQL

Unknown directive Warning

Unknown directive: 'use client'.
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.

None yet

2 participants