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

[APPINT-1044] Feature/app setup #563

Open
wants to merge 20 commits into
base: feature/app-setup
Choose a base branch
from

Conversation

manu-d
Copy link

@manu-d manu-d commented Nov 2, 2017

Add new endpoints on app instances to interact with mno-enterprise-angular and mno-hub
List of new endpoints:

  • GET setup_form
    => Returns the form generated by the add-on for the user to link their account
  • POST create_omniauth
    => Link the user's account to the add-on
  • POST sync
    => Launches a sync on the add-on
  • POST disconnect
    => Unlink the user's account
  • GET sync_history
    => History of add-on syncs
  • GET id_maps
    => List of add-on id-maps

@manu-d
Copy link
Author

manu-d commented Nov 2, 2017

@ouranos Please, review

Copy link
Contributor

@ouranos ouranos left a comment

Choose a reason for hiding this comment

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

Please add authorization

Rails.logger.info("Error on request #{url} with options #{options}: #{e}")
end
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

No newline at EOF

module MnoEnterprise
module AddOnHelper

def AddOnHelper.send_request(instance, method, path, options = {})
Copy link
Contributor

Choose a reason for hiding this comment

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

def self.send_request

@@ -16,7 +16,7 @@ module MnoEnterprise::Concerns::Controllers::Jpi::V1::AppInstancesController
# GET /mnoe/jpi/v1/organization/1/app_instances
def index
statuses = MnoEnterprise::AppInstance::ACTIVE_STATUSES.join(',')
@app_instances = MnoEnterprise::AppInstance.includes(:app).where(owner_id: parent_organization.id, 'status.in': statuses).to_a.select do |i|
@app_instances = MnoEnterprise::AppInstance.with_params(extra_fields: true).includes(:app).where(owner_id: parent_organization.id, 'status.in': statuses).to_a.select do |i|
Copy link
Contributor

Choose a reason for hiding this comment

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

What is extra_fields doing? Couldn't find it

def setup_form
app_instance = MnoEnterprise::AppInstance.find_one(params[:id], :app)
response = MnoEnterprise::AddOnHelper.send_request(app_instance, :get, '/setup_form')
MnoEnterprise::EventLogger.info('addon_form_request', current_user.id, 'Request add_on form', app_instance)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need an event every time the form is displayed?

# GET /mnoe/jpi/v1/organization/1/app_instances/11/setup_form
def setup_form
app_instance = MnoEnterprise::AppInstance.find_one(params[:id], :app)
response = MnoEnterprise::AddOnHelper.send_request(app_instance, :get, '/setup_form')
Copy link
Contributor

Choose a reason for hiding this comment

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

Authorization?

def create_omniauth
app_instance = MnoEnterprise::AppInstance.find_one(params[:id], :app)
body = params[:app_instance].merge!(org_uid: app_instance.channel_id)
response = MnoEnterprise::AddOnHelper.send_request(app_instance, :post, "/auth/#{app_instance.name.downcase}/request", body: body)
Copy link
Contributor

Choose a reason for hiding this comment

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

Authorization?

def stub_add_on(instance, method, path, status, response = {})
url = instance.metadata['app']['host'] + path
stub_options = {
headers: {
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need to specify all the headers

@ouranos ouranos requested a review from x4d3 November 6, 2017 07:19
Copy link
Contributor

@ouranos ouranos left a comment

Choose a reason for hiding this comment

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

Ok for me.
Waiting review from @x4d3 and @alexnoox

This is dependent on the mnohub PR

@@ -16,7 +16,7 @@ module MnoEnterprise::Concerns::Controllers::Jpi::V1::AppInstancesController
# GET /mnoe/jpi/v1/organization/1/app_instances
def index
statuses = MnoEnterprise::AppInstance::ACTIVE_STATUSES.join(',')
@app_instances = MnoEnterprise::AppInstance.with_params(extra_fields: true).includes(:app).where(owner_id: parent_organization.id, 'status.in': statuses).to_a.select do |i|
@app_instances = MnoEnterprise::AppInstance.select(MnoEnterprise::AppInstance::REQUIRED_INDEX_FIELDS).includes(:app).where(owner_id: parent_organization.id, 'status.in': statuses).to_a.select do |i|
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice use of select

@ouranos ouranos added this to the v4.0 milestone Nov 9, 2017
@manu-d manu-d force-pushed the feature/app-setup branch 2 times, most recently from 02f2137 to 4032d5b Compare November 26, 2017 02:00
@manu-d manu-d force-pushed the feature/app-setup branch from 76bf13a to b0803f2 Compare January 12, 2018 03:21
@manu-d manu-d force-pushed the feature/app-setup branch from d5e1961 to 4323618 Compare June 4, 2018 15:57
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.

4 participants