/juju-gui

Show local charm icon in inspector #698

Merged
merged 4 commits into from Mar 6, 2015

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@hatched @jujugui @makyo @huwshimi
@hatched
Owner

hatched commented Mar 4, 2015

Bug: https://bugs.launchpad.net/juju-gui/+bug/1427162

Fixes:

  • Local charms icons are not shown in their inspectors.
  • Opening the charm details pane of a local charm would throw error.
  • Opening and closing the charm details pane 4x would cause the defailt pane to stop opening.
@hatched
Owner

hatched commented Mar 4, 2015

To QA

Because the juju-gui-source has a bug in which it breaks if you try and use a repository other than the default one you will have to manually create a 'local charm' of the GUI.

  • Get a fresh check out of the GUI charm (follow the release notes).
  • Make a tarball of the GUI source (follow the release notes).
  • Copy the tarball into the GUI charm and create a zip of the local charm (follow the release notes).
  • Deploy this charm to a trusty environment.
  • Deploy another non-local charm to the environment.
  • Open the GUI and click on the inspector for the GUI service, the icon should show in the inspector.
  • Open the charm details pane by clicking on the charm details link.
  • It should open and close as normal
  • Open and close this at least 5x,
  • Repeat for the non-local charm

Note:
There is a known issue where the local charm details won't be loaded if you try and visit that url directly ie) /inspector/foo/charm . This is caused by a race condition that's deep in the startup procedure of the GUI.
app/views/viewlets/charm-details.js
- panel.removeClass('animate-in');
- container.hide();
-
- store.charm(charm.get('storeId'), {
@hatched

hatched Mar 4, 2015

Owner

This is not necessary because the BrowserCharmView handles fetching the data if it's not passed in.

test/test_inspector_charm.js
@@ -210,7 +211,8 @@ describe('Inspector Charm', function() {
assert.equal(hideSlot.calledOnce(), true);
assert.equal(Y.one('.charmbrowser').hasClass('animate-in'), false);
assert.equal(view.get('container').getHTML(), '');
-
+ assert.equal(destructor.callCount(), 1);
+ destructor.passThroughToOriginalMethod.call(view);
@hatched

hatched Mar 4, 2015

Owner

There are minor test changes because the existing tests only tested the code which still remains.

@jujugui
Member

jujugui commented Mar 4, 2015

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
http://ci.jujugui.org:8080//job/juju-gui/2383/
Test FAILed.
@hatched
Owner

hatched commented Mar 4, 2015

Spurious failure
@makyo
Member

makyo commented Mar 4, 2015

QA okay.

  • BUILD_IS_GOOD=1 make release in gui dir
  • copy GUI to a charm release dir
  • JUJU_ENV=local make deploy in charm dir
  • Deployed NTP.
  • Played around with the inspector of both services a bunch
@makyo
Member

makyo commented Mar 4, 2015

👍 on code, too
@huwshimi
Member

huwshimi commented Mar 5, 2015

Looks good, thanks @hatched 👍 No QA.
@hatched
Owner

hatched commented Mar 6, 2015

:shipit:
@jujugui
Member

jujugui commented Mar 6, 2015

Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/juju-gui-merge

jujugui added a commit that referenced this pull request Mar 6, 2015

@jujugui
Merge pull request #698 from hatched/charm-details-apiv4 
Show local charm icon in inspector

Bug: https://bugs.launchpad.net/juju-gui/+bug/1427162

Fixes:
- Local charms icons are not shown in their inspectors. 
- Opening the charm details pane of a local charm would throw error.
- Opening and closing the charm details pane 4x would cause the defailt pane to stop opening.
0495ee6

@jujugui jujugui merged commit 0495ee6 into juju:develop Mar 6, 2015
1 check failed

1 check failed

default Merged build finished.
Details

@hatched hatched deleted the hatched:charm-details-apiv4 branch Mar 9, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment