Bug 7493

Summary: Deleting a record with comments breaks
Product: Koha Reporter: Magnus Enger <magnus>
Component: OPACAssignee: Owen Leonard <oleonard>
Status: CLOSED FIXED QA Contact: Ian Walls <koha.sekjal>
Severity: normal    
Priority: P5 - low CC: julian.maurice, paul.poulain
Version: 3.6   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on:    
Bug Blocks: 8149    
Attachments: Bug 7493 - Deleting a record with comments breaks "Recent Comments"
Bug 7493 [REVISED] Deleting a record with comments breaks "Recent Comments"
[Signed-off] Bug 7493 [REVISED] Deleting a record with comments breaks "Recent Comments"
Bug 7493 [REVISED] Deleting a record with comments breaks "Recent Comments"

Description Magnus Enger 2012-02-02 11:50:18 UTC 
A library deleted a record that had comments attached to it, and now they get an error when they click on "Recent Comments" in the OPAC: 

Software error:
Can't call method "field" on an undefined value at /usr/share/koha/lib/C4/Koha.pm line 1216.

This is version 3.6.3.
Comment 1 Owen Leonard 2012-02-06 15:06:45 UTC 
I think the solution to this problem is to add a foreign key constraint so that when bibliographic or patron records are deleted the reviews table is updated. I propose to:

- delete reviews when bibliographic records are deleted
- set set reviews.borrowernumber to NULL when patron records are deleted.

The database update making these changes will not run properly if there are already instances of this bug--reviews with deleted biblios or patrons--so some kind of update will have to be run in addition to setting the constraint.

Does anyone have any comments or suggestions about the efficiency/efficacy of using these queries for that purpose?

DELETE FROM reviews WHERE biblionumber IN (SELECT biblionumber from deletedbiblio);

UPDATE reviews SET borrowernumber = NULL WHERE borrowernumber IN (SELECT borrowernumber FROM deletedborrowers);
Comment 2 Katrin Fischer 2012-02-12 20:00:16 UTC 
Hi Owen, 
I am not sure about the SQL, but I think the changes you suggest makes sense and are logical. I am not sure if we might want to delete reviews with the patron account too (privacy issues?).
Comment 3 Owen Leonard 2012-02-17 16:59:06 UTC Comment hidden (obsolete)
Comment 4 Julian Maurice 2012-02-23 10:26:37 UTC 
I don't know when or how it happened but I had in database 1 line in biblio and 1 line in deletedbiblio with the same biblionumber, which cause comments on this biblio to be deleted.
I'm sure this is not a common case, and it should not happen, but I would prefer that deletions are done with an SQL query like this

DELETE FROM reviews WHERE biblionumber **NOT** IN (SELECT biblionumber from
**biblio**);
(same for patrons)
Comment 5 Owen Leonard 2012-02-28 16:12:04 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2012-02-28 16:23:58 UTC 
The revised patch takes Julian's suggestion to use "NOT IN" biblio and borrowers.
Comment 7 Julian Maurice 2012-03-02 13:36:06 UTC Comment hidden (obsolete)
Comment 8 Ian Walls 2012-03-02 15:46:38 UTC 
Database constraints added in safe and sustainable fashion, with cleanup for upgrades.  Template changes are clean.  Marking as Passed QA.
Comment 9 Owen Leonard 2012-03-05 14:14:28 UTC 
Created attachment 8011 [details] [review]
Bug 7493 [REVISED] Deleting a record with comments breaks "Recent Comments"

Adding foreign key constraint on biblio and borrowers:

- delete reviews when bibliographic records are deleted
- set set reviews.borrowernumber to NULL when patron records are deleted.

Before these constraints are set the database update script will
clean up existing instances of these problems by deleting reviews of
deleted biblios and setting to NULL reviews.borrowernumber for deleted
patrons.

In comments moderation, the list of comments will indicate "(deleted patron")
if the review was submitted by a patron who has since been deleted.

In the OPAC display of comments will omit the patron name altogether
if the patron has since been deleted.

To test:

1. CONFIRM THAT THE DATABASE UPDATE RUNS CORRECTLY

Before applying the patch:

 - delete the record for a title which has a review
 - delete a patron who has submitted a review (on a different title).

Viewing /cgi-bin/koha/opac-showreviews.pl should result in an error.

Apply the patch and make sure the database update runs. Viewing
the recent comments page in the OPAC should no longer result in an
error. The title with the comment from a deleted patron should
show no patron name on these pages:

 - Recent comments (opac-showreviews.pl)
 - Recent comments RSS (opac-showreviews.pl?format=rss)
 - Detail page (opac-detail.pl)

Comments from other patrons should show correctly.

2. CONFIRM THAT NEW DELETIONS PERFORM CORRECTLY

After applying the patch:

 - delete the record for a title which has a review
 - delete a patron who has submitted a review (on a different title).

Viewing /cgi-bin/koha/opac-showreviews.pl should NOT result in an error.
The review of the title which you deleted should no longer appear in the
list of recent comments.

The title with the comment from a deleted patron should
show no patron name on these pages:

 - Recent comments (opac-showreviews.pl)
 - Recent comments RSS (opac-showreviews.pl?format=rss)
 - Detail page (opac-detail.pl)

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 10 Owen Leonard 2012-03-05 14:15:14 UTC 
The patch no longer applied so I rebased and fixed conflicts.
Comment 11 Jared Camins-Esakov 2012-05-09 17:04:54 UTC 
This bug will be included in the Koha 3.6.5 release.