-
Notifications
You must be signed in to change notification settings - Fork 801
feature(watching): Add support for ignoring or subscribing to repos #360
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| /** | ||
| * @file | ||
| * @copyright 2016 Matt Smith (Development Seed) | ||
| * @license Licensed under {@link https://spdx.org/licenses/BSD-3-Clause-Clear.html BSD-3-Clause-Clear}. | ||
| * Github.js is freely distributable. | ||
| */ | ||
|
|
||
| import Requestable from './Requestable'; | ||
| import debug from 'debug'; | ||
| const log = debug('github:watching'); | ||
|
|
||
| /** | ||
| * Watching a Repository registers the user to receive notifications on new discussions, as well as events in the user's | ||
| * activity feed. | ||
| */ | ||
| export default class Watching extends Requestable { | ||
| /** | ||
| * Watching wrapper | ||
| * @param {Requestable.auth} [auth] - information required to authenticate to Github | ||
| * @param {string} [apiBase=https://api.github.com] - the base Github API URL | ||
| */ | ||
| constructor(auth, apiBase) { | ||
| super(auth, apiBase); | ||
| } | ||
|
|
||
| /** | ||
| * Get Watchers for repo | ||
| * @see https://developer.github.com/v3/activity/watching/#list-watchers | ||
| * @param {string} owner - username or organization | ||
| * @param {string} repo - repo name | ||
| * @param {Requestable.callback} [cb] - list of watchers | ||
| * @return {Promise} - the promise for the http request | ||
| */ | ||
| listWatchers(owner, repo, cb) { | ||
| log(`Fetching watchers for ${owner}/${repo}`); | ||
| return this._requestAllPages(`/repos/${owner}/${repo}/subscribers`, undefined, cb); | ||
| } | ||
|
|
||
| /** | ||
| * Subscribe to repo | ||
| * @see https://developer.github.com/v3/activity/watching/#set-a-repository-subscription | ||
| * @param {string} owner - username or organization | ||
| * @param {string} repo - repo name | ||
| * @param {Requestable.callback} [cb] - subscription status | ||
| * @return {Promise} - the promise for the http request | ||
| */ | ||
| subscribe(owner, repo, cb) { | ||
| log(`Subscribing to ${owner}/${repo}`); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can remove this and the other logs. Possibly we might want to use them only for weird situations. Any opinion @clayreimann?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the logs only run if they are enabled, the idea with including them is that when we need extra information we have the tooling in place to help track down an end-user reported issue. (Logs are enabled via the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I meant was, is there any sense in this specific case in stating the obvious. For example you call
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since they don't get output unless you explicitly turn them on I don't see the harm here. This can be useful for diagnostic purposes when needed. There is logging of this sort in the base class, however if you enable it there you enable it for everything in this library. Having these here will allow you to only turn on logging in this section. |
||
| return this._setSubscription(owner, repo, true, cb); | ||
| } | ||
|
|
||
| /** | ||
| * Ignore a repo | ||
| * @see https://developer.github.com/v3/activity/watching/#set-a-repository-subscription | ||
| * @param {string} owner - username or organization | ||
| * @param {string} repo - repo name | ||
| * @param {Requestable.callback} [cb] - subscription status | ||
| * @return {Promise} - the promise for the http request | ||
| */ | ||
| ignore(owner, repo, cb) { | ||
| log(`Ignoring ${owner}/${repo}`); | ||
| return this._setSubscription(owner, repo, false, cb); | ||
| } | ||
|
|
||
| /** | ||
| * Remove all subscription settings for repo | ||
| * @see https://developer.github.com/v3/activity/watching/#delete-a-repository-subscription | ||
| * @param {string} owner - username or organization | ||
| * @param {string} repo - repo name | ||
| * @param {Requestable.callback} [cb] - subscription status | ||
| * @return {Promise} - the promise for the http request | ||
| */ | ||
| removeSubscription(owner, repo, cb) { | ||
| log(`Removing subscription settings for ${owner}/${repo}`); | ||
| return this._request204or404(`/repos/${owner}/${repo}/subscription`, undefined, cb, 'DELETE'); | ||
| } | ||
|
|
||
| /** | ||
| * General subscription method | ||
| * @private | ||
| * @see https://developer.github.com/v3/activity/watching/#set-a-repository-subscription | ||
| * @param {string} owner - username or organization | ||
| * @param {string} repo - repo name | ||
| * @param {boolean} subscribed - subscribed or ignored | ||
| * @param {Requestable.callback} [cb] - subscription status | ||
| * @return {Promise} - the promise for the http request | ||
| */ | ||
| _setSubscription(owner, repo, subscribed, cb) { | ||
| log(`Subscribing to ${owner}/${repo}`); | ||
| return this._request('PUT', `/repos/${owner}/${repo}/subscription`, { | ||
| subscribed, | ||
| ignored: !subscribed | ||
| }, cb); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "USERNAME": "mtscout6-test", | ||
| "PASSWORD": "test1324" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| import expect from 'must'; | ||
|
|
||
| import Github from '../lib/GitHub'; | ||
| import testUser from './fixtures/user.json'; | ||
| import altUser from './fixtures/alt-user.json'; | ||
| import getTestRepoName from './helpers/getTestRepoName'; | ||
|
|
||
| describe('Watching', function() { | ||
| const testRepoName = getTestRepoName(); | ||
| let githubUser; | ||
| let githubAltUser; | ||
| let organization; | ||
|
|
||
| before(function() { | ||
| githubUser = new Github({ | ||
| username: testUser.USERNAME, | ||
| password: testUser.PASSWORD, | ||
| auth: 'basic' | ||
| }); | ||
|
|
||
| githubAltUser = new Github({ | ||
| username: altUser.USERNAME, | ||
| password: altUser.PASSWORD, | ||
| auth: 'basic' | ||
| }); | ||
|
|
||
| organization = githubUser.getOrganization(testUser.ORGANIZATION); | ||
|
|
||
| const options = { | ||
| name: testRepoName, | ||
| description: 'test repo watchers', | ||
| homepage: 'https://github.com/', | ||
| private: false, | ||
| has_issues: false, // eslint-disable-line | ||
| has_wiki: false, // eslint-disable-line | ||
| has_downloads: false // eslint-disable-line | ||
| }; | ||
|
|
||
| return organization.createRepo(options); | ||
| }); | ||
|
|
||
| it('should list watchers', function() { | ||
| return githubUser.watching() | ||
| .listWatchers(testUser.ORGANIZATION, 'fixed-test-repo-1') | ||
| .then(({data}) => { | ||
| const watchers = data.map((x) => x.login); | ||
| expect(watchers).must.include('mikedeboertest'); | ||
| }); | ||
| }); | ||
|
|
||
| it('should add a subscription to a repo', function() { | ||
| const watching = githubAltUser.watching(); | ||
|
|
||
| return watching.subscribe(testUser.ORGANIZATION, testRepoName) | ||
| .then(({data}) => { | ||
| expect(data.subscribed).to.be.true(); | ||
| expect(data.ignored).to.be.false(); | ||
| }); | ||
| }); | ||
|
|
||
| it('should ignore a repo', function() { | ||
| const watching = githubAltUser.watching(); | ||
|
|
||
| return watching.ignore(testUser.ORGANIZATION, testRepoName) | ||
| .then(({data}) => { | ||
| expect(data.subscribed).to.be.false(); | ||
| expect(data.ignored).to.be.true(); | ||
| }); | ||
| }); | ||
|
|
||
| it('should remove subscription settings', function() { | ||
| const watching = githubAltUser.watching(); | ||
|
|
||
| return watching.removeSubscription(testUser.ORGANIZATION, testRepoName) | ||
| .then((result) => { | ||
| expect(result).to.be.true(); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment we use
module.exportsfor exporting classes. I'm not sure about the reason as this was made by @clayreimann but I think it's because we aren't using the babel plugin to revert the export of defaults to the behavior of Babel 5.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know there was such a plugin. I forget the exact reason that we're using
module.exportsbut it boiled down to nothing works if you use theexport defaultsyntax.If there's a plugin that fixes things while allowing us to use the new syntax then I'm all for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use babel-plugin-add-module-exports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've used that plugin before as well. I'll get it wired up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to do it in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commit is already in, it was a two line change and doesn't require me to change this. I can do that if you like though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I do a separate PR for this, then I'll tackle the babel-runtime issue as well. I'll crank that out real quick.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright it is in PR #366 We should hold off on this PR until that one is done because of this line.