GUVNOR-3557: [Guided Decision Table] User is able to bind two fields …

…to same name (apache#652) * GUVNOR-3557: [Guided Decision Table] User is able to bind two fields to same name * GUVNOR-3557: [Guided Decision Table] User is able to bind two fields to same name. Fixes following peer review * GUVNOR-3557: [Guided Decision Table] User is able to bind two fields to same name. More fixes following peer review. * GUVNOR-3557: [Guided Decision Table] User is able to bind two fields to same name. Add different PatternPageView descriptions depending on context.
Rikkola · Oct 17, 2017 · 89a2f9f · 89a2f9f
1 parent 6bc5fed
commit 89a2f9f
Show file tree

Hide file tree

Showing 16 changed files with 283 additions and 21 deletions.
diff --git a/...kbench/screens/guided/dtable/client/resources/i18n/GuidedDecisionTableErraiConstants.java b/...kbench/screens/guided/dtable/client/resources/i18n/GuidedDecisionTableErraiConstants.java
@@ -152,6 +152,12 @@ public class GuidedDecisionTableErraiConstants {
     @TranslationKey(defaultValue = "")
     public static final String PatternPageView_SelectPattern = "PatternPageView.SelectPattern";
 
+    @TranslationKey(defaultValue = "")
+    public static final String PatternPageView_PatternPageDescriptionConditions = "PatternPageView.PatternPageDescriptionConditions";
+
+    @TranslationKey(defaultValue = "")
+    public static final String PatternPageView_PatternPageDescriptionActions = "PatternPageView.PatternPageDescriptionActions";
+
     @TranslationKey(defaultValue = "")
     public static final String PatternPageView_EntryPointDescription = "PatternPageView.EntryPointDescription";
 

diff --git a/...g/drools/workbench/screens/guided/dtable/client/wizard/column/commons/HasPatternPage.java b/...g/drools/workbench/screens/guided/dtable/client/wizard/column/commons/HasPatternPage.java
@@ -30,5 +30,7 @@ public interface HasPatternPage {
 
     String getEntryPointName();
 
+    String getPatternPageDescription();
+
     Set<PatternWrapper> getPatterns();
 }
diff --git a/...ols/workbench/screens/guided/dtable/client/wizard/column/commons/HasValueOptionsPage.java b/...ols/workbench/screens/guided/dtable/client/wizard/column/commons/HasValueOptionsPage.java
@@ -25,6 +25,8 @@ public interface HasValueOptionsPage {
 
     Boolean isValueOptionsPageCompleted();
 
+    Boolean isFieldBindingValid();
+
     int constraintValue();
 
     String getFactType();

diff --git a/...va/org/drools/workbench/screens/guided/dtable/client/wizard/column/pages/PatternPage.java b/...va/org/drools/workbench/screens/guided/dtable/client/wizard/column/pages/PatternPage.java
@@ -98,12 +98,17 @@ public void prepareView() {
 
         setupPattern();
         setupEntryPoint();
+        setPatternPageDescription();
     }
 
     private void setupEntryPoint() {
         view.setupEntryPointName(getEntryPointName());
     }
 
+    private void setPatternPageDescription() {
+        view.setPatternPageDescription(getPatternPageDescription());
+    }
+
     void setupPattern() {
         view.clearPatternList();
 
@@ -210,6 +215,10 @@ String getEntryPointName() {
         return plugin().getEntryPointName();
     }
 
+    String getPatternPageDescription() {
+        return plugin().getPatternPageDescription();
+    }
+
     void setEntryPoint() {
         plugin().setEntryPointName(view.getEntryPointName());
     }
@@ -239,6 +248,8 @@ public interface View extends HasList,
 
         void selectPattern(String currentPatternValue);
 
+        void setPatternPageDescription(final String description);
+
         void showPatternWarning();
 
         void hidePatternWarning();

diff --git a/...rg/drools/workbench/screens/guided/dtable/client/wizard/column/pages/PatternPageView.html b/...rg/drools/workbench/screens/guided/dtable/client/wizard/column/pages/PatternPageView.html
@@ -19,7 +19,7 @@
         <span class="pficon pficon-info"></span>
         <b><span data-i18n-key="Select"></span></b>
         <hr/>
-        <span data-i18n-key="PatternPageDescription"></span>
+        <span data-field="patternPageDescription"></span>
     </div>
 
     <div data-field="patternWarning" class="alert alert-warning">

diff --git a/...rg/drools/workbench/screens/guided/dtable/client/wizard/column/pages/PatternPageView.java b/...rg/drools/workbench/screens/guided/dtable/client/wizard/column/pages/PatternPageView.java
@@ -30,6 +30,7 @@
 import org.drools.workbench.screens.guided.dtable.client.wizard.column.commons.HasPatternPage;
 import org.drools.workbench.screens.guided.dtable.client.wizard.column.pages.common.DecisionTablePopoverUtils;
 import org.jboss.errai.common.client.dom.Div;
+import org.jboss.errai.common.client.dom.Span;
 import org.jboss.errai.ui.client.local.api.IsElement;
 import org.jboss.errai.ui.client.local.spi.TranslationService;
 import org.jboss.errai.ui.shared.api.annotations.DataField;
@@ -45,7 +46,9 @@ public class PatternPageView implements IsElement,
 
     private PatternPage<? extends HasPatternPage> page;
 
-    @Inject
+    @DataField("patternPageDescription")
+    private Span patternPageDescription;
+
     @DataField("patternWarning")
     private Div patternWarning;
 
@@ -66,12 +69,16 @@ public class PatternPageView implements IsElement,
     private DecisionTablePopoverUtils popoverUtils;
 
     @Inject
-    public PatternPageView(final ListBox patternList,
+    public PatternPageView(final Span patternPageDescription,
+                           final Div patternWarning,
+                           final ListBox patternList,
                            final TextBox entryPointName,
                            final Button createANewFactPattern,
                            final Div entryPointContainer,
                            final TranslationService translationService,
                            final DecisionTablePopoverUtils popoverUtils) {
+        this.patternPageDescription = patternPageDescription;
+        this.patternWarning = patternWarning;
         this.patternList = patternList;
         this.entryPointName = entryPointName;
         this.createANewFactPattern = createANewFactPattern;
@@ -161,6 +168,11 @@ public void addItem(final String itemName,
                             itemKey);
     }
 
+    @Override
+    public void setPatternPageDescription(final String description) {
+        this.patternPageDescription.setInnerHTML(description);
+    }
+
     @Override
     public void showPatternWarning() {
         patternWarning.setHidden(false);

diff --git a/...g/drools/workbench/screens/guided/dtable/client/wizard/column/pages/ValueOptionsPage.java b/...g/drools/workbench/screens/guided/dtable/client/wizard/column/pages/ValueOptionsPage.java
@@ -71,7 +71,17 @@ public String getTitle() {
 
     @Override
     public void isComplete(final Callback<Boolean> callback) {
-        callback.callback(plugin().isValueOptionsPageCompleted());
+        boolean isFieldBindingValid = plugin().isFieldBindingValid();
+        boolean isValueOptionPageComplete = plugin().isValueOptionsPageCompleted();
+        boolean isComplete = isValueOptionPageComplete && isFieldBindingValid;
+
+        if (!isFieldBindingValid) {
+            view.showFieldBindingWarning();
+        } else {
+            view.hideFieldBindingWarning();
+        }
+
+        callback.callback(isComplete);
     }
 
     @Override
@@ -384,5 +394,9 @@ public interface View extends UberElement<ValueOptionsPage> {
         void hideCepOperators();
 
         void setupCepOperators(IsWidget widget);
+
+        void showFieldBindingWarning();
+
+        void hideFieldBindingWarning();
     }
 }
diff --git a/...ools/workbench/screens/guided/dtable/client/wizard/column/pages/ValueOptionsPageView.html b/...ools/workbench/screens/guided/dtable/client/wizard/column/pages/ValueOptionsPageView.html
@@ -22,6 +22,11 @@
         <span data-i18n-key="ValueOptionsPageDescription"></span>
     </div>
 
+    <div data-field="fieldBindingWarning" class="alert alert-warning">
+        <span class="pficon pficon-warning-triangle-o"></span>
+        <span data-i18n-key="PleaseEnterANameThatIsNotAlreadyUsed"></span>
+    </div>
+
     <div class="form-horizontal">
         <div class="form-group" data-field="valueListGroupContainer">
             <label class="col-sm-4 form-control-label" data-i18n-key="ValueList"></label>

diff --git a/...ools/workbench/screens/guided/dtable/client/wizard/column/pages/ValueOptionsPageView.java b/...ools/workbench/screens/guided/dtable/client/wizard/column/pages/ValueOptionsPageView.java
@@ -69,6 +69,9 @@ public class ValueOptionsPageView implements IsElement,
     @DataField("bindingContainer")
     private Div bindingContainer;
 
+    @DataField("fieldBindingWarning")
+    private Div fieldBindingWarning;
+
     private ValueOptionsPage<?> page;
 
     private TranslationService translationService;
@@ -86,6 +89,7 @@ public ValueOptionsPageView(final Div valueListGroupContainer,
                                 final Div defaultValueContainer,
                                 final Div limitedValueContainer,
                                 final Div bindingContainer,
+                                final Div fieldBindingWarning,
                                 final TranslationService translationService,
                                 final DecisionTablePopoverUtils popoverUtils) {
         this.valueListGroupContainer = valueListGroupContainer;
@@ -98,6 +102,7 @@ public ValueOptionsPageView(final Div valueListGroupContainer,
         this.defaultValueContainer = defaultValueContainer;
         this.limitedValueContainer = limitedValueContainer;
         this.bindingContainer = bindingContainer;
+        this.fieldBindingWarning = fieldBindingWarning;
         this.translationService = translationService;
         this.popoverUtils = popoverUtils;
     }
@@ -187,6 +192,16 @@ public void hideCepOperators() {
         cepWindowOperatorsGroupContainer.setHidden(true);
     }
 
+    @Override
+    public void showFieldBindingWarning() {
+        fieldBindingWarning.setHidden(false);
+    }
+
+    @Override
+    public void hideFieldBindingWarning() {
+        fieldBindingWarning.setHidden(true);
+    }
+
     private String translate(final String key,
                              final Object... args) {
         return translationService.format(key,

diff --git a/...ols/workbench/screens/guided/dtable/client/wizard/column/plugins/ActionSetFactPlugin.java b/...ols/workbench/screens/guided/dtable/client/wizard/column/plugins/ActionSetFactPlugin.java
@@ -198,6 +198,11 @@ public Boolean isValueOptionsPageCompleted() {
         return valueOptionsPageCompleted;
     }
 
+    @Override
+    public Boolean isFieldBindingValid() {
+        return Boolean.TRUE;
+    }
+
     @Override
     public PatternWrapper patternWrapper() {
         return Optional.ofNullable(patternWrapper).orElse(new PatternWrapper());
@@ -219,6 +224,11 @@ public String getEntryPointName() {
         return "";
     }
 
+    @Override
+    public String getPatternPageDescription() {
+        return translate(GuidedDecisionTableErraiConstants.PatternPageView_PatternPageDescriptionActions);
+    }
+
     @Override
     public void setEntryPointName(final String entryPointName) {
     }

diff --git a/...ench/screens/guided/dtable/client/wizard/column/plugins/ActionWorkItemSetFieldPlugin.java b/...ench/screens/guided/dtable/client/wizard/column/plugins/ActionWorkItemSetFieldPlugin.java
@@ -404,6 +404,11 @@ public String getEntryPointName() {
         return "";
     }
 
+    @Override
+    public String getPatternPageDescription() {
+        return translate(GuidedDecisionTableErraiConstants.PatternPageView_PatternPageDescriptionActions);
+    }
+
     @Override
     public void setEntryPointName(final String entryPointName) {
         // empty

diff --git a/...s/workbench/screens/guided/dtable/client/wizard/column/plugins/ConditionColumnPlugin.java b/...s/workbench/screens/guided/dtable/client/wizard/column/plugins/ConditionColumnPlugin.java
@@ -35,11 +35,12 @@
 import org.drools.workbench.models.guided.dtable.shared.model.CompositeColumn;
 import org.drools.workbench.models.guided.dtable.shared.model.ConditionCol52;
 import org.drools.workbench.models.guided.dtable.shared.model.DTCellValue52;
-import org.drools.workbench.models.guided.dtable.shared.model.GuidedDecisionTable52.TableFormat;
 import org.drools.workbench.models.guided.dtable.shared.model.GuidedDecisionTable52;
+import org.drools.workbench.models.guided.dtable.shared.model.GuidedDecisionTable52.TableFormat;
 import org.drools.workbench.models.guided.dtable.shared.model.LimitedEntryCol;
 import org.drools.workbench.models.guided.dtable.shared.model.LimitedEntryConditionCol52;
 import org.drools.workbench.models.guided.dtable.shared.model.Pattern52;
+import org.drools.workbench.models.guided.dtable.shared.model.adaptors.FactPatternPattern52Adaptor;
 import org.drools.workbench.screens.guided.dtable.client.resources.i18n.GuidedDecisionTableErraiConstants;
 import org.drools.workbench.screens.guided.dtable.client.widget.Validator;
 import org.drools.workbench.screens.guided.dtable.client.widget.table.utilities.CellUtilities;
@@ -182,6 +183,7 @@ void appendColumn() {
         }
     }
 
+    @Override
     public Pattern52 editingPattern() {
         return wrappedPattern();
     }
@@ -256,6 +258,11 @@ public String getEntryPointName() {
         return patternWrapper().getEntryPointName();
     }
 
+    @Override
+    public String getPatternPageDescription() {
+        return translate(GuidedDecisionTableErraiConstants.PatternPageView_PatternPageDescriptionConditions);
+    }
+
     @Override
     public void setEntryPointName(final String entryPointName) {
         patternWrapper().setEntryPointName(entryPointName);
@@ -264,14 +271,17 @@ public void setEntryPointName(final String entryPointName) {
     @Override
     public Set<PatternWrapper> getPatterns() {
         final Set<PatternWrapper> patterns = new HashSet<>();
-        final BRLRuleModel brlRuleModel = new BRLRuleModel(presenter.getModel());
+        final BRLRuleModel brlRuleModel = new BRLRuleModel(getPresenter().getModel());
         final List<String> variables = brlRuleModel.getLHSPatternVariables();
         variables.forEach(var -> {
-            final String factType = brlRuleModel.getLHSBoundFact(var).getFactType();
-            final boolean isNegated = brlRuleModel.getLHSBoundFact(var).isNegated();
-            patterns.add(new PatternWrapper(factType,
-                                            var,
-                                            isNegated));
+            final Pattern52 pattern = getPresenter().getModel().getConditionPattern(var);
+            if (!(pattern instanceof FactPatternPattern52Adaptor)) {
+                final String factType = brlRuleModel.getLHSBoundFact(var).getFactType();
+                final boolean isNegated = brlRuleModel.getLHSBoundFact(var).isNegated();
+                patterns.add(new PatternWrapper(factType,
+                                                var,
+                                                isNegated));
+            }
         });
 
         return patterns;
@@ -383,6 +393,8 @@ public String getBinding() {
     @Override
     public void setBinding(final String binding) {
         editingCol().setBinding(binding);
+
+        fireChangeEvent(valueOptionsPage);
     }
 
     @Override
@@ -395,6 +407,7 @@ public boolean isBindable() {
         return tableFormat() == LIMITED_ENTRY || constraintValue() == BaseSingleFieldConstraint.TYPE_LITERAL;
     }
 
+    @Override
     public int constraintValue() {
         final boolean factHasEnums = presenter.getDataModelOracle().hasEnums(getFactType(),
                                                                              getFactField());
@@ -426,6 +439,7 @@ public String getValueList() {
         return editingCol().getValueList();
     }
 
+    @Override
     public void setValueList(final String valueList) {
         editingCol().setValueList(valueList);
 
@@ -463,6 +477,7 @@ public void setConstraintValue(final int constraintValue) {
         fireChangeEvent(operatorPage);
     }
 
+    @Override
     public void setValueOptionsPageAsCompleted() {
         if (!isValueOptionsPageCompleted()) {
             setValueOptionsPageCompleted();
@@ -471,10 +486,32 @@ public void setValueOptionsPageAsCompleted() {
         }
     }
 
+    @Override
     public Boolean isValueOptionsPageCompleted() {
         return valueOptionsPageCompleted;
     }
 
+    @Override
+    public Boolean isFieldBindingValid() {
+        if (!isBindable()) {
+            return Boolean.TRUE;
+        }
+
+        final String binding = getBinding();
+        if ((binding == null || binding.isEmpty())) {
+            return Boolean.TRUE;
+        }
+
+        if (!isNewColumn()) {
+            if (binding.equals(originalCondition().getBinding())) {
+                return Boolean.TRUE;
+            }
+        }
+
+        final BRLRuleModel brlRuleModel = new BRLRuleModel(getPresenter().getModel());
+        return !brlRuleModel.isVariableNameUsed(binding);
+    }
+
     public String getFactType() {
         return patternWrapper().getFactType();
     }

diff --git a/.../screens/guided/dtable/client/resources/i18n/GuidedDecisionTableErraiConstants.properties b/.../screens/guided/dtable/client/resources/i18n/GuidedDecisionTableErraiConstants.properties
@@ -120,7 +120,10 @@ PatternPageView.Metadata=Metadata:
 PatternPageView.SelectPattern=Select a pattern
 PatternPageView.EntryPoint=Entry Point:
 PatternPageView.YouMustEnterAColumnPattern=You must select a Pattern.
-PatternPageView.PatternPageDescription=Select from the list of fact patterns already used in your table or create a new fact pattern. A fact pattern is a combination of \
+PatternPageView.PatternPageDescriptionConditions=Select from the list of fact patterns already used in <i>Conditions</i> in your table or create a new fact pattern. A fact pattern is a combination of \
+                                       an available data object in the package and a model class binding that you specify. (Examples: <b>LoanApplication [application]</b> or \
+                                       <b>IncomeSource [income]</b> where the bracketed portion is the binding to the given fact type).
+PatternPageView.PatternPageDescriptionActions=Select from the list of fact patterns already used in <i>Conditions</i> or <i>Condition BRL fragments</i> in your table or create a new fact pattern. A fact pattern is a combination of \
                                        an available data object in the package and a model class binding that you specify. (Examples: <b>LoanApplication [application]</b> or \
                                        <b>IncomeSource [income]</b> where the bracketed portion is the binding to the given fact type).
 PatternPageView.EntryPointDescription=Define the entry point for the fact pattern, if applicable. An entry point is a gate or stream through which facts enter the rule engine, \
@@ -145,6 +148,7 @@ ValueOptionsPageView.Binding=Binding:
 ValueOptionsPageView.ValueOptionsPageDescription=(optional) Enter a list of value options, delimited by a comma and space, to limit table input data for the condition ("WHEN") portion of the rule. \
                                                  When this value list is defined, the values will be provided in the table cells for that column as a drop-down list, from which users can select only one option. \
                                                  (Example list: <b>Monday</b>, <b>Wednesday</b>, <b>Friday </b>to specify only these three options)
+ValueOptionsPageView.PleaseEnterANameThatIsNotAlreadyUsed=That binding name is already in use by another Pattern or Field - please pick another.
 WorkItemPageView.Select=Select a Work Item
 WorkItemPageView.WorkItem=Work Item:
 WorkItemPageView.NoWorkItemsAvailable=<None available>
@@ -161,7 +165,7 @@ NewPatternView.Cancel=Cancel
 NewPatternView.OK=OK
 NewPatternPresenter.PleaseEnterANameForFact=You have to bind the fact to a name. Please enter a binding name.
 NewPatternPresenter.PleaseEnterANameThatIsNotTheSameAsTheFactType=Please enter a name that is not the same as the fact type.
-NewPatternPresenter.PleaseEnterANameThatIsNotAlreadyUsedByAnotherPattern=That binding name is already in use by another Pattern or Field - please pick another
+NewPatternPresenter.PleaseEnterANameThatIsNotAlreadyUsedByAnotherPattern=That binding name is already in use by another Pattern or Field - please pick another.
 AdditionalInfoPage.AdditionalInfo=Additional info
 AdditionalInfoPage.HideColumnDescription=Select this to hide the column, or clear this to display the column.
 AdditionalInfoPage.LogicalInsertDescription=Select this to insert the fact pattern logically into the inference engine, or clear this to insert it regularly. \