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

fixes #8040 - Product content changes are updated in UI, BZ1112747 #4856

Merged
merged 1 commit into from Jan 30, 2015

Conversation

cfouant
Copy link
Member

@cfouant cfouant commented Dec 8, 2014

No description provided.

@@ -94,6 +95,7 @@ angular.module('Bastion.activation-keys').controller('ActivationKeyProductDetail
},
function () {
$scope.success(content);
$scope.reloadActivationKey();
Copy link
Member

Choose a reason for hiding this comment

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

Better to avoid scope inheritance and place the get of the activation key here in this controller. That being said, it appears that this API returns the activation key anyway -- https://github.com/Katello/katello/blob/master/app/controllers/katello/api/v2/activation_keys_controller.rb#L220

Copy link
Member

Choose a reason for hiding this comment

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

So i worked with christine on this, and due to the nature of the issue, she would have to do something like:

$scope.$parent.$parent.$parent = data_from_response;

The reason being is that "$scope.activationKey = " did not seem to be working on the great-grand parent's scope, but seemed to be setting it on the current scope. When she navigated away from the tab and back the great-grand parent's scope was stale.

Copy link
Member

Choose a reason for hiding this comment

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

That said, something like $scope.setActivatoinKey(data_from_response) would be more ideal (and not take a 2nd request).

Copy link
Member

Choose a reason for hiding this comment

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

We really should pull this out into a service so there isn't scope chaining funny business.That being said, I am willing to accept your solution and to start contemplating some re-factoring work for myself :)

Copy link
Member Author

Choose a reason for hiding this comment

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

@ehelms - I do agree that some refactoring would work better than either the great-grandparent scope or a 2nd request. There are some new functions inside the model which may assist with making that easier (namely available content).

$scope.success(content);
$scope.setActivationKey(response);
Copy link
Member

Choose a reason for hiding this comment

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

Test to check that setActivationKey is called?

@cfouant
Copy link
Member Author

cfouant commented Jan 27, 2015

[test]

@ehelms
Copy link
Member

ehelms commented Jan 30, 2015

ACK

cfouant added a commit that referenced this pull request Jan 30, 2015
fixes #8040 - Product content changes are updated in UI, BZ1112747
@cfouant cfouant merged commit 78f2700 into Katello:master Jan 30, 2015
@cfouant cfouant deleted the ui-product-content branch January 30, 2015 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants