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 Cookie Handling, Fix Publish Date, and Update Element Selectors #88

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

VasilisTako
Copy link
Contributor

TODO: Further improve selector usage by incorporating wildcards where appropriate for better code readability.

During the development process, @TheJoin95 and I discussed the possibility of using class-based elements or selector-based elements for referencing xPath elements. I considered using wildcards in the selectors to improve code readability, but I haven't implemented them yet due to uncertainty about whether to adopt selector-based or class-based elements. For now, I've kept it simple and used selectors without wildcards.

@abe-101, I'd appreciate your feedback on these changes and any suggestions you might have on whether to pursue selector-based or class-based elements for future updates.

VT

…ish date referred to issue Schrodinger-Hat#84. Changed text based element xpath to selectors because with different languages on browser it gets stuck. TODO: Change where possible full selector with wildcards
Copy link
Member

@TheJoin95 TheJoin95 left a comment

Choose a reason for hiding this comment

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

Thanks for handling the cookie panel, it will solves a lot of issue I believe.

On the publish/confirm/days selectors: you are using some classes defined by the bundler, which will change at every spotify release.

We should avoid using those kind of classes.

Instead, since the issue may came from a different language set on the account, we should fix this by set at the start (when we are accepting the cookies) the english language.

I see there is this button here:
<button aria-label="Change language" data-encore-id="buttonSecondary" class="Button-sc-y0gtbx-0 jHYozi"> that will trigger the language modal then we can click on the data-testid="language-option-en" selector which is relative to the english language.

Doing this will secure that the selectors containing the text() functions will be always valid.

What do you think on this?

@VasilisTako
Copy link
Contributor Author

Thanks for handling the cookie panel, it will solves a lot of issue I believe.

On the publish/confirm/days selectors: you are using some classes defined by the bundler, which will change at every spotify release.

We should avoid using those kind of classes.

Instead, since the issue may came from a different language set on the account, we should fix this by set at the start (when we are accepting the cookies) the english language.

I see there is this button here: <button aria-label="Change language" data-encore-id="buttonSecondary" class="Button-sc-y0gtbx-0 jHYozi"> that will trigger the language modal then we can click on the data-testid="language-option-en" selector which is relative to the english language.

Doing this will secure that the selectors containing the text() functions will be always valid.

What do you think on this?

That would certainly solve the problem without having to change all the selectors, you are right. I'm getting back to text based xpath elements and forcing the language to be us using the button you suggested.

Thank you, i didn't see that. Would have been easier too :D

Going to fix that,

VT

Copy link
Member

@TheJoin95 TheJoin95 left a comment

Choose a reason for hiding this comment

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

Looking good on my side, did not try it locally: could you please attach the successful upload terminal log or a video in headless: false mode? Thank you, great effort there

@VasilisTako
Copy link
Contributor Author

Looking good on my side, did not try it locally: could you please attach the successful upload terminal log or a video in headless: false mode? Thank you, great effort there

Yep, don't know where to upload the video so i'm dumping here the log for now.

vaso@MacBook-Pro-di-Vasilis youtube-to-anchorfm % npm start 

> youtube-to-anchorfm@2.0.0 start
> node src/index.js

Getting JSON video info for video id j839QWYQ5xk
title: IMBEVIBILE, MA fa 250MLN in 12 mesi
description: Prime, KSI, Logan Paul
Upload date: {"year":"2023","month":"Apr","day":"18"}
Downloading thumbnail for video id j839QWYQ5xk
Downloading audio for video id j839QWYQ5xk
Downloaded thumbnail for video id j839QWYQ5xk
Downloaded audio for video id j839QWYQ5xk
Posting episode to anchorfm
Launching puppeteer
Trying to log in
Logged in
Uploading audio file
Waiting for upload to finish
-- Adding title
-- Adding description
-- Selecting content type
-- Saving draft
Yay
Finished successfully.

If needed i can upload the video on yt as private or something like this? don't know which one is the best option here.

