Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Fixed unplaced units dropped on existing containers. #334

Merged
merged 2 commits into from May 21, 2014

Conversation

hatched
Copy link
Contributor

@hatched hatched commented May 20, 2014

  1. Fixes a bug with ecs.placeUnit which would cause dropping an unplaced unit on an existing container to fail.
  2. Adds tests for ecs.placeUnit.

To QA

  • Open config-debug.js and disable simulateEvents.
  • Open app.js and add this.fakebackend = state after line 418
  • Load the GUI using the mv and il flags.
  • Drag and drop a charm to the canvas.
  • Switch to the machine view.
  • Drag and drop the unplaced unit to the New Machine header.
  • Click deploy and confirm.
  • Switch back to the service view.
  • Run app.db.machines.add({id: '0/lxc/0'}); app.fakebackend.db.machines.add({id: '0/lxc/0'}) in the console.
  • Drag and drop a new charm to the canvas.
  • Switch to the machine view.
  • Select the 0 machine. You should now see an empty container and 'Bare Metal' with your previous service.
  • Drag the unplaced unit and drop it on the empty container.
  • Click deploy and confirm.
  • Switch to service view and back to machine view. You should now have placed your new service on that container.

@jujugui
Copy link
Contributor

jujugui commented May 20, 2014

Test PASSed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/juju-gui/1055/

@huwshimi
Copy link
Member

QA OK.

@@ -604,6 +604,130 @@ describe('Environment Change Set', function() {
assert.equal(envObj._add_relation.callCount(), 0);
});
});

describe('placeUnit', function() {
it('throws if it can\'t find the unit being placed', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think cannot would read a bit nicer.

@mitechie
Copy link
Contributor

👍 on the code with one comment and running with Huw's QA.

@hatched
Copy link
Contributor Author

hatched commented May 21, 2014

Thanks for the reviews and qa!
:shipit:

@jujugui
Copy link
Contributor

jujugui commented May 21, 2014

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

jujugui added a commit that referenced this pull request May 21, 2014
1. Fixes a bug with `ecs.placeUnit` which would cause dropping an unplaced unit on an existing container to fail.
2. Adds tests for `ecs.placeUnit`.

#### To QA
- Open `config-debug.js` and disable `simulateEvents`.
- Open `app.js` and add `this.fakebackend = state` after line 418
- Load the GUI using the mv and il flags.
- Drag and drop a charm to the canvas. 
- Switch to the machine view.
- Drag and drop the unplaced unit to the `New Machine` header.
- Click `deploy` and `confirm`.
- Switch back to the service view.
- Run `app.db.machines.add({id: '0/lxc/0'}); app.fakebackend.db.machines.add({id: '0/lxc/0'})` in the console.
- Drag and drop a new charm to the canvas.
- Switch to the machine view.
- Select the `0` machine. You should now see an empty container and 'Bare Metal' with your previous service.
- Drag the unplaced unit and drop it on the empty container.
- Click `deploy` and `confirm`.
- Switch to service view and back to machine view. You should now have placed your new service on that container.
@jujugui jujugui merged commit 2aec332 into juju:develop May 21, 2014
@hatched hatched deleted the unplaced-to-existing branch May 22, 2014 17:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants