-
Notifications
You must be signed in to change notification settings - Fork 35
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
[MNOE-1280] Cleared instances cache to refresh apps & products #541
base: 2.0
Are you sure you want to change the base?
[MNOE-1280] Cleared instances cache to refresh apps & products #541
Conversation
RidaAhmad
commented
Aug 28, 2018
•
edited
Loading
edited
- Emptied out apps & products instances' cache upon subscription's validation so the instances are fetched again and hence the updated apps are displayed on dock & app management page.
- Removed redundant reloading of app instances
@@ -77,6 +82,7 @@ angular.module 'mnoEnterpriseAngular' | |||
MnoeProvisioning.refreshCartSubscriptions() | |||
$state.go("home.subscriptions", {subType: 'cart'}) | |||
else | |||
clearInstancesCache() | |||
# Reload dock apps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MAhsenArif Please if you can confirm if we're good to remove this reload dock apps block of code as it no longer seems to be required now, will appreciate your input on this so you can confirm if it ain’t needed for cart or any other related functionalities.
Thank you,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RidaAhmad We still need this for the impac dashboard(if it is enabled) as this code is responsible for reloading the dock apps.
It isnt, however related to the cart subscriptions reloading as that is a separate condition branch(the if
part of this code).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RidaAhmad On a deeper look, it seems that since we have a watcher added in the app docks component, it does do the appInstance cache refresh on it's own. This watcher will always get triggered because the app dock component is completely removed from DOM and re-added whenever we switch to and fro from Impac Dasboard.
The same behaviour is happening for the apps dashboard(tile view)
So it should be safe to remove the dock reload from here. and just keep the clearInstancesCache()
method.
FYI @adamaziz15 @ouranos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MAhsenArif Yeah, seems to work fine without having to reload dock apps from here.
We can remove it for now and revert in case of any issue later.
Thank you for your input.
303bfbc
to
3ee96a3
Compare
@@ -1,5 +1,5 @@ | |||
angular.module 'mnoEnterpriseAngular' | |||
.controller('ProvisioningConfirmCtrl', ($scope, $state, $stateParams, $log, MnoeOrganizations, MnoeProvisioning, MnoeAppInstances, MnoeConfig, ProvisioningHelper, schemaForm, toastr) -> | |||
.controller('ProvisioningConfirmCtrl', ($scope, $state, $stateParams, $log, MnoeOrganizations, MnoeProvisioning, MnoeAppInstances, MnoeConfig, ProvisioningHelper, schemaForm, toastr, MnoeProductInstances) -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line size increased. Might need to adjust indentation of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feedback incorporated, thanks.