Merge lp://qastaging/~ricgard/maas/edit-and-save-subnet-details into lp://qastaging/~maas-committers/maas/trunk

Proposed by Richard McCartney
Status: Merged
Approved by: Richard McCartney
Approved revision: no longer in the source branch.
Merged at revision: 6097
Proposed branch: lp://qastaging/~ricgard/maas/edit-and-save-subnet-details
Merge into: lp://qastaging/~maas-committers/maas/trunk
Diff against target: 220 lines (+114/-35)
3 files modified
src/maasserver/static/js/angular/controllers/subnet_details.js (+22/-0)
src/maasserver/static/js/angular/controllers/tests/test_subnet_details.js (+20/-0)
src/maasserver/static/partials/subnet-details.html (+72/-35)
To merge this branch: bzr merge lp://qastaging/~ricgard/maas/edit-and-save-subnet-details
Reviewer Review Type Date Requested Status
Mike Pontillo (community) Approve
Richard McCartney (community) Needs Resubmitting
Andres Rodriguez (community) Needs Information
Blake Rouse Pending
Review via email: mp+324874@code.qastaging.launchpad.net

Commit message

Remove auto save on blur for the Subnet details summary row. Applied static content when not in edit mode.

Description of the change

- Removes the auto saving on blur for the Subnet details summary row.
- Applied and updates the correct static content for the subnet when not in edit mode

---

TO TEST

- Visit a subnet details page, amend any content and save. Content should have updated accordingly.

To post a comment you must log in.
Revision history for this message
Andres Rodriguez (andreserl) wrote :

What about validation and surfacing errors? I would imagine that these forms do not have validation in the backend atm.

review: Needs Information
Revision history for this message
Richard McCartney (ricgard) wrote :

> What about validation and surfacing errors? I would imagine that these forms
> do not have validation in the backend atm.

Hi Andres,

the maas-obj-form already has taken care of the forms and validation created by team in the original development of this page. If you were to enter in incorrect information / remove content from a require field you'll still get the same error handling.

Rich

Revision history for this message
Mike Pontillo (mpontillo) wrote :

This seems to break validation.

