Fixes #37861 - Update web UI for multi-CV activation key display (#11161

)

* Fixes #37861 - Update web UI for multi-CV activation key display

* Refs #37861 - fix issue with AngularJS editing

* Refs #37861 - bring back CVEDetailsBareCard to new AK details page

* Refs #37861 - handle mixed validity CVE params

* Refs #37861 - Add column to AK list page

* Refs #37861 - add N/A to CVE card when no CVEs

* Refs #37861 - update ouiaID and rubocop

app/controllers/katello/api/v2/activation_keys_controller.rb

@@ -300,11 +300,23 @@ def find_cve_for_single
       @content_view_environments = [cve]
     end
 
+    def params_likely_not_from_angularjs?
+      # AngularJS sends back the activation key's existing API response values.
+      # A side effect of this is that when it sends params[:content_view_environments] or params[:content_view_environment_ids],
+      # it incorrectly sends the nested objects from the rabl response, instead of the required single comma-separated string of Candlepin names.
+      # This would cause fetch_content_view_environments to fail.
+      # Therefore, we need a way to (a) detect if the request is from AngularJS, and (b) avoid trying to handle the nested objects as if they were strings.
+      # So we look at params[:multi_content_view_environment]. This is a computed value, not meant to be submitted as part of an update request.
+      # If it's true or false, it's likely AngularJS.
+      # And if the key is omitted, it's likely from Hammer or API, so it's safe to proceed.
+      !params.key?(:multi_content_view_environment)
+    end
+
     def find_content_view_environments
       @content_view_environments = []
       if params[:environment_id] || params[:environment]
         find_cve_for_single
