Details

      Description

      I noticed that the first visist element in a controller.xml is never been called.
      Here is a proposal patch to solve this issue.

      Now everytime when a session is created, the first visit element will be called.
      I don't now if this is the best solution, but maybe we can discuss

      So long
      Sascha

        Activity

        Hide
        Sascha Rodekamp added a comment -

        Hey Thanks Scott. And hava a good Weekend!!

        Show
        Sascha Rodekamp added a comment - Hey Thanks Scott. And hava a good Weekend!!
        Scott Gray made changes -
        Fix Version/s Release Branch 10.04 [ 12314832 ]
        Scott Gray made changes -
        Status Open [ 1 ] Closed [ 6 ]
        Resolution Fixed [ 1 ]
        Hide
        Scott Gray added a comment -

        Thanks Sascha, fixed (and tested) in revs:
        trunk - 980747
        10.04 - 980764

        Show
        Scott Gray added a comment - Thanks Sascha, fixed (and tested) in revs: trunk - 980747 10.04 - 980764
        Scott Gray committed 980764 (1 file)
        Reviews: none

        Merged from trunk r980747:
        Fix for OFBIZ-3786 reported by Sascha Rodekamp, first-visit controller events were not being executed. Fix based on a simplified version of the patch provided by Sascha.

        Scott Gray committed 980747 (1 file)
        Reviews: none

        Fix for OFBIZ-3786 reported by Sascha Rodekamp, first-visit controller events were not being executed. Fix based on a simplified version of the patch provided by Sascha.

        Scott Gray made changes -
        Assignee Jacques Le Roux [ jacques.le.roux ] Scott Gray [ lektran ]
        Hide
        Scott Gray added a comment -

        Hi Sascha,

        Sorry maybe I didn't do a good job at explaining my feedback, instead of trying again I'll just commit what I was talking about and you can see what I mean

        Show
        Scott Gray added a comment - Hi Sascha, Sorry maybe I didn't do a good job at explaining my feedback, instead of trying again I'll just commit what I was talking about and you can see what I mean
        Hide
        Sascha Rodekamp added a comment -

        Hi Arun, Hi Scott,

        Atul thanks for testing.
        Scott, i know this is really not the best solution, but it was necassary in our project.

        • idon't think that you know about the first visit when looking on the absence of the value, because the value is set in the context filter
        • i agree on you're second point maybe there is a better solution But i think at the moment it's a better solution.

        Cheers
        Sascha

        Show
        Sascha Rodekamp added a comment - Hi Arun, Hi Scott, Atul thanks for testing. Scott, i know this is really not the best solution, but it was necassary in our project. idon't think that you know about the first visit when looking on the absence of the value, because the value is set in the context filter i agree on you're second point maybe there is a better solution But i think at the moment it's a better solution. Cheers Sascha
        Hide
        Scott Gray added a comment -

        I don't see a better approach but a couple of small comments on the patch:

        • Why bother setting firstVisit when the session is created? We will know if it is the first visit anyway because of an absence of the value
        • If the visit has already been created then there is no point in leaving behind the call to VisitHandler.getVisit(...) under the condition
        Show
        Scott Gray added a comment - I don't see a better approach but a couple of small comments on the patch: Why bother setting firstVisit when the session is created? We will know if it is the first visit anyway because of an absence of the value If the visit has already been created then there is no point in leaving behind the call to VisitHandler.getVisit(...) under the condition
        Hide
        Arun Patidar added a comment -

        Thanks Sascha,

        Your patch is working fine for me. I think It should be committed in trunk to fix bug.

        Thanks & Regards

        Arun Patidar

        Show
        Arun Patidar added a comment - Thanks Sascha, Your patch is working fine for me. I think It should be committed in trunk to fix bug. Thanks & Regards — Arun Patidar
        Jacques Le Roux made changes -
        Assignee Jacques Le Roux [ jacques.le.roux ]
        Hide
        Sascha Rodekamp added a comment -

        Hi Jacques i'll try to explain what's in my mind

        controller.xml specialpurpose/ecommerce

         
        <!-- Events run from here for the first hit in a visit --> 
        <firstvisit> 
                <event name="autoLoginCheck" type="java" path="org.ofbiz.webapp.control.LoginWorker" invoke="autoLoginCheck"/> 
              <event name="checkTrackingCodeCookies" type="java" path="org.ofbiz.marketing.tracking.TrackingCodeEvents" invoke="checkTrackingCodeCookies"/> 
              <event name="setDefaultStoreSettings" type="java" path="org.ofbiz.product.product.ProductEvents" invoke="setDefaultStoreSettings"/> 
        </firstvisit> 
         

        The Code in firstVisit is never used, because of the wrong if-statement in the RequestHandler.

        if (this.trackVisit(request) && session.getAttribute("visit") == null) { 
        ... 
        } 
        
        

        session.getAttribute("visit") is never null. It is set in the ControlServlet before. (VisitHandler.getVisitId(session); )

        Show
        Sascha Rodekamp added a comment - Hi Jacques i'll try to explain what's in my mind controller.xml specialpurpose/ecommerce <!-- Events run from here for the first hit in a visit --> <firstvisit> <event name= "autoLoginCheck" type= "java" path= "org.ofbiz.webapp.control.LoginWorker" invoke= "autoLoginCheck" /> <event name= "checkTrackingCodeCookies" type= "java" path= "org.ofbiz.marketing.tracking.TrackingCodeEvents" invoke= "checkTrackingCodeCookies" /> <event name= "setDefaultStoreSettings" type= "java" path= "org.ofbiz.product.product.ProductEvents" invoke= "setDefaultStoreSettings" /> </firstvisit> The Code in firstVisit is never used, because of the wrong if-statement in the RequestHandler. if ( this .trackVisit(request) && session.getAttribute( "visit" ) == null ) { ... } session.getAttribute("visit") is never null. It is set in the ControlServlet before. (VisitHandler.getVisitId(session); )
        Hide
        Jacques Le Roux added a comment -

        But Sascha, this is already hanlded by the VisitHandler.java and it does not depend only on a session created. What are you trying to achieve?

        Show
        Jacques Le Roux added a comment - But Sascha, this is already hanlded by the VisitHandler.java and it does not depend only on a session created. What are you trying to achieve?
        Hide
        Sascha Rodekamp added a comment -

        So have nobody an opinion to this issue?

        Show
        Sascha Rodekamp added a comment - So have nobody an opinion to this issue?
        Sascha Rodekamp made changes -
        Field Original Value New Value
        Attachment OFBIZ-3786_RequestHandler.java.patch [ 12445752 ]
        Sascha Rodekamp created issue -

          People

          • Assignee:
            Scott Gray
            Reporter:
            Sascha Rodekamp
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development