1553266: When d-bus methods are unavailable, show appropriate message (ENT-576) #1836
Conversation
nikosmoum
commented
Jun 12, 2018
nikosmoum
commented
- Added a "safe call" mechanism that makes the initial dbus calls (entitlementService, configService, productsService) only if the service is available, tries to restart the rhsm service if possible, and otherwise failing gracefully.
- Added new UI curtain that provides a meaningful message and advice to the end user.
- Re-added utility method statusUpdateFailed that was accidentally deleted.
cockpit/src/subscriptions-client.js
Outdated
| /* Utility function to check if an object has a specific property */ | ||
| function hasProperty(object, property) { | ||
| return Object.prototype.hasOwnProperty.call(object, property); | ||
| } |
There was a problem hiding this comment.
Is it really necessary to have special function for this testing? This function is used only on one place in the code.
cockpit/src/subscriptions-client.js
Outdated
| }); | ||
| } | ||
|
|
||
| function callProductsService() { |
There was a problem hiding this comment.
Why do you create special function for this? Is this reason any unit testing in the future? IMHO you needn't to create special function, because you can do it in this way:
safeDBusCall(productsService, 'ListInstalledProducts', () => {
productsService.ListInstalledProducts('', {}, userLang) // FIXME: use proxy settings
// ...
});There was a problem hiding this comment.
Done. It is because of the Java developer in me, that isn't used to how anonymous high-order functions look 😄
cockpit/src/subscriptions-client.js
Outdated
| getSubscriptionDetails(); | ||
| needRender(); | ||
| }); | ||
| safeDBusCall(entitlementService, 'GetStatus', function () { |
There was a problem hiding this comment.
I would use arrow function here too () => {};. It will not be necessary to do explicit binding of this. You will be able to use this instead of that in following code.
cockpit/src/subscriptions-client.js
Outdated
| client.readConfig = () => { | ||
| return configService.wait(() => configService.GetAll(userLang).then(config => { | ||
| return configService.wait(() => { | ||
| safeDBusCall(configService, 'GetAll', callConfigService); |
There was a problem hiding this comment.
Consider using arrow function here too.
cockpit/src/subscriptions-client.js
Outdated
| * @param delegateMethod the method that we delegate the actual call to dbus | ||
| */ | ||
| function safeDBusCall(serviceProxy, methodName, delegateMethod) { | ||
| if (!hasProperty(serviceProxy, methodName)) { |
There was a problem hiding this comment.
You can simplify the code to use something like this:
if (!Object.prototype.hasOwnProperty.call(serviceProxy, methodName)) {
* Added a "safe call" mechanism that makes the initial dbus calls (entitlementService, configService, productsService) only if the service is available, tries to restart the rhsm service if possible, and otherwise failing gracefully. * Added new UI curtain that provides a meaningful message and advice to the end user. * Re-added utility method statusUpdateFailed that was accidentally deleted.
nikosmoum
force-pushed
the
nikos/m/1553266
branch
from
71b2470
to
cdf0357
Compare
