[Freeipa-devel] [PATCH] 928-936 webui: topology visualization

[Petr Vobornik]

On 11/25/2015 05:05 PM, Martin Babinsky wrote:
> On 11/25/2015 03:28 PM, Petr Vobornik wrote:
>> On 11/24/2015 02:09 PM, Martin Babinsky wrote:
>>> On 11/24/2015 12:17 PM, Petr Vobornik wrote:
>>>> On 11/24/2015 12:10 PM, Ludwig Krispenz wrote:
>>>>> Hi Petr,
>>>>>
>>>>> I'm testing these patches.Two observations so far:
>>>>> - in Topology->IPA Servers I see a table of my servers and the managed
>>>>> suffix column I see both suffixes, ipaca and the realm,
>>>>> but if I select one of the servers I Only see the realm suffix, this
>>>>> was
>>>>> different in the demo video
>>>>
>>>> implemented in patch 927 (separate thread)
>>>>
>>>>> - the graph layou is not stable. If I arrange it and the try some
>>>>> actions on a segment, eg try to remove one (which is rejected), the
>>>>> graph does change again-
>>>>
>>>> Not sure if I understand it correctly. One can set static position to a
>>>> node by double clicking on it (this needs to be somehow communicated)
>>>> otherwise it is random - each refresh.
>>>>
>>>> Maybe we should not refresh on failed operation because there is no
>>>> change.
>>> +1, failed operations on segments should not trigger graph redraws.
>>
>> Fixed in patch 936-1.
>>
> Confirmed.
>>>
>>>>
>>>>>
>>>>> Ludwig
>>>>>
>>>>> On 11/20/2015 04:00 PM, Petr Vobornik wrote:
>>>>>> Patches 928-931 are prerequisites.
>>>>>>
>>>>>> Patches 932-934 implements the visualization
>>>>>>
>>>>>> Patches 935-936 adds 'add' and 'remove' segment functionality to the
>>>>>> visualization page.
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/4286
>>>>
>>>
>>> I have tested the new feature a bit (I have looked also on the code, but
>>> I do not speak JavaScript very well) and so far I have found these
>>> issues:
>>>
>>> 1.) If you first setup topology w/ domain level 0 and try to render
>>> topology graph, you are correctly informed that managed topology is
>>> available on domain level 1 only. IF you raise your domainlevel and
>>> click refresh, you get the same error message, probably because the
>>> request to get domainlevel is sent only in the beginning of session. You
>>> have to logout and then login back to see the graph.
>>
>> Login/Logout should not be required. Refresh of UI(F5) should be
>> sufficient.
>>
>> Anyway, fixed in new patch 937.
>>
> Confirmed works as advertised.
>
>>>
>>> 2.) If I add a new segment using Add dialog and clicking 'Add & edit'
>>> button, I get the following error:
>>>
>>> """
>>> IPA Error 3007: RequirementError
>>>
>>> 'topologysuffixcn' is required
>>> """
>>>
>>> However the segment is created normally. I guess that there is some bug
>>> when viewing the segment immediately after creation.
>>
>> Fixed in patch 935-1.
>>
>
> Indeed.
>
>>>
>>> 3.) The key with suffix colors should be more descriptive. I would like
>>> to propose the following format:
>>>
>>> """
>>> Color coding of segments by suffix:
>>> {suffix_name} ({suffix_dn})
>>> e.g.
>>> ipaca (o=ipaca)
>>> realm (dc=ipa,dc=test)
>>> """
>>>
>>> 4.) It would be nice if the graph canvas could be dynamically resized
>>> based on the container size, but I am not sure how difficult it is to
>>> implement it.
>>> (Graphical nitpick: I would change the color of nodes, green does not
>>> fit very well with the bluish/grey  palette of other WebUI elements)
>>
>> I would like this, #3 and #4 to go trough additional designer review and
>> therefore fixed in other patch.
>>
> I'm OK with that, it is a cosmetic change anyway so we shouldn't
> bikeshed over it.
>> Right now I'm not sure what is the correct behavior for the re-sizing.
>>
>> Atm the canvas size is fixed: 960x500.
>>
>> 1. What should be the canvas size when size of the container is very
>> small, e.g. 330x90? With static positions, nodes would not be visible.
>> 2. Similar issue when nodes are put into corners and then the window is
>> shrunk.
>>
>> Probably there could be a minimum size and then enlarge the cavas when
>> nodes are dragged.
>>
> Minimum canvas size makes sense IMHO. I'm not sure about the behavior of
> the auto-resize during node manipulation. I would begin with a canvas
> that has some reasonable minimal size and expands to the size of the
> container/facet (not sure about the terminology here) during resize with
> some reasonable padding added. Then we can implement some
> zooming/panning feature on top.
>
>>>
>>> Otherwise I think that it is a good start and additional features can be
>>> built on top of this patchset.
>>>
>
> All in all I think that all these points can be addressed in followup
> patches after discussion with UX people. If nobody else objects I would
> ACK these patches.
>

pushed to master:
* 148083f26a51110860f58b1610ae032f74e6465c webui: add Deferred/Promise 
API to rpc.command
* fc4b33d05017fac255aa8600fc45b5795f9a51c6 webui: split facet header 
into two classes
* 74b7c00365e66967fa6a7a62a3d0eaab99443824 webui: extract header and 
action logic from facet to separate mixins
* b8059001a5c57fd06bd977f6cc2448d3b401cabd webui: allow to update 
action_state directly
* ce1645ceeca577d1c73ba77f3eb8cb13c2bef2a1 webui: add d3 library - 
version 3.5.6
* 24fead79cbe5ef168a42f08da6fd99b4ec2b1bab webui: topology graph component
* 68f6c2c7dc1277d9efd3e729a924d4c9cadc90ec webui: topology graph facet
* eb242ac9a9cc1e8dea16788a64030adbb333a4b2 webui: add segments on 
topology graph page
* 768d1965aaa79625b5c6e16d999d65dd7fd4372e webui: remove segments on 
topology graph page
* b4aa222e426c2b3f294432be64d95750dd86bc03 webui: update topology graph 
after raising domain level

-- 
Petr Vobornik