This patch is build on top of my DNS patches 218-220 ---- Add 2 new features to DNS record interactive help to increase its usability and also make its behavior more consistent with standard parameter interactive help:
1) Ask for missing DNS parts When a required part of a newly added DNS record was missing, we just returned a ValidationError. Now, the interactive help rather asks for all missing required parts of all DNS records that were being added by its parts. 2) Let user amend invalid part When an interactive help asked for a DNS record part value and user enters an invalid value, the entire interactive help exits with an error. This may upset a user if he already entered several correct DNS record part values. Now, the help rather tells user what's wrong and give him an opportunity to amend the value. https://fedorahosted.org/freeipa/ticket/2386 ------------- A demonstration of the new features: # ipa dnsrecord-add example.com foo --mx-exchanger=mx.example.com. MX Preference: 0 <<< we don't fail now Record name: foo MX record: 0 mx.example.com. # ipa dnsrecord-add example.com foo Please choose a type of DNS resource record to be added The most common types for this type of zone are: A, AAAA DNS resource record type: LOC LOC Degrees Latitude: 1 [LOC Minutes Latitude]: 1000 <<< we don't fail with invalid values! >>> LOC Minutes Latitude: can be at most 59 [LOC Minutes Latitude]: 50 [LOC Seconds Latitude]: LOC Direction Latitude: E >>> LOC Direction Latitude: must be one of (u'N', u'S') LOC Direction Latitude: N LOC Degrees Longtitude: 2 [LOC Minutes Longtitude]: [LOC Seconds Longtitude]: LOC Direction Longtitude: E LOC Altitude: 123 [LOC Size]: [LOC Horizontal Precision]: [LOC Vertical Precision]: Record name: foo LOC record: 1 50 N 2 E 123.00 MX record: 0 mx.example.com.
>From 2373c2e30b99ee3f344bc9ed2daba6b90373bc0c Mon Sep 17 00:00:00 2001 From: Martin Kosek <mko...@redhat.com> Date: Tue, 28 Feb 2012 15:04:05 +0100 Subject: [PATCH] Improve dnsrecord interactive help Add 2 new features to DNS record interactive help to increase its usability and also make its behavior more consistent with standard parameter interactive help: 1) Ask for missing DNS parts When a required part of a newly added DNS record was missing, we just returned a ValidationError. Now, the interactive help rather asks for all missing required parts of all DNS records that were being added by its parts. 2) Let user amend invalid part When an interactive help asked for a DNS record part value and user enters an invalid value, the entire interactive help exits with an error. This may upset a user if he already entered several correct DNS record part values. Now, the help rather tells user what's wrong and give him an opportunity to amend the value. https://fedorahosted.org/freeipa/ticket/2386 --- ipalib/cli.py | 6 ++- ipalib/plugins/dns.py | 123 +++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 109 insertions(+), 20 deletions(-) diff --git a/ipalib/cli.py b/ipalib/cli.py index e232c3ed24c3b35d4db5180e302b4e0e29c89f08..737ae001573af0f614783fe69add5711362da21e 100644 --- a/ipalib/cli.py +++ b/ipalib/cli.py @@ -529,6 +529,9 @@ class textui(backend.Backend): print raise PromptFailed(name=label) + def print_prompt_attribute_error(self, attribute, error): + self.print_plain('>>> %s: %s' % (attribute, error)) + def prompt(self, label, default=None, get_values=None, optional=False): """ Prompt user for input. @@ -1160,7 +1163,8 @@ class cli(backend.Executioner): error = None while True: if error is not None: - print '>>> %s: %s' % (unicode(param.label), unicode(error)) + self.Backend.textui.print_prompt_attribute_error(unicode(param.label), + unicode(error)) raw = self.Backend.textui.prompt(param.label, default, optional=param.alwaysask or not param.required) try: value = param(raw, **kw) diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index 07d44dde76d513881eeb5762626af229549f8e52..8ffc2b4ba17c3ec883e044fd2ad815974984f162 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -672,6 +672,25 @@ class DNSRecord(Str): return tuple(self._convert_dnsrecord_extra(extra) for extra in self.extra) + def __get_part_param(self, backend, part, output_kw, default=None): + name = self.part_name_format % (self.rrtype.lower(), part.name) + label = self.part_label_format % (self.rrtype, unicode(part.label)) + optional = not part.required + + while True: + try: + raw = backend.textui.prompt(label, + optional=optional, + default=default) + if not raw.strip(): + raw = default + + output_kw[name] = part(raw) + break + except (errors.ValidationError, errors.ConversionError), e: + backend.textui.print_prompt_attribute_error( + unicode(label), unicode(e.error)) + def prompt_parts(self, backend, mod_dnsvalue=None): mod_parts = None if mod_dnsvalue is not None: @@ -682,21 +701,33 @@ class DNSRecord(Str): return user_options for part_id, part in enumerate(self.parts): - name = self.part_name_format % (self.rrtype.lower(), part.name) - label = self.part_label_format % (self.rrtype, unicode(part.label)) - optional = not part.required if mod_parts: default = mod_parts[part_id] else: default = None - raw = backend.textui.prompt(label, - optional=optional, - default=default) - if not raw.strip(): - raw = default + self.__get_part_param(backend, part, user_options, default) - user_options[name] = part(raw) + return user_options + + def prompt_missing_parts(self, backend, kw, prompt_optional=False): + user_options = {} + if self.parts is None: + return user_options + + for part in self.parts: + name = self.part_name_format % (self.rrtype.lower(), part.name) + label = self.part_label_format % (self.rrtype, unicode(part.label)) + + if name in kw: + continue + + optional = not part.required + if optional and not prompt_optional: + continue + + default = part.get_default(**kw) + self.__get_part_param(backend, part, user_options, default) return user_options @@ -1919,6 +1950,51 @@ class dnsrecord(LDAPObject): record.setdefault('dnsrecords', []).append(dnsentry) del record[attr] + def get_rrparam_from_part(self, part_name): + """ + Get an instance of DNSRecord parameter that has part_name as its part. + If such parameter is not found, None is returned + + :param part_name Part parameter name + """ + try: + param = self.params[part_name] + + if not any(flag in param.flags for flag in \ + ('dnsrecord_part', 'dnsrecord_extra')): + return None + + # All DNS record part or extra parameters contain a name of its + # parent RR parameter in its hint attribute + rrparam = self.params[param.hint] + except (KeyError, AttributeError): + return None + + return rrparam + + def iterate_rrparams_by_parts(self, kw, skip_extra=False): + """ + Iterates through all DNSRecord instances that has at least one of its + parts or extra options in given dictionary. It returns the DNSRecord + instance only for the first occurence of part/extra option. + + :param kw Dictionary with DNS record parts or extra options + :param skip_extra Skip DNS record extra options, yield only DNS records + with a real record part + """ + processed = [] + for opt in kw: + rrparam = self.get_rrparam_from_part(opt) + if rrparam is None: + continue + + if skip_extra and 'dnsrecord_extra' in self.params[opt].flags: + continue + + if rrparam.name not in processed: + processed.append(rrparam) + yield rrparam + api.register(dnsrecord) @@ -1943,11 +2019,22 @@ class dnsrecord_add(LDAPCreate): def interactive_prompt_callback(self, kw): try: self.obj.has_cli_options(kw, self.no_option_msg) + + # Some DNS records were entered, do not use full interactive help + # We should still ask user for required parts of DNS parts he is + # trying to add in the same way we do for standard LDAP parameters + # + # Do not ask for required parts when any "extra" option is used, + # it can be used to fill all required params by itself + new_kw = {} + for rrparam in self.obj.iterate_rrparams_by_parts(kw, skip_extra=True): + user_options = rrparam.prompt_missing_parts(self.Backend, kw, + prompt_optional=False) + new_kw.update(user_options) + kw.update(new_kw) + return except errors.OptionError: pass - else: - # some record type entered, skip this helper - return # check zone type if kw['idnsname'] == _dns_zone_record: @@ -1999,13 +2086,11 @@ class dnsrecord_add(LDAPCreate): except KeyError: continue + rrparam = self.obj.get_rrparam_from_part(option) + if rrparam is None: + continue + if 'dnsrecord_part' in param.flags: - # check if any record part was added - try: - rrparam = self.params[param.hint] - except (KeyError, AttributeError): - continue - if rrparam.name in entry_attrs: # this record was already entered continue @@ -2020,7 +2105,7 @@ class dnsrecord_add(LDAPCreate): if isinstance(param, Flag) and not options[option]: continue # extra option is passed, run per-type pre_callback for given RR type - precallback_attrs.append(param.hint) + precallback_attrs.append(rrparam.name) # run precallback also for all new RR type attributes in entry_attrs for attr in entry_attrs: -- 1.7.7.6
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
