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

Add Client.getIdSites() #536

Merged
merged 9 commits into from
Sep 14, 2016
Merged

Add Client.getIdSites() #536

merged 9 commits into from
Sep 14, 2016

Conversation

the-overengineer
Copy link

@the-overengineer the-overengineer commented Sep 12, 2016

Add support for getting a list of Tenant's Id Sites, as well as a helper directly on Client, as well as tests for these features.

Usage example:

var stormpath = require('stormpath');

var client = new stormpath.Client();

function doneCallback(err, idSites) {
   if (!err) {
      console.log('Loaded ID sites for tenant');
   }
}

function idSitePrintIterator(idSite, next) {
  console.log('Loaded ID site with href:', idSite.href);
  next();
}

function onGetIdSites(err, idSites) {
  if (err) {
    return console.log('Error:', err);
  }

  idSites.each(idSitePrintIterator, doneCallback);
}

client.getIdSites(onGetIdSites);

Fixes #424

Count not drag the card as I am not a collaborator.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 89.074% when pulling 6aa10fc on feature-client-get-id-sites into 9ac26eb on master.

1 similar comment
@coveralls
Copy link

coveralls commented Sep 12, 2016

Coverage Status

Coverage increased (+0.04%) to 89.074% when pulling 6aa10fc on feature-client-get-id-sites into 9ac26eb on master.

* be a list of {@link IdSiteModel} objects.
*
*/
Tenant.prototype.getIdSiteModels = function getTenantIdSiteModels(/* [options,] callback */) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be getIdSites() instead of getIdSiteModels(). Your reasoning is correct. But the underlying model is named incorrectly so we'll have to fix that in another PR.

#537

Copy link
Author

Choose a reason for hiding this comment

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

Regarding this, I was following a comment in the IdSiteModel class file which references {@link Tenant#getIdSiteModels Tenant.getIdSiteModels()} (which did not exist at the time). Updating this as well.

@typerandom
Copy link
Contributor

Besides my comments this PR is looking good! 👍

@coveralls
Copy link

coveralls commented Sep 12, 2016

Coverage Status

Coverage increased (+0.04%) to 89.074% when pulling 2e56dd5 on feature-client-get-id-sites into 9ac26eb on master.

@typerandom
Copy link
Contributor

@Tweety-FER Awesome. Thank you! I'm happy with the changes. Noticed a small typo in the tests (Tennant -> Tenant). But I fixed that. Going to merge this in.

🎉

@typerandom typerandom merged commit 9e9a06f into master Sep 14, 2016
@typerandom typerandom deleted the feature-client-get-id-sites branch September 14, 2016 12:18
@coveralls
Copy link

coveralls commented Sep 14, 2016

Coverage Status

Coverage increased (+0.04%) to 89.074% when pulling 4204769 on feature-client-get-id-sites into 9ac26eb on master.

@robertjd
Copy link
Member

I'm pulling this PR and it's issue onto the backlog, as this will be part of 1.0, so I don't want to visualize it on the board right now

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

Successfully merging this pull request may close these issues.

4 participants