Changes to exclude service templates marked as service_type = internal #4274

h-kataria · 2018-07-10T18:51:14Z

This is to exclude Migration Plan Service Templates from Catalogs Explorer. Backend changes to support this are in ManageIQ/manageiq#17681

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1594408

Depends on ManageIQ/manageiq#17681, ManageIQ/manageiq#17748

@bzwei please review/test

h-kataria · 2018-07-10T18:56:34Z

@martinpovolny i will be adding spec test for this later today

bzwei · 2018-07-12T13:42:00Z

@h-kataria I cannot test your PR locally. I forked your branch and referenced it in my bundler.d/Gemfile.dev.rb, but I cannot resolve error Bundler could not find compatible versions for gem "faraday"
It is OK to me if you have tested it locally with my backend PR.

gmcculloug

@h-kataria I believe we need to review all the where class in this PR and determine if there are existing scopes or do we need new scopes that can be used.

cc @bzwei

gmcculloug · 2018-07-13T18:55:20Z

app/controllers/catalog_controller.rb

@@ -1776,7 +1777,7 @@ def get_node_info_handle_stc_node(id)

  def get_node_info_handle_leaf_node_stcat(id)
    @record = ServiceTemplateCatalog.find(id)
-    @record_service_templates = Rbac.filtered(@record.service_templates)
+    @record_service_templates = Rbac.filtered(@record.service_templates.where("service_type != 'internal'"))


@h-kataria I think you want to use the new scope here instead of recreating the where clause.

@gmcculloug will clean those up.

h-kataria · 2018-07-14T16:59:08Z

@gmcculloug please re-review, replaced where clauses with named_Scope wherever possible.

gmcculloug · 2018-07-16T12:28:10Z

app/controllers/catalog_controller.rb

@@ -1455,7 +1454,7 @@ def rearrange_groups_array

  def get_available_resources(kls)
    @edit[:new][:available_resources] = {}
-    Rbac.filtered(kls.constantize.where("type is null or type != 'ServiceTemplateAnsiblePlaybook'")).select(:id, :name).each do |r|
+    Rbac.filtered(kls.constantize.where("type is null or type not in (?)", ['ServiceTemplateAnsiblePlaybook', 'ServiceTemplateTransformationPlan'])).select(:id, :name).each do |r|


Is kls.constantize always ServiceTemplate here? If so it would be better to use the scope here as well:

Rbac.filtered(kls.constantize.public_service_templates.where("type is null or type != 'ServiceTemplateAnsiblePlaybook'")).select(:id, :name).each do |r|

gmcculloug

@h-kataria One additional change I commented on and then please address rubocop comments.

h-kataria · 2018-07-16T13:17:08Z

@lgalis can you please test

h-kataria · 2018-07-16T13:35:01Z

@gmcculloug addressed your comments and rubocop warning, please take a look.

lgalis · 2018-07-16T18:01:40Z

@h-kataria - looks good. Verified that the migration plans are no longer displayed in the Catalog Items list with this fix.

bthurber · 2018-07-23T15:51:20Z

@gmcculloug any remaining items to solve to merge?

gmcculloug · 2018-07-23T17:27:10Z

@bthurber Yes, this PR still requires backend changes which are being worked on. We are taking a slightly different approach then what was done in the dependent PR ManageIQ/manageiq#17681 and those changes are in progress.

First PR is here: ManageIQ/manageiq-schema#238

miq-bot · 2018-07-24T01:31:43Z

This pull request is not mergeable. Please rebase and repush.

h-kataria · 2018-07-30T15:41:00Z

@AparnaKarve can you please review/test this blocker PR

This is to exclude Migration Plan Service Templates from Catalogs Explorer. Backend changes to support this are in ManageIQ/manageiq#17681 Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1594408

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1594408

AparnaKarve

Validated in the UI successfully.

service templates that already existed prior to this PR and ManageIQ/manageiq#17681, ManageIQ/manageiq#17748 still show up in Catalog Items; but the new ones (new migration plans) created after applying these PR changes, do not show up in Catalog items, which is the expected behavior

