ICEfaces
  1. ICEfaces
  2. ICE-5975

The "Bid declined." message does not display when invalid bid is submitted using the accept bid button

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.8.2-EE-GA_P01, 2.0-Beta1
    • Fix Version/s: 1.8.3, 1.8.2-EE-GA_P02, 2.0.0
    • Component/s: Sample Apps
    • Labels:
      None
    • Environment:
      ICEfaces 1 ICEfaces
    • Assignee Priority:
      P1
    • Affects:
      Sample App./Tutorial

      Description

      In both the 1.8 version and ICEfaces 2 versions of the Auction Monitor, the "Bid declined." message only appears if you submit an invalid bid using the Enter key. If you submit an invalid bid using the accept bid button (ie the green checkmark), the bid will be rejected but the message will not appear.

        Activity

        Deryk Sinotte created issue -
        Deryk Sinotte made changes -
        Field Original Value New Value
        Salesforce Case []
        Assignee Priority P3
        Assignee Ken Fyten [ ken.fyten ]
        Priority Major [ 3 ] Minor [ 4 ]
        Hide
        Deryk Sinotte added a comment -

        This issue was originally flagged as a regression in ICE-5915.

        Show
        Deryk Sinotte added a comment - This issue was originally flagged as a regression in ICE-5915 .
        Ken Fyten made changes -
        Salesforce Case []
        Fix Version/s 2.0.0 [ 10230 ]
        Fix Version/s 1.8.2-EE-GA_P02 [ 10226 ]
        Fix Version/s 1.8.3 [ 10211 ]
        Assignee Priority P3 P2
        Assignee Ken Fyten [ ken.fyten ] Patrick Corless [ patrick.corless ]
        Hide
        Patrick Corless added a comment -

        This is a perfect example of why we don't tell users to do work on setters and getters.

        The inputText for a bid is value bound to the instance variable tempLocalBid in the AuctionMonitorItemBean. The setter for the tempLocalBid checks to see if the bid is valid. If valid the setter assigns the new value and then calls the action Method setLocalBid. If not valid no value assignment is made and a boolean flag to show the bid error message is set to true. From a JSF point of view a lot of rules are broken with this approach but the application works as expected and the error message will show up if the bid is invalid and the original value is kept.

        When the bid action event is processed with an invalid bid we don't see the error message because of the work done on the setter. When the bid button is pressed a new JSF lifecycle is kicked off. Because we don't use an JSF validator the Bean setters all get called at which point the above logic kicks in effectively wiping the bad value. Next our action event is processed but the model has the old original value because of the setter interfered with the data. The action logic sees that there is nothing wrong with the bid and sets the boolean flag to hide the message.

        The proper way to handle this logic is to avoid doing any work on the setter. Instead opting for a bidValitor that if fails prevents the action from being executed. And avoids the doubling up of logic like we are currently seeing.

        Show
        Patrick Corless added a comment - This is a perfect example of why we don't tell users to do work on setters and getters. The inputText for a bid is value bound to the instance variable tempLocalBid in the AuctionMonitorItemBean. The setter for the tempLocalBid checks to see if the bid is valid. If valid the setter assigns the new value and then calls the action Method setLocalBid. If not valid no value assignment is made and a boolean flag to show the bid error message is set to true. From a JSF point of view a lot of rules are broken with this approach but the application works as expected and the error message will show up if the bid is invalid and the original value is kept. When the bid action event is processed with an invalid bid we don't see the error message because of the work done on the setter. When the bid button is pressed a new JSF lifecycle is kicked off. Because we don't use an JSF validator the Bean setters all get called at which point the above logic kicks in effectively wiping the bad value. Next our action event is processed but the model has the old original value because of the setter interfered with the data. The action logic sees that there is nothing wrong with the bid and sets the boolean flag to hide the message. The proper way to handle this logic is to avoid doing any work on the setter. Instead opting for a bidValitor that if fails prevents the action from being executed. And avoids the doubling up of logic like we are currently seeing.
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #22387 Tue Sep 21 07:46:22 MDT 2010 patrick.corless ICE-5975 updated bean logic to used standard jsf validation for the bid input which fixes bid error message from not showing up.
        Files Changed
        Commit graph MODIFY /icefaces2/trunk/icefaces/compat/samples/auctionMonitor/web/auctionMonitor.jspx
        Commit graph MODIFY /icefaces2/trunk/icefaces/compat/samples/auctionMonitor/src/com/icesoft/applications/faces/auctionMonitor/beans/AuctionMonitorItemBean.java
        Commit graph ADD /icefaces2/trunk/icefaces/compat/samples/auctionMonitor/src/com/icesoft/applications/faces/auctionMonitor/validators
        Commit graph MODIFY /icefaces2/trunk/icefaces/compat/samples/auctionMonitor/web/WEB-INF/faces-config.xml
        Commit graph MODIFY /icefaces2/trunk/icefaces/compat/samples/auctionMonitor/web/auctionMonitor.xhtml
        Commit graph ADD /icefaces2/trunk/icefaces/compat/samples/auctionMonitor/src/com/icesoft/applications/faces/auctionMonitor/validators/BidValidator.java
        Commit graph MODIFY /icefaces2/trunk/icefaces/compat/samples/auctionMonitor/src/com/icesoft/applications/faces/auctionMonitor/beans/AuctionBean.java
        Hide
        Patrick Corless added a comment -

        Updated the input field to use a custom bid validator to validate bid values. For the most part standard validation is used but we still store the error message in a bean to avoid the message from getting cleared on a server push.

        Show
        Patrick Corless added a comment - Updated the input field to use a custom bid validator to validate bid values. For the most part standard validation is used but we still store the error message in a bean to avoid the message from getting cleared on a server push.
        Patrick Corless made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Ken Fyten added a comment -

        Please apply these fixes to the icefaces/trunk/samples/auction-monitor version also.

        Show
        Ken Fyten added a comment - Please apply these fixes to the icefaces/trunk/samples/auction-monitor version also.
        Ken Fyten made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Hide
        Ken Fyten added a comment -

        Oops. Testing shows that a "Bid declined, too small" message appears even when you enter a valid bid.
        Also, there doesn't seem to be any max. bid, when the app. says you can't bid more than 1,000,000 more than the last bid.

        Also, suggest using "too low/high" instead of "small.big" in error messages.

        Show
        Ken Fyten added a comment - Oops. Testing shows that a "Bid declined, too small" message appears even when you enter a valid bid. Also, there doesn't seem to be any max. bid, when the app. says you can't bid more than 1,000,000 more than the last bid. Also, suggest using "too low/high" instead of "small.big" in error messages.
        Ken Fyten made changes -
        Salesforce Case []
        Affects [Sample App./Tutorial]
        Assignee Patrick Corless [ patrick.corless ] Carlo Guglielmin [ carlo.guglielmin ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #23081 Wed Nov 10 10:25:11 MST 2010 carlo.guglielmin ICE-5975 - Changed messages to 'too low/high' instead of 'too small/large'.
        Files Changed
        Commit graph MODIFY /icefaces2/trunk/icefaces/compat/samples/auctionMonitor/src/com/icesoft/applications/faces/auctionMonitor/validators/BidValidator.java
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #23083 Wed Nov 10 10:39:45 MST 2010 carlo.guglielmin ICE-5975 - Ported BidValidator change to the original 1.8.x auctionMonitor example, instead of just in the 2.0 compat version
        Files Changed
        Commit graph MODIFY /icefaces/trunk/icefaces/samples/auctionMonitor/src/com/icesoft/applications/faces/auctionMonitor/beans/AuctionBean.java
        Commit graph MODIFY /icefaces/trunk/icefaces/samples/auctionMonitor/web/auctionMonitor.jspx
        Commit graph MODIFY /icefaces/trunk/icefaces/samples/auctionMonitor/web/WEB-INF/faces-config.xml
        Commit graph ADD /icefaces/trunk/icefaces/samples/auctionMonitor/src/com/icesoft/applications/faces/auctionMonitor/validators/BidValidator.java
        Commit graph MODIFY /icefaces/trunk/icefaces/samples/auctionMonitor/src/com/icesoft/applications/faces/auctionMonitor/beans/AuctionMonitorItemBean.java
        Commit graph ADD /icefaces/trunk/icefaces/samples/auctionMonitor/src/com/icesoft/applications/faces/auctionMonitor/validators
        Hide
        Carlo Guglielmin added a comment -

        So the BidValidator added by Pat seems to have fixed this issue. In my testing the "Bid declined, too small" message appears properly if the bid is less than the current bid, otherwise no message appears (as expected). So that looks like it's working.

        I did notice that the original icefaces/trunk/ didn't receive these changes, so I'll port those across.

        Show
        Carlo Guglielmin added a comment - So the BidValidator added by Pat seems to have fixed this issue. In my testing the "Bid declined, too small" message appears properly if the bid is less than the current bid, otherwise no message appears (as expected). So that looks like it's working. I did notice that the original icefaces/trunk/ didn't receive these changes, so I'll port those across.
        Hide
        Ken Fyten added a comment -

        This seems to be working fine now on icefaces/trunk. Can you change it so the default next bid value is always $1 higher than the current bid?

        Show
        Ken Fyten added a comment - This seems to be working fine now on icefaces/trunk. Can you change it so the default next bid value is always $1 higher than the current bid?
        Hide
        Ken Fyten added a comment -

        Also noticing this warning in the tomcat log after testing IF2 compat auctionMonitor. There is one entry for each invalid response, although the messages did show up on the browser correctly:

        Nov 10, 2010 12:25:25 PM com.sun.faces.renderkit.RenderKitUtils renderUnhandledMessages
        INFO: WARNING: FacesMessage(s) have been enqueued, but may not have been displayed.
        sourceId=iceForm:iceTable:1:item_localBid[severity=(ERROR 2), summary=(null), detail=(<br />Bid declined, too low.)]

        Show
        Ken Fyten added a comment - Also noticing this warning in the tomcat log after testing IF2 compat auctionMonitor. There is one entry for each invalid response, although the messages did show up on the browser correctly: Nov 10, 2010 12:25:25 PM com.sun.faces.renderkit.RenderKitUtils renderUnhandledMessages INFO: WARNING: FacesMessage(s) have been enqueued, but may not have been displayed. sourceId=iceForm:iceTable:1:item_localBid [severity=(ERROR 2), summary=(null), detail=(<br />Bid declined, too low.)]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #23088 Wed Nov 10 14:27:01 MST 2010 carlo.guglielmin ICE-5975 - Added code to ensure the current bid autodefaults to 'high bid + 1'. Also put a maxlength on the bid field
        Files Changed
        Commit graph MODIFY /icefaces2/trunk/icefaces/compat/samples/auctionMonitor/src/com/icesoft/applications/faces/auctionMonitor/beans/AuctionMonitorItemBean.java
        Commit graph MODIFY /icefaces2/trunk/icefaces/compat/samples/auctionMonitor/web/auctionMonitor.xhtml
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #23089 Wed Nov 10 14:29:43 MST 2010 carlo.guglielmin ICE-5975 - Added code to ensure the current bid autodefaults to 'high bid + 1'. Also put a maxlength on the bid field
        Files Changed
        Commit graph MODIFY /icefaces/trunk/icefaces/samples/auctionMonitor/web/auctionMonitor.jspx
        Commit graph MODIFY /icefaces/trunk/icefaces/samples/auctionMonitor/src/com/icesoft/applications/faces/auctionMonitor/beans/AuctionMonitorItemBean.java
        Hide
        Carlo Guglielmin added a comment -

        I've added functionality so the "new bid" field defaults to $1 higher than the current max bid, for both the 1.8.x version and the 2.0 compat version.

        Show
        Carlo Guglielmin added a comment - I've added functionality so the "new bid" field defaults to $1 higher than the current max bid, for both the 1.8.x version and the 2.0 compat version.
        Hide
        Carlo Guglielmin added a comment -

        Marking as fixed as validation works and a few improvements listed under this bug have been checked in.

        Show
        Carlo Guglielmin added a comment - Marking as fixed as validation works and a few improvements listed under this bug have been checked in.
        Carlo Guglielmin made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Ken Fyten added a comment -

        One last thing, the app. states that any individual bid cannot be more than $1,000,000 more than the current price, but the validator is not enforcing this now.

        I'd suggest that we reduce that limit to $10,000 and enforce it. The intent is to reduce the immediate expansion of the amount fields such that they no longer render nicely in the space provided, etc.

        Show
        Ken Fyten added a comment - One last thing, the app. states that any individual bid cannot be more than $1,000,000 more than the current price, but the validator is not enforcing this now. I'd suggest that we reduce that limit to $10,000 and enforce it. The intent is to reduce the immediate expansion of the amount fields such that they no longer render nicely in the space provided, etc.
        Ken Fyten made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #23134 Tue Nov 16 13:17:32 MST 2010 carlo.guglielmin ICE-5975 - Changed max bid increase to 10,000 in the BidValidator and in the page level message
        Files Changed
        Commit graph MODIFY /icefaces2/trunk/icefaces/compat/samples/auctionMonitor/web/auctionMonitor.xhtml
        Commit graph MODIFY /icefaces2/trunk/icefaces/compat/samples/auctionMonitor/src/com/icesoft/applications/faces/auctionMonitor/validators/BidValidator.java
        Hide
        Carlo Guglielmin added a comment -

        r23134 - I updated the BidValidator so the max bid increase is $10,000. Also I changed the message on the page to reflect this. Only the 1.8.x compat version of auctionMonitor needed this change, as it was existing in the current 2.0 version already.

        Show
        Carlo Guglielmin added a comment - r23134 - I updated the BidValidator so the max bid increase is $10,000. Also I changed the message on the page to reflect this. Only the 1.8.x compat version of auctionMonitor needed this change, as it was existing in the current 2.0 version already.
        Carlo Guglielmin made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Repository Revision Date User Message
        ICEsoft Public SVN Repository #23147 Wed Nov 17 14:46:55 MST 2010 carlo.guglielmin ICE-5975 - Changed max bid increase to 10,000 in the BidValidator and in the page level message
        Files Changed
        Commit graph MODIFY /icefaces/trunk/icefaces/samples/auctionMonitor/web/auctionMonitor.jspx
        Commit graph MODIFY /icefaces/trunk/icefaces/samples/auctionMonitor/src/com/icesoft/applications/faces/auctionMonitor/validators/BidValidator.java
        Hide
        Ken Fyten added a comment -

        Carlo, the commit for the max $10K bid limit was made to icefaces2/trunk, not icefaces/trunk. Please review and correct.

        Show
        Ken Fyten added a comment - Carlo, the commit for the max $10K bid limit was made to icefaces2/trunk, not icefaces/trunk. Please review and correct.
        Ken Fyten made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Assignee Priority P2 P1
        Hide
        Carlo Guglielmin added a comment -

        r23147 - Ported max bid change to icefaces/trunk.

        Show
        Carlo Guglielmin added a comment - r23147 - Ported max bid change to icefaces/trunk.
        Carlo Guglielmin made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Mandeep Hayher added a comment -

        Tested successfully on:
        Icefaces1.8 trunk revision# 23161
        Server: Tomcat6
        Browsers: FF3.6, IE8 & Opera10.60

        Show
        Mandeep Hayher added a comment - Tested successfully on: Icefaces1.8 trunk revision# 23161 Server: Tomcat6 Browsers: FF3.6, IE8 & Opera10.60
        Mandeep Hayher made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Carlo Guglielmin
            Reporter:
            Deryk Sinotte
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: