Re: [Freeipa-devel] [PATCH] 0036-38 webui: Server roles

2016-06-14 Thread Petr Vobornik
On 06/09/2016 05:39 PM, Pavel Vomacka wrote:
> 
> 
> On 06/08/2016 04:09 PM, Petr Vobornik wrote:
>> On 06/05/2016 07:22 PM, Pavel Vomacka wrote:
>>>
>>> On 06/03/2016 03:10 PM, Petr Vobornik wrote:
 On 06/02/2016 01:40 PM, Pavel Vomacka wrote:
> Hello,
>
> please review my patches which add webui for server roles.
>
 Did not test yet. I'm waiting for rebase of backend.

 Patch 36: ACK (assuming it works when ^^ is available)

 Patch 37:

 1. typo: 'overriden' - twice
>>> Fixed.
 2. 'create_column_link' is a bad name for the method. The method
 doesn't
 create a column link. It is a link's click handler. So the name should
 be e.g. on_column_link_click
>>> Yes, this is better. Fixed.
 Patch 38:

 1. in serverroles_nested_search_facet wouldn't it be better to override
 only get_refresh_command_options and maybe get_refresh_command_args
 instead of full create_refresh_command?

>>> Fixed.
>>>
>>> Attached the whole patchset with edited patches.
>>>
>>> -- 
>>> Pavel^3 Vomacka
>> 1. Following line breaks navigation:
>> that.on_column_link_click(value, entity);
>> it should be:
>> return that.on_column_link_click(value, entity);
>>
> Attached patches are rebased and updated according to new Server Role
> patches.

ACK for all 4 patches.

pushed to master:
* 95c61c6a0b7d97a9f78fc5b83f38ce2b43cbebc4 Association table can be read
only
* 72fe7e3294fd2f0acdab161180609e9868c4a943 Extend table facet
* 1eb57600185f96e61d0894148a5f50870173c7cd Add server roles on topology page
* 31faf1c21d5635edfa9da23005a1942452a0248c Search facet can be without
search field

-- 
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


Re: [Freeipa-devel] [PATCH] 0036-38 webui: Server roles

2016-06-09 Thread Pavel Vomacka



On 06/08/2016 04:09 PM, Petr Vobornik wrote:

On 06/05/2016 07:22 PM, Pavel Vomacka wrote:


On 06/03/2016 03:10 PM, Petr Vobornik wrote:

On 06/02/2016 01:40 PM, Pavel Vomacka wrote:

Hello,

please review my patches which add webui for server roles.


Did not test yet. I'm waiting for rebase of backend.

Patch 36: ACK (assuming it works when ^^ is available)

Patch 37:

1. typo: 'overriden' - twice

Fixed.

2. 'create_column_link' is a bad name for the method. The method doesn't
create a column link. It is a link's click handler. So the name should
be e.g. on_column_link_click

Yes, this is better. Fixed.

Patch 38:

1. in serverroles_nested_search_facet wouldn't it be better to override
only get_refresh_command_options and maybe get_refresh_command_args
instead of full create_refresh_command?


Fixed.

Attached the whole patchset with edited patches.

--
Pavel^3 Vomacka

1. Following line breaks navigation:
that.on_column_link_click(value, entity);
it should be:
return that.on_column_link_click(value, entity);

Attached patches are rebased and updated according to new Server Role 
patches.
From 968b650028543e3dd00f1bfd8a7d68dd29c24e9a Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Thu, 2 Jun 2016 13:07:00 +0200
Subject: [PATCH 1/4] Association table can be read only

When it is read only it does not show Add and Delete buttons.

Part of: https://fedorahosted.org/freeipa/ticket/5906
---
 install/ui/src/freeipa/association.js | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/install/ui/src/freeipa/association.js b/install/ui/src/freeipa/association.js