lgalis · 2018-07-30T18:59:16Z

@h-kataria - Looks good - verified in the UI that with this PR and ManageIQ/manageiq#17748, a newly created migration plan will not show in the list of Catalog Items . ( Migration plans created prior to these code changes will still be visible)

gmcculloug · 2018-07-31T17:31:11Z

@h-kataria dependent PR ManageIQ/manageiq#17748 has been merged.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1594408

miq-bot · 2018-07-31T21:32:58Z

Checked commits h-kataria/manageiq-ui-classic@5fc42f8~...e61f273 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
4 files checked, 0 offenses detected
Everything looks fine. 🏆

simaishi · 2018-08-01T10:27:23Z

@h-kataria @gmcculloug Please confirm that the only PR this depends on for Gaprindashvili backport is ManageIQ/manageiq#17748 and ManageIQ/manageiq#17681 isn't needed.

gmcculloug · 2018-08-01T12:21:03Z

@simaishi Your statement is correct, you only need to backport ManageIQ/manageiq#17748.

The confusion is that the work was initially started in ManageIQ/manageiq#17681 and we ended up changing direction on the solution. However, this PR had a refactoring, creating constants for service_type, that we wanted to keep so the PR was reworked to keep just that part.

…migration_plans Changes to exclude service templates marked as service_type = internal (cherry picked from commit 3acd763) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1610729

simaishi · 2018-08-06T13:32:44Z

Gaprindashvili backport details:

$ git log -1
commit 24bc296be8024b360cb14837ac60e34d3c741c93
Author: Dan Clarizio <dclarizi@redhat.com>
Date:   Tue Jul 31 15:24:17 2018 -0700

    Merge pull request #4274 from h-kataria/service_templates_to_exclude_migration_plans
    
    Changes to exclude service templates marked as service_type = internal
    (cherry picked from commit 3acd7630eadecd477892075a2d4b222a16547940)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1610729

h-kataria added bug wip pending core services gaprindashvili/no labels Jul 10, 2018

h-kataria requested a review from martinpovolny July 10, 2018 18:55

h-kataria force-pushed the service_templates_to_exclude_migration_plans branch from 42b1f0e to 8352bf6 Compare July 11, 2018 20:12

gmcculloug reviewed Jul 13, 2018

View reviewed changes

gmcculloug reviewed Jul 16, 2018

View reviewed changes

h-kataria force-pushed the service_templates_to_exclude_migration_plans branch from 9703fe2 to d1baacf Compare July 16, 2018 12:55

h-kataria changed the title ~~[WIP] - Changes to exclude service templates marked as service_type = internal~~ Changes to exclude service templates marked as service_type = internal Jul 16, 2018

h-kataria removed the wip label Jul 16, 2018

miq-bot added the unmergeable label Jul 24, 2018

h-kataria force-pushed the service_templates_to_exclude_migration_plans branch from f8a1575 to cef6662 Compare July 27, 2018 23:22

h-kataria mentioned this pull request Jul 27, 2018

Refactor service_type to use constants ManageIQ/manageiq#17681

Merged

h-kataria added the wip label Jul 27, 2018

h-kataria changed the title ~~Changes to exclude service templates marked as service_type = internal~~ [WIP] - Changes to exclude service templates marked as service_type = internal Jul 27, 2018

miq-bot removed the unmergeable label Jul 29, 2018

h-kataria requested a review from AparnaKarve July 30, 2018 15:39

h-kataria added the blocker label Jul 30, 2018

h-kataria added 2 commits July 30, 2018 11:41

Changes to exclude service templates marked as service_type = internal

5fc42f8

This is to exclude Migration Plan Service Templates from Catalogs Explorer. Backend changes to support this are in ManageIQ/manageiq#17681 Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1594408

replaced where clauses to use named_scope where possible

f7144c5

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1594408

h-kataria force-pushed the service_templates_to_exclude_migration_plans branch from cef6662 to 4e232c3 Compare July 30, 2018 15:41

