[jira] Created: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
Locked
[jira] Created: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
 
|
sendMailFrom Screen don't need to load uiLabels
----------------------------------------------- Key: OFBIZ-3969 URL: https://issues.apache.org/jira/browse/OFBIZ-3969 Project: OFBiz Issue Type: Bug Components: order Affects Versions: SVN trunk Reporter: Sascha Rodekamp Fix For: SVN trunk Hi, i noticed an issues in the sendOrderNotificationScreen method. There is no need of loading uiLabels in the method, because the labels will be loaded in the screen (which will be used for the email). The point is, when someone (for whatever reason) don't add English uiLabels and the service try to access the default English labels the an exception will be thrown :-). This can be simply avoided. Cheers -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
[jira] Updated: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
|
|
[ https://issues.apache.org/jira/browse/OFBIZ-3969?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Sascha Rodekamp updated OFBIZ-3969: ----------------------------------- Attachment: OFBIZ-3969_OrderServices.patch > sendMailFrom Screen don't need to load uiLabels > ----------------------------------------------- > > Key: OFBIZ-3969 > URL: https://issues.apache.org/jira/browse/OFBIZ-3969 > Project: OFBiz > Issue Type: Bug > Components: order > Affects Versions: SVN trunk > Reporter: Sascha Rodekamp > Fix For: SVN trunk > > Attachments: OFBIZ-3969_OrderServices.patch > > > Hi, > i noticed an issues in the sendOrderNotificationScreen method. There is no need of loading uiLabels in the method, because the labels will be loaded in the screen (which will be used for the email). > The point is, when someone (for whatever reason) don't add English uiLabels and the service try to access the default English labels the an exception will be thrown :-). This can be simply avoided. > Cheers -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
[jira] Commented: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
|
|
In reply to this post by Nicolas Malin (Jira)
[ https://issues.apache.org/jira/browse/OFBIZ-3969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12917255#action_12917255 ] Jacques Le Roux commented on OFBIZ-3969: ---------------------------------------- Sascha, I committed your patch in trunk at r1003836, but I finally reverted it at r1003909, as I'm not sure the EcommerceUiLabels will be there in all cases. And I guess if it's there, there should be a reason... I must say I did not take the time to dig it further... Also looks like your use case is specific (no English Labels) and OFBiz OOTB is not specific. > sendMailFrom Screen don't need to load uiLabels > ----------------------------------------------- > > Key: OFBIZ-3969 > URL: https://issues.apache.org/jira/browse/OFBIZ-3969 > Project: OFBiz > Issue Type: Bug > Components: order > Affects Versions: SVN trunk > Reporter: Sascha Rodekamp > Fix For: SVN trunk > > Attachments: OFBIZ-3969_OrderServices.patch > > > Hi, > i noticed an issues in the sendOrderNotificationScreen method. There is no need of loading uiLabels in the method, because the labels will be loaded in the screen (which will be used for the email). > The point is, when someone (for whatever reason) don't add English uiLabels and the service try to access the default English labels the an exception will be thrown :-). This can be simply avoided. > Cheers -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
[jira] Commented: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
|
|
In reply to this post by Nicolas Malin (Jira)
[ https://issues.apache.org/jira/browse/OFBIZ-3969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12917256#action_12917256 ] Jacques Le Roux commented on OFBIZ-3969: ---------------------------------------- I let it open in case someone is interested and wants to have a closer look > sendMailFrom Screen don't need to load uiLabels > ----------------------------------------------- > > Key: OFBIZ-3969 > URL: https://issues.apache.org/jira/browse/OFBIZ-3969 > Project: OFBiz > Issue Type: Bug > Components: order > Affects Versions: SVN trunk > Reporter: Sascha Rodekamp > Fix For: SVN trunk > > Attachments: OFBIZ-3969_OrderServices.patch > > > Hi, > i noticed an issues in the sendOrderNotificationScreen method. There is no need of loading uiLabels in the method, because the labels will be loaded in the screen (which will be used for the email). > The point is, when someone (for whatever reason) don't add English uiLabels and the service try to access the default English labels the an exception will be thrown :-). This can be simply avoided. > Cheers -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
[jira] Commented: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
|
|
In reply to this post by Nicolas Malin (Jira)
[ https://issues.apache.org/jira/browse/OFBIZ-3969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12917294#action_12917294 ] Sascha Rodekamp commented on OFBIZ-3969: ---------------------------------------- OK Jacques, thanks for reviewing! > sendMailFrom Screen don't need to load uiLabels > ----------------------------------------------- > > Key: OFBIZ-3969 > URL: https://issues.apache.org/jira/browse/OFBIZ-3969 > Project: OFBiz > Issue Type: Bug > Components: order > Affects Versions: SVN trunk > Reporter: Sascha Rodekamp > Fix For: SVN trunk > > Attachments: OFBIZ-3969_OrderServices.patch > > > Hi, > i noticed an issues in the sendOrderNotificationScreen method. There is no need of loading uiLabels in the method, because the labels will be loaded in the screen (which will be used for the email). > The point is, when someone (for whatever reason) don't add English uiLabels and the service try to access the default English labels the an exception will be thrown :-). This can be simply avoided. > Cheers -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
[jira] Closed: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
|
|
In reply to this post by Nicolas Malin (Jira)
[ https://issues.apache.org/jira/browse/OFBIZ-3969?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Le Roux closed OFBIZ-3969. ---------------------------------- Assignee: Jacques Le Roux Resolution: Won't Fix Finally I close, it's too specific > sendMailFrom Screen don't need to load uiLabels > ----------------------------------------------- > > Key: OFBIZ-3969 > URL: https://issues.apache.org/jira/browse/OFBIZ-3969 > Project: OFBiz > Issue Type: Bug > Components: order > Affects Versions: SVN trunk > Reporter: Sascha Rodekamp > Assignee: Jacques Le Roux > Fix For: SVN trunk > > Attachments: OFBIZ-3969_OrderServices.patch > > > Hi, > i noticed an issues in the sendOrderNotificationScreen method. There is no need of loading uiLabels in the method, because the labels will be loaded in the screen (which will be used for the email). > The point is, when someone (for whatever reason) don't add English uiLabels and the service try to access the default English labels the an exception will be thrown :-). This can be simply avoided. > Cheers -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
[jira] Commented: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
|
|
In reply to this post by Nicolas Malin (Jira)
[ https://issues.apache.org/jira/browse/OFBIZ-3969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12918843#action_12918843 ] Sascha Rodekamp commented on OFBIZ-3969: ---------------------------------------- Hi Jacques, another thought to this issue comes in my mind: what happen when the ecommerce application is disabled (because it is not needed), do this code crash? [code] ResourceBundleMapWrapper uiLabelMap = (ResourceBundleMapWrapper) UtilProperties.getResourceBundleMap("EcommerceUiLabels", locale); [code] From my point of view the code is more clean when it's only loads the labels which are really needed directly in the screens and not in the java code?! Have a good day Sascha > sendMailFrom Screen don't need to load uiLabels > ----------------------------------------------- > > Key: OFBIZ-3969 > URL: https://issues.apache.org/jira/browse/OFBIZ-3969 > Project: OFBiz > Issue Type: Bug > Components: order > Affects Versions: SVN trunk > Reporter: Sascha Rodekamp > Assignee: Jacques Le Roux > Fix For: SVN trunk > > Attachments: OFBIZ-3969_OrderServices.patch > > > Hi, > i noticed an issues in the sendOrderNotificationScreen method. There is no need of loading uiLabels in the method, because the labels will be loaded in the screen (which will be used for the email). > The point is, when someone (for whatever reason) don't add English uiLabels and the service try to access the default English labels the an exception will be thrown :-). This can be simply avoided. > Cheers -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
[jira] Issue Comment Edited: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
|
|
In reply to this post by Nicolas Malin (Jira)
[ https://issues.apache.org/jira/browse/OFBIZ-3969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12918843#action_12918843 ] Sascha Rodekamp edited comment on OFBIZ-3969 at 10/7/10 5:02 AM: ----------------------------------------------------------------- Hi Jacques, another thought to this issue comes in my mind: what happen when the ecommerce application is disabled (because it is not needed), do this code crash? {code} ResourceBundleMapWrapper uiLabelMap = (ResourceBundleMapWrapper) UtilProperties.getResourceBundleMap("EcommerceUiLabels", locale); {code} From my point of view the code is more clean when it's only loads the labels which are really needed directly in the screens and not in the java code?! Have a good day Sascha was (Author: sascha): Hi Jacques, another thought to this issue comes in my mind: what happen when the ecommerce application is disabled (because it is not needed), do this code crash? [code] ResourceBundleMapWrapper uiLabelMap = (ResourceBundleMapWrapper) UtilProperties.getResourceBundleMap("EcommerceUiLabels", locale); [code] From my point of view the code is more clean when it's only loads the labels which are really needed directly in the screens and not in the java code?! Have a good day Sascha > sendMailFrom Screen don't need to load uiLabels > ----------------------------------------------- > > Key: OFBIZ-3969 > URL: https://issues.apache.org/jira/browse/OFBIZ-3969 > Project: OFBiz > Issue Type: Bug > Components: order > Affects Versions: SVN trunk > Reporter: Sascha Rodekamp > Assignee: Jacques Le Roux > Fix For: SVN trunk > > Attachments: OFBIZ-3969_OrderServices.patch > > > Hi, > i noticed an issues in the sendOrderNotificationScreen method. There is no need of loading uiLabels in the method, because the labels will be loaded in the screen (which will be used for the email). > The point is, when someone (for whatever reason) don't add English uiLabels and the service try to access the default English labels the an exception will be thrown :-). This can be simply avoided. > Cheers -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
[jira] Commented: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
|
|
In reply to this post by Nicolas Malin (Jira)
[ https://issues.apache.org/jira/browse/OFBIZ-3969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12918965#action_12918965 ] Jacques Le Roux commented on OFBIZ-3969: ---------------------------------------- Sascha, It depends if the component has beeen disabled (as it should be done) or removed. If it's removed then an IllegalArgumentException("Could not find resource bundle [" + resource + "] in the locale [" + locale + "]"); will be thrown. I understand though that this introduces a dependency from the eCommercer component (in Order or also Accounting) and this should not be (an appplications component should not depend on a specialpurpose component). In other word this is bad! So I agree we should rather have the uiLabel Map set in the calling screen(*+s+*). Now I agree to change it if you can prove me that this will not be any side-effects. I don't want to break anything and I have not enough time right now to check it by myself, sorry to put the burden on you ;) > sendMailFrom Screen don't need to load uiLabels > ----------------------------------------------- > > Key: OFBIZ-3969 > URL: https://issues.apache.org/jira/browse/OFBIZ-3969 > Project: OFBiz > Issue Type: Bug > Components: order > Affects Versions: SVN trunk > Reporter: Sascha Rodekamp > Assignee: Jacques Le Roux > Fix For: SVN trunk > > Attachments: OFBIZ-3969_OrderServices.patch > > > Hi, > i noticed an issues in the sendOrderNotificationScreen method. There is no need of loading uiLabels in the method, because the labels will be loaded in the screen (which will be used for the email). > The point is, when someone (for whatever reason) don't add English uiLabels and the service try to access the default English labels the an exception will be thrown :-). This can be simply avoided. > Cheers -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
[jira] Reopened: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
|
|
In reply to this post by Nicolas Malin (Jira)
[ https://issues.apache.org/jira/browse/OFBIZ-3969?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Le Roux reopened OFBIZ-3969: ------------------------------------ BTW you asked :D > sendMailFrom Screen don't need to load uiLabels > ----------------------------------------------- > > Key: OFBIZ-3969 > URL: https://issues.apache.org/jira/browse/OFBIZ-3969 > Project: OFBiz > Issue Type: Bug > Components: order > Affects Versions: SVN trunk > Reporter: Sascha Rodekamp > Assignee: Jacques Le Roux > Fix For: SVN trunk > > Attachments: OFBIZ-3969_OrderServices.patch > > > Hi, > i noticed an issues in the sendOrderNotificationScreen method. There is no need of loading uiLabels in the method, because the labels will be loaded in the screen (which will be used for the email). > The point is, when someone (for whatever reason) don't add English uiLabels and the service try to access the default English labels the an exception will be thrown :-). This can be simply avoided. > Cheers -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
[jira] Commented: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
|
|
In reply to this post by Nicolas Malin (Jira)
[ https://issues.apache.org/jira/browse/OFBIZ-3969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12919009#action_12919009 ] Scott Gray commented on OFBIZ-3969: ----------------------------------- +1 for this patch (well I didn't review it closely but I agree with the idea put forward). IMO it should be perfectly acceptable to remove the ecommerce application but still retain a working order manager and I agree that loading uiLabels is the responsibility of the screen rendering the email and not the code calling the email service. In theory the calling code should have no idea of the contents of the screen being emailed and hence no idea what uiLabels would be necessary. Regards Scott > sendMailFrom Screen don't need to load uiLabels > ----------------------------------------------- > > Key: OFBIZ-3969 > URL: https://issues.apache.org/jira/browse/OFBIZ-3969 > Project: OFBiz > Issue Type: Bug > Components: order > Affects Versions: SVN trunk > Reporter: Sascha Rodekamp > Assignee: Jacques Le Roux > Fix For: SVN trunk > > Attachments: OFBIZ-3969_OrderServices.patch > > > Hi, > i noticed an issues in the sendOrderNotificationScreen method. There is no need of loading uiLabels in the method, because the labels will be loaded in the screen (which will be used for the email). > The point is, when someone (for whatever reason) don't add English uiLabels and the service try to access the default English labels the an exception will be thrown :-). This can be simply avoided. > Cheers -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
[jira] Commented: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
|
|
In reply to this post by Nicolas Malin (Jira)
[ https://issues.apache.org/jira/browse/OFBIZ-3969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12919061#action_12919061 ] Jacques Le Roux commented on OFBIZ-3969: ---------------------------------------- Thanks for your comment Scott, It's good to have another commiter opinion...For the moment I have bigger fishes to fry but ASAP I will bet back to this... What annoy me is that there is certainly a (bad) reason to be like that, and I'm afraid to break this (bad even if it works) design, because we need to check where it can be used, and it's not easy to track... > sendMailFrom Screen don't need to load uiLabels > ----------------------------------------------- > > Key: OFBIZ-3969 > URL: https://issues.apache.org/jira/browse/OFBIZ-3969 > Project: OFBiz > Issue Type: Bug > Components: order > Affects Versions: SVN trunk > Reporter: Sascha Rodekamp > Assignee: Jacques Le Roux > Fix For: SVN trunk > > Attachments: OFBIZ-3969_OrderServices.patch > > > Hi, > i noticed an issues in the sendOrderNotificationScreen method. There is no need of loading uiLabels in the method, because the labels will be loaded in the screen (which will be used for the email). > The point is, when someone (for whatever reason) don't add English uiLabels and the service try to access the default English labels the an exception will be thrown :-). This can be simply avoided. > Cheers -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
[jira] Commented: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
|
|
In reply to this post by Nicolas Malin (Jira)
[ https://issues.apache.org/jira/browse/OFBIZ-3969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12921989#action_12921989 ] Sascha Rodekamp commented on OFBIZ-3969: ---------------------------------------- Ok, i tested and it seems, that all needed labels will be loaded in the screens before triggering the mail service. So it's ready for commit. > sendMailFrom Screen don't need to load uiLabels > ----------------------------------------------- > > Key: OFBIZ-3969 > URL: https://issues.apache.org/jira/browse/OFBIZ-3969 > Project: OFBiz > Issue Type: Bug > Components: order > Affects Versions: SVN trunk > Reporter: Sascha Rodekamp > Assignee: Jacques Le Roux > Fix For: SVN trunk > > Attachments: OFBIZ-3969_OrderServices.patch > > > Hi, > i noticed an issues in the sendOrderNotificationScreen method. There is no need of loading uiLabels in the method, because the labels will be loaded in the screen (which will be used for the email). > The point is, when someone (for whatever reason) don't add English uiLabels and the service try to access the default English labels the an exception will be thrown :-). This can be simply avoided. > Cheers -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
[jira] Closed: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
|
|
In reply to this post by Nicolas Malin (Jira)
[ https://issues.apache.org/jira/browse/OFBIZ-3969?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Le Roux closed OFBIZ-3969. ---------------------------------- Resolution: Fixed Thanks for checking Sascha, I reapplied your patch at r1023719 > sendMailFrom Screen don't need to load uiLabels > ----------------------------------------------- > > Key: OFBIZ-3969 > URL: https://issues.apache.org/jira/browse/OFBIZ-3969 > Project: OFBiz > Issue Type: Bug > Components: order > Affects Versions: SVN trunk > Reporter: Sascha Rodekamp > Assignee: Jacques Le Roux > Fix For: SVN trunk > > Attachments: OFBIZ-3969_OrderServices.patch > > > Hi, > i noticed an issues in the sendOrderNotificationScreen method. There is no need of loading uiLabels in the method, because the labels will be loaded in the screen (which will be used for the email). > The point is, when someone (for whatever reason) don't add English uiLabels and the service try to access the default English labels the an exception will be thrown :-). This can be simply avoided. > Cheers -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
[jira] Updated: (OFBIZ-3969) sendMailFrom Screen don't need to load uiLabels
|
|
In reply to this post by Nicolas Malin (Jira)
[ https://issues.apache.org/jira/browse/OFBIZ-3969?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Le Roux updated OFBIZ-3969: ----------------------------------- Issue Type: Improvement (was: Bug) > sendMailFrom Screen don't need to load uiLabels > ----------------------------------------------- > > Key: OFBIZ-3969 > URL: https://issues.apache.org/jira/browse/OFBIZ-3969 > Project: OFBiz > Issue Type: Improvement > Components: order > Affects Versions: SVN trunk > Reporter: Sascha Rodekamp > Assignee: Jacques Le Roux > Fix For: SVN trunk > > Attachments: OFBIZ-3969_OrderServices.patch > > > Hi, > i noticed an issues in the sendOrderNotificationScreen method. There is no need of loading uiLabels in the method, because the labels will be loaded in the screen (which will be used for the email). > The point is, when someone (for whatever reason) don't add English uiLabels and the service try to access the default English labels the an exception will be thrown :-). This can be simply avoided. > Cheers -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. |
«
Return to OFBiz - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |