Re: [Freeipa-devel] [PATCH] 670-675 webui: dns forward zones

2014-06-27 Thread Petr Vobornik

On 27.6.2014 06:34, Endi Sukma Dewata wrote:

On 6/24/2014 9:39 AM, Petr Vobornik wrote:

On 24.6.2014 13:02, Petr Vobornik wrote:

ticket: https://fedorahosted.org/freeipa/ticket/4357

- patch 673 is compressed
- CI patches functionally depends on #667, #668

== PATCH] 670 webui: add confirmation for dns zone permission actions ==
All header actions should require confirmation.

== [PATCH] 671 webui: dns forward zones ==
Add DNS Forward Zones Web UI.

- pages under: Identity/DNS/DNS Forward Zones

== [PATCH] 672 webui-ci: dns forward zone tests ==
Selenium CI sanity tests for DNS Forward Zones

== [PATCH] 673 webui-test: static metadata update ==
Regular update of static metadata for testing and presentation purposes.
It should also contain new DNS Forward Zones metadata.

== [PATCH] 674 webui-test: dns forward zone json data ==
Fake API results for testing and presentation purposes of DNS Forward
Zones.

== [PATCH] 675 webui: fix detection of RPC command ==
old detection did not work with the static version used for test and
demonstration purposes.


Attaching an updated version of #675 with a fix for unit tests.


ACK. Some comments below.


Pushed to master:
* 8ca5793160cf24268c405e51b0bb8ce267608b6b webui: add confirmation for 
dns zone permission actions

* 7a25168a3ceb8677a01b3bce6d2c10ac52d4d584 webui: dns forward zones
* c7c13965e3604ea3f51684875ba252cc2d8bfbf1 webui-ci: dns forward zone tests
* db2666d276ae60f2c3f1f5f1eaec87e55e32bf81 webui-test: static metadata 
update
* e6a373e930ae483bc2eb384da0bba5f2b4b8f6c2 webui-test: dns forward zone 
json data
* 59f66a156be393dba92ab6ca999cdc39c6787c36 webui: fix detection of RPC 
command






Btw I'm
not very satisfied with patch #675's approach. I'm open to suggestions
for better approaches.


How about adding another parameter to get_record() to indicate the type
of the data?

Possible improvements:

1. In the Add DNS Forward Zone dialog, if the Zone forwarders is empty
and you click Add, there is no error message.


new ticket: https://fedorahosted.org/freeipa/ticket/4406



2. In the same dialog, by default there probably should be an empty
field to enter the Zone forwarders because it's required. The admin
can click Add to add additional forwarders.


my patch #681 fixes it



3. The permission name is only displayed briefly after creation. It
would be nice to display the permission name or a link to it in the
details page.


Interesting idea. The same applies for Master DNS Zones.



4. Unrelated. Should undo and undo all be capitalized? They seem to
be inconsistent with other buttons.



Fixed in new patch #692


--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 670-675 webui: dns forward zones

2014-06-26 Thread Endi Sukma Dewata

On 6/24/2014 9:39 AM, Petr Vobornik wrote:

On 24.6.2014 13:02, Petr Vobornik wrote:

ticket: https://fedorahosted.org/freeipa/ticket/4357

- patch 673 is compressed
- CI patches functionally depends on #667, #668

== PATCH] 670 webui: add confirmation for dns zone permission actions ==
All header actions should require confirmation.

== [PATCH] 671 webui: dns forward zones ==
Add DNS Forward Zones Web UI.

- pages under: Identity/DNS/DNS Forward Zones

== [PATCH] 672 webui-ci: dns forward zone tests ==
Selenium CI sanity tests for DNS Forward Zones

== [PATCH] 673 webui-test: static metadata update ==
Regular update of static metadata for testing and presentation purposes.
It should also contain new DNS Forward Zones metadata.

== [PATCH] 674 webui-test: dns forward zone json data ==
Fake API results for testing and presentation purposes of DNS Forward
Zones.

== [PATCH] 675 webui: fix detection of RPC command ==
old detection did not work with the static version used for test and
demonstration purposes.


Attaching an updated version of #675 with a fix for unit tests.


ACK. Some comments below.


