Bug 6331 - Obsolete marc column in deleteditems
Summary: Obsolete marc column in deleteditems
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Fridolin Somers
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-12 07:04 UTC by Fridolin Somers
Modified: 2015-06-04 23:24 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 6331 - Obsolete marc column in deleteditems (1.81 KB, patch)
2013-11-20 13:43 UTC, Fridolin Somers
Details | Diff | Splinter Review
[PATCH][SIGNED OFF] Bug 6331 - Obsolete marc column in deleteditems (2.08 KB, patch)
2013-11-30 10:48 UTC, Mathieu Saby
Details | Diff | Splinter Review
Supplementary patch (1.21 KB, patch)
2013-12-03 11:44 UTC, Colin Campbell
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 6331 Do not populate marc in deleted items (1.27 KB, patch)
2013-12-05 13:02 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 6331 - Update timestamp when deleting items (1.53 KB, patch)
2013-12-05 14:40 UTC, Fridolin Somers
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 6331 - Update timestamp when deleting items (1.61 KB, patch)
2013-12-09 14:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 6331 - Obsolete marc column in deleteditems (2.09 KB, patch)
2013-12-20 17:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 6331 Do not populate marc in deleted items (1.34 KB, patch)
2013-12-20 17:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 6331 - Update timestamp when deleting items (1.72 KB, patch)
2013-12-20 17:50 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2011-05-12 07:04:46 UTC
There is a difference between "items" and "deleteditems" tables in "kohastructure.sql" :
"deleteditems" has a field "marc" not existing in "items".

This columns appears to be obsolete.
Comment 1 Owen Leonard 2013-03-26 16:18:24 UTC
This bug is still valid in master as of 3.12 beta1.
Comment 2 Fridolin Somers 2013-11-20 13:43:35 UTC Comment hidden (obsolete)
Comment 3 Mathieu Saby 2013-11-30 10:48:03 UTC Comment hidden (obsolete)
Comment 4 Mathieu Saby 2013-11-30 10:48:35 UTC
Test :
- suppressing an item, checking the deleted item is properly stored in deleteditems table
- checking the column marc has been deleted from deleteditems table


I sign off

Mathieu
Comment 5 Colin Campbell 2013-12-03 11:44:23 UTC Comment hidden (obsolete)
Comment 6 Martin Renvoize 2013-12-03 11:49:34 UTC
This patch needs a followup to remove any references in code to the marc column in deleteditems.

I spotted one in C4/Items.pm at least. I'm also wondering if this affects the output of tools/export.pl (or rather assume it affects) so that will need correcting to.
Comment 7 Martin Renvoize 2013-12-03 11:51:18 UTC
Looks like Colin did the first followup I wanted whilst I was failing.. can someone take care of the second one?

I'll then nudge Katrin to do QA as with Colin's input I no longer can..
Comment 8 Martin Renvoize 2013-12-03 12:06:21 UTC
Setting back to Needs Signoff, I've gone through the code and don't think the third followup is required.. I believe the marc column is grabbed from biblioitems in the join so this isn't a regression.
Comment 9 Fridolin Somers 2013-12-05 13:02:27 UTC Comment hidden (obsolete)
Comment 10 Fridolin Somers 2013-12-05 13:04:10 UTC
Comment on attachment 23316 [details] [review]
[SIGNED-OFF]Bug 6331 Do not populate marc in deleted items

Tested by applying the 2 patches, deleting an item and check that item is moved from table items to table deleteditems.
Comment 11 Mathieu Saby 2013-12-05 13:12:04 UTC
hello
I suspect this line for copying marc column was only here to update the timestamp of deleteditems table.
With your patch, is the timestamp still updated?

Mathieu
Comment 12 Marcel de Rooy 2013-12-05 13:57:45 UTC
(In reply to mathieu saby from comment #11)
> hello
> I suspect this line for copying marc column was only here to update the
> timestamp of deleteditems table.
> With your patch, is the timestamp still updated?
> 
> Mathieu

Just jumping in this discussion: But I would not like to loose the timestamp in deleteditems or deletedbiblio..
Comment 13 Fridolin Somers 2013-12-05 14:13:36 UTC
(In reply to mathieu saby from comment #11)
In my test, timestamp value is copied from table items. It does not get the current timestamp.

@M. de Rooy :
If timestamp is not updated, how do we know the date/time of deletion ?
Comment 14 Mathieu Saby 2013-12-05 14:24:23 UTC
(In reply to Fridolin SOMERS from comment #13)
> (In reply to mathieu saby from comment #11)
> In my test, timestamp value is copied from table items. It does not get the
> current timestamp.
> 
> @M. de Rooy :
> If timestamp is not updated, how do we know the date/time of deletion ?



I made some reports recently on my Koha (v 3.11). Timestamp of deleteditems were updated when we delete them.
So this behavior must be keep in 3.14 :-)

Mathieu
Comment 15 Fridolin Somers 2013-12-05 14:40:19 UTC Comment hidden (obsolete)
Comment 16 Fridolin Somers 2013-12-05 14:40:48 UTC
Set as need signoff for 3rd patch.
Comment 17 Martin Renvoize 2013-12-09 14:34:55 UTC Comment hidden (obsolete)
Comment 18 Martin Renvoize 2013-12-09 14:35:29 UTC
Everything now works as advertised for me.. signing off.
Comment 19 Kyle M Hall 2013-12-20 17:49:37 UTC
Created attachment 23725 [details] [review]
Bug 6331 - Obsolete marc column in deleteditems

There is a difference between "items" and "deleteditems" tables in "kohastructure.sql" :
"deleteditems" has a field "marc" not existing in "items".

This patch removes this obsolete column.
===
Test :
- suppressing an item, checking the deleted item is properly stored in deleteditems table
- checking the column marc has been deleted from deleteditems table

Signed-off-by: Mathieu Saby <mathieu.saby@univ-rennes2.fr>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 20 Kyle M Hall 2013-12-20 17:49:51 UTC
Created attachment 23726 [details] [review]
Bug 6331 Do not populate marc in deleted items

This is supplementary to the main patch for
bug 6331. Having removed the attribute marc from
items DelItem should not populate it

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 21 Kyle M Hall 2013-12-20 17:50:08 UTC
Created attachment 23727 [details] [review]
Bug 6331 - Update timestamp when deleting items

When item is transfered from items table to deleted items, all fields must be copies but "timestamp".
This value must be updated to know when item has been deleted.

Test plan:
- Look a an item timestamp :
mysql> select timestamp from items where itemnumber = 2690;
+---------------------+
| timestamp           |
+---------------------+
| 2011-09-09 15:30:21 |
+---------------------+
1 row in set (0.00 sec)
- Delete this item in cataloguing module
- Check it is not in items table anymore :
mysql> select timestamp from items where itemnumber = 2690;
Empty set (0.00 sec)
- Look in deleteditems table :
mysql> select timestamp from deleteditems where itemnumber = 2690;
+---------------------+
| timestamp           |
+---------------------+
| 2013-12-05 15:33:20 |
+---------------------+
1 row in set (0.00 sec)
=> timestamp as been set to actual date/time

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Patch set passes koha-qa.pl, works as advertised!
Comment 22 Galen Charlton 2013-12-23 16:42:57 UTC
Pushed to master.  Thanks, Fridolin and Colin!
Comment 23 Fridolin Somers 2014-01-03 08:50:28 UTC
Patches pushed to 3.14.x, will be in 3.14.2.
(created version 3.14.01.004)