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

Add backend and API functionality for uploaded videos #2581

Merged
merged 1 commit into from
Jan 18, 2022

Conversation

dbelokon
Copy link
Contributor

@dbelokon dbelokon commented Dec 8, 2021

Issue This PR Addresses

#1026, since this issue has a lot of features involved, this PR will not cover all of them.

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

This PR focuses on the backend for tracking and processing feeds of video channels.

The frontend is addressed in PR #2596.

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: A test has been added to make sure that the post is of type 'video'.
  • Screenshots: There is no PR changes.
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@gitpod-io
Copy link

gitpod-io bot commented Dec 8, 2021

@humphd
Copy link
Contributor

humphd commented Dec 8, 2021

@dbelokon it's really cool to see you starting work on this. My initial look at what you've done is making we wonder if we should be exploring how to evolve Posts vs. duplicating it in order to create Videos. It looks like they share a ton of code, which isn't an ideal starting point from a maintenance point of view.

Can you tell me about how this differs from the existing feed/post system? What would it take for us to support a YouTube feed? For example, here's my YouTube channel's feed:

https://www.youtube.com/feeds/videos.xml?channel_id=UCqaMbMDf01BLttof1lHAo2A

An example entry looks like this:

<entry>
<id>yt:video:mNuHA7vH6Wc</id>
<yt:videoId>mNuHA7vH6Wc</yt:videoId>
<yt:channelId>UCqaMbMDf01BLttof1lHAo2A</yt:channelId>
<title>
DPS909 OSD600 Week 03 - Fixing a Bug in the Azure JS SDK
</title>
<link rel="alternate" href="https://www.youtube.com/watch?v=mNuHA7vH6Wc"/>
<author>
<name>David Humphrey</name>
<uri>
https://www.youtube.com/channel/UCqaMbMDf01BLttof1lHAo2A
</uri>
</author>
<published>2021-09-24T19:04:25+00:00</published>
<updated>2021-10-27T10:46:10+00:00</updated>
<media:group>
<media:title>
DPS909 OSD600 Week 03 - Fixing a Bug in the Azure JS SDK
</media:title>
<media:content url="https://www.youtube.com/v/mNuHA7vH6Wc?version=3" type="application/x-shockwave-flash" width="640" height="390"/>
<media:thumbnail url="https://i2.ytimg.com/vi/mNuHA7vH6Wc/hqdefault.jpg" width="480" height="360"/>
<media:description>
Walkthrough and discussion of fixing a bug in https://github.com/Azure/azure-sdk-for-js. Issue at https://github.com/Azure/azure-sdk-for-js/issues/15772. PR at https://github.com/Azure/azure-sdk-for-js/pull/17820.
</media:description>
<media:community>
<media:starRating count="0" average="0.00" min="1" max="5"/>
<media:statistics views="344"/>
</media:community>
</media:group>
</entry>

We have a title, author, description, link, uri, etc. similar to a post in a blog feed. Can we figure out how to ingest this along with a blog feed so that we can share the same code path?

@dbelokon
Copy link
Contributor Author

dbelokon commented Dec 8, 2021

@humphd, I noticed the similarities when I started to process them in the backend, but I wasn't sure if I wanted to change the Post codepath, so that's why I ended up creating a new codepath😅.

The major difference between a blog post and a video, is that the only thing we will store in the DB is the metadata of the video (url, description, title, author), while in the Post path, we assume that the feed already provides us with the contents of the blog.

One change we might apply to join the two concepts is considering that a VideoPost (?) has no contents to cache, while a BlogPost (?) does. Thus, there is a simple check of doesPostHaveContent(), or something along those lines. This may be fine, but I do not know what would be the major changes we would have to take care of. The API for the posts would have to be expanded or slightly improved (should we provide an endpoint that fetches blogposts only, and another endpoint that fetches videoposts only? Should we provide a response where we return an array of both, leaving the frontend to decide how to represent them?)

This is, like, the easy way to merge the two concepts without an unnecessary design.

@humphd
Copy link
Contributor

humphd commented Dec 9, 2021

One quick fix for this is to store the video's Description:

<media:description>
Walkthrough and discussion of fixing a bug in https://github.com/Azure/azure-sdk-for-js. Issue at https://github.com/Azure/azure-sdk-for-js/issues/15772. PR at https://github.com/Azure/azure-sdk-for-js/pull/17820.
</media:description>

Another is to synthesize some HTML that loads a kind of "card" for the video when it loads it.

Solving the "how is the post different between blog and video" problem is a lot easier than "duplicate and maintain two nearly identical services." Let's try and figure it out, and I would suggest that we start with something simple that lets us experiment.

@humphd
Copy link
Contributor

humphd commented Dec 9, 2021

Another thought: we could add a new property to a Post that either indicates its type, or add a property like isMedia or something. Both would let us make a different rendering choice in the front-end, and avoid trying to display text if what we really need is a video.

We should do this with some sense of the need to later include "streams" (i.e., live streams) as separate from "videos."

@dbelokon
Copy link
Contributor Author

@humphd, I simplified it a lot, it actually went smoother than I thought. I am already trying it out in the frontend.

Here's a screenshot of how it is looking like in the frontend:
image

@cindyorangis
Copy link
Contributor

Wow, this is looking amazing. How do I go about reproducing the same result you have in this PR? I don't know how to view David's feed in the timeline

Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

I like the degree to which you've been able to simplify things for this, and it's cool to see a screenshot of a prototype.

I'd like to simplify it further. Reading the code, there's a lot of churn on the back-end and in the data modelling; however, we never use any of that new type info outside of the front-end. Whether a "post" represents a blog or a video (or something else we don't have yet...GitHub release?) is really just a rendering hint to the front-end.

As a result, why don't we synthesize vs. store this info. That is, either in the front-end, or at the REST API level (e.g., when we return a post JSON object), we decorate it using the same logic you have here:

/**
 * Determine the type of the post depending on the given article.
 * The possible types are 'video' or 'blogpost'.
 *
 * @param {*} article
 * @returns a String representing the type
 */
function determinePostType(article) {
  try {
    const associatedLink = new URL(article.link);

    if (associatedLink.hostname.includes('youtube.com')) {
      return 'video';
    }
    // Assume that we are dealing with a blogpost if we
    // are not dealing with videos
    return 'blogpost';
  } catch {
    return 'blogpost';
  }
}

Consider this example:

curl -s https://api.telescope.cdot.systems/v1/posts/e8ebfd9e56 | jq
{
  "id": "e8ebfd9e56",
  "title": "Progress for release 0.4",
  "html": "<h1>\n  <a href=\"#telescopes-issue\">\n  </a>\n  Telescope's issue\n</h1>\n\n<p><a href=\"https://github.com/Seneca-CDOT/telescope/issues/2550\">issue-2550</a></p>\n\n<p>Started by running into an issue related to fetching from mainstream. Somehow there was an unwanted bug related to the packages. I tried to remove the <code>node_modules</code> folder and re-installed all the packages. Unfortunately, I did not go well, so I decided to re-fork the repo and start everything again. By following the instruction from Telescope's Github, I managed to run the site locally, but it took me nearly an hour to do so. The issue was really simple, I just had to modify all properties named <code>justify</code> to <code>justifyContent</code>. What I did was going to the search option of Visual Studio Code and search all the names of <code>justify</code> and changed them to <code>justifyContent</code>. <br>\n<a href=\"https://res.cloudinary.com/practicaldev/image/fetch/s--CkzPJjCI--/c_limit%2Cf_auto%2Cfl_progressive%2Cq_auto%2Cw_880/https://dev-to-uploads.s3.amazonaws.com/uploads/articles/06vrbkm1n9nj6rkyabzf.png\"><img src=\"https://res.cloudinary.com/practicaldev/image/fetch/s--CkzPJjCI--/c_limit%2Cf_auto%2Cfl_progressive%2Cq_auto%2Cw_880/https://dev-to-uploads.s3.amazonaws.com/uploads/articles/06vrbkm1n9nj6rkyabzf.png\" loading=\"lazy\"></a><br>\n(This was after the change, but this picture will help you to understand how searching in Visual Studio Code works) <br>\nI submitted an <a href=\"https://github.com/Seneca-CDOT/telescope/pull/2552\">PR</a> after that and it got accepted. </p>\n\n<h1>\n  <a href=\"#codepeaks-issue\">\n  </a>\n  Codepeak's issue\n</h1>\n\n<p><a href=\"https://github.com/VishalIITP/CodepeakSuperDuper/issues/13\">LeaderboardDesign</a></p>\n\n<p>This issue was challenging since it was more related to designing. I will leave it for my final week. But for now, I will take a look at some templates to get inspired. The hardest part is that the design of the leaderboard should match the design of the site in general, this might include the color, alignment, etc. The owners of the project added me to their Discord so I will send them the design first before touching the coding part.</p>\n\n<h1>\n  <a href=\"#final-thought\">\n  </a>\n  Final thought\n</h1>\n\n<p>So that is it for the second week. I cannot wait to see what issues I will have and what interesting things I will face next week.</p>\n\n",
  "published": "2021-12-13T10:32:02.000Z",
  "updated": "2021-12-13T10:32:02.000Z",
  "url": "https://dev.to/nguyenhung15913/progress-for-release-04-2f46",
  "guid": "https://dev.to/nguyenhung15913/progress-for-release-04-2f46",
  "feed": {
    "id": "b0ea6fdde8",
    "author": "Hung Nguyen",
    "url": "https://dev.to/feed/nguyenhung15913",
    "user": "",
    "link": "https://dev.to/nguyenhung15913",
    "etag": "W/\"2953c77874ab917417a5e680d2e409df\"",
    "lastModified": null
  }
}

Imagine if we added a "type": "video" to the outgoing JSON representation of a post based on the fact that it's from youtube.com. Later when we add support for Vimeo or Twitch we can do the same, and the front-end doesn't have to figure out what this is.

Doing so would mean we could add this more quickly, and not need to update so much of our back-end.

@dbelokon
Copy link
Contributor Author

@cindyledev

Wow, this is looking amazing. How do I go about reproducing the same result you have in this PR? I don't know how to view David's feed in the timeline

This may require a little bit of a setup...

The way I made it work is by adding a feed-like object in the processAllFeeds() in the src/backend/index.js file.

async function processAllFeeds() {
  try {
    // Get an Array of Feed objects from the wiki feed list and Redis
    /* const [all, wiki] = await Promise.all([Feed.all(), getWikiFeeds()]); */
    // Process these feeds into the database and feed queue
    await processFeeds([/*...all, ...wiki*/ { author: 'David Humphrey', url: 'https://www.youtube.com/feeds/videos.xml?channel_id=UCqaMbMDf01BLttof1lHAo2A' }]);
  } catch (err) {
    logger.error({ err }, 'Error queuing feeds');
  }
}

I commented out the other feeds because I focused on the YouTube feeds.

You may also flush the redis cache by writing:

const { redis } = require('./lib/redis');

redis.flushall();

That way, it will only have YouTube feeds. After running the backend with pnpm run start, you would want to pull the changes in #2596 and start the frontend, using the local redis database.

You should have something similar in the screenshot. Lemme know if you find any issues.

@dbelokon
Copy link
Contributor Author

@humphd

I decided to keep the type property in the API response, as it may be useful for any other future clients of the API. Right now, the backend only has the necessary changes to extract the information, but the type synthesis happens in the API.

Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

This is looking great. A few things and I think it's pretty much ready!

src/api/posts/src/data/post.js Outdated Show resolved Hide resolved
src/api/posts/src/data/post.js Show resolved Hide resolved
src/api/posts/src/data/post.js Show resolved Hide resolved
src/backend/feed/processor.js Outdated Show resolved Hide resolved
test/post.test.js Show resolved Hide resolved
JerryHue
JerryHue previously approved these changes Jan 18, 2022
DukeManh
DukeManh previously approved these changes Jan 18, 2022
Copy link
Contributor

@DukeManh DukeManh left a comment

Choose a reason for hiding this comment

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

Nice work!

@dbelokon
Copy link
Contributor Author

Thank you everyone for reviewing!!😄

YouTube videos are tracked like regular blogposts,
as they also come from a type of RSS feed. We make
some changes to adjust to the type of RSS response we
get so we can extract the necessary data.
@dbelokon dbelokon merged commit d2be98c into Seneca-CDOT:master Jan 18, 2022
TueeNguyen pushed a commit to TueeNguyen/telescope that referenced this pull request Feb 6, 2022
- Initial work from @manekenpix
- Edited `npm run dev`
- Export Redis constructor instead
- Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581
- Bumped node-fetch to 2.6.7
@TueeNguyen TueeNguyen mentioned this pull request Feb 6, 2022
8 tasks
TueeNguyen pushed a commit to TueeNguyen/telescope that referenced this pull request Feb 10, 2022
- Initial work from @manekenpix
- Edited `npm run dev`
- Export Redis constructor instead
- Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581
- Bumped node-fetch to 2.6.7
TueeNguyen pushed a commit to TueeNguyen/telescope that referenced this pull request Feb 10, 2022
- Initial work from @manekenpix
- Edited `npm run dev`
- Export Redis constructor instead
- Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581
- Bumped node-fetch to 2.6.7
TueeNguyen pushed a commit to TueeNguyen/telescope that referenced this pull request Feb 12, 2022
- Initial work from @manekenpix
- Edited `npm run dev`
- Export Redis constructor instead
- Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581
- Bumped node-fetch to 2.6.7
TueeNguyen pushed a commit to TueeNguyen/telescope that referenced this pull request Feb 12, 2022
- Initial work from @manekenpix
- Edited `npm run dev`
- Export Redis constructor instead
- Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581
- Bumped node-fetch to 2.6.7
TueeNguyen pushed a commit to TueeNguyen/telescope that referenced this pull request Feb 15, 2022
- Initial work from @manekenpix
- Edited `npm run dev`
- Export Redis constructor instead
- Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581
- Bumped node-fetch to 2.6.7
TueeNguyen pushed a commit to TueeNguyen/telescope that referenced this pull request Feb 15, 2022
- Initial work from @manekenpix
- Edited `npm run dev`
- Export Redis constructor instead
- Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581
- Bumped node-fetch to 2.6.7
TueeNguyen pushed a commit to TueeNguyen/telescope that referenced this pull request Feb 16, 2022
- Initial work from @manekenpix
- Edited `npm run dev`
- Export Redis constructor instead
- Added changes based on backend and Seneca-CDOT#2706 Seneca-CDOT#2581
- Bumped node-fetch to 2.6.7
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.

5 participants