Btw I'm
not very satisfied with patch #675's approach. I'm open to suggestions
for better approaches.


How about adding another parameter to get_record() to indicate the type 
of the data?


Possible improvements:

1. In the Add DNS Forward Zone dialog, if the Zone forwarders is empty 
and you click Add, there is no error message.


2. In the same dialog, by default there probably should be an empty 
field to enter the Zone forwarders because it's required. The admin 
can click Add to add additional forwarders.


3. The permission name is only displayed briefly after creation. It 
would be nice to display the permission name or a link to it in the 
details page.


4. Unrelated. Should undo and undo all be capitalized? They seem to 
be inconsistent with other buttons.


--
Endi S. Dewata

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 670-675 webui: dns forward zones

2014-06-24 Thread Petr Vobornik

On 24.6.2014 13:02, Petr Vobornik wrote:

ticket: https://fedorahosted.org/freeipa/ticket/4357

- patch 673 is compressed
- CI patches functionally depends on #667, #668

== PATCH] 670 webui: add confirmation for dns zone permission actions ==
All header actions should require confirmation.

== [PATCH] 671 webui: dns forward zones ==
Add DNS Forward Zones Web UI.

- pages under: Identity/DNS/DNS Forward Zones

== [PATCH] 672 webui-ci: dns forward zone tests ==
Selenium CI sanity tests for DNS Forward Zones

== [PATCH] 673 webui-test: static metadata update ==
Regular update of static metadata for testing and presentation purposes.
It should also contain new DNS Forward Zones metadata.

== [PATCH] 674 webui-test: dns forward zone json data ==
Fake API results for testing and presentation purposes of DNS Forward
Zones.

== [PATCH] 675 webui: fix detection of RPC command ==
old detection did not work with the static version used for test and
demonstration purposes.



Attaching an updated version of #675 with a fix for unit tests. Btw I'm 
not very satisfied with patch #675's approach. I'm open to suggestions 
for better approaches.

--
Petr Vobornik
From d39c27e9aeb85bdae0f10a3bf16a5d8aca42b4ff Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 18 Jun 2014 18:59:44 +0200
Subject: [PATCH] webui: fix detection of RPC command

old detection did not work with the static version used for test and
demonstration purposes.

https://fedorahosted.org/freeipa/ticket/4357
---
 install/ui/src/freeipa/field.js |  4 ++--
 install/ui/test/aci_tests.js| 16 
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/install/ui/src/freeipa/field.js b/install/ui/src/freeipa/field.js
index 1ae3ddf3ee98493426a1776f8a315206c5a0f8f6..0bc8c6f5eb633463ca829b0e46a0fbc0ffacefd7 100644
--- a/install/ui/src/freeipa/field.js
+++ b/install/ui/src/freeipa/field.js
@@ -775,8 +775,8 @@ field.Adapter = declare(null, {
 get_record: function(data) {
 
 // detection if it's result or raw RPC command response
-// all raw responses should contain `version` and `principal`
-if (!data.version || !data.principal) {
+// each RPC response should define properties as follows
+if (data.id === undefined || data.result === undefined || data.error === undefined) {
 return data;
 }
 
diff --git a/install/ui/test/aci_tests.js b/install/ui/test/aci_tests.js
index e82dd86df1f61ad98413492d687d14d4c73a9e79..1cfc2fb4fa059b464928a2c5cc25972e1d19c907 100644
--- a/install/ui/test/aci_tests.js
+++ b/install/ui/test/aci_tests.js
@@ -229,10 +229,10 @@ var get_visible_rows = function(section) {
 
 
 test(Testing type target., function() {
-var data = {};
-data.result = {};
-data.result.result = {
-type: 'hostgroup'
+var data = {
+id: null,
+error: null,
+result: { result: { type: 'hostgroup'} }
 };
 
 target_facet.load(data);
@@ -266,10 +266,10 @@ test(Testing type target., function() {
 
 test(Testing general target., function() {
 
-var data = {};
-data.result = {};
-data.result.result = {
-extratargetfilter: 'somevalue'
+var data = {
+id: null,
+error: null,
+result: { result: { extratargetfilter: 'hostgroup'} }
 };
 
 target_facet.load(data);
-- 
1.9.0

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel