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.

Sign up for GitHub

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

Jump to bottom

Build badgeKit API client #26

Closed
aid29 opened this issue May 26, 2015 · 24 comments
Closed

Build badgeKit API client #26

aid29 opened this issue May 26, 2015 · 24 comments
Assignees
@aid29

Comments

@aid29
Copy link
Contributor

aid29 commented May 26, 2015

Extract the codes using the badgeKit API to a separated client class, make easy to test this client against real api server or a fake one.
@abbycabs
Copy link
Member

Thanks for this too @aid29!

@abbycabs abbycabs mentioned this issue May 29, 2015
Closed
3 tasks
@aid29
Copy link
Contributor Author

aid29 commented Jun 3, 2015

No problem! Like to work on this.

@aid29
Copy link
Contributor Author

aid29 commented Jun 3, 2015

But may not finished before Mozilla Global Sprint, what is your plan on Sprint?

@abbycabs
Copy link
Member

abbycabs commented Jun 3, 2015

During the sprint, I'd like to finish everything in Short Term on the Roadmap: #17

Just to clarify, this issue is separating the badgekit code out of app.js into a separate module? We are technically already using badgekit-api-client (http://github.com/mozilla/badgekit-api-client) to interface with the badgekit-api

@aid29
Copy link
Contributor Author

aid29 commented Jun 3, 2015

Yes, that's right. We can call it "BadgerService" or something which use badgekit-api-client.
In your sort term plan, it looks like a lot of works...

@aid29
Copy link
Contributor Author

aid29 commented Jun 15, 2015

@acabunoc I found this request parameter :badges is not used in this endpoint. https://github.com/mozillascience/PaperBadger/blob/master/src/app.js#L279

@aid29
Copy link
Contributor Author

aid29 commented Jun 15, 2015

Hi @audy "nock" is great!

Where you find the url "/systems/badgekit/badges?archived=any" which you have faked for calling by BadgeClientApi?

@abbycabs
Copy link
Member

@aid29 that should definitely be 'users' not ':badges'...

#60

@audy
Copy link
Member

audy commented Jun 15, 2015

Where you find the url ...

I don't recall. I think I looked at the tests for badgekit-api.

@aid29
Copy link
Contributor Author

aid29 commented Jun 16, 2015

@audy when I tried to stub this endpoint "/systems/badgekit/badges?archived=any" with some fake JSON or real badge JSON, the badge Client always return '{}'

If you don't 'nock' this endpoint, definitely the Client return a error.

Do you have successfully cheat badge Client to return some JSON data?

1 similar comment
@aid29
Copy link
Contributor Author

aid29 commented Jun 16, 2015

@audy when I tried to stub this endpoint "/systems/badgekit/badges?archived=any" with some fake JSON or real badge JSON, the badge Client always return '{}'

If you don't 'nock' this endpoint, definitely the Client return a error.

Do you have successfully cheat badge Client to return some JSON data?

@aid29
Copy link
Contributor Author

aid29 commented Jun 16, 2015

I think I need read more BadgeClientAPi source code to understand what happen there, maybe try a integration test with real Badge server in this issue first.

@abbycabs
Copy link
Member

Hey @aid29, this might help you:
https://github.com/mozilla/badgekit-api-client

Also: https://github.com/mozilla/badgekit-api/tree/master/docs

Thanks for looking into this!

@abbycabs
Copy link
Member

Also, this is the badgekit-api instance we have running / we are using on http://paperbadger.herokuapp.com/:

http://badgekit-api-sciencelab.herokuapp.com/

We successfully fetch / store badges from there. Can you make sure you have the correct environment variables?

@aid29
Copy link
Contributor Author

aid29 commented Jun 16, 2015

@acabunoc When I connect the project to http://badgekit-api-sciencelab.herokuapp.com/, everything is worked.

But I think audy's solution is cool, so I tried to make the badgekit-api-client talking to a "nock" http server in the test. but it's failed. badgekit-api-client always return a empty json.

I will start writing the tests against the http://badgekit-api-sciencelab.herokuapp.com/

Better to change the tests running against a local badgekit server late.

aid29 added a commit to aid29/PaperBadger that referenced this issue Jun 19, 2015
@aid29
mozillascience#26 wip
0165a1f
aid29 added a commit to aid29/PaperBadger that referenced this issue Jun 19, 2015
@aid29
mozillascience#26 wip 2 endpoints worked with IT
100360b
aid29 added a commit to aid29/PaperBadger that referenced this issue Jun 19, 2015
@aid29
mozillascience#26 3 endpoints works with IT
7213aa6
aid29 added a commit to aid29/PaperBadger that referenced this issue Jun 19, 2015
@aid29
mozillascience#26 refactroing all the badges read endpoints + IT
06075ec
aid29 added a commit to aid29/PaperBadger that referenced this issue Jun 19, 2015
@aid29
mozillascience#26 use Habitat to load the enviroment variables in Bad…
23d065a 
…geClient and Servicce, comment out the api.js test (config conflict with env.dist)
aid29 added a commit to aid29/PaperBadger that referenced this issue Jun 19, 2015
@aid29
mozillascience#26 refactoring the create Badge code to BadgeService, …
144a033 
…clean up the app.js (the creation is not working? )
aid29 added a commit to aid29/PaperBadger that referenced this issue Jun 19, 2015
@aid29
mozillascience#26 lint and clean up the codes
e6f2d55
@aid29
Copy link
Contributor Author

aid29 commented Jun 19, 2015

Hi @acabunoc I have pushed my branch for #26, please have a look.
here is my notes:

  • I finally choice to write directly the integration tests (run against the real OpenBadges server) to cover all our endpoints. Sorry, @audy You can't run all the tests locally for the moment. I think we can start one local Badges server in our build script lately to run all our tests on local.
  • I got a problem with "Habit" to load the different environment variables in different tests, so I comment out the api.js in the branch.
  • I have built a test for create the badge, but it's see not working? the response return is empty. I commented this test for the moment.

All the feedback or question are welcome, thanks.

@abbycabs
Copy link
Member

Hey @aid29! Can you make a pull request?

https://guides.github.com/activities/hello-world/#pr

Thanks!

@audy
Copy link
Member

audy commented Jun 19, 2015

I think we can start one local Badges server in our build script lately to run all our tests on local.

I agree. This is a better approach.

@aid29
Copy link
Contributor Author

aid29 commented Jun 19, 2015

@acabunoc Sorry, forgot that, just done. (see #61 -- @audy)

@aid29
Copy link
Contributor Author

aid29 commented Jun 19, 2015

The build are failed on CI https://travis-ci.org/mozillascience/PaperBadger/builds/67547510
I think the test running base on the configs load from .env file which don't exist in project.
The problem is we don't want to commit the secret code to the source. Maybe we need block this until we have a local Badges server setup.

@audy
Copy link
Member

audy commented Jun 19, 2015

You can encrypt the key and store it in .travis.yml: http://docs.travis-ci.com/user/encryption-keys/. Let me know if you need help.

@aid29
Copy link
Contributor Author

aid29 commented Jun 20, 2015

Thanks @audy
But looks like it have the restriction about "encrypted environment variables are not available for pull requests from forks." Let's wait @acabunoc to do this :-p

@abbycabs
Copy link
Member

abbycabs commented Jul 2, 2015

Hey both! I added the relevant environment variables to Travis -- not sure if this is the best approach

BADGES_ENDPOINT
BADGES_KEY
BADGES_SECRET
BADGES_SYSTEM

aid29 added a commit to aid29/PaperBadger that referenced this issue Jul 2, 2015
@aid29
mozillascience#26 tidy up the code and package.json
3850cb6
aid29 added a commit to aid29/PaperBadger that referenced this issue Jul 3, 2015
@aid29
mozillascience#26 load all the test's necessaire environment variable…
3b78ff1 
…s from system only
aid29 added a commit to aid29/PaperBadger that referenced this issue Jul 5, 2015
@aid29
mozillascience#26 push Travis to build ...
db9fa53
aid29 added a commit to aid29/PaperBadger that referenced this issue Jul 7, 2015
@aid29
mozillascience#26 wip
62133fb
aid29 added a commit to aid29/PaperBadger that referenced this issue Jul 7, 2015
@aid29
mozillascience#26 wip 2 endpoints worked with IT
e4f21a4
aid29 added a commit to aid29/PaperBadger that referenced this issue Jul 7, 2015
@aid29
mozillascience#26 3 endpoints works with IT
b1c6af3
aid29 added a commit to aid29/PaperBadger that referenced this issue Jul 7, 2015
@aid29
mozillascience#26 refactroing all the badges read endpoints + IT
79ad734
aid29 added a commit to aid29/PaperBadger that referenced this issue Jul 7, 2015
@aid29
mozillascience#26 use Habitat to load the enviroment variables in Bad…
43a5dcc 
…geClient and Servicce, comment out the api.js test (config conflict with env.dist)
aid29 added a commit to aid29/PaperBadger that referenced this issue Jul 7, 2015
@aid29
mozillascience#26 refactoring the create Badge code to BadgeService, …
e1eec8a 
…clean up the app.js (the creation is not working? )
aid29 added a commit to aid29/PaperBadger that referenced this issue Jul 7, 2015
@aid29
mozillascience#26 lint and clean up the codes
c6815b8
aid29 added a commit to aid29/PaperBadger that referenced this issue Jul 7, 2015
@aid29
mozillascience#26 tidy up the code and package.json
f48c399
aid29 added a commit to aid29/PaperBadger that referenced this issue Jul 7, 2015
@aid29
mozillascience#26 load all the test's necessaire environment variable…
23b7fe7 
…s from system only
aid29 added a commit to aid29/PaperBadger that referenced this issue Jul 7, 2015
@aid29
mozillascience#26 push Travis to build ...
79066e9
@aid29 aid29 mentioned this issue Jul 7, 2015
Merged
aid29 added a commit to aid29/PaperBadger that referenced this issue Jul 9, 2015
@aid29
mozillascience#26 Fix a error come from git rebase process
ca625a4
@abbycabs abbycabs mentioned this issue Jul 10, 2015
Merged
@abbycabs
Copy link
Member

resolved with #67!

abbycabs added a commit that referenced this issue Jul 21, 2015
@abbycabs
Merge branch 'master' into paper-submission
35849d2 
* master:
  use src/environments.js
  #26 Fix a error come from git rebase process
  #26 push Travis to build ...
  #26 load all the test's necessaire environment variables from system only
  #26 tidy up the code and package.json
  #26 lint and clean up the codes
  #26 refactoring the create Badge code to BadgeService, clean up the app.js (the creation is not working? )
  #26 use Habitat to load the enviroment variables in BadgeClient and Servicce, comment out the api.js test (config conflict with env.dist)
  #26 refactroing all the badges read endpoints + IT
  #26 3 endpoints works with IT
  #26 wip 2 endpoints worked with IT
  #26 wip
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aid29 aid29

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@audy @abbycabs @aid29