index abb032fbb73b33553f38e0fcd78fafbaf72ab095..7579bb095842b80422c94cbe06eb2c8225a2944d 100644
--- a/install/ui/src/freeipa/association.js
+++ b/install/ui/src/freeipa/association.js
@@ -428,6 +428,7 @@ IPA.association_table_widget = function (spec) {
 that.add_method = spec.add_method || 'add_member';
 that.remove_method = spec.remove_method || 'remove_member';
 
+that.read_only = spec.read_only === undefined ? false : spec.read_only;
 that.add_title = text.get(spec.add_title || '@i18n:association.add.member');
 that.remove_title = text.get(spec.remove_title || '@i18n:association.remove.member');
 
@@ -501,6 +502,8 @@ IPA.association_table_widget = function (spec) {
 
 that.table_create(container);
 
+if (that.read_only) return;
+
 that.remove_button = IPA.button_widget({
 name: 'remove',
 label: '@i18n:buttons.remove',
-- 
2.5.5

From a48ba17edf3624e055db79fc09a5df49c7523a33 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Thu, 2 Jun 2016 13:08:27 +0200
Subject: [PATCH 3/4] Extend table facet

There is new attribute of table facet which allows to set which column of
table will be its primary key. This patch also move some code into separate
method - it will be easier to overide some functionality in child classes.

Part of: https://fedorahosted.org/freeipa/ticket/5906
---
 install/ui/src/freeipa/facet.js | 71 +
 1 file changed, 57 insertions(+), 14 deletions(-)

diff --git a/install/ui/src/freeipa/facet.js b/install/ui/src/freeipa/facet.js
index 9d71487c970368279dc2812b14d8171cf795c8bc..cc51a1531d875882414c0392fabff2dbf4ce315a 100644
--- a/install/ui/src/freeipa/facet.js
+++ b/install/ui/src/freeipa/facet.js
@@ -360,6 +360,13 @@ exp.facet = IPA.facet = function(spec, no_init) {
 that.state = new FacetState();
 
 
+/**
+ * Sets the name of attribute which will be used as primary key in case
+ * that primary key is missing in metadata.
+ * @type {String}
+ */
+that.primary_key_name = spec.primary_key_name;
+
 that.get_full_name = function() {
 if (that.entity) {
 return that.entity.create_facet_type(that.name);
@@ -2011,6 +2018,23 @@ exp.table_facet = IPA.table_facet = function(spec, no_init) {
 };
 
 /**
+ * Method which is called before adding the record into array which will be
+ * displayed. This is place where filters can be implemented. If this
+ * method returns true then the record will be shown in table. if returns
+ * false then the record won't be shown.
+ *
+ * It is created to be overridden by child classes.
+ *
+ * @param records_map {Array} array of already added entries
+ * @param pkey {string} primary_key
+ * @param record {Object} result from API call response
+ * @return {boolean} true when the record should be shown, otherwise false
+ */
+that.filter_records = function(records_map, pkey, record) {
+return true;
+};
+
+/**
  * Create a map with records as values and pkeys as keys
  *
  * Extracts records from data, where data originates from RPC command.
@@ -2024,13 +2048,16 @@ exp.table_facet = IPA.table_facet = function(spec, no_init) {
 var records_map = $.ordered_map();
 
 var result = data.result.result;
-

Re: [Freeipa-devel] [PATCH] 0036-38 webui: Server roles

2016-06-08 Thread Petr Vobornik
On 06/05/2016 07:22 PM, Pavel Vomacka wrote:
> 
> 
> On 06/03/2016 03:10 PM, Petr Vobornik wrote:
>> On 06/02/2016 01:40 PM, Pavel Vomacka wrote:
>>> Hello,
>>>
>>> please review my patches which add webui for server roles.
>>>
>> Did not test yet. I'm waiting for rebase of backend.
>>
>> Patch 36: ACK (assuming it works when ^^ is available)
>>
>> Patch 37:
>>
>> 1. typo: 'overriden' - twice
> Fixed.
>>
>> 2. 'create_column_link' is a bad name for the method. The method doesn't
>> create a column link. It is a link's click handler. So the name should
>> be e.g. on_column_link_click
> Yes, this is better. Fixed.
>> Patch 38:
>>
>> 1. in serverroles_nested_search_facet wouldn't it be better to override
>> only get_refresh_command_options and maybe get_refresh_command_args
>> instead of full create_refresh_command?
>>
> Fixed.
> 
> Attached the whole patchset with edited patches.
> 
> -- 
> Pavel^3 Vomacka

1. Following line breaks navigation:
   that.on_column_link_click(value, entity);
it should be:
   return that.on_column_link_click(value, entity);

-- 
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


Re: [Freeipa-devel] [PATCH] 0036-38 webui: Server roles

2016-06-05 Thread Pavel Vomacka



On 06/03/2016 03:10 PM, Petr Vobornik wrote:

On 06/02/2016 01:40 PM, Pavel Vomacka wrote:

Hello,

please review my patches which add webui for server roles.


Did not test yet. I'm waiting for rebase of backend.

Patch 36: ACK (assuming it works when ^^ is available)

Patch 37:

1. typo: 'overriden' - twice

Fixed.


2. 'create_column_link' is a bad name for the method. The method doesn't
create a column link. It is a link's click handler. So the name should
be e.g. on_column_link_click

Yes, this is better. Fixed.

Patch 38:

1. in serverroles_nested_search_facet wouldn't it be better to override
only get_refresh_command_options and maybe get_refresh_command_args
instead of full create_refresh_command?


Fixed.

Attached the whole patchset with edited patches.

--
Pavel^3 Vomacka
From 142e46045f6fe4f64ce20a1b822508fece767579 Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Thu, 2 Jun 2016 13:07:00 +0200
Subject: [PATCH 1/3] Association table can be read only

When it is read only it does not show Add and Delete buttons.

Part of: https://fedorahosted.org/freeipa/ticket/5906
---
 install/ui/src/freeipa/association.js | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/install/ui/src/freeipa/association.js b/install/ui/src/freeipa/association.js
index abb032fbb73b33553f38e0fcd78fafbaf72ab095..7579bb095842b80422c94cbe06eb2c8225a2944d 100644
--- a/install/ui/src/freeipa/association.js
+++ b/install/ui/src/freeipa/association.js
@@ -428,6 +428,7 @@ IPA.association_table_widget = function (spec) {
 that.add_method = spec.add_method || 'add_member';
 that.remove_method = spec.remove_method || 'remove_member';
 
+that.read_only = spec.read_only === undefined ? false : spec.read_only;
 that.add_title = text.get(spec.add_title || '@i18n:association.add.member');
 that.remove_title = text.get(spec.remove_title || '@i18n:association.remove.member');
 
@@ -501,6 +502,8 @@ IPA.association_table_widget = function (spec) {
 
 that.table_create(container);
 
+if (that.read_only) return;
+
 that.remove_button = IPA.button_widget({
 name: 'remove',
 label: '@i18n:buttons.remove',
-- 
2.5.5

From e5891c1955d48b1236d2cca47f6b9c8ec687e9ba Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Thu, 2 Jun 2016 13:08:27 +0200
Subject: [PATCH 1/2] Extend table facet

There is new attribute of table facet which allows to set which collumn of
table will be its primary key. This patch also move some code into separate
method - it will be easier to overide some functionality in child classes.

Part of: https://fedorahosted.org/freeipa/ticket/5906
---
 install/ui/src/freeipa/facet.js | 71 +
 1 file changed, 57 insertions(+), 14 deletions(-)

diff --git a/install/ui/src/freeipa/facet.js b/install/ui/src/freeipa/facet.js
index 9d71487c970368279dc2812b14d8171cf795c8bc..70602667cb0cd016326d53c6071ab17bc4e18820 100644
--- a/install/ui/src/freeipa/facet.js
+++ b/install/ui/src/freeipa/facet.js
@@ -360,6 +360,13 @@ exp.facet = IPA.facet = function(spec, no_init) {
 that.state = new FacetState();
 
 
+/**
+ * Sets the name of attribute which will be used as primary key in case
+ * that primary key is missing in metadata.
+ * @type {String}
+ */
+that.primary_key_name = spec.primary_key_name;
+
 that.get_full_name = function() {
 if (that.entity) {
 return that.entity.create_facet_type(that.name);
@@ -2011,6 +2018,23 @@ exp.table_facet = IPA.table_facet = function(spec, no_init) {
 };
 
 /**
+ * Method which is called before adding the record into array which will be
+ * displayed. This is place where filters can be implemented. If this
+ * method returns true then the record will be shown in table. if returns
+ * false then the record won't be shown.
+ *
+ * It is created to be overridden by child classes.
+ *
+ * @param records_map {Array} array of already added entries
+ * @param pkey {string} primary_key
+ * @param record {Object} result from API call response
+ * @return {boolean} true when the record should be shown, otherwise false
+ */
+that.filter_records = function(records_map, pkey, record) {
+return true;
+};
+
+/**
  * Create a map with records as values and pkeys as keys
  *
  * Extracts records from data, where data originates from RPC command.
@@ -2024,13 +2048,16 @@ exp.table_facet = IPA.table_facet = function(spec, no_init) {
 var records_map = $.ordered_map();
 
 var result = data.result.result;
-var pkey_name = that.managed_entity.metadata.primary_key;
+var pkey_name = that.managed_entity.metadata.primary_key ||
+that.primary_key_name;
 var adapter = builder.build('adapter', 'adapter', {context: that});
 
 for (var i=0; 

Re: [Freeipa-devel] [PATCH] 0036-38 webui: Server roles

2016-06-03 Thread Martin Basti



On 03.06.2016 15:10, Petr Vobornik wrote:

On 06/02/2016 01:40 PM, Pavel Vomacka wrote:

Hello,

please review my patches which add webui for server roles.


Did not test yet. I'm waiting for rebase of backend.

Patch 36: ACK (assuming it works when ^^ is available)

Patch 37:

1. typo: 'overriden' - twice

2. 'create_column_link' is a bad name for the method. The method doesn't
create a column link. It is a link's click handler. So the name should
be e.g. on_column_link_click

Patch 38:

1. in serverroles_nested_search_facet wouldn't it be better to override
only get_refresh_command_options and maybe get_refresh_command_args
instead of full create_refresh_command?



Works for me, but I borrow my VM to Petr to be sure if you meet all 
webUI requirements :)


Martin^2

--
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


Re: [Freeipa-devel] [PATCH] 0036-38 webui: Server roles

2016-06-03 Thread Petr Vobornik
On 06/02/2016 01:40 PM, Pavel Vomacka wrote:
> Hello,
> 
> please review my patches which add webui for server roles.
> 

Did not test yet. I'm waiting for rebase of backend.

Patch 36: ACK (assuming it works when ^^ is available)

Patch 37:

1. typo: 'overriden' - twice

2. 'create_column_link' is a bad name for the method. The method doesn't
create a column link. It is a link's click handler. So the name should
be e.g. on_column_link_click

Patch 38:

1. in serverroles_nested_search_facet wouldn't it be better to override
only get_refresh_command_options and maybe get_refresh_command_args
instead of full create_refresh_command?

-- 
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