If I type in an invalid CIDR, I used to see "invalid IPNetwork" in red text underneath the CIDR field. (I just re-tested to be sure it wasn't broken.)

With this branch, I no longer see the validation errors.

review: Needs Fixing
Revision history for this message
Richard McCartney (ricgard) wrote :

> This seems to break validation.
>
> If I type in an invalid CIDR, I used to see "invalid IPNetwork" in red text
> underneath the CIDR field. (I just re-tested to be sure it wasn't broken.)
>
> With this branch, I no longer see the validation errors.

Hi Mike,

So after talking to Blake, I've restored the maas-obj-errors back on the form. As certain inputs such as the CIDR don't follow the usual error handling. Blake informed me this is more of a backend issue.

review: Needs Resubmitting
Revision history for this message
Mike Pontillo (mpontillo) wrote :

Just one minor issue below. (The text "Manage" should be "Managed".) I'll give you an approval vote so you can just fix that and land it. See my miniature rant inline below.

We normally wait to land branches on 2.2 until the trunk branches merge, so I won't look at the 2.2 branch just yet. (Is it significantly different, or just a trivial port?)

Overall, this is looking great; thanks for doing this work!

review: Approve
Revision history for this message
Mike Pontillo (mpontillo) wrote :

Never mind regarding my comment about the 2.2 branch, I didn't realize that was a completely separate fix.

Revision history for this message
MAAS Lander (maas-lander) wrote :
Download full text (2.1 MiB)

The attempt to merge lp:~ricgard/maas/edit-and-save-subnet-details into lp:maas failed. Below is the output from the failed tests.

Get:1 http://security.ubuntu.com/ubuntu xenial-security InRelease [102 kB]
Hit:2 http://archive.ubuntu.com/ubuntu xenial InRelease
Get:3 http://archive.ubuntu.com/ubuntu xenial-updates InRelease [102 kB]
Get:4 http://archive.ubuntu.com/ubuntu xenial-backports InRelease [102 kB]
Get:5 http://archive.ubuntu.com/ubuntu xenial-updates/main amd64 Packages [558 kB]
Get:6 http://archive.ubuntu.com/ubuntu xenial-updates/universe amd64 Packages [486 kB]
Fetched 1,350 kB in 2s (604 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind avahi-utils bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common isc-dhcp-server libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libnss-wrapper libpq-dev make nodejs-legacy npm postgresql psmisc pxelinux python3-all python3-apt python3-attr python3-bson python3-convoy python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-netaddr python3-netifaces python3-novaclient python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
authbind is already the newest version (2.1.1+nmu1).
avahi-utils is already the newest version (0.6.32~rc+dfsg-1ubuntu2).
build-essential is already the newest version (12.1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the newest version (9.5+173).
psmisc is already the newest version (22.21-2.1build1).
pxelinux is already the newest version (3:6.03+dfsg-11ubuntu1).
python-formencode is already the newest version (1.3.0-0ubuntu5).
python-lxml is already the newest version (3.5.0-1build1).
python-netaddr i...

Revision history for this message
MAAS Lander (maas-lander) wrote :
Download full text (2.0 MiB)

The attempt to merge lp:~ricgard/maas/edit-and-save-subnet-details into lp:maas failed. Below is the output from the failed tests.

Get:1 http://security.ubuntu.com/ubuntu xenial-security InRelease [102 kB]
Hit:2 http://archive.ubuntu.com/ubuntu xenial InRelease
Get:3 http://archive.ubuntu.com/ubuntu xenial-updates InRelease [102 kB]
Get:4 http://archive.ubuntu.com/ubuntu xenial-backports InRelease [102 kB]
Get:5 http://archive.ubuntu.com/ubuntu xenial-updates/main amd64 Packages [565 kB]
Get:6 http://archive.ubuntu.com/ubuntu xenial-updates/universe amd64 Packages [488 kB]
Fetched 1,359 kB in 2s (533 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind avahi-utils bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common isc-dhcp-server libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libnss-wrapper libpq-dev make nodejs-legacy npm postgresql psmisc pxelinux python3-all python3-apt python3-attr python3-bson python3-convoy python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-netaddr python3-netifaces python3-novaclient python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
authbind is already the newest version (2.1.1+nmu1).
avahi-utils is already the newest version (0.6.32~rc+dfsg-1ubuntu2).
build-essential is already the newest version (12.1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the newest version (9.5+173).
psmisc is already the newest version (22.21-2.1build1).
pxelinux is already the newest version (3:6.03+dfsg-11ubuntu1).
python-formencode is already the newest version (1.3.0-0ubuntu5).
python-lxml is already the newest version (3.5.0-1build1).
python-netaddr i...

Revision history for this message
Mike Pontillo (mpontillo) wrote :

Ah, at first I thought this was a lander issue, but it looks like the JavaScript needs to be double-checked:

======================================================================
ERROR: bin/test.js
----------------------------------------------------------------------
log: {{{
20 06 2017 22:24:48.354:INFO [karma]: Karma v0.13.19 server started at http://localhost:9876/
20 06 2017 22:24:48.372:INFO [launcher]: Starting browser PhantomJS
20 06 2017 22:24:48.386:INFO [launcher]: Starting browser Chrome
20 06 2017 22:24:48.648:INFO [PhantomJS 2.1.1 (Linux 0.0.0)]: Connected on socket _lFztJE5OZp8X3UEAAAA with id 2011828
20 06 2017 22:24:52.407:INFO [Chromium 58.0.3029 (Ubuntu 0.0.0)]: Connected on socket AMNDjPIGxejdTWA8AAAB with id 73392963
PhantomJS 2.1.1 (Linux 0.0.0) LOG: undefined
Chromium 58.0.3029 (Ubuntu 0.0.0) LOG: undefined

PhantomJS 2.1.1 (Linux 0.0.0) failed specs:
 NodeStorageController > availableConfirmEdit > resets name to original if empty
        Expected undefined to be 'name_aOwsbTO0eR'.
        /tmp/tarmac/branch.Ql9goU/src/maasserver/static/js/angular/controllers/tests/test_node_details_storage.js:3054:35

Chromium 58.0.3029 (Ubuntu 0.0.0) failed specs:
 NodeStorageController > availableConfirmEdit > resets name to original if empty
        Expected undefined to be 'name_rpNgEZMNLr'.
            at Object.<anonymous> (/tmp/tarmac/branch.Ql9goU/src/maasserver/static/js/angular/controllers/tests/test_node_details_storage.js:3054:31)

Coverage.py warning: No data was collected.
}}}

Revision history for this message
Richard McCartney (ricgard) wrote :

> Ah, at first I thought this was a lander issue, but it looks like the
> JavaScript needs to be double-checked:
>
> ======================================================================
> ERROR: bin/test.js
> ----------------------------------------------------------------------
> log: {{{
> 20 06 2017 22:24:48.354:INFO [karma]: Karma v0.13.19 server started at
> http://localhost:9876/
> 20 06 2017 22:24:48.372:INFO [launcher]: Starting browser PhantomJS
> 20 06 2017 22:24:48.386:INFO [launcher]: Starting browser Chrome
> 20 06 2017 22:24:48.648:INFO [PhantomJS 2.1.1 (Linux 0.0.0)]: Connected on
> socket _lFztJE5OZp8X3UEAAAA with id 2011828
> 20 06 2017 22:24:52.407:INFO [Chromium 58.0.3029 (Ubuntu 0.0.0)]: Connected on
> socket AMNDjPIGxejdTWA8AAAB with id 73392963
> PhantomJS 2.1.1 (Linux 0.0.0) LOG: undefined
> Chromium 58.0.3029 (Ubuntu 0.0.0) LOG: undefined
>
> PhantomJS 2.1.1 (Linux 0.0.0) failed specs:
>  NodeStorageController > availableConfirmEdit >
> resets name to original if empty
> Expected undefined to be 'name_aOwsbTO0eR'.
> /tmp/tarmac/branch.Ql9goU/src/maasserver/static/js/angular/controllers
> /tests/test_node_details_storage.js:3054:35
>
>
>
> Chromium 58.0.3029 (Ubuntu 0.0.0) failed specs:
>  NodeStorageController > availableConfirmEdit >
> resets name to original if empty
> Expected undefined to be 'name_rpNgEZMNLr'.
> at Object.<anonymous> (/tmp/tarmac/branch.Ql9goU/src/maasserver/st
> atic/js/angular/controllers/tests/test_node_details_storage.js:3054:31)
>
>
>
> Coverage.py warning: No data was collected.
> }}}

This is a very strange issue considering I haven't touched the node storage part of the application, I'll investigate.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.