[Freeipa-devel] [PATCH] 080-085 DNS UI update

[Endi Sukma Dewata]

On 2/15/2012 9:58 AM, Petr Vobornik wrote:
> Tickets: #2349 #2350 #2351 #2367
>
> Attached patches reflect recent dns plugin changes:
>
> * DNS Zone UI: added new attributes
> * DNS UI: added A,AAAA create reverse options to adder dialog
> * Fixed displaying of A6 Record
> * New UI for DNS global configuration
>
> Two prerequisites:
>
> * Static metadata update - new DNS options
> * New checkboxes option: Mutual exclusive
>
> If acked, should be pushed after Martin's 195-199, 202

I tested this on the 2.2 branch with Martin's patches (rebased). Some 
issues that needs fixing:

1. The server accepts IP addresses without masks for idnsallowquery and 
idnsallowtransfer. The UI doesn't accept IP addresses without masks. If 
you add the IP addresses without masks via CLI, it will appear as 
invalid in UI.

2. The deleting all values of idnsallowquery and idnsallowtransfer 
doesn't work. When there's no values left for these attributes the UI 
doesn't send the modify operation.

3. When adding an A/AAAA record and checking the 'create reverse' 
option, if the reverse zone doesn't exist it will show an error dialog 
box saying it cannot create the reverse record. The A/AAAA record is 
actually created but the page is not refreshed. I think it should detect 
the error 4304 and refresh the page.

4. In #3 the error dialog shows a Retry button but if you click it it 
will say 'no modifications to be performed'. This is because the A/AAAA 
record is already added. I think we can use the message dialog that only 
has an OK button.

5. When you click Add to add the second value in the new multi-valued 
fields (allow query, allow transfer, zone forwarders, global 
forwarders), it will show an error message immediately although it's 
still empty. I think we should either not do the validation if it's a 
new and empty, or change the validation to accept empty values.

Possible enhancements:

6. The server doesn't support 'localhost' and 'localnets' for 
idnsallowquery and idnsallowtransfer. Right now the UI allows these 
values, but they will be rejected by the server. We probably can reject 
these values in the UI too saying it's unsupported (instead of invalid).

7. Not being a DNS expert, I'm not sure if it makes sense to allow 'any' 
and 'none' together, or in combination with IP addresses/networks. The 
server currently allows it, but suppose later we change that, the UI 
probably should show a combination of radio buttons and multivalued text 
fields or table like this:

  ( ) None
  ( ) Any
  (*) IP Addresses/Networks
      Allow                 [Add] [Delete]
      [ ] 192.168.1.10

      Deny                  [Add] [Delete]
      [ ] 192.168.1.1/24

8. The behavior of the checkboxes for idnsforwardpolicy is a bit unusual 
because you can only select at most one value. Usually checkboxes allow 
you to select multiple values. It might be better to use 3 radio buttons 
for all possible values: only, first, and none/default. BTW, the CLI 
docs doesn't really mention that empty value is possible.

7. We should plan to separate the entity from the navigation because the 
navigation map should not be dependent on the entities. Later we might 
have pages that do not correspond to any entities (e.g. login/logout 
pages). Also entities should not need to be defined in the navigation 
map if it's not part of the map. I think the 'hidden' and the 'depth' 
parameters are temporary workarounds for these issues.

-- 
Endi S. Dewata