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 #5858,bz 109961 - Updated cv published content count #4467

Merged
merged 1 commit into from Jul 28, 2014

Conversation

parthaa
Copy link
Contributor

@parthaa parthaa commented Jul 21, 2014

After a content view is published, the package/errata counts of the
newly created content view version was not getting updated in the ui.
This commit aims to fix that by forcefully reloading that specific
version of the content view when the publish is completed.

versionIds = _.map($scope.contentView.versions, function(ver) {
return ver.id;
}),
versionIndex = _.indexOf(versionIds, version.id);
Copy link
Member

Choose a reason for hiding this comment

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

I think you could use the native versionIds.indexOf(version.id) here.

Copy link
Member

Choose a reason for hiding this comment

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

ContentViewVersion.get is an async operation. You'll want to put the update logic into a then function to ensure that new version has been loaded from the server properly.

After a content view is published, the package/errata counts of the
newly created content view version was not getting updated in the ui.
This commit aims to fix that by forcefully reloading that specific
version of the content view when the publish is completed.
@@ -106,6 +109,18 @@ angular.module('Bastion.content-views').controller('ContentViewDetailsController
.replace('%ver', version.version);
}

function updateVersion(version) {
Copy link
Member

Choose a reason for hiding this comment

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

I would change this to pass in the versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not as straight forward because there are 2 "versions" objects that need to be in sync .

                $scope.contentView.versions[versionIndex] = newVersion;
                $scope.versions[versionIndex] = newVersion;

Copy link
Member

Choose a reason for hiding this comment

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

Why are we managing two separate sets of the same information?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ehelms theres some code in content-views/deletion/content-view-deletion.controller.js that is relying on this versions variable to be set with code that looks like,

       if ($scope.versions === undefined) {
            $scope.reloadVersions();
        }

and other references. This PR may not be the right place to remove these references.. That being said I can look into it further if you want me to play with changing the versions references in all the sub controllers and testing em..

@jlsherrill
Copy link
Member

ACK from me, tested and worked great

@ehelms
Copy link
Member

ehelms commented Jul 25, 2014

ACK

parthaa added a commit that referenced this pull request Jul 28, 2014
Fixes #5858,bz 109961 - Updated cv published content count
@parthaa parthaa merged commit 817a383 into Katello:master Jul 28, 2014
@parthaa parthaa deleted the cv-publish-fix branch November 3, 2018 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants