[Bug 36002] New: Get rid of aqorders.purchaseordernumber
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Bug ID: 36002 Summary: Get rid of aqorders.purchaseordernumber Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Acquisitions Assignee: koha-bugs@lists.koha-community.org Reporter: m.de.rooy@rijksmuseum.nl QA Contact: testopia@bugs.koha-community.org This field is unused. Any attempt to use it was inactive since 2014. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=11010 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 --- Comment #1 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 161779 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161779&action=edit Bug 36002: Database revision for aqorders.purchaseordernumber Test plan: Run dbrev. Check if field is no longer there. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 --- Comment #2 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 161780 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161780&action=edit Bug 36002: DBIx schema changes for Aqorder No test plan. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 161781 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161781&action=edit Bug 36002: Remove unused subroutine GetParcels That did refer to our aqorders.purchaseordernumber.. Test plan: Git grep for GetParcels. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 161782 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161782&action=edit Bug 36002: Remove last references to purchaseordernumber Test plan: Git grep for it. Ignore update22to30.pl and atomicupdate. Verify that you can still create/update/delete an acq order. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |m.de.rooy@rijksmuseum.nl |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161779|0 |1 is obsolete| | Attachment #161780|0 |1 is obsolete| | Attachment #161781|0 |1 is obsolete| | Attachment #161782|0 |1 is obsolete| | --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 164938 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164938&action=edit Bug 36002: Database revision for aqorders.purchaseordernumber Test plan: Run dbrev. Check if field is no longer there. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 164939 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164939&action=edit Bug 36002: DBIx schema changes for Aqorder No test plan. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 164940 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164940&action=edit Bug 36002: Remove unused subroutine GetParcels That did refer to our aqorders.purchaseordernumber.. Test plan: Git grep for GetParcels. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 164941 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164941&action=edit Bug 36002: Remove last references to purchaseordernumber Test plan: Git grep for it. Ignore update22to30.pl and atomicupdate. Verify that you can still create/update/delete an acq order. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Kyle M Hall from comment #8)
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Great. Thanks. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164938|0 |1 is obsolete| | --- Comment #10 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 165386 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165386&action=edit Bug 36002: Database revision for aqorders.purchaseordernumber Test plan: Run dbrev. Check if field is no longer there. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164939|0 |1 is obsolete| | --- Comment #11 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 165387 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165387&action=edit Bug 36002: DBIx schema changes for Aqorder No test plan. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164940|0 |1 is obsolete| | --- Comment #12 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 165388 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165388&action=edit Bug 36002: Remove unused subroutine GetParcels That did refer to our aqorders.purchaseordernumber.. Test plan: Git grep for GetParcels. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #164941|0 |1 is obsolete| | --- Comment #13 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 165389 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165389&action=edit Bug 36002: Remove last references to purchaseordernumber Test plan: Git grep for it. Ignore update22to30.pl and atomicupdate. Verify that you can still create/update/delete an acq order. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@ptfs-europe.co | |m --- Comment #14 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Code makes sense. Tested creating/editing and deleting a purchase order through UI, all works. QA script is showing what I believe is a false positive for POD coverage: Before applying patchset: perl -MPod::Coverage=C4::Acquisition -e666 C4::Acquisition has a Pod::Coverage rating of 0.94 The following are uncovered: GetOrderFromItemnumber, NotifyOrderUsers, ReOpenBasketgroup After applying patchset: perl -MPod::Coverage=C4::Acquisition -e666 C4::Acquisition has a Pod::Coverage rating of 0.938775510204082 The following are uncovered: GetOrderFromItemnumber, NotifyOrderUsers, ReOpenBasketgroup -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I have to admit that we have used this column in migrations to allow for reporting. And I feel we are not the only ones. Right now we just drop the column without checking for any contents. Could there be a more clever way of doing this, that raises a warning first? In light of us wanting to do more clean-up I think it would be worth to agree on how to handle this in general and then apply it to every removal moving forward. Maybe something like we did for items.paidfor? if ( column_exists( 'items', 'paidfor' ) ) { my ($count) = $dbh->selectrow_array( q| SELECT COUNT(*) FROM items WHERE paidfor IS NOT NULL AND paidfor <> "" | ); if ($count) { warn "Warning - Cannot remove column items.paidfor. At least one value exists"; } else { $dbh->do(q|ALTER TABLE items DROP COLUMN paidfor|); $dbh->do(q|UPDATE marc_subfield_structure SET kohafield = '' WHERE kohafield = 'items.paidfor'|); } } Database update is missing the "success" say out line. Otherwise this does a good clean-up :) Didn't find any other references and amazed we missed removing GetParcels! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_24_05_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Katrin Fischer from comment #15)
I have to admit that we have used this column in migrations to allow for reporting. And I feel we are not the only ones. Right now we just drop the column without checking for any contents.
Did you really? :)
Could there be a more clever way of doing this, that raises a warning first?
Well, will add a follow-up to meet your request. But have to admit that I was a bit hesitant to do so. You could theoretically say that people filling Koha columns with their own data should not depend on the development team to recover that data.
In light of us wanting to do more clean-up I think it would be worth to agree on how to handle this in general and then apply it to every removal moving forward.
I do not hope that this argument will hijack this simple cleanup operation. Please start that discussion elsewhere.
Maybe something like we did for items.paidfor?
I chose another approach.
Database update is missing the "success" say out line.
I have never seen that as a requirement. Which coding guideline is that? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA Patch complexity|Trivial patch |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 165853 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165853&action=edit Bug 36002: (follow-up) Save purchaseordernumbers from aqorders We copy them here to another (temporary) table. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #16)
Database update is missing the "success" say out line.
I have never seen that as a requirement. Which coding guideline is that?
DEV atomic update /usr/share/koha/installer/data/mysql/atomicupdate/bug_36002.pl [11:41:13]: Bug 36002 - Remove aqorders.purchaseordernumber We found 1 order lines where field purchaseordernumber was filled! These records have been copied to table: zzaqorders_purchaseordernumber. Please examine the data and remove this table. The last line here is kind of a success line btw ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
I have never seen that as a requirement. Which coding guideline is that?
It's in our skeleton.pl template with tons of examples. We tell what we are going to do before running the SQL and we tell what we did after. This is a standard thing, nothing I made up. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_24_05_candidate |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Needs documenting CC| |fridolin.somers@biblibre.co | |m --- Comment #21 from Fridolin Somers <fridolin.somers@biblibre.com> --- I prefer not to impact stable version 23.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36002 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED CC| |caroline.cyr-la-rose@inlibr | |o.com Resolution|--- |FIXED --- Comment #22 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- This looks like an architecture change. Nothing to add/edit in the Koha manual. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org