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

[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