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

Reuse integrations from across apps #712

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
12 changes: 10 additions & 2 deletions app/components/v2/integration_card_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,13 @@
<% end %>
</div>
<% end %>
<div class="flex gap-1">
<% if disconnected? %>
<% if connectable? %>
<div class="flex gap-1">
<%= connectable_form_partial %>
<% if repeated_integration.present? %>
<%= reusable_integration_form_partial(repeated_integration) %>
<% end %>
</div>
<% end %>
<% if creatable? %>
<%= render V2::ModalComponent.new(title: creatable_modal_title) do |modal| %>
Expand All @@ -50,6 +49,15 @@
<% end %>
<% end %>
<% end %>
<% if repeated_integrations_across_app.present? %>
<%= render V2::ModalComponent.new(title: "Reuse Previous") do |modal| %>
<% modal.with_button(label: "reuse across apps", scheme: :supporting, type: :action, size: :xxs, arrow: :none)
.with_icon("v2/repeat.svg", size: :md) %>
<% modal.with_body do %>
<%= reusable_integrations_across_app_form_partial(repeated_integrations_across_app) %>
<% end %>
<% end %>
<% end %>
<% end %>
</div>
</footer>
Expand Down
10 changes: 10 additions & 0 deletions app/components/v2/integration_card_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ def repeated_integration
Integration.existing_integration(@app, providable_type)
end

# Retrieves repeated integrations across apps for the given app and providable type
def repeated_integrations_across_app
Integration.existing_integrations_across_app(@app, providable_type)
end

def connect_path
connect_app_integrations_path(@app, integration)
end
Expand Down Expand Up @@ -54,6 +59,11 @@ def reusable_integration_form_partial(existing_integration)
locals: {app: @app, integration: @integration, category: @category, url: reuse_existing_integration_path(existing_integration), type: providable_type, provider: provider})
end

def reusable_integrations_across_app_form_partial(existing_integration)
render(partial: "integrations/app_reuseable",
locals: {app: @app, existing_integration: existing_integration, category: @category, url: reuse_integration_across_app_app_integrations_path, type: providable_type, provider: provider})
end

def disconnectable?
integration.disconnectable? && ci_cd?
end
Expand Down
20 changes: 19 additions & 1 deletion app/controllers/integrations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ class IntegrationsController < SignedInApplicationController

before_action :require_write_access!, only: %i[connect create index build_artifact_channels destroy]
before_action :set_app_config_tabs, only: %i[index]
before_action :set_integration, only: %i[connect create reuse]
before_action :set_integration, only: %i[connect create reuse reuse_integration_across_app]
before_action :set_existing_integration, only: %i[reuse]
before_action :set_providable, only: %i[connect create]
before_action :set_existing_integration_across_app, only: %i[reuse_integration_across_app]

def connect
redirect_to(@integration.install_path, allow_other_host: true)
Expand All @@ -17,6 +18,18 @@ def index
set_integrations_by_categories
end

# This method handles the process of reusing an existing app integration to create a new one
# Attempts to create a new integration based on the existing one across app
def reuse_integration_across_app
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: I think we can use the reuse action here, afaict, they do the same thing; we don't need a separate route for it.

Choose a reason for hiding this comment

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

Sure. We will make the changes and get back to you.

return redirect_to app_integrations_path(@app), alert: "Integration not found or not connected." unless @existing_app_integration&.connected?
new_integration = initiate_integration(@existing_app_integration)
if new_integration.save
redirect_to app_integrations_path(@app), notice: "#{@existing_app_integration.providable_type} across app integration reused successfully."
else
redirect_to app_integrations_path(@app), flash: {error: new_integration.errors.full_messages.to_sentence}
end
end

def reuse
return redirect_to app_integrations_path(@app), alert: "Integration not found or not connected." unless @existing_integration&.connected?
new_integration = initiate_integration(@existing_integration)
Expand Down Expand Up @@ -82,6 +95,11 @@ def set_existing_integration
@existing_integration = Integration.find_by(id: params[:id])
end

# Sets the existing app integration based on the provided integration ID in the params
def set_existing_integration_across_app
@existing_app_integration = Integration.find_by(id: params[:integration][:id])
end

def set_providable
@integration.providable = providable_type.constantize.new(integration: @integration)
set_providable_params
Expand Down
5 changes: 5 additions & 0 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ def toggle_for(hide, full_width: false, &block)
)
end

# Displays label which combines the app's bundle identifier, type, and ID
def integrations_across_app_options(integration)
"#{integration.app.bundle_identifier} - #{integration.providable_type} - #{integration.providable_id}"
end

def version_in_progress(version)
version.to_semverish.to_s(patch_glob: true)
end
Expand Down
5 changes: 5 additions & 0 deletions app/models/integration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,11 @@ def existing_integration(app, providable_type)
app.integrations.connected.find_by(providable_type: providable_type)
end

