On 05/15/2013 04:02 PM, Ana Krivokapic wrote:
On 05/15/2013 12:09 PM, Petr Vobornik wrote:
[PATCH] 414 Move spec modifications from facet factories to pre_ops
---
Spec modifications in factories makes inheritance and extensibility
more difficult.
Moving them to pre_ops allows modification of their output by other
pre_ops.
https://fedorahosted.org/freeipa/ticket/3605
[PATCH] 415 Unite and move facet pre_ops to related modules
---
Facet pre_ops defined in ./facet module were moved to modules where
facet are actually defined. Moved pre_ops were united with the ones
defined for the facets in these modules.
The move simplifies module dependencies - there is no reason to have
general facet module dependent on specialized facet modules.
Pre_ops uniting makes the code simpler.
https://fedorahosted.org/freeipa/ticket/3605
A minor nitpick - in patch 415:
+if (indirect_members) {
+if (indirect_members.indexOf(spec.other_entity) -1) {
+return true;
+}
+}
This construct:
if (A) {
if (B) {
do_something();
}
}
Can be replaced with:
if (A B) {
do_something();
}
The second option is more readable, and avoids nested ifs.
Changed:
-if (indirect_members) {
-if (indirect_members.indexOf(spec.other_entity) -1) {
-return true;
-}
-}
-return false;
+var has_indirect = !!(indirect_members
indirect_members.indexOf(spec.other_entity) -1);
+return has_indirect;
Updated patch attached.
I have found no other issues, so ACK from me.
--
Regards,
Ana Krivokapic
--
Petr Vobornik
From 92d704bb6a7227fe4df2ab1daede3748b22b54ff Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Tue, 14 May 2013 17:36:28 +0200
Subject: [PATCH] Move spec modifications from facet factories to pre_ops
Spec modifications in factories makes inheritance and extensibility more difficult.
Moving them to pre_ops allows modification of their output by other pre_ops.
https://fedorahosted.org/freeipa/ticket/3605
---
install/ui/src/freeipa/association.js | 54 ---
install/ui/src/freeipa/details.js | 21 --
install/ui/src/freeipa/facet.js | 47 +-
install/ui/src/freeipa/search.js | 46 +++--
install/ui/test/details_tests.js | 8 --
install/ui/test/entity_tests.js | 8 --
6 files changed, 99 insertions(+), 85 deletions(-)
diff --git a/install/ui/src/freeipa/association.js b/install/ui/src/freeipa/association.js
index 71ee71d488e4b8f042884762f0e73d994dba5011..9e200ecf27410e7cfabb9773336c687861d43d55 100644
--- a/install/ui/src/freeipa/association.js
+++ b/install/ui/src/freeipa/association.js
@@ -30,9 +30,12 @@ define([
'./phases',
'./reg',
'./text',
+'./facet',
'./search',
'./dialog'],
-function(Deferred, IPA, $, navigation, phases, reg, text) {
+function(Deferred, IPA, $, navigation, phases, reg, text, mod_facet) {
+
+var exp = {};
IPA.associator = function (spec) {
@@ -760,16 +763,14 @@ IPA.association_table_field = function (spec) {
return that;
};
-IPA.association_facet = function (spec, no_init) {
+exp.association_facet_pre_op = function(spec, context) {
-spec = spec || {};
-
-/*
- Link parameter is used to turn off the links in selfservice mode.
+ /*
+ Link parameter is used to turn off the links in self-service mode.
Default it to true if not set so that facets that would not otherwise
link by default get links set.
- link must be set before the call to the base class, to affect the table.
+ link must be set before the call to the base class, to affect the table.
*/
spec.link = spec.link === undefined ? true : spec.link;
spec.managed_entity = IPA.get_entity(spec.other_entity);
@@ -823,6 +824,13 @@ IPA.association_facet = function (spec, no_init) {
IPA.association_type_state_evaluator,
IPA.read_only_state_evaluator);
+return spec;
+};
+
+exp.association_facet = IPA.association_facet = function (spec, no_init) {
+
+spec = spec || {};
+
var that = IPA.table_facet(spec, true);
that.attribute_member = spec.attribute_member;
@@ -1156,9 +1164,7 @@ IPA.association_facet = function (spec, no_init) {
return that;
};
-IPA.attribute_facet = function(spec, no_init) {
-
-spec = spec || {};
+exp.attribute_facet_pre_op = function(spec, context) {
//default buttons and their actions
spec.actions = spec.actions || [];
@@ -1212,6 +1218,13 @@ IPA.attribute_facet = function(spec, no_init) {
spec.columns = spec.columns || [ spec.attribute ];
spec.table_name = spec.table_name || spec.attribute;
+return spec;
+};
+