-      elsif params[:content_view_environments] || params[:content_view_environment_ids]
+      elsif params_likely_not_from_angularjs? && (params[:content_view_environments] || params[:content_view_environment_ids])
         @content_view_environments = ::Katello::ContentViewEnvironment.fetch_content_view_environments(
           labels: params[:content_view_environments],
           ids: params[:content_view_environment_ids],

app/models/katello/content_view_environment.rb

@@ -69,20 +69,37 @@ def priority(content_object)
     end
 
     def self.fetch_content_view_environments(labels: [], ids: [], organization:)
-      # Must do maps here to ensure CVEs remain in the same order.
+      # Must ensure CVEs remain in the same order.
       # Using ActiveRecord .where will return them in a different order.
+      id_errors = []
+      label_errors = []
+      cves = []
       if ids.present?
-        cves = ids.map do |id|
-          ::Katello::ContentViewEnvironment.find_by(id: id)
+        ids.each do |id|
+          cve = ::Katello::ContentViewEnvironment.find_by(id: id)
+          if cve.blank?
+            id_errors << id
+          else
+            cves << cve
+          end
         end
-        cves.compact
       elsif labels.present?
         environment_names = labels.map(&:strip)
-        environment_names.map! do |name|
-          with_candlepin_name(name, organization: organization)
+        environment_names.each do |name|
+          cve = with_candlepin_name(name, organization: organization)
+          if cve.blank?
+            label_errors << name
+          else
+            cves << cve
+          end
         end
-        environment_names.compact
       end
+      if labels.present? && labels.length != cves.length
+        fail HttpErrors::UnprocessableEntity, _("No content view environments found with names: %{names}") % {names: label_errors.join(', ')} if label_errors.present?
+      elsif ids.present? && ids.length != cves.length
+        fail HttpErrors::UnprocessableEntity, _("No content view environments found with ids: %{ids}") % {ids: id_errors.join(', ')} if id_errors.present?
+      end
+      cves
     end
 
     private

app/views/katello/api/v2/activation_keys/base.json.rabl

@@ -7,6 +7,30 @@ node :multi_content_view_environment do |ak|
   ak.multi_content_view_environment?
 end
 
+child :content_view_environments => :content_view_environments do
+  node :content_view do |cve|
+    {
+      id: cve.content_view&.id,
+      name: cve.content_view&.name,
+      composite: cve.content_view&.composite,
+      content_view_version: cve.content_view_version&.version,
+      content_view_version_id: cve.content_view_version&.id,
+      content_view_version_latest: cve.content_view_version&.latest?,
+      content_view_default: cve.content_view&.default?
+    }
+  end
+  node :lifecycle_environment do |cve|
+    {
+      id: cve.lifecycle_environment&.id,
+      name: cve.lifecycle_environment&.name,
+      lifecycle_environment_library: cve.lifecycle_environment&.library?
+    }
+  end
+  node :candlepin_name do |cve|
+    cve.candlepin_name
+  end
+end
+
 # single cv/lce for backward compatibility
 node :content_view_id do |ak|
   ak.single_content_view&.id

engines/bastion_katello/app/assets/javascripts/bastion_katello/activation-keys/details/views/activation-key-info.html

@@ -8,7 +8,7 @@
 
 <div bst-alert="warning" ng-if="activationKey.multi_content_view_environment">
   <span translate>
-    This activation key has multiple content view environments, which are not yet displayed in the web UI. Changing the content view or lifecycle environment here will overwrite the activation key's multiple environments. To avoid this, update the activation key via Hammer.
+    This activation key has multiple content view environments, which cannot yet be edited in the web UI. To change this key's content view environments, update the activation key via Hammer.
   </span>
 </div>
 
@@ -57,7 +57,6 @@ <h4 translate>Basic Information</h4>
         </div>
       </dd>
 
-
       <div class="divider"></div>
       <h4 translate>System Purpose</h4>
 
@@ -123,17 +122,18 @@ <h4 translate>Activation Key Content</h4>
           on-delete="clearReleaseVersion()"
           on-save="save(activationKey)">
       </dd>
-
+    </dl>
+    <dl class="dl-horizontal dl-horizontal-left" ng-if="!activationKey.multi_content_view_environment">
       <dt bst-feature-flag="lifecycle_environments">
         <span translate>Environment</span>
       </dt>
       <dd bst-feature-flag="lifecycle_environments">
         <div path-selector="environments"
-             ng-model="activationKey.environment"
-             mode="singleSelect"
-             selection-required="selectionRequired"
-             disabled="denied('edit_activation_keys', activationKey)"
-             disable-trigger="disableEnvironmentSelection">
+            ng-model="activationKey.environment"
+            mode="singleSelect"
+            selection-required="selectionRequired"
+            disabled="denied('edit_activation_keys', activationKey)"
+            disable-trigger="disableEnvironmentSelection">
         </div>
       </dd>
 
@@ -142,14 +142,14 @@ <h4 translate>Activation Key Content</h4>
       </dt>
       <dd bst-feature-flag="lifecycle_environments">
         <div bst-edit-select="activationKey.content_view.name"
-             readonly="denied('edit_activation_keys', activationKey) || activationKey.environment === undefined || activationKey.environment === null"
-             selector="activationKey.content_view.id"
-             options="contentViews()"
-             on-cancel="cancelContentViewUpdate()"
-             on-save="saveContentView(activationKey)"
-             deletable="activationKey.content_view"
-             on-delete="resetEnvironment(activationKey)"
-             edit-trigger="editContentView">
+            readonly="denied('edit_activation_keys', activationKey) || activationKey.environment === undefined || activationKey.environment === null"
+            selector="activationKey.content_view.id"
+            options="contentViews()"
+            on-cancel="cancelContentViewUpdate()"
+            on-save="saveContentView(activationKey)"
+            deletable="activationKey.content_view"
+            on-delete="resetEnvironment(activationKey)"
+            edit-trigger="editContentView">
         </div>
 
         <div bst-alert="info" ng-show="editEnvironment">
@@ -160,6 +160,14 @@ <h4 translate>Activation Key Content</h4>
         </div>
       </dd>
     </dl>
+    <dl class="dl-horizontal dl-horizontal-left">
+      <span id="ak-cve-details" data-ak-details="{{ activationKey }}" class="hidden"></span>
+      <foreman-react-component
+          name="CVEDetailsCard"
+          data-props=""
+          ng-if="activationKey.multi_content_view_environment"
+      ></foreman-react-component>
+    </dl>
   </div>
 </div>
 

engines/bastion_katello/app/assets/javascripts/bastion_katello/activation-keys/views/activation-keys.html

@@ -29,6 +29,7 @@ <h2 translate>Activation Keys</h2>
         <th bst-table-column="consumed"><span translate>Host Limit</span></th>
         <th bst-table-column="environment"><span translate>Environment</span></th>
         <th bst-table-column="contentView"><span translate>Content View</span></th>
+        <th bst-table-column="multi-cv"><span translate>Multi Content View Environment</span></th>
       </tr>
       </thead>
 
@@ -48,6 +49,7 @@ <h2 translate>Activation Keys</h2>
         <td bst-table-cell>{{ activationKey | activationKeyConsumedFilter }}</td>
         <td bst-table-cell>{{ activationKey.environment.name || "" }}</td>
         <td bst-table-cell>{{ activationKey.content_view.name || "" }}</td>
+        <td bst-table-cell>{{ activationKey.multi_content_view_environment ? 'Yes' : 'No' }}</td>
       </tr>
       </tbody>
     </table>

test/models/content_view_environment_test.rb

@@ -54,8 +54,26 @@ def test_fetch_content_view_environments_ids
     def test_fetch_content_view_environments_invalid_ids_does_not_mutate_array
       dev = katello_environments(:dev)
       input_ids = [0, 999]
-      assert_equal [], ContentViewEnvironment.fetch_content_view_environments(ids: input_ids, organization: dev.organization)
+      assert_raises(HttpErrors::UnprocessableEntity) do
+        ContentViewEnvironment.fetch_content_view_environments(ids: input_ids, organization: dev.organization)
+      end
       assert_equal [0, 999], input_ids # should not have a map! which mutates the input array
     end
+
+    def test_fetch_content_view_environments_mixed_validity_candlepin_names
+      dev = katello_environments(:dev)
+      assert_raises(HttpErrors::UnprocessableEntity) do
+        ContentViewEnvironment.fetch_content_view_environments(labels: ['published_dev_view_dev, bogus'], organization: dev.organization)
+      end
+    end
+
+    def test_fetch_content_view_environments_mixed_validity_ids
+      dev = katello_environments(:dev)
+      view = katello_content_views(:library_dev_view)
+      cve = Katello::ContentViewEnvironment.where(:environment_id => dev, :content_view_id => view).first
+      assert_raises(HttpErrors::UnprocessableEntity) do
+        ContentViewEnvironment.fetch_content_view_environments(ids: [cve.id, 9999], organization: dev.organization)
+      end
+    end
   end
 end