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

Update FK api #85

Merged
merged 4 commits into from Jul 27, 2017

Conversation

@VandorpeDavid
Copy link
Contributor

commented Jul 27, 2017

No description provided.

VandorpeDavid added some commits Jul 27, 2017

clubs = hash['clubs'].map do |club| club['internal_name'] end
timestamp = hash['timestamp']

return [] unless (Time.now - DateTime.parse(timestamp)).abs < 300 # Timestamp can't differ by more than 5 minutes

This comment has been minimized.

Copy link
@feliciaan

feliciaan Jul 27, 2017

Member

300 is a magic number should be abstracted.


return [] unless (Time.now - DateTime.parse(timestamp)).abs < 300 # Timestamp can't differ by more than 5 minutes
dig = digest(Rails.application.secrets.fk_auth_salt, ugent_login, timestamp, clubs)
byebug

This comment has been minimized.

Copy link
@feliciaan

feliciaan Jul 27, 2017

Member

what does byebug do?

@coveralls

This comment has been minimized.

Copy link

commented Jul 27, 2017

Coverage Status

Coverage decreased (-0.9%) to 83.899% when pulling eb2895d on update_fk_api into 89f9029 on master.

@coveralls

This comment has been minimized.

Copy link

commented Jul 27, 2017

Coverage Status

Coverage decreased (-0.9%) to 83.899% when pulling eb2895d on update_fk_api into 89f9029 on master.

@@ -40,21 +40,28 @@ def request_clubnames
return Club.pluck(:internal_name) if Rails.env.development?

ugent_login = session[:cas]['user']

This comment has been minimized.

Copy link
@feliciaan

feliciaan Jul 27, 2017

Member

random line

@@ -40,21 +40,28 @@ def request_clubnames
return Club.pluck(:internal_name) if Rails.env.development?

ugent_login = session[:cas]['user']

def digest(*args)
Digest::SHA256.hexdigest args.join('-')

This comment has been minimized.

Copy link
@feliciaan

feliciaan Jul 27, 2017

Member

256 is a magic number, please abstract!

Also digest function should be set in config, because it can change

})
resp = HTTParty.get("#{ Rails.application.secrets.fk_auth_url }/#{ ugent_login }/FKEnrolment",
headers: {
'X-Authorization': Rails.application.secrets.fk_auth_key,

This comment has been minimized.

Copy link
@feliciaan

feliciaan Jul 27, 2017

Member

X-Authorization is a magic string.

clubs = hash['clubs'].map do |club| club['internal_name'] end
timestamp = hash['timestamp']

max_time_difference = 5*60 # Timestamp can't differ by more than 5 minutes

This comment has been minimized.

Copy link
@feliciaan

feliciaan Jul 27, 2017

Member

2 magic numbers for the price of one 5.minutes


max_time_difference = 5*60 # Timestamp can't differ by more than 5 minutes
return [] unless (Time.now - DateTime.parse(timestamp)).abs < max_time_difference
dig = digest(Rails.application.secrets.fk_auth_salt, ugent_login, timestamp, clubs)

This comment has been minimized.

Copy link
@feliciaan

feliciaan Jul 27, 2017

Member

where you gonna start digging?

@coveralls

This comment has been minimized.

Copy link

commented Jul 27, 2017

Coverage Status

Coverage decreased (-0.9%) to 83.899% when pulling 4b7764c on update_fk_api into 89f9029 on master.

@feliciaan

This comment has been minimized.

Copy link
Member

commented Jul 27, 2017

Maybe a test should be added because the coverage decreased.

@VandorpeDavid VandorpeDavid merged commit a506606 into master Jul 27, 2017

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
@feliciaan feliciaan referenced this pull request Jul 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.