Skip to content

Add ProxyBackends collection#1457

Merged
frenchbread merged 118 commits intodevelopfrom
feature/proxy-backend
Sep 8, 2016
Merged

Add ProxyBackends collection#1457
frenchbread merged 118 commits intodevelopfrom
feature/proxy-backend

Conversation

@brylie
Copy link
Copy Markdown
Contributor

@brylie brylie commented Aug 29, 2016

Closes #1233
Closes #1344

Changes

  • Add ProxyBackends collection
  • Add ProxyBackends schema
  • Add ApiUmbrellaSchema
  • Add Proxy tab on view API Backend page
  • Add Proxy Backend Configuration panel
  • Add Proxy Backend configuration form
    • insert and update methods
    • labels, help texts, and validation messages
    • Regular expression for Proxy/API base path fields
  • Add ApiProxyConfiguration publication/subscription
  • Add i18n tokens/strings
  • Cleanup
    • lint various files, as they are saved
    • Change apiBackend to api where possible
    • Refactor ApiMetadata collection to follow project filename convention

@brylie brylie added the WIP label Aug 29, 2016
@brylie brylie added this to the Sprint 30 milestone Aug 29, 2016
@frenchbread frenchbread self-assigned this Aug 30, 2016
"apiId": {
type: String
},
"apiUmbrella": {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@brylie Is this going to be like this, that type of the proxy is hardcoded in schema? Or are you changing it?

@brylie
Copy link
Copy Markdown
Contributor Author

brylie commented Sep 1, 2016

@jykae we have decided to have specific fields based on the type of proxy, e.g. apiUmbrella.host.

This is similar to how @frenchbread implemented the Proxy feature, where he created sub-schemas for each supported proxy. Right now we are only aiming to support API Umbrella, but this approach allows us to easily enable more proxies.

@brylie brylie removed the WIP label Sep 1, 2016
@brylie
Copy link
Copy Markdown
Contributor Author

brylie commented Sep 7, 2016

Suggested changes made. Please review.

@bajiat bajiat mentioned this pull request Sep 7, 2016
@brylie
Copy link
Copy Markdown
Contributor Author

brylie commented Sep 7, 2016

@frenchbread please also note that in your Proxy module we should have only one url field for API Umbrella. That url field should not allow values ending with /, or it should be cleaned up on submit.

We will be appending /api-umbrella/ within the API Umbrella Admin API integration methods.

@frenchbread
Copy link
Copy Markdown
Contributor

That url field should not allow values ending with /, or it should be cleaned up on submit.

@brylie Does this require any changes for #1459 ?

@brylie
Copy link
Copy Markdown
Contributor Author

brylie commented Sep 7, 2016

@brylie Does this require any changes for #1459 ?

@frenchbread it looks like it. I will make some inline comments.

import _ from 'lodash';

Apis.helpers({
currentUserCanEdit () {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd suggest writing this helper shorter, e.g:

currentUserCanEdit () {
    const userId - Meteor.userId();
    if (userId) {
        const isManager = _.includes(this.managerIds, userId);
        const isAdmin = Roles.userIsInRole(userId, ['admin']);
        return (isManager || is Admin);
    }
    return false;
}

I think this looks more compact & easy to read 😃

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@brylie
Copy link
Copy Markdown
Contributor Author

brylie commented Sep 7, 2016

@frenchbread I made the suggested changes. Please review.

@brylie
Copy link
Copy Markdown
Contributor Author

brylie commented Sep 7, 2016

I don't want to do the 'base path live preview' at this point, so that we can merge this work soon.

If desirable, we can file the 'base path live preview' as an enhancement task.

@frenchbread
Copy link
Copy Markdown
Contributor

@brylie On Api details tab, shouldn't the the base URL display the actual URL for requests?

@frenchbread
Copy link
Copy Markdown
Contributor

Everything else looks cool. Good job on refactoring bunch of files to fix eslist 👍

@bajiat
Copy link
Copy Markdown
Contributor

bajiat commented Sep 8, 2016

@frenchbread There is a continuation task for the details tab. If that is the only thing missing, please merge this. Illya is waiting for this task to be merged.

@bajiat
Copy link
Copy Markdown
Contributor

bajiat commented Sep 8, 2016

@frenchbread Is it possible for you to merge during the afternoon? If not, I can reassign the task

@frenchbread
Copy link
Copy Markdown
Contributor

Ok, merging. @brylie @bajiat

@frenchbread frenchbread merged commit ab85c0a into develop Sep 8, 2016
@frenchbread frenchbread deleted the feature/proxy-backend branch September 8, 2016 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants