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

api/src/services/url: support for longer Loom URL variant containing content ID #832

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

styxnanda
Copy link

Background

This is in response to #799.

It's not really a bug, but the original code intended the ID for Loom videos to be at most 32 characters. For example, https://www.loom.com/share/4a2a8baf124c4390954dcbb46a58cfd7.

However, apparently Loom has a longer URL variant that has the title encoded as well such as https://www.loom.com/share/Unlocking-Incredible-Organizational-Velocity-with-Async-Video-4a2a8baf124c4390954dcbb46a58cfd7.

However, I did notice that in this longer variant URL form, the URL actually contains the ID right at the end of the path. For example in https://www.loom.com/share/Unlocking-Incredible-Organizational-Velocity-with-Async-Video-4a2a8baf124c4390954dcbb46a58cfd7, the ID of the video is 4a2a8baf124c4390954dcbb46a58cfd7.

Solving Attempt

So, what I did is pretty much utilize the function aliasURL in url.js to provide support for this URL variant. Then, I changed the URL pathname to the correct format so the patternMatch is the exact ID to be later on tested by the tester pattern by extracting the last 32 characters from the supposed id containing more than 32 characters. Here's the code mentioned.

case "loom":
    const idPart = parts[parts.length - 1];
    if(idPart.length > 32){
        const actualIdPart = idPart.slice(-32);
        url.pathname = `/share/${actualIdPart}`;
    }
    break;

Test Result

I've done a manual test using Yaak and it succeeds in the supposedly unyielding URLs.
image

I've run npm run test as well and met some failures in reddit, streamable, and vimeo test cases but this is because those sites are banned in my country. It shouldn't affect those at all though.

Copy link
Contributor

@KwiatekMiki KwiatekMiki left a comment

Choose a reason for hiding this comment

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

lgtm

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