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

Fix Default Views settings for Service Catalogs #3487

Merged

Conversation

hstastna
Copy link

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1404207
More info: #3469


Fix Default Views settings for Services > Catalogs > Service Catalogs accordion.

Setting up new Default Views settings for Service Catalogs (tile view):
service_cat_tile

Before: (still list view)
service_cat_bad

After:(tile view as expected)
service_cat_ok

Note:
Changing between views in Service Catalogs now also works properly.

@hstastna
Copy link
Author

@miq-bot add_label bug

@miq-bot miq-bot added the bug label Feb 27, 2018
@@ -1426,6 +1426,9 @@ def get_view(db, options = {}, fetch_data = false)
sortcol_sym = "#{sort_prefix}_sortcol".to_sym
sortdir_sym = "#{sort_prefix}_sortdir".to_sym

# Change db_sym for proper setting of @gtl_type as Service Catalogs and Catalog Items use the same db
db_sym = :catalog if x_active_tree == :svccat_tree && %w(servicetemplate).include?(db_sym.to_s)
Copy link
Contributor

@himdel himdel Feb 28, 2018

Choose a reason for hiding this comment

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

Look at line 1391:

db_sym = (options[:gtl_dbname] || dbname).to_sym # Get db name as symbol

you should be able to pass gtl_dbname in options to override db_sym there.

(So that this very service-specific code can live in the service(?) controller, instead of in application controller.)

Any reason you can't do that?

Copy link
Author

@hstastna hstastna Feb 28, 2018

Choose a reason for hiding this comment

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

I was expecting similar comment on this because yes, it would be nicer to have this change implemented in catalog controller (catalog for now, not service), not here in app controller. App controller is more general but I tried to push there this specific thing, because it is simple, straight, clear and it works well comparing to what I was trying to add in catalog controller. I better pushed here what really works. What I forgot was to move this PR to WIP till I find nicer and working solution (we both know now that I probably was doing something wrong that such a simple thing as passing gtl_dbname in options did not work for me). Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 thanks, looks much better now :)

@hstastna hstastna changed the title Fix Default Views settings for Service Catalogs [WIP] Fix Default Views settings for Service Catalogs Feb 28, 2018
@miq-bot miq-bot added the wip label Feb 28, 2018
@hstastna hstastna force-pushed the Default_view_settings_service_catalogs branch 2 times, most recently from 95f5879 to 42bb5ba Compare February 28, 2018 13:13
@hstastna hstastna changed the title [WIP] Fix Default Views settings for Service Catalogs Fix Default Views settings for Service Catalogs Feb 28, 2018
@miq-bot miq-bot removed the wip label Feb 28, 2018
@hstastna hstastna force-pushed the Default_view_settings_service_catalogs branch from 42bb5ba to e6d1947 Compare February 28, 2018 15:18
@h-kataria
Copy link
Contributor

@hstastna when i click on a Service Catalog name in the tree, that does not honor the list view type setting, see attached screenshots
screenshot from 2018-02-28 17-07-17

screenshot from 2018-02-28 17-05-50

@hstastna hstastna changed the title Fix Default Views settings for Service Catalogs [WIP] Fix Default Views settings for Service Catalogs Mar 1, 2018
@miq-bot miq-bot added the wip label Mar 1, 2018
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1404207

Fix Default Views settings for Services > Catalogs > Service Catalogs accordion.
@hstastna hstastna force-pushed the Default_view_settings_service_catalogs branch from e6d1947 to 971c8f3 Compare March 1, 2018 10:59
@hstastna hstastna changed the title [WIP] Fix Default Views settings for Service Catalogs Fix Default Views settings for Service Catalogs Mar 1, 2018
@hstastna hstastna force-pushed the Default_view_settings_service_catalogs branch from b3e0f32 to 08d4647 Compare March 1, 2018 13:44
@miq-bot miq-bot removed the wip label Mar 1, 2018
@hstastna
Copy link
Author

hstastna commented Mar 1, 2018

@h-kataria Thank you for your catch, Harpreet, Now it should work well in every node in Service Catalogs. I also added spec test for my change :) (rubocop issue also fixed)

Add spec test to check options for Service Catalogs page view in service_template_list
method, for proper setting of @gtl_type variable in application controller.
@hstastna hstastna force-pushed the Default_view_settings_service_catalogs branch from 08d4647 to e54d3b9 Compare March 1, 2018 13:49
@miq-bot
Copy link
Member

miq-bot commented Mar 1, 2018

Checked commits hstastna/manageiq-ui-classic@971c8f3~...e54d3b9 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@h-kataria h-kataria added this to the Sprint 81 Ending Mar 12, 2018 milestone Mar 1, 2018
@h-kataria h-kataria merged commit 819ebed into ManageIQ:master Mar 1, 2018
simaishi pushed a commit that referenced this pull request Mar 8, 2018
…catalogs

Fix Default Views settings for Service Catalogs
(cherry picked from commit 819ebed)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1553336
@simaishi
Copy link
Contributor

simaishi commented Mar 8, 2018

Gaprindashvili backport details:

$ git log -1
commit 625fe381f20dc738d6d30556607df273ce4388cc
Author: Harpreet Kataria <[email protected]>
Date:   Thu Mar 1 12:51:48 2018 -0500

    Merge pull request #3487 from hstastna/Default_view_settings_service_catalogs
    
    Fix Default Views settings for Service Catalogs
    (cherry picked from commit 819ebed8aed5b9d17d7b62c4ff282fbab9af0512)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1553336

@hstastna
Copy link
Author

hstastna commented Apr 6, 2018

@miq-bot add_label test

@miq-bot miq-bot added the test label Apr 6, 2018
simaishi pushed a commit that referenced this pull request Apr 10, 2018
…catalogs

Fix Default Views settings for Service Catalogs
(cherry picked from commit 819ebed)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1553337
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit 888bfb6429b9200e47e2b151bd81ffd716255ec8
Author: Harpreet Kataria <[email protected]>
Date:   Thu Mar 1 12:51:48 2018 -0500

    Merge pull request #3487 from hstastna/Default_view_settings_service_catalogs
    
    Fix Default Views settings for Service Catalogs
    (cherry picked from commit 819ebed8aed5b9d17d7b62c4ff282fbab9af0512)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1553337

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants