[Freeipa-devel] [PATCH] 070 Modifying DNS UI to benefit from new DNS API

[Endi Sukma Dewata]

On 1/27/2012 8:51 AM, Petr Vobornik wrote:
> Updated patch attached.

ACK and pushed to master and ipa-2-2. See also the comments below.

>> 5. The following attributes probably should be shown as text areas in
>> the edit dialog and should not be displayed in the table because it
>> could be too long:
>> * cert_part_certificate_or_crl
>> * ds_part_digest
>> * key_part_public_key
>> * sig_part_signature
>> * rrsig_part_signature
>> * sshfp_part_fingerprint
>
> Fixed.

The sig_part_signature and rrsig_part_signature actually still appear in 
the table since we're showing the raw data but I think we can deal with 
it later once we decide which columns to show in the table.

Another possibility, instead of hiding these fields we can also show 
just the first few characters.

>> 7. The record type header (e.g. A record) on the left of the table
>> probably can be moved above the table to allow wider table. For
>> comparison, in HBAC/sudo rule details page the tables occupy the entire
>> width of the page.
>
> Maybe. I will wait for Kyle's review (WWKR). In this implementation the
> table is too narrow, if we moved the headers and extend the table the
> space for one type will be too height a user will have to scroll a lot.
> Both are not ideal.

OK'd by Kyle in ticket #2208. Let's leave it as is for now.

One thing though, in HBAC/sudo rules the Add/Delete buttons occupy the 
same box as the last column header. In the DNS records page the buttons 
occupy a separate box. This can be fixed later.

>> 9. The Edit links in the tables could be replaced with icons. Another
>> option is to convert the record values into links which will open the
>> dialog.
>
> I agree. WWKR

Kyle said links are OK for now.

>>> 2) hide empty tables (as I suggested in ticket description)
>>
>> OK. We probably can move the Add and Delete buttons next to the type
>> header and keep it there even when the table is visible.
>
> WWKR

Same reason, leaving it as is.

Additional improvements:

13. In the details page the title of the Add/Delete/Edit dialogs could 
be modified to mention the record type.

14. The IPA.dns.record_metadata is lazily initialized in 
get_record_metadata(). I suppose this is because the validators are 
dependent on the metadata loaded from the server. However, being a 
standalone function, it doesn't prevent someone from calling 
get_record_metadata() too early and still get an error.

We might be able to address this by moving the initialization into 
IPA.dns.record_entity.init() because it's guaranteed to be called after 
entity creation which will only happen after loading the metadata.

15. Currently when calling IPA.dns.get_record_type() you'd have to 
append the 'record' manually. The function can be modified to take the 
record type only, then internally append the 'record' prefix to find the 
metadata for that type.

16. Existing issue: some parameters in the specs (e.g. validators, 
policies, formats) are objects instead of the class/factory. For example:

     factory: IPA.dnszone_adder_dialog,
     policies: [
         IPA.dnsrecord_adder_dialog_type_policy({
             type_field: 'record_type'
         })
     ]

Compare it with:

     factory: IPA.dnszone_adder_dialog,
     policies: [
         {
             factory: IPA.dnsrecord_adder_dialog_type_policy,
             type_field: 'record_type'
         }
     ]

This is not an issue now, but if we want to make the spec more 
declarative (see ticket #2052) I think we should avoid executing a code 
in order to construct the spec. We also still need to figure out how to 
handle messages and dynamic specs properly (see user.js:35-36).

Suppose we have a fully declarative spec we can move the spec out of the 
entity class (and maybe into a separate json file) to simplify UI 
customization, but I'm still not sure if it's possible due to the 
dynamic nature of the UI. Suppose it's not possible, we might as well 
create the objects directly instead of using 2 stages (spec -> object):

     that.adder_dialog(IPA.dnszone_adder_dialog({
         policies: [
             IPA.dnsrecord_adder_dialog_type_policy({
                 type_field: 'record_type'
             })
         ]
     });

17. The code in dns.js:1751,1759 could be replaced with callback 
functions supplied by the facet.

     var widget = ... get the table widget ...
     widget.remove_on_success(function(data, text_status, xhr) {
         that.load(data); // load facet
     });
     widget.remove_on_error(function(xhr, text_status, error_thrown) {
         that.refresh(); // load facet
     });

-- 
Endi S. Dewata