Merge pull request #2616 from okriuchykhin/ok_SCI_4698_v2

Fix handling of deleted repositories in reports [SCI-4698]

app/helpers/reports_helper.rb

@@ -122,10 +122,14 @@ module ReportsHelper
     )
   end
 
-  def assign_repository_or_snapshot(my_module, element_id, snapshot, repository)
-    original_repository = Repository.find_by(id: element_id) if element_id
-    repository ||= snapshot
-    repository || my_module.active_snapshot_or_live(original_repository) || original_repository
+  def assigned_repository_or_snapshot(my_module, element_id, snapshot, repository)
+    if element_id
+      repository = Repository.accessible_by_teams(my_module.experiment.project.team).find_by(id: element_id)
+      repository ||= RepositorySnapshot.joins(my_module: { experiment: :project })
+                                       .where(my_module: { experiments: { project: my_module.experiment.project } })
+                                       .find_by(id: element_id)
+    end
+    repository || snapshot
   end
 
   def step_status_label(step)

app/models/my_module.rb

@@ -226,32 +226,15 @@ class MyModule < ApplicationRecord
     (live_repositories + selected_snapshots).sort_by { |r| r.name.downcase }
   end
 
-  def active_snapshot_or_live(rep_or_snap, exclude_snpashot_ids: [])
-    return unless rep_or_snap
-
-    parent_id = rep_or_snap.is_a?(Repository) ? rep_or_snap.id : rep_or_snap.parent_id
-
-    selected_snapshot_for_repo(parent_id, exclude_snpashot_ids: exclude_snpashot_ids) ||
-      assigned_repositories&.where(id: parent_id)&.first ||
-      repository_snapshots
-        .where(parent_id: parent_id)
-        .where.not(id: exclude_snpashot_ids)
-        .order(updated_at: :desc).first
-  end
-
-  def update_report_repository_references(rep_or_snap)
-    ids = if rep_or_snap.is_a?(Repository)
-            RepositorySnapshot.where(parent_id: rep_or_snap.id).pluck(:id)
+  def update_report_repository_references(repository)
+    ids = if repository.is_a?(Repository)
+            RepositorySnapshot.where(parent_id: repository.id).pluck(:id)
           else
-            Repository.where(id: rep_or_snap.parent_id).pluck(:id) +
-              RepositorySnapshot.where(parent_id: rep_or_snap.parent_id).pluck(:id)
+            Repository.where(id: repository.parent_id).pluck(:id) +
+              RepositorySnapshot.where(parent_id: repository.parent_id).pluck(:id)
           end
 
-    report_elements.where(repository_id: ids).update(repository_id: rep_or_snap.id)
-  end
-
-  def selected_snapshot_for_repo(repository_id, exclude_snpashot_ids: [])
-    repository_snapshots.where(parent_id: repository_id).where.not(id: exclude_snpashot_ids).where(selected: true).first
+    report_elements.where(repository_id: ids).update(repository: repository)
   end
 
   def unassigned_users

app/models/repository.rb

@@ -15,6 +15,7 @@ class Repository < RepositoryBase
            inverse_of: :original_repository
 
   before_save :sync_name_with_snapshots, if: :name_changed?
+  before_destroy :refresh_report_references_on_destroy, prepend: true
 
   validates :name,
             presence: true,
@@ -216,4 +217,15 @@ class Repository < RepositoryBase
   def sync_name_with_snapshots
     repository_snapshots.update(name: name)
   end
+
+  def refresh_report_references_on_destroy
+    report_elements.find_each do |report_element|
+      repository_snapshot = report_element.my_module
+                                          .repository_snapshots
+                                          .where(original_repository: self)
+                                          .order(:selected, created_at: :desc)
+                                          .first
+      report_element.update(repository: repository_snapshot) if repository_snapshot
+    end
+  end
 end

app/models/repository_snapshot.rb

@@ -3,7 +3,7 @@
 class RepositorySnapshot < RepositoryBase
   enum status: { provisioning: 0, ready: 1, failed: 2 }
   after_save :refresh_report_references, if: :saved_change_to_selected
-  before_destroy :refresh_report_references_for_destroy, prepend: true
+  before_destroy :refresh_report_references_on_destroy, prepend: true
 
   belongs_to :original_repository, foreign_key: :parent_id,
                                    class_name: 'Repository',
@@ -44,19 +44,17 @@ class RepositorySnapshot < RepositoryBase
 
   def refresh_report_references
     if selected
-      ids = Repository.where(id: parent_id).pluck(:id) +
-            RepositorySnapshot.where(parent_id: parent_id).pluck(:id)
-
-      ReportElement.where(my_module: my_module).where(repository_id: ids).update(repository_id: id)
-    elsif original_repository && !my_module.selected_snapshot_for_repo(original_repository.id)
+      repositories = RepositoryBase.where(id: parent_id).or(RepositoryBase.where(parent_id: parent_id))
+      my_module.report_elements.where(repository: repositories).update(repository: self)
+    elsif original_repository
       my_module.update_report_repository_references(original_repository)
     end
   end
 
-  def refresh_report_references_for_destroy
-    repository_or_snap = original_repository || self
-    default_view_candidate =
-      my_module.active_snapshot_or_live(repository_or_snap, exclude_snpashot_ids: [id])
-    my_module.update_report_repository_references(default_view_candidate) if default_view_candidate
+  def refresh_report_references_on_destroy
+    return my_module.update_report_repository_references(original_repository) if original_repository
+
+    repository_snapshot = my_module.repository_snapshots.where(parent_id: parent_id).order(created_at: :desc).first
+    my_module.update_report_repository_references(repository_snapshot) if repository_snapshot
   end
 end

app/views/reports/elements/_my_module_repository_element.html.erb

@@ -1,58 +1,55 @@
-<% if my_module.blank? and @my_module.present? then my_module = @my_module end %>
+<% my_module ||= @my_module %>
 <% element_id ||= nil %>
 <% repository ||= nil %>
 <% repository_snapshot ||= nil %>
-<% repository = assign_repository_or_snapshot(my_module, element_id, repository, repository_snapshot) %>
-<% if order.blank? and @order.present? then order = @order end %>
+<% repository = assigned_repository_or_snapshot(my_module, element_id, repository, repository_snapshot) %>
+<% order ||= @order %>
 <% timestamp = Time.current + 1.year - 1.days %>
-<% if repository.present? %>
-  <% rows_json = my_module.repository_json_hot(repository, order) %>
-  <div class="report-element report-module-repository-element"
-       data-sort-hot="1"
-       data-ts="<%= timestamp.to_i %>"
-       data-type="my_module_repository"
-       data-id='{ "my_module_id": <%= my_module.id %>, "repository_id": <%= repository.id %> }'
-       data-scroll-id="<%= "#{my_module.id}_#{repository.id}" %>"
-       data-order="<%= order == :asc ? "asc" : "desc" %>"
-       data-name="<%= repository.name %>"
-       data-icon-class="fas fa-list-alt">
-    <div class="report-element-header">
-      <div class="row">
-        <div class="pull-left repository-icon">
-          <i class="fas fa-list-alt"></i>
-        </div>
-        <div class="pull-left repository-name">
-          <%=t "projects.reports.elements.module_repository.name", repository: repository.name, my_module: my_module.name %>
-          <i><%= t('projects.reports.index.deleted') if repository.is_a?(RepositorySnapshot) && !repository.original_repository %></i>
-        </div>
-        <% if defined? export_all and export_all %>
-          <div class="pull-left table-name">
-              <a href="<%= path %>">
-                <em><%=t "projects.reports.elements.module_repository.table_name",
-                         name: filename %></em>
-              </a>
-          </div>
-        <% end %>
-        <div class="pull-right controls">
-          <%= render partial: "reports/elements/element_controls.html.erb", locals: { show_sort: true } %>
+<% rows_json = my_module.repository_json_hot(repository, order) %>
+<div class="report-element report-module-repository-element"
+     data-sort-hot="1"
+     data-ts="<%= timestamp.to_i %>"
+     data-type="my_module_repository"
+     data-id='{ "my_module_id": <%= my_module.id %>, "repository_id": <%= repository.id %> }'
+     data-scroll-id="<%= "#{my_module.id}_#{repository.id}" %>"
+     data-order="<%= order == :asc ? 'asc' : 'desc' %>"
+     data-name="<%= repository.name %>"
+     data-icon-class="fas fa-list-alt">
+  <div class="report-element-header">
+    <div class="row">
+      <div class="pull-left repository-icon">
+        <i class="fas fa-list-alt"></i>
+      </div>
+      <div class="pull-left repository-name">
+        <%= t('projects.reports.elements.module_repository.name', repository: repository.name, my_module: my_module.name) %>
+        <i><%= t('projects.reports.index.deleted') if repository.is_a?(RepositorySnapshot) && !repository.original_repository %></i>
+      </div>
+      <% if defined?(export_all) && export_all %>
+        <div class="pull-left table-name">
+            <a href="<%= path %>">
+              <em><%= t('projects.reports.elements.module_repository.table_name', name: filename) %></em>
+            </a>
         </div>
+      <% end %>
+      <div class="pull-right controls">
+        <%= render partial: "reports/elements/element_controls.html.erb", locals: { show_sort: true } %>
       </div>
     </div>
-    <div class="report-element-body">
-      <% if rows_json[:data].count > 0 %>
+  </div>
+  <div class="report-element-body">
+    <% if rows_json[:data].count > 0 %>
       <input type="hidden" class="hot-table-contents hot-samples" value='<%= rows_json.to_json.force_encoding(Encoding::UTF_8) %>' />
       <div class="hot-table-container"></div>
       <table class="report-common-table-format"></table>
-      <% else %>
-        <div class="row">
-          <div class="col-xs-12">
-            <em><%=t "projects.reports.elements.module_repository.no_items" %></em>
-          </div>
+    <% else %>
+      <div class="row">
+        <div class="col-xs-12">
+          <em><%= t('projects.reports.elements.module_repository.no_items') %></em>
         </div>
-      <% end %>
-    </div>
-    <div class="report-element-children">
-      <%= children if (defined? children and children.present?) %>
-    </div>
+      </div>
+    <% end %>
   </div>
-<% end %>
+  <div class="report-element-children">
+    <%= children if defined?(children) && children.present? %>
+  </div>
+</div>