Allow VCS integrations to be disconnected (#747)

Author: kitallis

.dockerignore

@@ -19,6 +19,7 @@
 # Ignore all logfiles and tempfiles.
 /log/*
 /tmp/*
+!/tmp/caching-dev.txt
 !/log/.keep
 !/tmp/.keep
 

app/components/integration_card_component.html.erb

@@ -20,7 +20,7 @@
         <% if connected? %>
           <div class="flex justify-between w-full items-baseline">
             <%= render BadgeComponent.new(text: "Connected", status: :success) %>
-            <% if ci_cd? %>
+            <% if disconnectable_categories? %>
               <%= render ButtonComponent.new(
                 scheme: :danger,
                 options: app_integration_path(@app, integration),

app/components/integration_card_component.rb

@@ -17,7 +17,7 @@ def initialize(app, integration, category)
   end
 
   attr_reader :integration
-  delegate :connected?, :disconnected?, :providable, :connection_data, :providable_type, :ci_cd?, to: :integration, allow_nil: true
+  delegate :connected?, :disconnected?, :providable, :connection_data, :providable_type, :disconnectable_categories?, to: :integration, allow_nil: true
   alias_method :provider, :providable
   delegate :creatable?, :connectable?, to: :provider
 
@@ -70,6 +70,6 @@ def reusable_integrations_form_partial(existing_integrations)
   end
 
   def disconnectable?
-    integration.disconnectable? && ci_cd?
+    integration.disconnectable? && disconnectable_categories?
   end
 end

app/components/live_release/finalize_component.rb

@@ -79,7 +79,8 @@ def subtitle
   end
 
   def tag_link
-    link = release.tag_url || release.app.config.code_repo_url
+    link = release.tag_url || release.app.config&.code_repo_url
+    return NOT_AVAILABLE if link.blank?
     link_to_external train.vcs_provider.display, link, class: "underline"
   end
 end

app/components/release_list_component.html.erb

@@ -2,7 +2,7 @@
   <% if train.hotfixable? %>
     <% container.with_action do %>
       <%= render ModalComponent.new(title: "Start a hotfix release",
-                                        subtitle: "This will be against your last successful release #{hotfix_from.release_version}") do |modal| %>
+                                    subtitle: "This will be against your last successful release #{hotfix_from.release_version}") do |modal| %>
         <% button = modal.with_button(scheme: :light,
                                       type: :action,
                                       size: :xxs,
@@ -62,6 +62,7 @@
       label: "Configure",
       authz: false,
       options: edit_app_train_path(app, train),
+      disabled: !app.ready?,
       type: :link) do |b|
       b.with_icon("cog.svg")
     end %>
@@ -111,15 +112,8 @@
   <% end %>
 
   <% container.with_body do %>
-    <% unless app.ready? %>
-      <%= render AlertComponent.new(kind: :banner,
-                                        type: :notice,
-                                        title: "App is not ready",
-                                        info: { label: "Configure", link: app_integrations_path(app) },
-                                        full_screen: false) do %>
-        Please finish configuring the required integrations before you can start creating releases.
-      <% end %>
-    <% end %>
+    <%= render partial: "shared/app_not_ready", locals: { app: } %>
+
     <% if train.automatic? %>
       <%= render ScheduledTrainComponent.new(train) %>
     <% end %>

app/components/release_list_component.rb

@@ -123,6 +123,14 @@ def ios_enabled?
   end
 
   def no_release_empty_state
+    unless app.ready?
+      return {
+        title: "App is not ready",
+        text: "There are required integrations that need to be configured before you can start creating releases.",
+        content: render(ButtonComponent.new(scheme: :light, type: :link, label: "Configure Integration", options: app_integrations_path(app), size: :xxs, authz: false))
+      }
+    end
+
     if train.automatic?
       if train.activatable?
         {

app/controllers/config/release_platforms_controller.rb

@@ -8,6 +8,7 @@ class Config::ReleasePlatformsController < SignedInApplicationController
   before_action :set_release_platform, only: %i[edit update]
   before_action :set_config, only: %i[edit update]
   before_action :set_train_config_tabs, only: %i[edit update]
+  before_action :ensure_app_ready, only: %i[edit update]
   before_action :set_ci_actions, only: %i[edit update]
   before_action :set_submission_types, only: %i[edit update]
   around_action :set_time_zone
@@ -43,7 +44,7 @@ def set_config
   end
 
   def set_ci_actions
-    @ci_actions = @app.config.ci_cd_workflows
+    @ci_actions = @train.workflows || []
   end
 
   def set_submission_types

app/controllers/notification_settings_controller.rb

@@ -5,6 +5,7 @@ class NotificationSettingsController < SignedInApplicationController
 
   before_action :require_write_access!, only: %i[update]
   before_action :set_train, only: %i[index update edit]
+  before_action :ensure_app_ready, only: %i[index edit update]
   before_action :set_notification_setting, only: %i[update edit]
   around_action :set_time_zone
 

app/controllers/release_health_rules_controller.rb

@@ -2,6 +2,7 @@ class ReleaseHealthRulesController < SignedInApplicationController
   before_action :require_write_access!, only: %i[create destroy]
   before_action :set_train, only: %i[create destroy]
   before_action :set_release_platform, only: %i[create destroy]
+  before_action :ensure_app_ready, only: %i[create destroy]
 
   def create
     @rule = @release_platform.release_health_rules.new(rule_params)

app/controllers/release_indices_controller.rb

@@ -5,6 +5,7 @@ class ReleaseIndicesController < SignedInApplicationController
   before_action :set_train, only: %i[edit update]
   before_action :set_app_from_train, only: %i[edit update]
   before_action :set_train_config_tabs, only: %i[edit update]
+  before_action :ensure_app_ready, only: %i[edit update]
 
   def edit
     @release_index = @train.release_index

app/controllers/signed_in_application_controller.rb

@@ -39,6 +39,12 @@ class SignedInApplicationController < ApplicationController
 
   private
 
+  def ensure_app_ready
+    unless @app.ready?
+      redirect_to app_path(@app), flash: {error: "App is not ready (not fully configured)."}
+    end
+  end
+
   def set_page_name
     user_facing_actions = %w[new index show edit]
 
@@ -185,11 +191,11 @@ def new_app
   end
 
   def vcs_provider_logo
-    @vcs_provider_logo ||= "integrations/logo_#{@app.vcs_provider}.png"
+    @vcs_provider_logo ||= "integrations/logo_#{@app.vcs_provider.presence || "deprecated"}.png"
   end
 
   def ci_cd_provider_logo
-    @ci_cd_provider_logo ||= "integrations/logo_#{@app.ci_cd_provider}.png"
+    @ci_cd_provider_logo ||= "integrations/logo_#{@app.ci_cd_provider || "deprecated"}.png"
   end
 
   def teams_supported?

app/controllers/trains_controller.rb

@@ -7,6 +7,7 @@ class TrainsController < SignedInApplicationController
   before_action :require_write_access!, only: %i[new create update destroy activate deactivate]
   before_action :set_train, only: %i[edit update destroy activate deactivate rules]
   before_action :set_train_config_tabs, only: %i[edit update rules destroy activate deactivate]
+  before_action :ensure_app_ready, only: %i[new create edit update activate deactivate rules]
   before_action :validate_integration_status, only: %i[new create]
   before_action :set_notification_channels, only: %i[new create edit update]
   around_action :set_time_zone

app/models/app_config.rb

@@ -6,12 +6,10 @@
 #  bitbucket_workspace     :string
 #  bugsnag_android_config  :jsonb
 #  bugsnag_ios_config      :jsonb
-#  ci_cd_workflows         :jsonb
 #  code_repository         :json
 #  firebase_android_config :jsonb
 #  firebase_ios_config     :jsonb
 #  jira_config             :jsonb            not null
-#  notification_channel    :json
 #  created_at              :datetime         not null
 #  updated_at              :datetime         not null
 #  app_id                  :uuid             not null, indexed
@@ -22,7 +20,7 @@ class AppConfig < ApplicationRecord
   include AppConfigurable
 
   PLATFORM_AWARE_CONFIG_SCHEMA = Rails.root.join("config/schema/platform_aware_integration_config.json")
-  self.ignored_columns += %w[bugsnag_project_id firebase_crashlytics_android_config firebase_crashlytics_ios_config notification_channel]
+  self.ignored_columns += %w[bugsnag_project_id firebase_crashlytics_android_config firebase_crashlytics_ios_config notification_channel ci_cd_workflows]
 
   belongs_to :app
   has_many :variants, class_name: "AppVariant", dependent: :destroy
@@ -47,28 +45,27 @@ def ready?
   end
 
   def code_repository_name
-    return if code_repository.blank?
-    code_repository["full_name"]
+    code_repository&.fetch("full_name", nil)
   end
 
   def code_repo_url
-    code_repository["repo_url"]
+    code_repository&.fetch("repo_url", nil)
   end
 
   def code_repo_namespace
-    code_repository["namespace"]
+    code_repository&.fetch("namespace", nil)
   end
 
   def code_repo_name_only
-    code_repository["name"]
+    code_repository&.fetch("name", nil)
   end
 
   def bitrise_project
     bitrise_project_id&.fetch("id", nil)
   end
 
   def further_setup_by_category?
-    integrations = app.integrations
+    integrations = app.integrations.connected
     categories = {}.with_indifferent_access
 
     if integrations.version_control.present?
@@ -131,10 +128,14 @@ def ci_cd_workflows
     super&.map(&:with_indifferent_access)
   end
 
-  def set_ci_cd_workflows(workflows)
-    return if code_repository.nil?
-    return if app.ci_cd_provider.blank?
-    update(ci_cd_workflows: workflows)
+  def disconnect!(integration)
+    if integration.version_control?
+      self.code_repository = nil
+    elsif integration.ci_cd?
+      self.bitrise_project_id = nil
+    end
+
+    save!
   end
 
   private

app/models/app_variant.rb

@@ -33,6 +33,10 @@ def display_text
 
   def config = self
 
+  def disconnect!(_)
+    raise NotImplementedError
+  end
+
   private
 
   def duplicate_bundle_identifier

app/models/google_play_store_integration.rb

@@ -16,6 +16,7 @@ class GooglePlayStoreIntegration < ApplicationRecord
   include Displayable
   include Loggable
 
+  delegate :cache, to: Rails
   delegate :integrable, to: :integration, allow_nil: true
   delegate :refresh_external_app, :bundle_identifier, to: :integrable, allow_nil: true
 
@@ -26,6 +27,7 @@ class GooglePlayStoreIntegration < ApplicationRecord
   after_create_commit :draft_check
   after_create_commit :refresh_external_app
 
+  CACHE_EXPIRY = 1.month
   LOCK_ACQUISITION_FAILURE_REASON = :lock_acquisition_failed
   LOCK_ACQUISITION_FAILURE_MSG = "Failed to acquire an internal lock to make Play Store calls"
   LOCK_NAME_PREFIX = "google_play_store_edit_"
@@ -150,15 +152,6 @@ def connection_data
     "Bundle Identifier: #{bundle_identifier}"
   end
 
-  def channels
-    default_channels = CHANNELS.map(&:with_indifferent_access)
-    channel_data.each do |chan|
-      next if default_channels.pluck(:id).map(&:to_s).include?(chan[:name])
-      default_channels << {id: chan[:name], name: "Closed testing - #{chan[:name]}", is_production: false}.with_indifferent_access
-    end
-    default_channels
-  end
-
   def pick_default_beta_channel
     BETA_CHANNEL
   end
@@ -229,6 +222,8 @@ def build_in_progress?(channel, build_number)
     response.present? && GooglePlayStoreIntegration::IN_PROGRESS_STORE_STATUS.include?(response[:status])
   end
 
+  private
+
   def project_id
     JSON.parse(json_key)["project_id"]&.split("-")&.third
   end
@@ -276,4 +271,19 @@ def api_lock_params(priority: :high)
       raise ArgumentError, "Invalid priority: #{priority}"
     end
   end
+
+  def channels
+    default_channels = CHANNELS.map(&:with_indifferent_access)
+    cache.fetch(tracks_cache_key, expires_in: CACHE_EXPIRY) do
+      channel_data&.each do |chan|
+        next if default_channels.pluck(:id).map(&:to_s).include?(chan[:name])
+        default_channels << {id: chan[:name], name: "Closed testing - #{chan[:name]}", is_production: false}.with_indifferent_access
+      end
+    end
+    default_channels
+  end
+
+  def tracks_cache_key
+    "google_play_store_integration/#{id}/tracks"
+  end
 end

app/models/integration.rb

@@ -229,9 +229,20 @@ def disconnectable?
     app.active_runs.none?
   end
 
+  def disconnectable_categories?
+    ci_cd? || version_control?
+  end
+
   def disconnect
     return unless disconnectable?
-    update(status: :disconnected, discarded_at: Time.current)
+    transaction do
+      integrable.config.disconnect!(self)
+      update!(status: :disconnected, discarded_at: Time.current)
+      true
+    end
+  rescue ActiveRecord::RecordInvalid => e
+    errors.add(:base, e.message)
+    false
   end
 
   def set_metadata!

app/models/release_platform.rb

@@ -109,7 +109,7 @@ def set_default_config
     return if Rails.env.test?
     return if platform_config.present?
 
-    rc_ci_cd_channel = app.config.ci_cd_workflows.first
+    rc_ci_cd_channel = train.workflows.first || {}
     base_config_map = {
       release_platform: self,
       workflows: {

app/models/train.rb

@@ -106,7 +106,7 @@ class Train < ApplicationRecord
   after_initialize :set_backmerge_config, if: :persisted?
   after_initialize :set_notifications_config, if: :persisted?
   before_validation :set_version_seeded_with, if: :new_record?
-  before_create :set_ci_cd_workflows
+  before_create :fetch_ci_cd_workflows
   before_create :set_current_version
   before_create :set_default_status
   after_create :create_release_platforms
@@ -157,7 +157,7 @@ def temporarily_allow_workflow_errors?
   end
 
   def workflows
-    @workflows ||= ci_cd_provider.workflows(working_branch)
+    @workflows ||= ci_cd_provider&.workflows(working_branch)
   end
 
   def version_ahead?(release)
@@ -454,9 +454,8 @@ def stop_failed_ongoing_release!
     ongoing_release.stop!
   end
 
-  def set_ci_cd_workflows
-    config.set_ci_cd_workflows(workflows)
-  end
+  # call workflows to memoize and cache the result
+  def fetch_ci_cd_workflows = workflows
 
   def last_finished_release
     releases.where(status: "finished").reorder(completed_at: :desc).first

app/views/config/release_platforms/_form.html.erb

@@ -1,5 +1,11 @@
 <%# locals: (app:, train:, platform:, platform_config:, platform_name:, platform_label:, ci_actions:, submission_types:) %>
 
+<% if @edit_allowed %>
+  <%= render AlertComponent.new(kind: :banner, type: :notice, title: "Updates will be applied on a new release", full_screen: false) do %>
+    The submissions settings can be edited while there are active releases. But they will only be applied on the next release.
+  <% end %>
+<% end %>
+
 <%= render FormComponent.new(model: [app, train, platform, platform_config],
                              url: app_train_platform_submission_config_path(app, train, platform_name)) do |f| %>
   <% f.with_section(heading: "Configure RC (Release Candidate) Workflow") do |section| %>
@@ -183,6 +189,6 @@
   <% end %>
 
   <% f.with_action do %>
-    <%= f.F.authz_submit "Save", "archive.svg" %>
+    <%= f.F.authz_submit "Save", "archive.svg", disabled: !@app.ready? %>
   <% end %>
 <% end %>