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

Fixed idownloadcoupon scraping #433

Merged
merged 3 commits into from Mar 8, 2024
Merged

Conversation

samin-irtiza
Copy link

@samin-irtiza samin-irtiza commented May 4, 2023

Description

Fixed idownloadcoupon scraping by redirecting redeem button url manually and parsing the redirected udemy link with the coupon code.

Fixes #432

Type of change

Added a new function in http_utils.py called http_get_no_redirect that doesn't follow redirected url
Modified idownloadcoupon.py file and function called get_udemy_course_link. changed the url parsing code.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Running the script in idownloadcoupon only mode by running python.exe run_enroller.py --idownloadcoupon

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • I have added tests that prove my fix is effective or that my feature works (Optional)
  • New and existing unit tests pass locally with my changes (Optional)

Added a function named http_get_no_redirect that doesn't follow redirect and returns raw http response.
fixed idownloadcoupon scraping by handling manual link redirection.
@samin-irtiza
Copy link
Author

samin-irtiza commented May 4, 2023

Hi,
I haven't used 'black' to format code before. Trying to patch provided by restyled.io fails and produces error similar to this open issue involving Visual studio. That is why I haven't pushed code styling commit to the PR. Otherwise my code is working fine and locally its running smoothly.

@cullzie
Copy link
Collaborator

cullzie commented Jun 7, 2023

Hey @samin-irtiza
Thanks for the PR.
I think this could be added to the existing http_get method with an argument like redirect=True being the default. In cases where we don't want the redirect we can pass redirect=False

@samin-irtiza
Copy link
Author

Hi @cullzie
Thanks for the feedback. I always try not to modify existing code as much as possible in a PR that's why I created the new method.

@cullzie
Copy link
Collaborator

cullzie commented Jun 26, 2023

Hey @samin-irtiza, The problem at the moment is there are too many diffs showing in this PR meaning its hard to see the actual changes.
I think the only change you made was to add in that method but we could make the change in maybe 2-3 lines if we update the existing method to accept redirect param similar to how it takes headers and then modifying the specific scraper with the issue to pass redirect=false.
If you run black and isort it should resolve the majority of the diffs that are showing here.

@cullzie
Copy link
Collaborator

cullzie commented Feb 21, 2024

@samin-irtiza Can you please tidy up this PR to only show changes you made. At the moment everything is showing up as changed. Thanks

@samin-irtiza
Copy link
Author

Hi @cullzie, I linted the code with black to tidy it up, but that seems to have caused more changes. Not sure what else I can do.

@fakeid30
Copy link
Collaborator

fakeid30 commented Mar 6, 2024

আমার কাছে ঠিকই লাগছে, আশা করি কালজি আপনার চেঞ্জ অ্যাক্সেপ্ট করবে। @samin-irtiza

@cullzie
Copy link
Collaborator

cullzie commented Mar 8, 2024

Ok thanks @samin-irtiza
I will merge since it will get reformatted at another stage if there is an issue.
Thanks for your contribution 🦖

@cullzie cullzie merged commit a30baf5 into aapatre:develop Mar 8, 2024
1 of 2 checks passed
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

3 participants