Apex:commandButton action not being invoked

Question

I am currently trying to create an app to track workouts and I've hit a snag.

I have a visualforce page:

<apex:page standardController="Exercise__c" extensions="ExerciseListController" recordSetVar="Exercises"  sidebar="true" showheader="false">
  <apex:form >
      <apex:pageBlock id="ExerciseList">
          <apex:pageBlockTable Value="{!Exercises}" var="e" rendered="{!NOT(ISBLANK(Exercises))}">
              <apex:column HeaderValue="Add to Wod" width="60">
                 <apex:inputCheckbox value="{!selected[e.id]}"/>
              </apex:column>
              <apex:column headerValue="Exercise Name">
                  <apex:outputLink value="/{!e.id}" target="_blank">
                      {!e.Name}
                  </apex:outputLink>
              </apex:column>
              <apex:column HeaderValue="Exercise ID">
                  <apex:outputLink value="/{!e.id}" target="_blank">
                      {!e.id}
                  </apex:outputLink>
              </Apex:column>
          </apex:pageBlockTable>
          <apex:outputLabel value="No record to display" rendered="{!(ISBLANK(Exercises))}" styleClass="noRowsHeader"></apex:outputLabel>
          <apex:pageBlockButtons >
              <apex:commandButton reRender="all" value="Save" action="{!SelectExercisesForWod}"/>
          </apex:pageBlockButtons>
      </apex:pageBlock>
  </apex:form>
</apex:page>

and a controller extension:

public class ExerciseListController{

    public Map<Id, Boolean> selected {get; set;}

    List<Exercise__c> Exercises;

    public String WodID {get; set;}

    public ExerciseListController(ApexPages.StandardSetController controller) {

    }

    public ApexPages.StandardSetController ExerciseListController{
        get{
            if(ExerciseListController == null) {
                 ExerciseListController= new apexPages.standardSetController(
                    Database.getQueryLocator(
                        [Select Name FROM Exercise__c]
                    )
                );    
            }
            return ExerciseListController;
        }
        set;
    }

    public pageReference SelectExercisesForWod(){
        system.debug('test');
        return null;
    }

    public List<Exercise__c> getExercises(){
        return (List<Exercise__c>) ExerciseListController.getRecords();
    }

    public void setExercises(Exercise__c[] exercises){
        this.exercises = exercises;
    }

    public List<SelectOption> getItems(){
        List<SelectOption> options = new List<SelectOption>();
        for(Exercise__c e : [select name from Exercise__c]){
            options.add(new selectOption(e.id, e.name));
        }
        return options;        
    }
}

When I click on the commandbutton in the page the action method does not fire. Their is no redirect and the system.debug does not show "test". Any clue why?

Answer 1

Heres how I'd write the page from looking at your code. Unless you have a good reason to use it, I'd drop the standard controller, the standard set controller, and some other code. I assume theres some reason for functions like getOptions but since they're unused for the demo you posted I removed them.

So first off, I'm using a custom controller, with a wrapper class to hold the selected value. You could use the standard controller, but Im generally not a fan of it. The custom controller gets its own data & doesn't create a standard Controller (its usually overkill at that point). I modified your save method to set a fake id from the WodID on the selected records & the return either a page error or a fake results page.

The key point here - don't mix and match custom / standard code. Don't create a page using the standard controller, but also using the standardSetController, but not using the one passed along in the controller, instead you created a totally separate setController and then didn't use any of its functions (such as standard save, standard selected, or pagination). Try to avoid names overlapping with each other too. Don't make getters/ setters that are external to the variable. Use the same data in all places - the standard controller, the getOptions method, and your own controller data.

---

Rewritten to use a component instead of a page. Compatible with standard set controller & other callers.

Page

<apex:page standardController="Exercise__c" recordSetVar="Exercises">
  <c:ExerciseList records_param="{!selected}" />
</apex:page>

Component

