OFBiz
  1. OFBiz
  2. OFBIZ-4715

Invoice PDF and Contact Information show Region Code instead of Country Code before the Zip Code

    Details

      Description

      The Invoice PDF and the Contact Information in the Party application show the Region Code instead of the Country Code before the Zip Code. This will be fine for the US, here in Germany it is rather unusual.

      I propose to avoid this by removing the Region Code if the country is different then US.

        Issue Links

          Activity

          Hide
          Pierre Smits added a comment -

          The location and composition of the postal code is dependent on national definition. For instance, some countries have the country code before the postal code before the name of the city. Others have country code and postal code on a new line below the name of the city.

          See http://en.wikipedia.org/wiki/Postal_code

          In stead of removal, I suggest to look a bit deeper in all the variations and devise a solution that facilitates all.

          Regards,

          Pierre Smits

          Show
          Pierre Smits added a comment - The location and composition of the postal code is dependent on national definition. For instance, some countries have the country code before the postal code before the name of the city. Others have country code and postal code on a new line below the name of the city. See http://en.wikipedia.org/wiki/Postal_code In stead of removal, I suggest to look a bit deeper in all the variations and devise a solution that facilitates all. Regards, Pierre Smits
          Hide
          Markus M. May added a comment -

          Determine the country with the locale, remove stateProvinceGeoId if country is not equal US in Contact.ftl

          Show
          Markus M. May added a comment - Determine the country with the locale, remove stateProvinceGeoId if country is not equal US in Contact.ftl
          Hide
          Markus M. May added a comment -

          These are a lot of different ways on how to do this. How should this be implemented in a "common" way. I am thinking about a configuration via an XML-Locale file. This is on the other hand a misuse of locales, since I would retrieve the locale via the country code and not from the user locale.

          What do you think?

          Show
          Markus M. May added a comment - These are a lot of different ways on how to do this. How should this be implemented in a "common" way. I am thinking about a configuration via an XML-Locale file. This is on the other hand a misuse of locales, since I would retrieve the locale via the country code and not from the user locale. What do you think?
          Hide
          Jose F. Fernandez added a comment -

          I think that the address template must be personalized by locales, but not by user locale or country, because is a user from Spain (for example, me) send an invoice to a USA costumer, the address must be formatted according to the USA locale. So it must be by the address locale.

          I'm thinking right?

          Show
          Jose F. Fernandez added a comment - I think that the address template must be personalized by locales, but not by user locale or country, because is a user from Spain (for example, me) send an invoice to a USA costumer, the address must be formatted according to the USA locale. So it must be by the address locale. I'm thinking right?
          Hide
          Jacopo Cappellato added a comment -

          Maybe a better way is to create a simple dynamic mechanism to lookup, based on the postalAddress.countryGeoId, if there is a country specific ftl; if not use the default one.
          Steps to implement this feature could be:

          • extract from Contact.ftl the existing code that formats the postal address into a separate PostalAddress.ftl (or similar) template;
          • in Contact.ftl, where the postal address needs to be rendered, get the postalAddress.countryGeoId and try to locate a template with file name: PostalAddress_$ {postalAddress.countryGeoId}

            .ftl (e.g. PostalAddress_USA.ftl or PostalAddress_FRA.ftl); if not found then default to PostalAddress.ftl

          • the same mechanism could be reused for invoices etc...
          • it would be nice (but maybe not too easy) to try to implement all the PostalAddress_*.ftl templates in a way that will let them to be reused in various screens, possibly using the <@htmlTemplate/> tags, with minimal to no formatting/styling; but it would be also nice to be able to reuse them for PDF rendering (form widgets?)

          Just my 2 cents

          Show
          Jacopo Cappellato added a comment - Maybe a better way is to create a simple dynamic mechanism to lookup, based on the postalAddress.countryGeoId, if there is a country specific ftl; if not use the default one. Steps to implement this feature could be: extract from Contact.ftl the existing code that formats the postal address into a separate PostalAddress.ftl (or similar) template; in Contact.ftl, where the postal address needs to be rendered, get the postalAddress.countryGeoId and try to locate a template with file name: PostalAddress_$ {postalAddress.countryGeoId} .ftl (e.g. PostalAddress_USA.ftl or PostalAddress_FRA.ftl); if not found then default to PostalAddress.ftl the same mechanism could be reused for invoices etc... it would be nice (but maybe not too easy) to try to implement all the PostalAddress_*.ftl templates in a way that will let them to be reused in various screens, possibly using the <@htmlTemplate/> tags, with minimal to no formatting/styling; but it would be also nice to be able to reuse them for PDF rendering (form widgets?) Just my 2 cents
          Hide
          Markus M. May added a comment -

          Jacopo: This solution is interesting.
          The only problem I do see is that we do need a new database entity (PostalAddressFTL) which references the CountryCode and also the ftl for this (e.g. PostalAddress_USA.ftl). Furthermore if an FTL does not exist for a countryCode we need a default ftl (PostalAddress.ftl). Then this data needs to get retrieved from the DB (which is quite easy), and the seed data needs to get extended for each FTL.
          Sounds like a lot of effort and also a lot of things, which could be forgotten or could go wrong.

          WDYT?

          Show
          Markus M. May added a comment - Jacopo: This solution is interesting. The only problem I do see is that we do need a new database entity (PostalAddressFTL) which references the CountryCode and also the ftl for this (e.g. PostalAddress_USA.ftl). Furthermore if an FTL does not exist for a countryCode we need a default ftl (PostalAddress.ftl). Then this data needs to get retrieved from the DB (which is quite easy), and the seed data needs to get extended for each FTL. Sounds like a lot of effort and also a lot of things, which could be forgotten or could go wrong. WDYT?
          Hide
          Anil K Patel added a comment -

          Ofbiz is designed to be customized. Templates used for rendering data are some of the objects in Ofbiz that gets customized the most. Keeping these two things in mind, its better to not try to make templates do to much. Instead if they are simple, users (in this case developers implementing ofbiz) will be able to understand the code much easy and might customize them to fit their project needs.

          My two cents

          Show
          Anil K Patel added a comment - Ofbiz is designed to be customized. Templates used for rendering data are some of the objects in Ofbiz that gets customized the most. Keeping these two things in mind, its better to not try to make templates do to much. Instead if they are simple, users (in this case developers implementing ofbiz) will be able to understand the code much easy and might customize them to fit their project needs. My two cents
          Hide
          Jacopo Cappellato added a comment -

          Markus,

          I was not suggesting to add new entities but simply to adopt the naming convention I explained above: nothing more or less than this; all the country variant templates could stay in the same folder and the system could easily try to locate them in the filesystem without the need to lookup custom entities.
          And adding a custom template would be a matter of copying it in that folder with the proper naming convention.
          I hope it clarifies what I was suggesting.

          Show
          Jacopo Cappellato added a comment - Markus, I was not suggesting to add new entities but simply to adopt the naming convention I explained above: nothing more or less than this; all the country variant templates could stay in the same folder and the system could easily try to locate them in the filesystem without the need to lookup custom entities. And adding a custom template would be a matter of copying it in that folder with the proper naming convention. I hope it clarifies what I was suggesting.
          Hide
          Joe Eckard added a comment - - edited

          I agree with the general idea (see this dev list message from 2008), but something else to consider is that you will most like want different templates for HTML display, HTML form input & XSL:FO.

          Show
          Joe Eckard added a comment - - edited I agree with the general idea (see this dev list message from 2008 ), but something else to consider is that you will most like want different templates for HTML display, HTML form input & XSL:FO.
          Hide
          Jacopo Cappellato added a comment -

          Yeah, I agree... we may simply use the same lookup mechanism for XSL-FO but search for files with the fo.ftl suffix.

          Show
          Jacopo Cappellato added a comment - Yeah, I agree... we may simply use the same lookup mechanism for XSL-FO but search for files with the fo.ftl suffix.
          Hide
          Markus M. May added a comment - - edited

          I am currently a little lost in this task.

          My current setup is as follows:

          1) A groovy component to determine the countryCode and with this determines the ftl-name and if it is available, if not, it will return the usual PostalAddress.ftl
          2) new PostalAddressScreen, which calls the above mentioned groovy component and calls the html-template PostalAddress* from the groovy script from the context
          3) Adopt the Contact.ftl to call the new screen with the postalAddress in the context, so that I do have access to this in the screen
          <#assign postalAddress = contactMechMap.postalAddress>
          $

          {setRequestAttribute("postalAddress", postalAddress)}


          $

          {screens.render("component://party/widget/partymgr/PostalAddressScreens.xml#postalAddress")}


          4) new PostalAddress.ftl, which can have several instances (PostalAddress_DE.ftl, PostalAddress_USA.ftl, ...)

          Am I walking in the right direction?

          Show
          Markus M. May added a comment - - edited I am currently a little lost in this task. My current setup is as follows: 1) A groovy component to determine the countryCode and with this determines the ftl-name and if it is available, if not, it will return the usual PostalAddress.ftl 2) new PostalAddressScreen, which calls the above mentioned groovy component and calls the html-template PostalAddress* from the groovy script from the context 3) Adopt the Contact.ftl to call the new screen with the postalAddress in the context, so that I do have access to this in the screen <#assign postalAddress = contactMechMap.postalAddress> $ {setRequestAttribute("postalAddress", postalAddress)} $ {screens.render("component://party/widget/partymgr/PostalAddressScreens.xml#postalAddress")} 4) new PostalAddress.ftl, which can have several instances (PostalAddress_DE.ftl, PostalAddress_USA.ftl, ...) Am I walking in the right direction?
          Hide
          Markus M. May added a comment -

          Initial commit, I still receive errors that the parameter PostalAddress is empty. Any help on this one?

          Show
          Markus M. May added a comment - Initial commit, I still receive errors that the parameter PostalAddress is empty. Any help on this one?
          Hide
          Markus M. May added a comment -

          Upload with all git related stuff removed.

          Show
          Markus M. May added a comment - Upload with all git related stuff removed.
          Hide
          Jacopo Cappellato added a comment -

          Hi Markus,

          I have completed and improved your initial patch and then committed it in revision 1293204.
          You should follow a similar approach to implement a similar mechanism for fo.ftl templates (for PDF files).

          Show
          Jacopo Cappellato added a comment - Hi Markus, I have completed and improved your initial patch and then committed it in revision 1293204. You should follow a similar approach to implement a similar mechanism for fo.ftl templates (for PDF files).
          Hide
          Markus M. May added a comment -

          The attached patch should fix this issue.

          I have implemented a common groovy-script for both the quote as well as the company header. This should work for all other implementations as well, so probably we could even adopt the one in the already implemented one for the profile.

          We would just need to edit the folder, but the groovy script could be the same for all these screens

          Show
          Markus M. May added a comment - The attached patch should fix this issue. I have implemented a common groovy-script for both the quote as well as the company header. This should work for all other implementations as well, so probably we could even adopt the one in the already implemented one for the profile. We would just need to edit the folder, but the groovy script could be the same for all these screens
          Hide
          Jacopo Cappellato added a comment -

          Hi Markus,

          I have some suggestions to further improve your patch (and this new feature in general):

          • the two screens companyPostalAddress and postalAddress can be unified into one generic screen with name "postalAddressPdf" or similar; the screen definitions should be moved to the party component
          • more generally, the new generic artifacts for fo-ftl that you have created should be defined in the party component in the same locations where I have committed the ones for fo formatting: in this way the party component will offer a small library of templates for the formatting of html/PDF postal addresses properly localized; then the order component (order PDF and quote PDF) will reuse them to render the documents; could you please also move to party component the GetPostalAddressTemplate.groovy script, the templates PostalAddress.fo.ftl and PostalAddress_USA.fo.ftl there?
          • the script that you have created with name GetPostalAddressTemplate.groovy is very similar to the one I have committed; could you please try to merge them into the same script? the script could accept as an additional parameter the template suffix (.ftl or .fo.ftl) and do the lookup accordingly
          • please also try to use, in the generic script/templates neutral names for variables: please use "postalAddress" rather than "usedPostalAddress" etc...

          Thanks

          Show
          Jacopo Cappellato added a comment - Hi Markus, I have some suggestions to further improve your patch (and this new feature in general): the two screens companyPostalAddress and postalAddress can be unified into one generic screen with name "postalAddressPdf" or similar; the screen definitions should be moved to the party component more generally, the new generic artifacts for fo-ftl that you have created should be defined in the party component in the same locations where I have committed the ones for fo formatting: in this way the party component will offer a small library of templates for the formatting of html/PDF postal addresses properly localized; then the order component (order PDF and quote PDF) will reuse them to render the documents; could you please also move to party component the GetPostalAddressTemplate.groovy script, the templates PostalAddress.fo.ftl and PostalAddress_USA.fo.ftl there? the script that you have created with name GetPostalAddressTemplate.groovy is very similar to the one I have committed; could you please try to merge them into the same script? the script could accept as an additional parameter the template suffix (.ftl or .fo.ftl) and do the lookup accordingly please also try to use, in the generic script/templates neutral names for variables: please use "postalAddress" rather than "usedPostalAddress" etc... Thanks
          Hide
          Markus M. May added a comment -

          Hi Jacopo,

          thanks for the Input.

          I can try to use postalAddress instead of usedPostalAddress, I just wanted to avoid Naming Conflicts for the variables, so that we are able to use the same script for multiple Screens. I was thinking along the lines that the same script multiple could only be called multiple times, if the corresponding variable is not already used in the same Screen. Otherwise the variable in the Screen would probably overwrite an already existing variable and therefor would return a wrong result. Am I wrong with this assumption?
          Other then that, I will dig into this and will try to solve this asap.

          Show
          Markus M. May added a comment - Hi Jacopo, thanks for the Input. I can try to use postalAddress instead of usedPostalAddress, I just wanted to avoid Naming Conflicts for the variables, so that we are able to use the same script for multiple Screens. I was thinking along the lines that the same script multiple could only be called multiple times, if the corresponding variable is not already used in the same Screen. Otherwise the variable in the Screen would probably overwrite an already existing variable and therefor would return a wrong result. Am I wrong with this assumption? Other then that, I will dig into this and will try to solve this asap.
          Hide
          Jacopo Cappellato added a comment -

          Hi Markus,

          if the variable name "postalAddress" will cause conflicts you can use a different name that still makes sense and it is generic.

          And after thinking more about this, I would suggest also to change the names of the generic screens in the following way (or similar):
          postalAddress ---> postalAddressHtmlFormatter
          postalAddressPdf ---> postalAddressPdfFormatter

          then the name of the input "postalAddress" variable (i.e. the GenericValue that contains the postal address) could be renamed:

          postalAddress ---> postalAddressFormatter_postalAddress (or similar)

          I hope it helps

          Show
          Jacopo Cappellato added a comment - Hi Markus, if the variable name "postalAddress" will cause conflicts you can use a different name that still makes sense and it is generic. And after thinking more about this, I would suggest also to change the names of the generic screens in the following way (or similar): postalAddress ---> postalAddressHtmlFormatter postalAddressPdf ---> postalAddressPdfFormatter then the name of the input "postalAddress" variable (i.e. the GenericValue that contains the postal address) could be renamed: postalAddress ---> postalAddressFormatter_postalAddress (or similar) I hope it helps
          Hide
          Markus M. May added a comment -

          This patch provides a central solution for the address-formatting of HTML as well as PDFs in the partymgr application.

          Show
          Markus M. May added a comment - This patch provides a central solution for the address-formatting of HTML as well as PDFs in the partymgr application.
          Hide
          Markus M. May added a comment -

          No problem using the same variable all over The patch calls now postalAddressHtmlFormatter or postalAddressPdfFormatter in the PartyScreens.xml from Contact.ftl and companyHeader.fo.ftl and ordercontactinfo.ftl as well as quoteReportContactMechs.fo.ftl.

          A $

          {setRequestAttribute("postalAddress", toPostalAddress)}

          is called before the screen is rendered, to set the postalAddress accordingly. The postalAddress.fo.ftl as well as postalAddress.ftl are aggregated into the applications/party/webapp/partymgr/party/contactmechtemplates/ folder.

          Hope this helps

          Show
          Markus M. May added a comment - No problem using the same variable all over The patch calls now postalAddressHtmlFormatter or postalAddressPdfFormatter in the PartyScreens.xml from Contact.ftl and companyHeader.fo.ftl and ordercontactinfo.ftl as well as quoteReportContactMechs.fo.ftl. A $ {setRequestAttribute("postalAddress", toPostalAddress)} is called before the screen is rendered, to set the postalAddress accordingly. The postalAddress.fo.ftl as well as postalAddress.ftl are aggregated into the applications/party/webapp/partymgr/party/contactmechtemplates/ folder. Hope this helps
          Hide
          Jacques Le Roux added a comment -

          Markus,

          Dont forget to mark the issues as "Patch provided" when attaching patch/es

          Thanks

          Show
          Jacques Le Roux added a comment - Markus, Dont forget to mark the issues as "Patch provided" when attaching patch/es Thanks
          Hide
          Markus M. May added a comment -

          Fixed minor issue with not empty context variable.

          Show
          Markus M. May added a comment - Fixed minor issue with not empty context variable.
          Hide
          Jacopo Cappellato added a comment -

          Thank you Markus, I have committed your patch in rev. 1305898

          Show
          Jacopo Cappellato added a comment - Thank you Markus, I have committed your patch in rev. 1305898

            People

            • Assignee:
              Jacopo Cappellato
              Reporter:
              Markus M. May
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development