VT

TheJoin95
TheJoin95 previously approved these changes Apr 21, 2023
@TheJoin95
Copy link
Member

@abe-101 @matevskial could you please review this? Thanks

@VasilisTako
Copy link
Contributor Author

@abe-101 @matevskial could you please review this? Thanks

i've noticed that i missed to revert the xpath of save_as_draft button. Now should be all right

@matevskial
Copy link
Contributor

matevskial commented Apr 23, 2023

Thank you for the contribution.

I checked the code and I have some suggestions.

  • Could you tell me when the language of the page would be different than English? Is it when the operating system language is different? This feature is going to be very useful since the language of the page is not set per-account as far as I can see.

  • Could you provide a screenshot of the cookie popup? Which buttons would the script need to click? I can't see those buttons. Here is mine screenshot

Screenshot_20230423_133003

  • Could you elaborate on why would the cookie popup need to be resolved in order for login to work? As far as I can see, we can fill the login form even without dealing with the cookie popup

  • Lastly, could you create separate PRs for each issue/change that you tackle, in particular:

    • one PR for the refactor where you abstract clicking xpath or selector into function(if there is a real advantage of doing this)
    • one PR tackling the cookie popup
    • one PR tackling the language
    • I would suggest not to create a PR for fixing the publish date since, there is already one PR for that

@VasilisTako
Copy link
Contributor Author

VasilisTako commented Apr 23, 2023

Thank you @matevskial for the review.

  1. I think the language is set based on IP. Because my system and browser languages are set on english but if i start the script with headless: false I get Italian language that is where my ip is located.
    Selector based element's are no more needed to solve this because there is a language modal that we can use to set language to english at the start of everything as @TheJoin95 suggested and maintain the text based elements.

  2. Here are the screenshots, it will click on cookie options and then accept only 'forced' cookies.

image

image

On the related discord channel i dropped a video with headless false too.

  1. That's true most of the times it's not requested to check the cookie before the login because we can evaluate it in the DOM without problems. But i've noticed that on local mac OS env some times (it's not replicable every time) if i don't have the cookie accepted it doesn't evaluate correctly the login button and so it remains in the login page and never finds the input field. I've tried to run it several times commenting the piece of code cookie related and got this. I'm dumping a screen here.

Never noticed this error instead when i've already accepted the cookie before the login. But here would be nice to test with other local envs and see if the behaviour is the same.

image

  1. Yes I can surely split the PR into smaller ones, and omit the date picker fix, if needed.

one PR for the refactor where you abstract clicking xpath or selector into function(if there is a real advantage of doing this)
what do you mean here? I think that would be an advantage on code readability and sustainability, if you are talking about performance, nope that's completely useless.

@matevskial
Copy link
Contributor

matevskial commented Apr 23, 2023

Thank you for the response.

About point 1: Thank you for the elaboration, the language handling is definitely going to be useful, do it wholeheartedly :D

About point 2: I think I now understand why the cookie popup appears for you, because you are located in an EU country.

About point 3: My question to this point: you don't ever see the issue randomly appearing after you implement cookie popup handling in your local Mac env?

About point 4: Do the refactor then :)

@VasilisTako
Copy link
Contributor Author

about point 3: Nope i've never seen that error with the cookie handler implemented.

about point 4: That's sure :D

@matevskial
Copy link
Contributor

Okay, in that case, After you are done, I am going to implement the handling of my cookie popup :D.

Because I also see the same issue that you try to fix with the cookie popup.

I guess my cookie popup causes the same thing too?

In that case, after you are done, I am going to implement cookie handling for my cookie popup too,

@VasilisTako
Copy link
Contributor Author

Yep you can try, in your case you just need to click the X at the top right corner? Or need to interact with a popup like EU based cookie?

Because we could add an env variable that uses my cookie handler or your cookie handler based on a boolean 'EUZONE' var

@TheJoin95
Copy link
Member

Could you please rebase and verify if this is still applicable ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants