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

Add BaseManager class #743

Merged
merged 1 commit into from Sep 28, 2022
Merged

Add BaseManager class #743

merged 1 commit into from Sep 28, 2022

Conversation

adamjmcgrath
Copy link
Member

Changes

Adding a BaseManager class to DRY up the arguments parsing and rest client creation of all the Manager classes.

This should make it easier to customise the rest client and add other features in the future.

Before:

class SomeManager {
  constructor(options) {
    if (options === null || typeof options !== 'object') {
      throw new ArgumentError('Must provide client options');
    }

    if (options.baseUrl === null || options.baseUrl === undefined) {
      throw new ArgumentError('Must provide a base URL for the API');
    }

    if ('string' !== typeof options.baseUrl || options.baseUrl.length === 0) {
      throw new ArgumentError('The provided base URL is invalid');
    }

    const clientOptions = {
      errorFormatter: { message: 'message', name: 'error' },
      headers: options.headers,
      query: { repeatParams: false },
    };

    const auth0RestClient = new Auth0RestClient(
      `${options.baseUrl}/some/path/:id`,
      clientOptions,
      options.tokenProvider
    );
    this.resource = new RetryRestClient(auth0RestClient, options.retry);
  }
}

After:

class SomeManager extends BaseManager {
  constructor(options) {
    super(options);

    this.resource = this._getRestClient('/some/path/:id');
  }
}

@adamjmcgrath adamjmcgrath added the review:medium Medium review label Sep 22, 2022
@adamjmcgrath adamjmcgrath requested a review from a team as a code owner September 22, 2022 11:35
@adamjmcgrath
Copy link
Member Author

image

☝️ This is one of those cases where removing covered code drops the coverage percentage, even though you're not introducing uncovered code

@adamjmcgrath adamjmcgrath merged commit 9e87337 into master Sep 28, 2022
@adamjmcgrath adamjmcgrath deleted the base-manager branch September 28, 2022 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review:medium Medium review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants