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

Community Bounty no.17 - Discord Video Bot #196

Merged
merged 6 commits into from
Jul 5, 2021
Merged

Conversation

singulart
Copy link
Contributor

The implementation of a bot that is expected to fulfil the requirements laid out here

Example of the bot announcement: https://imgur.com/a/MXPkC2z

Supported data points:

  • video ID
  • video title
  • video description,
  • video category,
  • channel author,
  • license,
  • video URL,
  • category URL,
  • video duration

Tags are not supported, because this feature is not yet implemented in Joystream Studio.

@freakstatic
Copy link
Collaborator

Mine "Developer mode" was in User Settings -> Advanced -> Developer Mode.

Got this error after uploading this video (https://play.joystream.org/video/520):
TypeError: Cannot read property 'liaison' of null at joystreamvideobot/lib/src/bot.js:48:108

Note:
Node 16 seems to be required

@freakstatic
Copy link
Collaborator

By commenting the setAuthor I was able to test it and seems to post on discord just fine!
Great job 👍

@freakstatic
Copy link
Collaborator

Mine "Developer mode" was in User Settings -> Advanced -> Developer Mode.

Got this error after uploading this video (https://play.joystream.org/video/520):
TypeError: Cannot read property 'liaison' of null at joystreamvideobot/lib/src/bot.js:48:108

Note:
Node 16 seems to be required

oh seems to be because my channel doesn't have an avatar :)

@singulart
Copy link
Contributor Author

Got this error after uploading this video (https://play.joystream.org/video/520):
TypeError: Cannot read property 'liaison' of null at joystreamvideobot/lib/src/bot.js:48:108

Your channel has no avatar, this was the root cause of an issue. I'll try to fix this today

@freakstatic
Copy link
Collaborator

I think that is a bit important to clear the ids set
probably not a issue but after various months of the bot running that set will keep increasing and consuming more memory...
I think you can remove the video from the set if the createdAgo is higher than the video "uploaded date" the next iteration will not catch that video anyway :)

@singulart
Copy link
Contributor Author

I think that is a bit important to clear the ids set
probably not a issue but after various months of the bot running that set will keep increasing and consuming more memory...
I think you can remove the video from the set if the createdAgo is higher than the video "uploaded date" the next iteration will not catch that video anyway :)

Agreed

@freakstatic
Copy link
Collaborator

LGTM! 👍

@mochet
Copy link
Collaborator

mochet commented Jun 19, 2021

this was approved by the council in the following proposal: https://testnet.joystream.org/#/proposals/154

@mochet mochet requested a review from bwhm June 19, 2021 14:52
@mochet
Copy link
Collaborator

mochet commented Jun 24, 2021

@bwhm as this involves code I won't merge it and will wait for your review

Copy link
Member

@bwhm bwhm left a comment

Choose a reason for hiding this comment

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

Looks great @singulart!

Was about to test deploy it now, but I get:

this integration is private. only the application owner can add it.

I assume this means only the bot owner, not the server owner?

@singulart
Copy link
Contributor Author

Yes, the bot is an application refered to in this message I think

@bwhm
Copy link
Member

bwhm commented Jul 2, 2021

Yes, the bot is an application refered to in this message I think

Thanks for the quick reply. Are you able to grant us the permission required?

@singulart
Copy link
Contributor Author

You don't need any specific permission from me) My assumption was that Jsgenesis would create the bot in discord guided by the instructions in the community repo: https://github.com/Joystream/community-repo/tree/master/community-contributions/discordbot

@bwhm
Copy link
Member

bwhm commented Jul 2, 2021

Sorry for the confusion, I thought it was already deployed as I got a link for it earlier :)

Copy link
Member

@bwhm bwhm left a comment

Choose a reason for hiding this comment

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

Seems to work well, but I'm seeing some issues:

Deployment issues

Running failed due to replaceAll I had to manually add it with
yarn add string.prototype.replaceall

Then, the build failed before I created a new file:
node_modules/string.prototype.replaceall/index.d.ts
with
declare module 'string.prototype.replaceall';

Errors

I'm getting this error all the time:

Video 1853 already announced. 
TypeError: Cannot read property 'metadata' of null
    at /home/joystream/singulart-community-repo/community-contributions/joystreamvideobot/lib/src/bot.js:55:85
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at async main (/home/joystream/singulart-community-repo/community-contributions/joystreamvideobot/lib/src/bot.js:33:9)

It doesn't crash, but I don't think it's great to have it running like that.


These are rather small issues in the grand scheme of things, but would be great if you could fix them :)

community-contributions/joystreamvideobot/README.md Outdated Show resolved Hide resolved
@singulart
Copy link
Contributor Author

Deployment issue mentioned can be fixed by upgrading the node version, I believe.

I will look into the missing metadata

@singulart
Copy link
Contributor Author

Comments of @bwhm were addressed.

@singulart singulart closed this Jul 2, 2021
@singulart singulart reopened this Jul 2, 2021
@singulart singulart requested a review from bwhm July 2, 2021 15:28
Copy link
Member

@bwhm bwhm left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the quick fixes!

@bwhm bwhm merged commit b6baebe into Joystream:master Jul 5, 2021
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

4 participants