Details

      Description

      In the following issue : OFBIZ-7922, there was a problem with the pagination of the Marketing Campaing list. Because there is not enough data in the demo, it was more difficult to find this bug.

      To help the testing of this component, we can add some Marketing Campaign in the demo data (at least enough to unlock a second page of search).

      1. OFBIZ-7923.patch
        6 kB
        Montalbano Florian
      2. OFBIZ-7923.patch
        6 kB
        Montalbano Florian

        Issue Links

          Activity

          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          I quickly tried to move the DemoMarketing.xml file in marketing/data/ but using component syntax does not work in ofbiz-component.xml files:

          <entity-resource type="data" reader-name="demo" loader="main" location="component://marketing/data/DemoMarketing.xml"/>
          

          does not work, you get

          2016-08-01 16:07:31,434 |main                 |EntityDataLoader              |W| Could not get URL for Component ResourceHandler: org.apache.ofbiz.base.component.ComponentException: File Resource not found: 
          C:/projectASF-Mars/ofbiz/specialpurpose/ecommerce/component://marketing/data/DemoMarketing.xml
          

          The loaded in ofbiz-component.xml refers to the local component, hence C:/projectASF-Mars/ofbiz/specialpurpose/ecommerce/component://marketing/data/DemoMarketing.xml

          We need another solution, or to code that there...later...

          Show
          jacques.le.roux Jacques Le Roux added a comment - I quickly tried to move the DemoMarketing.xml file in marketing/data/ but using component syntax does not work in ofbiz-component.xml files: <entity-resource type= "data" reader-name= "demo" loader= "main" location= "component: //marketing/data/DemoMarketing.xml" /> does not work, you get 2016-08-01 16:07:31,434 |main |EntityDataLoader |W| Could not get URL for Component ResourceHandler: org.apache.ofbiz.base.component.ComponentException: File Resource not found: C:/projectASF-Mars/ofbiz/specialpurpose/ecommerce/component: //marketing/data/DemoMarketing.xml The loaded in ofbiz-component.xml refers to the local component, hence C:/projectASF-Mars/ofbiz/specialpurpose/ecommerce/component://marketing/data/DemoMarketing.xml We need another solution, or to code that there...later...
          Hide
          pfm.smits Pierre Smits added a comment -

          Thanks!

          Show
          pfm.smits Pierre Smits added a comment - Thanks!
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Oh right, this was wrong in 1st place (pre Apache era). I'll do it!

          Show
          jacques.le.roux Jacques Le Roux added a comment - Oh right, this was wrong in 1st place (pre Apache era). I'll do it!
          Hide
          pfm.smits Pierre Smits added a comment -

          In light of removing the dependencies on the ecommerce commerce (OFBIZ-7016) , this patch should not have gone into the datasets of the ecommerce component. Instead, it should have gone into the demo data set of the marketing component.

          Will you correct this situation, Jacques Le Roux?

          Show
          pfm.smits Pierre Smits added a comment - In light of removing the dependencies on the ecommerce commerce ( OFBIZ-7016 ) , this patch should not have gone into the datasets of the ecommerce component. Instead, it should have gone into the demo data set of the marketing component. Will you correct this situation, Jacques Le Roux ?
          Hide
          Florian M Montalbano Florian added a comment -

          I found where this replacement was done but before opening a Jira, I would like to fully understand the comment put above the replace : (found in the file GenericEntity.java at line 1166 in the function writeXmlText)

          /*
          * Some common character for these invalid values, have seen these are mostly from MS Word, but may be part of some standard:
          * 5 = ... 18 = apostrophe 19 = left quotation mark 20 = right quotation mark 22 = – 23 = - 25 = tm
          */
          

          After this, there is a switch checking some specials characters such as ' , " , & , < and > .
          If useful, we could remove the replace for all those characters. But I don't know the range of this modification. Do you have more intel about this matter ?

          Show
          Florian M Montalbano Florian added a comment - I found where this replacement was done but before opening a Jira, I would like to fully understand the comment put above the replace : (found in the file GenericEntity.java at line 1166 in the function writeXmlText) /* * Some common character for these invalid values, have seen these are mostly from MS Word, but may be part of some standard: * 5 = ... 18 = apostrophe 19 = left quotation mark 20 = right quotation mark 22 = – 23 = - 25 = tm */ After this, there is a switch checking some specials characters such as ' , " , & , < and > . If useful, we could remove the replace for all those characters. But I don't know the range of this modification. Do you have more intel about this matter ?
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Thanks Florian, your patch is in trunk at revision: 1754657.

          Please if we should create another Jira for generalising a fix to the quote issue.

          Show
          jacques.le.roux Jacques Le Roux added a comment - Thanks Florian, your patch is in trunk at revision: 1754657. Please if we should create another Jira for generalising a fix to the quote issue.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Actually you don't even have to add more data. The size of the list is defined by this property in widget.properties

          # Default number of items to be displayed per page in a list form
          widget.form.defaultViewSize=20
          

          Locally you can change that by hand, clear caches, et voilà.

          Now if we would like to have the same on demos we would need to have this property in the DB and use EntityUtilProperties.getPropertyNumber() to retrieve where it's done at the moment. Unfortunately EntityUtilProperties.getPropertyNumber() simply uses for the moment UtilProperties.getPropertyNumber() so it means more work than adding those data. But once done, it's for all (though you would still have to replace the calls where UtilProperties is used instead of EntityUtilProperties. I don't remember if we have an umbrella task ans subtasks for that. It seems though a long term work, because we have much properties likely to benefit from this possibility.

          Another KISS option: maybe simply testing locally is sufficient

          Show
          jacques.le.roux Jacques Le Roux added a comment - Actually you don't even have to add more data. The size of the list is defined by this property in widget.properties # Default number of items to be displayed per page in a list form widget.form.defaultViewSize=20 Locally you can change that by hand, clear caches, et voilà. Now if we would like to have the same on demos we would need to have this property in the DB and use EntityUtilProperties.getPropertyNumber() to retrieve where it's done at the moment. Unfortunately EntityUtilProperties.getPropertyNumber() simply uses for the moment UtilProperties.getPropertyNumber() so it means more work than adding those data. But once done, it's for all (though you would still have to replace the calls where UtilProperties is used instead of EntityUtilProperties. I don't remember if we have an umbrella task ans subtasks for that. It seems though a long term work, because we have much properties likely to benefit from this possibility. Another KISS option: maybe simply testing locally is sufficient
          Hide
          Florian M Montalbano Florian added a comment -

          Thanks for the review
          By the hardest way, do you mean updating the function Exporting All Data ?
          I think it may be better to open a new issue for that. But I didn't check the code yet so it may be a specific issue to exporting this entity (or maybe the &quote appears when creating a marketing campaign ?).

          If those datas are ok, I'll add the remaining one. I'll stop when we can have at least a navigation to the second page (for the testing purpose).

          Show
          Florian M Montalbano Florian added a comment - Thanks for the review By the hardest way, do you mean updating the function Exporting All Data ? I think it may be better to open a new issue for that. But I didn't check the code yet so it may be a specific issue to exporting this entity (or maybe the &quote appears when creating a marketing campaign ?). If those datas are ok, I'll add the remaining one. I'll stop when we can have at least a navigation to the second page (for the testing purpose).
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          BTW your data look good to me, thanks!

          Show
          jacques.le.roux Jacques Le Roux added a comment - BTW your data look good to me, thanks!
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          So you did not go the hardest way May I ask why?

          Show
          jacques.le.roux Jacques Le Roux added a comment - So you did not go the hardest way May I ask why?
          Hide
          Florian M Montalbano Florian added a comment - - edited

          Replaced

          &quote;

          by

          '

          (simple quote)

          Show
          Florian M Montalbano Florian added a comment - - edited Replaced &quote; by ' (simple quote)
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Yes, I guess so, thanks!

          Show
          jacques.le.roux Jacques Le Roux added a comment - Yes, I guess so, thanks!
          Hide
          Florian M Montalbano Florian added a comment -

          Thanks for the improvment Jacques Le Roux !
          I'll do the change.

          Those " came from the functionnality "Export all XML data" in the webtools.
          If they can be replaced, maybe it can be done in this function too ?

          Show
          Florian M Montalbano Florian added a comment - Thanks for the improvment Jacques Le Roux ! I'll do the change. Those " came from the functionnality "Export all XML data" in the webtools. If they can be replaced, maybe it can be done in this function too ?
          Hide
          jacques.le.roux Jacques Le Roux added a comment - - edited

          1st I can say that

          &quot;

          can be replaced by ' inside ", too late tonight for the rest

          Show
          jacques.le.roux Jacques Le Roux added a comment - - edited 1st I can say that &quot; can be replaced by ' inside ", too late tonight for the rest
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          Thanks Florian, I have used your data to test OFBIZ-7922. I have though to check them because you put a lot of details

          Show
          jacques.le.roux Jacques Le Roux added a comment - Thanks Florian, I have used your data to test OFBIZ-7922 . I have though to check them because you put a lot of details
          Hide
          Florian M Montalbano Florian added a comment -

          Here are some extra datas.
          Can someone confirm if the datas are significant and respect the standard ?

          Thanks.

          Show
          Florian M Montalbano Florian added a comment - Here are some extra datas. Can someone confirm if the datas are significant and respect the standard ? Thanks.

            People

            • Assignee:
              jacques.le.roux Jacques Le Roux
              Reporter:
              Florian M Montalbano Florian
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development