From 51cbeb664036a448b8ad55b7e353d553f305c561 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Wed, 28 Jun 2017 20:31:06 +0100 Subject: [PATCH] Fix missing import package button in murano dashboard After https://review.openstack.org/#/c/476273/ was merged, murano dashboard is throwing errors related to listing categories when trying to import a package. Currently, python-muranoclient still contains a function [0] for listing categories using the deprecated API v1/packages/categories. The problem is that murano dashboard calls this function [1][2][3] in python-muranoclient which throws a 404, as the deprecated API was removed from murano. This commit replaces instances of client.packages.categories() with client.categories.list(). However, while the former returns a list of category names, the latter returns of a list of category objects -- so they then need to be iterated over to retrieve the list of category names. [0] https://github.com/openstack/python-muranoclient/blob/08411aa8d20993ac7c4a52b2aa0e73fb6fea4d40/muranoclient/v1/packages.py#L52 [1] https://github.com/openstack/murano-dashboard/blob/f98e08d827a006db5fd054ac4fb6abba786bb414/muranodashboard/packages/forms.py#L216 [2] https://github.com/openstack/murano-dashboard/blob/f98e08d827a006db5fd054ac4fb6abba786bb414/muranodashboard/packages/forms.py#L288 [3] https://github.com/openstack/murano-dashboard/blob/bc8e4b97d083bfccf916e5538fcdb6ee1cbc4405/muranodashboard/packages/tables.py#L54 Change-Id: I715b1c9ccfdd044980596cc6d966062e0386884c Partial-Bug: #1701067 --- muranodashboard/packages/forms.py | 12 +++++------ muranodashboard/packages/tables.py | 3 ++- .../tests/unit/packages/test_forms.py | 20 +++++++++++++------ .../tests/unit/packages/test_tables.py | 4 ++-- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/muranodashboard/packages/forms.py b/muranodashboard/packages/forms.py index 2a8461641..ab93fe666 100644 --- a/muranodashboard/packages/forms.py +++ b/muranodashboard/packages/forms.py @@ -213,10 +213,10 @@ class ModifyPackageForm(PackageParamsMixin, horizon_forms.SelfHandlingForm): choices=[('', 'No categories available')], required=False) try: - categories = api.muranoclient(request).packages.categories() + categories = api.muranoclient(request).categories.list() if categories: - self.fields['categories'].choices = [(c, c) - for c in categories] + category_names = [(c.name, c.name) for c in categories] + self.fields['categories'].choices = category_names if package.categories: self.fields['categories'].initial = dict( (key, True) for key in package.categories) @@ -285,10 +285,10 @@ class SelectCategories(forms.Form): super(SelectCategories, self).__init__(*args, **kwargs) try: - categories = api.muranoclient(request).packages.categories() + categories = api.muranoclient(request).categories.list() if categories: - self.fields['categories'].choices = [(c, c) - for c in categories] + category_names = [(c.name, c.name) for c in categories] + self.fields['categories'].choices = category_names except (exc.HTTPException, Exception): msg = _('Unable to get list of categories') LOG.exception(msg) diff --git a/muranodashboard/packages/tables.py b/muranodashboard/packages/tables.py index df49b7f30..72103c170 100644 --- a/muranodashboard/packages/tables.py +++ b/muranodashboard/packages/tables.py @@ -51,7 +51,8 @@ class ImportPackage(tables.LinkAction): _allowed = False with api.handled_exceptions(request): client = api.muranoclient(request) - _allowed = client.packages.categories() is not None + if client.categories.list(): + _allowed = True return _allowed diff --git a/muranodashboard/tests/unit/packages/test_forms.py b/muranodashboard/tests/unit/packages/test_forms.py index 40faa624d..db60b3035 100644 --- a/muranodashboard/tests/unit/packages/test_forms.py +++ b/muranodashboard/tests/unit/packages/test_forms.py @@ -149,8 +149,12 @@ class TestModifyPackageForm(helpers.APITestCase): self.mock_request = mock.MagicMock(return_value=(fake_response)) with mock.patch('muranodashboard.api.muranoclient') as mock_client: - mock_client().packages.categories.return_value =\ - ['c3', 'c4'] + mock_categories = [] + for cname in ['c3', 'c4']: + mock_category = mock.Mock() + mock_category.configure_mock(name=cname) + mock_categories.append(mock_category) + mock_client().categories.list.return_value = mock_categories self.modify_pkg_form = forms.ModifyPackageForm(self.mock_request, **self.kwargs) @@ -170,7 +174,7 @@ class TestModifyPackageForm(helpers.APITestCase): @mock.patch('muranodashboard.api.muranoclient') def test_init_except_http_exception(self, mock_client, mock_reverse, mock_exceptions): - mock_client().packages.categories.side_effect = exc.HTTPException + mock_client().categories.list.side_effect = exc.HTTPException mock_reverse.return_value = 'test_redirect' self.modify_pkg_form = forms.ModifyPackageForm(self.mock_request, @@ -265,8 +269,12 @@ class TestSelectCategories(helpers.APITestCase): self.kwargs = {'request': self.mock_request} with mock.patch('muranodashboard.api.muranoclient') as mock_client: - mock_client().packages.categories.return_value =\ - ['c1', 'c2'] + mock_categories = [] + for cname in ['c1', 'c2']: + mock_category = mock.Mock() + mock_category.configure_mock(name=cname) + mock_categories.append(mock_category) + mock_client().categories.list.return_value = mock_categories self.select_categories_form = forms.SelectCategories(**self.kwargs) def test_init(self): @@ -279,7 +287,7 @@ class TestSelectCategories(helpers.APITestCase): @mock.patch('muranodashboard.api.muranoclient') def test_init_except_http_exception(self, mock_client, mock_reverse, mock_exceptions): - mock_client().packages.categories.side_effect = exc.HTTPException + mock_client().categories.list.side_effect = exc.HTTPException mock_reverse.return_value = 'test_redirect' forms.SelectCategories(**self.kwargs) diff --git a/muranodashboard/tests/unit/packages/test_tables.py b/muranodashboard/tests/unit/packages/test_tables.py index bafcd7507..edb617f27 100644 --- a/muranodashboard/tests/unit/packages/test_tables.py +++ b/muranodashboard/tests/unit/packages/test_tables.py @@ -40,10 +40,10 @@ class TestImportPackage(testtools.TestCase): @mock.patch.object(tables, 'api') def test_allowed(self, mock_api): - mock_api.muranoclient().packages.categories.return_value = ['foo_cat'] + mock_api.muranoclient().categories.list.return_value = ['foo_cat'] self.assertTrue(self.import_package.allowed(None, None)) - mock_api.muranoclient().packages.categories.return_value = None + mock_api.muranoclient().categories.list.return_value = None self.assertFalse(self.import_package.allowed(None, None))