|
Description
Katrin Fischer
2016-04-14 06:05:58 UTC
+1 Yes a 'use edifact' syspreference which if set to N (default) hides the edifact screens and links - which in fact are few Hi Colin, EDIFACT appeared to be turnted on for me automatically. Are you sure this has been added? There is no mention of EDIFACT in the sysprefs.sql file: http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=installer/data/mysql/sysprefs.sql;h=ef4c3f0aef428b2cf2dd592a7273389333a818ed;hb=2a8c68936dd7fd5c82af339db968eb67cb93a082 I was also thinking about additional columns and buttons in other places - I haven't been able to get a total overview of the EDIFACT feature so far - but i seems there is something new on basket summary as well. Hm, I found 134 ('EDIInvoicesShippingBudget',NULL,NULL,'The budget code used to allocate shipping charges to when processing EDI Invoice messages','free'), now - recheckign for EDI, but nothing else.
In most places if there are no edifact vendors present the functionality gets disabled. The main feature to control is probably the admin screens. A syspref shouldnt be to turn it off but to enable it - I'll see if I can generate a patch to add an enabling system preference Created attachment 51185 [details] [review] Proposed Patch Patch to add the syspreference. Set to no the Edi related screens are hidden from the admin page. Also removes the edifact messages link in acq. Created attachment 51228 [details] [review] Proposed patch make an import explicit Created attachment 54539 [details] [review] Bug 16258 Add a syspref to govern EDIFACT functionality Adds an "Edifact" systempreference to govern whether edifact processing is enabled. In most places this is hidden if the current vendor does not appear in the edi vendors table. This preference hides the admin screens which define this and a couple of links. Also fixes an anomaly whereby the basketgroup screen was not making the same check on whether edi ordering should be enabled as the basket screen. Both now use the same logic. Rebase by mtompset@hotmail.com Created attachment 54567 [details]
Button Create EDIFACT order still appears
If syspref "Edifact" is disabled but the vendor has an Edifact account, the button "Create EDIFACT order" still displays.
I would expect that it does no longer appear.
This patch works well, however there are some things that should be considered and improved before signing off on this: 1) With Edifact disabled, the EDI links still appear in the Admin sidebar navigation 2) With Edifact disabled, I can still directly navigate to edi_accounts.pl etc., either through the sidebar nav or going straight to the URL. Patch no longer applies. Created attachment 130451 [details] [review] Bug 16258 Add a syspref to govern EDIFACT functionality Adds an "Edifact" systempreference to govern whether edifact processing is enabled. In most places this is hidden if the current vendor does not appear in the edi vendors table. This preference hides the admin screens which define this and a couple of links. Also fixes an anomaly whereby the basketgroup screen was not making the same check on whether edi ordering should be enabled as the basket screen. Both now use the same logic. Created attachment 130453 [details] [review] Bug 16258 Add a syspref to govern EDIFACT functionality Adds an "Edifact" systempreference to govern whether edifact processing is enabled. In most places this is hidden if the current vendor does not appear in the edi vendors table. This preference hides the admin screens which define this and a couple of links. Also fixes an anomaly whereby the basketgroup screen was not making the same check on whether edi ordering should be enabled as the basket screen. Both now use the same logic. Rebased-by: Mark Tompsett <mtompset@hotmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130454 [details] [review] Bug 16258: (follow-up) Handle admin-menu.inc Created attachment 130455 [details] [review] Bug 16258: (follow-up) Handle 'Create EDIFACT order' button Brought this back up to date and fixed the issues mentioned. Created attachment 130470 [details] [review] Bug 16258: Add a syspref to govern EDIFACT functionality Adds an "Edifact" systempreference to govern whether edifact processing is enabled. In most places this is hidden if the current vendor does not appear in the edi vendors table. This preference hides the admin screens which define this and a couple of links. Also fixes an anomaly whereby the basketgroup screen was not making the same check on whether edi ordering should be enabled as the basket screen. Both now use the same logic. Rebased-by: Mark Tompsett <mtompset@hotmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 130471 [details] [review] Bug 16258: (follow-up) Handle admin-menu.inc Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 130472 [details] [review] Bug 16258: (follow-up) Handle 'Create EDIFACT order' button Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 130473 [details] [review] Bug 16258: (QA follow-up) Add a guard to the cron script Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 130474 [details] [review] Bug 16258: (QA follow-up) Don't set to off for existing install, they may be using EDI Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 130475 [details] [review] Bug 16258: (QA follow-up) Include Koha plugin for basket.tt Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> There is 2 small things so far:
* When an order is added, there is a column "supplier report" in the basket summary table that I think should also be removed with the pref set to off.
* The doubled description here doesn't look right:
('EasyAnalyticalRecords','0','','If on, display in the catalogue screens tools to easily setup analytical record relationships','YesNo'),
+('Edifact','0','Enables edifact acquisitions functions','Enables use of Edifact electronic message exchange for acquisitions','YesNo'),
* There is now an EDIFACT heading on the acq syspref page, so I think we should move the pref there :)
* Spelling: EDIFACT is an abbreviation and we should spell it as such. Edifact for the syspref name makes sense, but in the descriptions we should use EDIFACT consistently (updatedatabase, syspref.sql, acquisitions.pref)
Rest looks great as far as I can tell!
Created attachment 130833 [details] [review] Bug 16258: Add a syspref to govern EDIFACT functionality Adds an "Edifact" systempreference to govern whether edifact processing is enabled. In most places this is hidden if the current vendor does not appear in the edi vendors table. This preference hides the admin screens which define this and a couple of links. Also fixes an anomaly whereby the basketgroup screen was not making the same check on whether edi ordering should be enabled as the basket screen. Both now use the same logic. Rebased-by: Mark Tompsett <mtompset@hotmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130834 [details] [review] Bug 16258: (follow-up) Handle admin-menu.inc Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130835 [details] [review] Bug 16258: (follow-up) Handle 'Create EDIFACT order' button Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130836 [details] [review] Bug 16258: (QA follow-up) Add a guard to the cron script Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130837 [details] [review] Bug 16258: (QA follow-up) Don't set to off for existing install, they may be using EDI Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130838 [details] [review] Bug 16258: (QA follow-up) Include Koha plugin for basket.tt Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130839 [details] [review] Bug 16258: (QA follow-up) Hide "Suppliers report" column As noted by Katrin, this column is only used for Edifact message data and so can be hidden when Edifact is disabled. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130840 [details] [review] Bug 16258: (QA follow-up) Remove superflous comment from sysprefs This patch removes the errant comment from default syspref SQL. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130841 [details] [review] Bug 16258: (QA follow-up) Move 'Edifact' preference This patch moves the global 'Edifact' switch into the 'EDIFACT' section of the sysprefs. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 130842 [details] [review] Bug 16258: (QA follow-up) Capitalise EDIFACT EDIFACT is an abreviation, so it should be ALLCAPS. * Electronic Data Interchange for Administration, Commerce and Transport Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> I checked and signed off on Kyles follow-ups and added some more to resolve the issues identified by Katrin :) Thank you for the fixes! Sadly removing the "Supplier report" column from the basket summary page breaks the table configuration on that page and as a consequence reopening and closing baskets. :( Not sure if we should just leave removing that column to the table configuration or if there is another good way of dealing with this. Created attachment 130952 [details] [review] Bug 16258: (QA follow-up) Fix columns_settings Splice out the relevant field from the columns_settings when EDIFACT is disabled Created attachment 130980 [details] [review] Bug 16258: Add a syspref to govern EDIFACT functionality Adds an "Edifact" systempreference to govern whether edifact processing is enabled. In most places this is hidden if the current vendor does not appear in the edi vendors table. This preference hides the admin screens which define this and a couple of links. Also fixes an anomaly whereby the basketgroup screen was not making the same check on whether edi ordering should be enabled as the basket screen. Both now use the same logic. Rebased-by: Mark Tompsett <mtompset@hotmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 130981 [details] [review] Bug 16258: (follow-up) Handle admin-menu.inc Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 130982 [details] [review] Bug 16258: (follow-up) Handle 'Create EDIFACT order' button Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 130983 [details] [review] Bug 16258: (QA follow-up) Add a guard to the cron script Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 130984 [details] [review] Bug 16258: (QA follow-up) Don't set to off for existing install, they may be using EDI Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 130985 [details] [review] Bug 16258: (QA follow-up) Include Koha plugin for basket.tt Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 130986 [details] [review] Bug 16258: (QA follow-up) Hide "Suppliers report" column As noted by Katrin, this column is only used for Edifact message data and so can be hidden when Edifact is disabled. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 130987 [details] [review] Bug 16258: (QA follow-up) Remove superflous comment from sysprefs This patch removes the errant comment from default syspref SQL. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 130988 [details] [review] Bug 16258: (QA follow-up) Move 'Edifact' preference This patch moves the global 'Edifact' switch into the 'EDIFACT' section of the sysprefs. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 130989 [details] [review] Bug 16258: (QA follow-up) Capitalise EDIFACT EDIFACT is an abreviation, so it should be ALLCAPS. * Electronic Data Interchange for Administration, Commerce and Transport Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 130990 [details] [review] Bug 16258: (QA follow-up) Fix columns_settings Splice out the relevant field from the columns_settings when EDIFACT is disabled Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 132391 [details] [review] Bug 16258: Add a syspref to govern EDIFACT functionality Adds an "Edifact" systempreference to govern whether edifact processing is enabled. In most places this is hidden if the current vendor does not appear in the edi vendors table. This preference hides the admin screens which define this and a couple of links. Also fixes an anomaly whereby the basketgroup screen was not making the same check on whether edi ordering should be enabled as the basket screen. Both now use the same logic. Rebased-by: Mark Tompsett <mtompset@hotmail.com> Rebased-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132392 [details] [review] Bug 16258: (follow-up) Handle admin-menu.inc Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132393 [details] [review] Bug 16258: (follow-up) Handle 'Create EDIFACT order' button Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132394 [details] [review] Bug 16258: (QA follow-up) Add a guard to the cron script Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132395 [details] [review] Bug 16258: (QA follow-up) Don't set to off for existing install, they may be using EDI Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132396 [details] [review] Bug 16258: (QA follow-up) Include Koha plugin for basket.tt Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132397 [details] [review] Bug 16258: (QA follow-up) Hide "Suppliers report" column As noted by Katrin, this column is only used for Edifact message data and so can be hidden when Edifact is disabled. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132398 [details] [review] Bug 16258: (QA follow-up) Remove superflous comment from sysprefs This patch removes the errant comment from default syspref SQL. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132399 [details] [review] Bug 16258: (QA follow-up) Move 'Edifact' preference This patch moves the global 'Edifact' switch into the 'EDIFACT' section of the sysprefs. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132400 [details] [review] Bug 16258: (QA follow-up) Capitalise EDIFACT EDIFACT is an abreviation, so it should be ALLCAPS. * Electronic Data Interchange for Administration, Commerce and Transport Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132401 [details] [review] Bug 16258: (QA follow-up) Fix columns_settings Splice out the relevant field from the columns_settings when EDIFACT is disabled Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132402 [details] [review] Bug 16258: (QA follow-up) Add note of disbaled EDIFACT to EDI accounts page One shouldn't end up here, but nice to have a note if you do Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed to master for 22.05, thanks to everybody involved [U+1F984] |