Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.4
    • Fix Version/s: 1.5 M1
    • Component/s: extras
    • Labels:
      None

      Description

      I have a strange PropertySelect problem. I have a CayenneForm with
      some TextFields and some PropertySelects. The page also have a table
      where users can list entries to edit. It is very much like
      http://www.avoka.com/click-examples/cayenne/cayenne-form-page.htm

      Pressing the edit-link in the table works fine. It does a
      form.setDataObject(theDataObject).

      I want kind of the same functionality for the cancel-button in the
      CayenneForm. If user does some editing and press cancel, the changes
      are lost, but the entry is still up for editing. This how our users
      want the cancel-button to work.. As this is kind of the same as the
      edit-link, I have implemented this the following way:

      public boolean onCancelClick()

      { form.clearErrors(); form.getDataContext().rollbackChanges(); form.setDataObject(dataObject); return true; }

      Should be almost the same thing as for the edit-button in the table
      row. It is working for all fields except the PropertySelects. Those
      are not reset to their original value, but set to blank.

      I raised this issue on the list and it was confirmed. See
      http://sourceforge.net/mailarchive/forum.php?thread_name=4A54D954-4067-4B74-ABCF-3419D6D06060%40pvv.ntnu.no&forum_name=click-user

        Activity

        Hide
        Malcolm Edgar added a comment -

        Fix checked in, also solves some behavioural issues with stateful pages. Tricky little bugs.

        Show
        Malcolm Edgar added a comment - Fix checked in, also solves some behavioural issues with stateful pages. Tricky little bugs.
        Hide
        Malcolm Edgar added a comment -

        I think this is a different problem, than the container concept, but I haven't given this much thought.

        Tore has concerns about the tight coupling between CayenneForm and PropertySelect. I am less concerned about this, but need to work through some design solutions, including Tores.

        regards Malcolm Edgar

        Show
        Malcolm Edgar added a comment - I think this is a different problem, than the container concept, but I haven't given this much thought. Tore has concerns about the tight coupling between CayenneForm and PropertySelect. I am less concerned about this, but need to work through some design solutions, including Tores. regards Malcolm Edgar
        Hide
        Bob Schellink added a comment -

        Hi Malcolm,

        Should we add the method copyFrom / copyTo as suggested by Tore?

        Just checking if this approach might be better with upcoming container concept.

        regards

        bob

        Show
        Bob Schellink added a comment - Hi Malcolm, Should we add the method copyFrom / copyTo as suggested by Tore? Just checking if this approach might be better with upcoming container concept. regards bob
        Hide
        Malcolm Edgar added a comment -

        Currently working on this item, slowly.

        Show
        Malcolm Edgar added a comment - Currently working on this item, slowly.
        Hide
        Tore Halset added a comment -

        Inspired by Malcolms CayenneForm.copyFrom, I have added this to my click installation. It seem to work fine for me.

        Added the following method to PropertySelect:
        public void copyFrom(Object object) {
        // TODO: super.copyFrom(object);
        CayenneForm form = (CayenneForm) getForm();
        Class doClass = form.getDataObjectClass();
        String getterName = ClickUtils.toGetterName(getName());

        try {
        Method method = doClass.getMethod(getterName, null);
        DataObject property = (DataObject) method.invoke(object, null);
        if (property == null)

        { setValue(null); }

        else

        { Object propPk = DataObjectUtils.pkForObject(property); setValue(propPk.toString()); }

        } catch (Exception e)

        { throw new RuntimeException(e); }

        }

        Added the following method to CayenneForm:
        public void copyFrom(Object object) {
        super.copyFrom(object);

        final List fieldList = ClickUtils.getFormFields(this);

        for (int i = 0; i < fieldList.size(); i++) {
        Field field = (Field) fieldList.get;

        // TODO: create Field.copyFrom(Object object) and make ClickUtils
        // call it so that we do not need
        // special handling here?
        if (field instanceof PropertySelect)

        { PropertySelect select = (PropertySelect) field; select.copyFrom(object); }

        }

        }

        As you see, I prefer adding a method to all Fields to get rid of special cases controlled by if...instanceby..

        I have seen the instanceby-if-sentence several times in click source. F.eks. ClickUtils.getFormFields that does not include Labels. Those special cases are likely to introduce bugs and makes it more difficult to expand the framework (yes, I know instanceof handles subclasses and such).

        Show
        Tore Halset added a comment - Inspired by Malcolms CayenneForm.copyFrom, I have added this to my click installation. It seem to work fine for me. Added the following method to PropertySelect: public void copyFrom(Object object) { // TODO: super.copyFrom(object); CayenneForm form = (CayenneForm) getForm(); Class doClass = form.getDataObjectClass(); String getterName = ClickUtils.toGetterName(getName()); try { Method method = doClass.getMethod(getterName, null); DataObject property = (DataObject) method.invoke(object, null); if (property == null) { setValue(null); } else { Object propPk = DataObjectUtils.pkForObject(property); setValue(propPk.toString()); } } catch (Exception e) { throw new RuntimeException(e); } } Added the following method to CayenneForm: public void copyFrom(Object object) { super.copyFrom(object); final List fieldList = ClickUtils.getFormFields(this); for (int i = 0; i < fieldList.size(); i++) { Field field = (Field) fieldList.get ; // TODO: create Field.copyFrom(Object object) and make ClickUtils // call it so that we do not need // special handling here? if (field instanceof PropertySelect) { PropertySelect select = (PropertySelect) field; select.copyFrom(object); } } } As you see, I prefer adding a method to all Fields to get rid of special cases controlled by if...instanceby.. I have seen the instanceby-if-sentence several times in click source. F.eks. ClickUtils.getFormFields that does not include Labels. Those special cases are likely to introduce bugs and makes it more difficult to expand the framework (yes, I know instanceof handles subclasses and such).

          People

          • Assignee:
            Malcolm Edgar
            Reporter:
            Tore Halset
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development