# This method retrieves a list of existing integrations for apps within the same organization
def existing_integrations_across_app(app, providable_type)
Integration.connected.where(integrable_id: app.organization.apps.where.not(id: app.id), providable_type: providable_type).select("DISTINCT ON (providable_id, integrable_id) id, providable_id, integrable_id, providable_type, created_at, updated_at, metadata, integrable_type")
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: let's change the DISTINCT ON to do DISTINCT ON (providable_id, integrable_id) id, * so we don't have to keep this query updated as new columns are added.

Choose a reason for hiding this comment

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

sure

end

private

def providable_error_message(meta)
Expand Down
16 changes: 16 additions & 0 deletions app/views/integrations/_app_reuseable.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<%= form_with model: [app, existing_integration], builder: EnhancedFormHelper::AuthzForm, url: url, method: :post do |f| %>
<div class="bg-main-50 p-4 rounded-lg border border-main-300">
<% existing_integration.each do |integration| %>
<div class="flex items-center mb-2">
<%= image_tag("integrations/logo_#{provider}.png", alt: "Logo", style: "width: 24px; height: 24px; margin-right: 8px;") %>
<%= f.radio_button :id,integration.id, style: "margin-right: 8px;" %>
<%= f.label :id, integrations_across_app_options(integration), value: integration.id %>
</div>
<% end %>
<%= f.hidden_field :category, value: category %>
<%= f.hidden_field "providable[type]", value: type %>
</div>
<div class="mt-4">
<%= f.authz_submit "Save", "v2/archive.svg", size: :sm %>
</div>
<% end %>
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@
post :reuse
end
collection do
post :reuse_integration_across_app
get :connect, to: "integrations#connect", as: :connect

resource :google_play_store, only: [:create],
Expand Down
45 changes: 39 additions & 6 deletions spec/controllers/integrations_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,34 @@
describe IntegrationsController do
let(:app) { create(:app, platform: :android) }
let(:organization) { app.organization }
let(:app_for_across_integration) { create(:app, platform: :android, organization: organization, bundle_identifier: "com.abc.com") }
let(:user) { create(:user, :with_email_authentication, :as_developer, member_organization: organization) }
let(:existing_integration) { create(:integration, status: "connected", category: "version_control", providable: create(:github_integration), integrable: app, metadata: {id: Faker::Number.number(digits: 8)}) }
let(:existing_integration_across_app) { create(:integration, status: "connected", category: "version_control", providable: create(:github_integration), integrable: app_for_across_integration, metadata: {id: Faker::Number.number(digits: 8)}) }
let(:integration) { create(:integration, category: "ci_cd", providable: create(:github_integration), integrable: app) }

before do
sign_in user.email_authentication
allow_any_instance_of(described_class).to receive(:current_user).and_return(user)
allow_any_instance_of(described_class).to receive(:set_integration)
end

describe "POST #reuse" do
before do
allow_any_instance_of(described_class).to receive(:set_integration)
allow(Integration).to receive(:find_by).with(id: existing_integration.id.to_s).and_return(existing_integration)
end

context "when the existing integration is not connected or does not exist" do
it "redirects to the integrations path with an error message" do
post :reuse, params: {id: Faker::Internet.uuid, app_id: app.id}
existing_integration.update(status: "disconnected")
post :reuse, params: {id: existing_integration.id, app_id: app.id}

expect(response).to redirect_to(app_integrations_path(app))
expect(flash[:alert]).to eq("Integration not found or not connected.")
end
end

context "when the existing integration is connected" do
before do
allow(Integration).to receive(:find_by).with(id: existing_integration.id.to_s).and_return(existing_integration)
end

it "reuses the integration and redirects to the integrations path with a success message" do
new_integration = instance_double(Integration, save: true)
allow(controller).to receive(:initiate_integration).and_return(new_integration)
Expand All @@ -49,4 +52,34 @@
end
end
end

describe "POST #reuse_integration_across_app" do
context "when the existing integration is connected" do
it "reuses the integration and redirects to the integrations path with a success message" do
post :reuse_integration_across_app, params: {app_id: app.id, integration: {id: existing_integration_across_app.id, category: existing_integration_across_app.category}}

expect(response).to redirect_to(app_integrations_path(app))
expect(flash[:notice]).to eq("#{existing_integration_across_app.providable_type} across app integration reused successfully.")
end
end

context "when the existing integration across app is not connected or does not exist" do
it "redirects to the integrations path with an error message" do
existing_integration_across_app.update(status: "disconnected")

post :reuse_integration_across_app, params: {app_id: app.id, integration: {id: existing_integration_across_app.id, category: existing_integration_across_app.category}}
expect(response).to redirect_to(app_integrations_path(app))
expect(flash[:alert]).to eq("Integration not found or not connected.")
end

it "fails to reuse the integration across app and redirects with an error message if saving the new integration fails" do
new_integration = instance_double(Integration, save: false, errors: instance_double(ActiveModel::Errors, full_messages: ["Save failed"]))
allow(controller).to receive(:initiate_integration).and_return(new_integration)

post :reuse_integration_across_app, params: {app_id: app.id, integration: {id: existing_integration_across_app.id, category: existing_integration_across_app.category}}
expect(response).to redirect_to(app_integrations_path(app))
expect(flash[:error]).to eq("Save failed")
end
end
end
end
Loading