Updates the handling of the create machine callback handlers to update the existing model instead of destroying and re-creating #504
Conversation
|
Test FAILed. |
…o the YUI source tree. Update the machine model id when the juju ACK comes in using the new lazy-model-list method updateModelId
hatched
force-pushed
the
update-ghost-models-1348163
branch
from
a7c3179
to
140b133
Compare
To QAThis will need to be QA'd under sandbox and a real env (LXC, EC2, etc).
Also do exploratory QA around things which create machines/containers to make sure they all still work. |
| @@ -1210,7 +1210,7 @@ YUI.add('juju-models', function(Y) { | |||
| @return {Object} The newly created model instance. | |||
| */ | |||
| addGhost: function(parentId, containerType, attrs) { | |||
| var obj = attrs ? Y.clone(attrs) : Object.create(null); | |||
| var obj = attrs ? Y.clone(attrs) : {}; | |||
There was a problem hiding this comment.
YUI uses the native object prototype methods when reviving a LML model to a Y.Model
|
Test PASSed. |
| @return {Object} The model object with the updated id. Or Throws if the | ||
| requested id is already in the id map. | ||
| */ | ||
| Y.LazyModelList.prototype.updateModelId = function(model, newId, revive) { |
There was a problem hiding this comment.
This is a totally new function, right, not overwriting anything?
Is there any argument for creating our own extension of the model list and extending that for service list &c?
There was a problem hiding this comment.
Totally new, correct.
The argument was that I didn't want to have to go and change everywhere in the app which uses it and then re-test the entire app :) This way anyone using LML will just get that functionality. Maybe if we end up adding more than a single method it'll make sense to do that, but atm I think it would be overkill.
There was a problem hiding this comment.
As per the discussion on IRC. The reason this is an issue in LML is that the LML has an internal ID map which is only updated on add and remove so you cannot update the id on an object within the LML and still have it work correctly.
|
So, most of this looks fine; I'm really not sold on monkey patching LazyModelList rather than just adding the updateId function to the existing db lists. Given its current usage you could actually add it to just the machine list, couldn't you? I'm willing to be convinced otherwise, but right now I don't see the logic in favor of globally patching YUI for this. |
|
Yes it could be added to the machine list, or anywhere really but imho it makes sense to put it on the lml as it's a limitation of the default functionality which is being patched by adding a method to fix said functionality. |
|
It seems like the container token(s) aren't being re-rendered after the ack comes back. The machine token switches from uncommitted to committed, but the container (bare metal) does not. To reproduce: Walk through the QA steps above. Once you complete them, the machine token will be as described, but the root container/bare metal container token will still have the blue uncommitted dot. I think it's just a rendering issue because if I deselect and reselect the machine (i.e., force the container token to re-render) the normal deployed state is used. |
|
After discussion on IRC, I'm good with the patch. Please add a test for it as we discussed. With that, 👍 |
…tead of destroying
hatched
force-pushed
the
update-ghost-models-1348163
branch
from
140b133
to
004b857
Compare
|
Thanks for the reviews. As discussed on IRC @kadams54 I'll be deferring the fix for that bug to an immediate follow-up as it was a pre-existing condition exposed by this work.
|
|
Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/juju-gui-merge |
|
Test PASSed. |
Updates the handling of the create machine callback handlers to update the existing model instead of destroying and re-creating Destroying and re-creating the machine model caused the tokens to disappear then re-appear when the deltas came in. Updating the models instead keeps them in the DOM. Created a new module called 'yui-patches' as a place to put monkey patches to the YUI codebase.
Destroying and re-creating the machine model caused the tokens to disappear then re-appear when the deltas came in. Updating the models instead keeps them in the DOM.
Created a new module called 'yui-patches' as a place to put monkey patches to the YUI codebase.