Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Guided Rule Editor doesn't show a popup for field name binding in case o... #34

Closed
wants to merge 1 commit into from

2 participants

@tkobayas

org.drools.guvnor.client.asseteditor.drools.modeldriven.ui.FactPatternWidget has 2 similar constructors (Look at the 5th argument type!):

public FactPatternWidget(RuleModeller mod,
                         EventBus eventBus,
                         IPattern p,
                         boolean isAll0WithLabel,
                         boolean canBind) {
    this( mod,
          eventBus,
          p,
          isAll0WithLabel,
          canBind,
          null );
}

public FactPatternWidget(RuleModeller ruleModeller,
                         EventBus eventBus,
                         IPattern pattern,
                         boolean canBind,
                         Boolean readOnly) {
    this( ruleModeller,
          eventBus,
          pattern,
          false,
          canBind,
          readOnly );
}

The former constructor is only used by org.drools.guvnor.client.asseteditor.drools.modeldriven.ui.FromCompositeFactPatternWidget.createFactPatternWidget()

private Widget createFactPatternWidget(FactPattern fact) {
    FactPatternWidget factPatternWidget;
    if ( this.readOnly ) {
        //creates a new read-only FactPatternWidget
        factPatternWidget = new FactPatternWidget( this.getModeller(),
                                                   this.getEventBus(),
                                                   fact,
                                                   false,
                                                   true );
        //this.layout.setWidget( 0, 0, factPatternWidget );
        return factPatternWidget;
    } else {
        factPatternWidget = new FactPatternWidget( this.getModeller(),
                                                   this.getEventBus(),
                                                   fact,
                                                   true,
                                                   false );

I believe that createFactPatternWidget() wants to call the latter constructor. This is the root cause of this issue.
So deleting the former constructor is the cleanest way to fix this issue. You don't have to fix createFactPatternWidget() thanks to autoboxing.

Could you review it?

Thanks!

@manstis manstis was assigned
@manstis
Owner

Hi, I had a quick look but couldn't fully understand the problem this is fixing partly because the "title" ("Guided Rule Editor doesn't show a popup for field name binding in case o...") is truncated. Could you please explain? I agree the constructors aren't a good text book example of overloading ;) Thanks, Mike

@tkobayas

Ah, I'm very sorry. I thought I had included the JIRA id in the subject but I hadn't.

It's
Guided Rule Editor doesn't show a popup for field name binding in case of 'From'
https://issues.jboss.org/browse/GUVNOR-1823

Please look at the JIRA for the issue.

When this issue occurs, FromCompositeFactPatternWidget.createFactPatternWidget() seems to want to create a FactPatternWidget with canBind=true and readOnly=false.

private Widget createFactPatternWidget(FactPattern fact) {
    FactPatternWidget factPatternWidget;
    if ( this.readOnly ) {
...
    } else {
        factPatternWidget = new FactPatternWidget( this.getModeller(),
                                                   this.getEventBus(),
                                                   fact,
                                                   true,
                                                   false );

But actually it calls the former constructor which falls into canBind=false and readOnly=null.
It is the reason why the field name label is not clickable.

FactPatternWidget.fieldLabel()

    private Widget fieldLabel(final SingleFieldConstraint con,
                              final HasConstraints hasConstraints,
                              boolean showBinding,
                              int padding) {
...
        if ( bindable && showBinding && !this.readOnly ) {
            ClickHandler click = new ClickHandler() {

                public void onClick(ClickEvent event) {
                    String[] fields = new String[0];
                    //If field name is "this" use parent FactPattern type otherwise we can use the Constraint's field type
                    String fieldName = con.getFieldName();
                    if ( fieldName.contains( "." ) ) {
                        fieldName = fieldName.substring( fieldName.indexOf( "." ) + 1 );
                    }
                    if ( SuggestionCompletionEngine.TYPE_THIS.equals( fieldName ) ) {
                        fields = connectives.getCompletions().getFieldCompletions( pattern.getFactType() );
                    } else {
                        fields = connectives.getCompletions().getFieldCompletions( con.getFieldType() );
                    }
                    popupCreator.showBindFieldPopup( (Widget) event.getSource(),
                                                     pattern,
                                                     con,
                                                     fields,
                                                     popupCreator );
                }
            };

Here, the value of 'bindable' is the same as 'canBind'.

@manstis
Owner

Pull request has been merged. See 536e63e. Thank-you.

@manstis manstis closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 4, 2012
  1. @tkobayas
This page is out of date. Refresh to see the latest.
View
13 ...in/java/org/drools/guvnor/client/asseteditor/drools/modeldriven/ui/FactPatternWidget.java
@@ -90,19 +90,6 @@ public FactPatternWidget(RuleModeller mod,
null );
}
- public FactPatternWidget(RuleModeller mod,
- EventBus eventBus,
- IPattern p,
- boolean isAll0WithLabel,
- boolean canBind) {
- this( mod,
- eventBus,
- p,
- isAll0WithLabel,
- canBind,
- null );
- }
-
/**
* Creates a new FactPatternWidget
*
Something went wrong with that request. Please try again.