On 06/14/2016 10:17 AM, Pavel Vomacka wrote: > > > On 06/14/2016 06:42 AM, Fraser Tweedale wrote: >> On Mon, Jun 13, 2016 at 07:48:58PM +0200, Pavel Vomacka wrote: >>> >>> On 06/13/2016 06:55 AM, Fraser Tweedale wrote: >>>> On Fri, Jun 10, 2016 at 04:34:33PM +0200, Pavel Vomacka wrote: >>>>> Hello, >>>>> >>>>> please review these new patches which add WebUI for Sub-CAs. >>>>> >>>>> https://fedorahosted.org/freeipa/ticket/5939 >>>>> >>>> Hi Pavel, I have reviewed the functionality of the patches. >>>> Functionality is good - a few minor comments below. >>> Hello, thank you for review. >>>> Patch 45: >>>> >>>> 1) In the main `Certificate Authorities' table, `Subject DN' is >>>> showing the DN of the IPA object, instead of the Subject DN. >>> Fixed. >>>> 2) In the `Certificate Authorities' detail table, there is an >>>> unlabelled row showing the DN of the IPA object. IMO we do not need >>>> to show this value at all. >>> The field removed. >>>> Patch 46: >>>> >>>> 3) I see a FIXME in certificate.js. The behaviour (default to IPA >>>> CA / 'ipa') is OK. Alternatively, you could allow the user to not >>>> specify a CA (this will allow the default - currently 'ipa' - to be >>>> controlled by server). >>> FIXME comment removed. >>>> Patch 47: >>>> >>>> 4) For backwards compatibility, a CA ACL without any specified CAs >>>> (and not cacat=all) implies the 'ipa' CA. It would be good to >>>> indicate this in the UI somehow, or include a notice to explain. >>> I added tooltip next to the checkbox on adder dialog and also note >>> above the >>> table with CAs in CA ACL details view. >>> >> The message has a small typo: "specificed" should be "specified" >> (the typo occurrs in both places). >> >> Once this is fixed, ACK. > Updated patches attached.
Also ACK from Web UI internals perspective. Patch 0045-2: ACK Patch 0046-2: ACK, could be split into two patches but don't bother... Patch 0047-3: ACK -- Petr Vobornik -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code