<apex:component allowDml="true" controller="ExerciseListController">
    <apex:attribute name="records_param" type="List<Exercise__c>" assignTo="{!Records}" description="Records from Standard Set Controller" required="true" /> 

    <apex:form >
      <apex:pageBlock id="ExerciseList">
      <!-- Important to have page messages in rerender area -->
      <apex:pageMessages />
          <apex:pageBlockTable Value="{!Exercises}" var="e" rendered="{!Excercise.size > 0}">
              <apex:column HeaderValue="Add to Wod" width="60">
              <!-- Uses its own property & not a map, either way is fine tbh --> 
                 <apex:inputCheckbox value="{!e.Selected}"/>
              </apex:column>
              <apex:column headerValue="Exercise Name">
                  <apex:outputLink value="/{!e.id}" target="_blank">
                      {!e.Name}
                  </apex:outputLink>
              </apex:column>
              <!-- removed dupe column can easily add it back --> 
          </apex:pageBlockTable>
          <!-- can directly acces size of list on page --> 
          <apex:outputLabel value="No record to display" rendered="{!Excercise.size == 0}" styleClass="noRowsHeader"></apex:outputLabel>
          <apex:pageBlockButtons >
            <!-- make sure your rerender points to a named panel --> 
              <apex:commandButton reRender="ExerciseList" value="Select" action="{!SelectExercisesForWod}"/>
          </apex:pageBlockButtons>
      </apex:pageBlock>
  </apex:form>
</apex:component>

Controller

public class ExerciseListController {

    // wrapper holds all data & extra, page only data we add to class 
    public class ExerciseWrapper {
        // returns based on given object 
        public Id Id { get { return Exercise.Id; } }
        public String Name { get { return Exercise.Name; } }

        // Custom value to store selected wrapper values 
        public Boolean Selected { get; set; }

        // Source of data, made public to access any fields not given above
        // can set just setter to private 
        public Exercise__c Exercise { get; private set; }

        public ExerciseWrapper(Exercise__c Exercise) {
            this.Exercise = Exercise; 
            this.selected = false; 
        }
    }

    // set via param 
    public List<Exercise__c> Records { get; set; }

    public List<ExerciseWrapper> Exercises { 
        get {
            // Null Check Records 
            if (Excersizes == null) {
                Exercises = new List<Exercise__c>();

                for (Exercise__c e : Records) {
                    Exercises.add(new ExerciseWrapper(e));
                }
            }
        }
        set; 
    }

    public String WodID { get {
        if (WodID == null) {
            // assume set somewhere not set in your code 
            WodId = (Id)ApexPages.CurrentPage.getParameters.get('WodID');
        }

        return WodID;
    } set; } 

    // params are not set at constructor time 
    public ExerciseListController() {}

    public pageReference SelectExercisesForWod() { 
        List<Exercise__c> udpateList = new List<Exercise__c>(); 

        for (ExerciseWrapper wrapper : Exercises) {
            if (wrapper.Selected) {
                // assume your setting some fields 
                Exercise__c e = wrapper.Exercise;
                // get records, set some field to given id 
                e.Wod_Id__c = WodId; 
                // add to update list 
                udpateList.add(e);
            }
        }

        // add page message "you must select at least one record" if size == 0

        try {
            // try update 
            update updateList; 
            // return to dummy result page 
            return new PageReference('/apex/someResultPage');
        } catch (Exception e) {
            // Set error on returned page 
            ApexPages.CurrentPage.addMessage(new ApexPages.Message(e));
        }
        // refreshes page & would show error in page message 
        return null;
    }
}

---

Move your oncomplete return into your action, so its returned when the server is done.

public pageReference SelectExercisesForWod(){
    insert someData; // youll probably want to return a conditional value based on your save result anyway 

    return new PageReference('/');
}

You could try to do both, but really you're only going to redirect to a single page at a time. Trying to use both oncomplete and return pageRef is asking for problems.

Answer 2

You will have to pass the page name when initializing the PageReference.

So, your controller code should be

public PageReference SelectExercisesForWod(){ 
    // Your code goes here
    return new PageReference('/apex/YourPageName'); 
}

Or you can set the page name directly, so that it verifies the page name at compile time.

public PageReference SelectExercisesForWod(){ 
    // Your code goes here
    return Page.YourPageName; 
}

If you currently don't have any visualforce page created, then I would advice you to create a dummy page to which you can redirect and can refer that dummy page till the time you create the actual page.

public PageReference SelectExercisesForWod(){ 
    // Your code goes here
    return Page.DummyPage; 
}

Also make sure that development mode is switched OFF when testing this, as redirect might not happen properly when development mode is ON.