h-kataria removed the wip label Jul 30, 2018

h-kataria changed the title ~~[WIP] - Changes to exclude service templates marked as service_type = internal~~ Changes to exclude service templates marked as service_type = internal Jul 30, 2018

AparnaKarve approved these changes Jul 30, 2018

View reviewed changes

h-kataria added gaprindashvili/yes and removed gaprindashvili/no labels Jul 30, 2018

h-kataria removed the pending core label Jul 31, 2018

h-kataria assigned dclarizio Jul 31, 2018

h-kataria force-pushed the service_templates_to_exclude_migration_plans branch from 4e232c3 to 59a7fda Compare July 31, 2018 21:23

addressed rubocop/codeclimate warnings

e61f273

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1594408

h-kataria force-pushed the service_templates_to_exclude_migration_plans branch from 59a7fda to e61f273 Compare July 31, 2018 21:31

dclarizio removed the request for review from martinpovolny July 31, 2018 21:37

dclarizio merged commit 3acd763 into ManageIQ:master Jul 31, 2018

dclarizio added this to the Sprint 92 Ending Aug 13, 2018 milestone Jul 31, 2018

h-kataria deleted the service_templates_to_exclude_migration_plans branch July 31, 2018 22:28

simaishi added gaprindashvili/backported and removed gaprindashvili/yes labels Aug 6, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes to exclude service templates marked as service_type = internal #4274

Changes to exclude service templates marked as service_type = internal #4274

h-kataria commented Jul 10, 2018 •

edited

Loading

h-kataria commented Jul 10, 2018

bzwei commented Jul 12, 2018

gmcculloug left a comment

gmcculloug Jul 13, 2018

h-kataria Jul 13, 2018

h-kataria commented Jul 14, 2018

gmcculloug Jul 16, 2018

gmcculloug left a comment •

edited

Loading

h-kataria commented Jul 16, 2018

h-kataria commented Jul 16, 2018

lgalis commented Jul 16, 2018

bthurber commented Jul 23, 2018

gmcculloug commented Jul 23, 2018

miq-bot commented Jul 24, 2018

h-kataria commented Jul 30, 2018

AparnaKarve left a comment •

edited

Loading

lgalis commented Jul 30, 2018

gmcculloug commented Jul 31, 2018

miq-bot commented Jul 31, 2018

simaishi commented Aug 1, 2018

gmcculloug commented Aug 1, 2018

simaishi commented Aug 6, 2018

Changes to exclude service templates marked as service_type = internal #4274

Changes to exclude service templates marked as service_type = internal #4274

Conversation

h-kataria commented Jul 10, 2018 • edited Loading

h-kataria commented Jul 10, 2018

bzwei commented Jul 12, 2018

gmcculloug left a comment

Choose a reason for hiding this comment

gmcculloug Jul 13, 2018

Choose a reason for hiding this comment

h-kataria Jul 13, 2018

Choose a reason for hiding this comment

h-kataria commented Jul 14, 2018

gmcculloug Jul 16, 2018

Choose a reason for hiding this comment

gmcculloug left a comment • edited Loading

Choose a reason for hiding this comment

h-kataria commented Jul 16, 2018

h-kataria commented Jul 16, 2018

lgalis commented Jul 16, 2018

bthurber commented Jul 23, 2018

gmcculloug commented Jul 23, 2018

miq-bot commented Jul 24, 2018

h-kataria commented Jul 30, 2018

AparnaKarve left a comment • edited Loading

Choose a reason for hiding this comment

lgalis commented Jul 30, 2018

gmcculloug commented Jul 31, 2018

miq-bot commented Jul 31, 2018

simaishi commented Aug 1, 2018

gmcculloug commented Aug 1, 2018

simaishi commented Aug 6, 2018

h-kataria commented Jul 10, 2018 •

edited

Loading

gmcculloug left a comment •

edited

Loading

AparnaKarve left a comment •

edited

Loading