Bug 7736 introduces an EDIFACT module that works great for many European vendors, does not work will for US vendors, which have a much different interpretation of 'standard'. In fact, each vendor may pick require different arrangements of values in EDIFACT messages. It would be impossible to encompass all these requirements within Koha's EDIFACT module itself. Instead, we should allow the module to be pluggable, so versions of the module can be developed for vendors that require EDIFACT messages that don't conform to the standard set by Koha's EDIFACT module.
Created attachment 47025 [details] [review] Bug 15630 - Make Edifact module pluggable Koha's EDIFACT module works great for many European vendors, but does not work will for US vendors, which have a much different interpretation of 'standard'. In fact, each vendor may require different arrangements of values in EDIFACT messages. It would be impossible to encompass all these requirements within Koha's EDIFACT module itself. Instead, we should allow the module to be pluggable, so versions of the module can be developed for vendors that require EDIFACT messages that don't conform to the standard set by Koha's EDIFACT module. Test Plan: 1) Apply this patch 2) Run updatedatabase 3) Enable Koha plugins 4) Install the Edifact stub plugin available at https://github.com/bywatersolutions/koha-plugin-edifact-stub 5) Edit the EDI Vendor account, assign the plugin to a Vendor EDI account 6) Test EDI functionality ( ORDER, INVOICE ), there should be no errors or changes to the EDIFACT message input or output
Created attachment 47026 [details] [review] Bug 15630 - Make Edifact module pluggable Koha's EDIFACT module works great for many European vendors, but does not work will for US vendors, which have a much different interpretation of 'standard'. In fact, each vendor may require different arrangements of values in EDIFACT messages. It would be impossible to encompass all these requirements within Koha's EDIFACT module itself. Instead, we should allow the module to be pluggable, so versions of the module can be developed for vendors that require EDIFACT messages that don't conform to the standard set by Koha's EDIFACT module. Test Plan: 1) Apply this patch 2) Run updatedatabase 3) Enable Koha plugins 4) Install the Edifact stub plugin available at https://github.com/bywatersolutions/koha-plugin-edifact-stub 5) Edit the EDI Vendor account, assign the plugin to a Vendor EDI account 6) Test EDI functionality ( ORDER, INVOICE ), there should be no errors or changes to the EDIFACT message input or output
Created attachment 49270 [details] [review] Bug 15630 - Make Edifact module pluggable Koha's EDIFACT module works great for many European vendors, but does not work will for US vendors, which have a much different interpretation of 'standard'. In fact, each vendor may require different arrangements of values in EDIFACT messages. It would be impossible to encompass all these requirements within Koha's EDIFACT module itself. Instead, we should allow the module to be pluggable, so versions of the module can be developed for vendors that require EDIFACT messages that don't conform to the standard set by Koha's EDIFACT module. Test Plan: 1) Apply this patch 2) Run updatedatabase 3) Enable Koha plugins 4) Install the Edifact stub plugin available at https://github.com/bywatersolutions/koha-plugin-edifact-stub 5) Edit the EDI Vendor account, assign the plugin to a Vendor EDI account 6) Test EDI functionality ( ORDER, INVOICE ), there should be no errors or changes to the EDIFACT message input or output
Created attachment 49271 [details] [review] Bug 15630 - Make Edifact module pluggable Koha's EDIFACT module works great for many European vendors, but does not work will for US vendors, which have a much different interpretation of 'standard'. In fact, each vendor may require different arrangements of values in EDIFACT messages. It would be impossible to encompass all these requirements within Koha's EDIFACT module itself. Instead, we should allow the module to be pluggable, so versions of the module can be developed for vendors that require EDIFACT messages that don't conform to the standard set by Koha's EDIFACT module. Test Plan: 1) Apply this patch 2) Run updatedatabase 3) Enable Koha plugins 4) Install the Edifact stub plugin available at https://github.com/bywatersolutions/koha-plugin-edifact-stub 5) Edit the EDI Vendor account, assign the plugin to a Vendor EDI account 6) Test EDI functionality ( ORDER, INVOICE ), there should be no errors or changes to the EDIFACT message input or output Signed-off-by: Jason DeShaw <JDeShaw@cityoffargo.com>
Created attachment 49788 [details] [review] Bug 15630 - Make Edifact module pluggable Koha's EDIFACT module works great for many European vendors, but does not work will for US vendors, which have a much different interpretation of 'standard'. In fact, each vendor may require different arrangements of values in EDIFACT messages. It would be impossible to encompass all these requirements within Koha's EDIFACT module itself. Instead, we should allow the module to be pluggable, so versions of the module can be developed for vendors that require EDIFACT messages that don't conform to the standard set by Koha's EDIFACT module. Test Plan: 1) Apply this patch 2) Run updatedatabase 3) Enable Koha plugins 4) Install the Edifact stub plugin available at https://github.com/bywatersolutions/koha-plugin-edifact-stub 5) Edit the EDI Vendor account, assign the plugin to a Vendor EDI account 6) Test EDI functionality ( ORDER, INVOICE ), there should be no errors or changes to the EDIFACT message input or output Signed-off-by: Jason DeShaw <JDeShaw@cityoffargo.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Nice enhancement, should make things much more extensible.. Go forth :)
Pushed to Master - Should be in the May 2016 release. Thanks! RM note - I am now running the dbix schema update for both this and bug 7736 at the same time.
aqbooksellers.plugin has not been added to existing installations
(In reply to Jonathan Druart from comment #8) > aqbooksellers.plugin has not been added to existing installations vendor_edi_accounts.plugin!
*Manually added in the missing updatedatabase.pl for .plugin
This patch makes the t/db_dependent/TestBuilder.t tests fail: t/db_dependent/TestBuilder.t .. 3/41 DBD::mysql::st execute failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha_empty`.`edifact_messages`, CONSTRAINT `emfk_basketno` FOREIGN KEY (`basketno`) REFERENCES `aqbasket` (`basketno`)) [for Statement "DELETE FROM `aqbasket` WHERE ( `basketno` = ? )" with ParamValues: 0='13'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1834. the foreign keys don't not have a ON * CASCADE clause.
(In reply to Jonathan Druart from comment #11) > This patch makes the t/db_dependent/TestBuilder.t tests fail: > > t/db_dependent/TestBuilder.t .. 3/41 DBD::mysql::st execute failed: Cannot > delete or update a parent row: a foreign key constraint fails > (`koha_empty`.`edifact_messages`, CONSTRAINT `emfk_basketno` FOREIGN KEY > (`basketno`) REFERENCES `aqbasket` (`basketno`)) [for Statement "DELETE FROM > `aqbasket` WHERE ( `basketno` = ? )" with ParamValues: 0='13'] at > /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1834. > > the foreign keys don't not have a ON * CASCADE clause. Filed bug 16354
Comment on attachment 49788 [details] [review] Bug 15630 - Make Edifact module pluggable Review of attachment 49788 [details] [review]: ----------------------------------------------------------------- ::: Koha/Plugins.pm @@ +20,4 @@ > use Modern::Perl; > > use Module::Load::Conditional qw(can_load); > +use Module::Pluggable search_path => ['Koha::Plugin'], except => qr/::Edifact(|::Line|::Message|::Order|::Segment|::Transport)$/; This seems suboptimal... since any plugin with additional packages would need an exception. Wouldn't it make more sense to use "only" to only find plugins like ^Koha::Plugin::Com::CompanyName::PluginName$. I suppose it might be too late for that now as there's any number of plugins that can be allowed so perhaps one does have to only use exceptions from now on... I'm thinking of making a plugin, but I'll probably just use a different namespace like Local::PluginName::Submodules...
(In reply to David Cook from comment #13) > > +use Module::Pluggable search_path => ['Koha::Plugin'], except => qr/::Edifact(|::Line|::Message|::Order|::Segment|::Transport)$/; > > This seems suboptimal... since any plugin with additional packages would > need an exception. Have the same feeling when I see these lines..
(In reply to Marcel de Rooy from comment #14) > (In reply to David Cook from comment #13) > > > > +use Module::Pluggable search_path => ['Koha::Plugin'], except => qr/::Edifact(|::Line|::Message|::Order|::Segment|::Transport)$/; > > > > This seems suboptimal... since any plugin with additional packages would > > need an exception. > > Have the same feeling when I see these lines.. I agree. I'm sure there is a better solution!