Skip to content
This repository has been archived by the owner on Dec 3, 2019. It is now read-only.

Add available scheme to CodeSchool url #452

Closed
wants to merge 1 commit into from
Closed

Add available scheme to CodeSchool url #452

wants to merge 1 commit into from

Conversation

AlexandrSorokin
Copy link

Description of changes

Before saving CodeSchool to the database, it takes the following steps:

  1. Substitutes the protocol http in url without protocol
  2. Make HTTP call to url with https
  3. If https is not available, then change protocol to http

The http call slows down saving to the database. It can be avoided by using only the http protocol.
This is my first commit to an open source project, so please tell me what I am doing wrong if you can 😊

Issue Resolved

Fixes #436

before_create :set_valid_scheme, :if => Proc.new{ |cs| URI(cs.url).scheme.nil? }

private

Copy link
Member

Choose a reason for hiding this comment

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


private

def set_valid_scheme
Copy link
Member

Choose a reason for hiding this comment

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

Would you be able to add a unit test showing what happens in these cases:

  1. Correct url
  2. Incorrect url with https being the valid route
  3. Incorrect url with http being the valid route

Thank you!

@apex-omontgomery
Copy link
Member

Thanks for the PR. If this is your first PR with us please fill this form out and ashley will send you some stickers and a handwritten letter.
https://docs.google.com/forms/d/e/1FAIpQLSfiBjBC3ALBMiWHQXNvYqNlr5ywW2fhw7i0n82w7gwqN_uc4g/viewform

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants