Bug 13535

Summary:	Table alert is missing FK and not deleted with the patron
Product:	Koha	Reporter:	Katrin Fischer <katrin.fischer>
Component:	Database	Assignee:	Katrin Fischer <katrin.fischer>
Status:	CLOSED FIXED	QA Contact:	Marcel de Rooy <m.de.rooy>
Severity:	enhancement
Priority:	P5 - low	CC:	jonathan.druart, lucas, m.de.rooy, martin.renvoize
Version:	Main
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	Small patch
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:	20.11.00
Circulation function:
Bug Depends on:
Bug Blocks:	5334
Attachments:	32# Attachment to Bug 13535 - Table alert is missing FK and not deleted with the patron Bug 13535: Add FK constraint on borrowrnumber to alert table Bug 13535: Add FK constraint on borrowrnumber to alert table Bug 13535: Add FK constraint on borrowrnumber to alert table Bug 13535: Add FK constraint on borrowernumber to alert table

Description Katrin Fischer 2015-01-07 17:37:52 UTC

The table alert contains subscriptions for email notifications for new serial issues. There is no FK on the borrowernumber and no other permission I can see to clean the table once the patron has been deleted.


2806 DROP TABLE IF EXISTS `alert`;
2807 CREATE TABLE `alert` (
2808   `alertid` int(11) NOT NULL auto_increment,
2809   `borrowernumber` int(11) NOT NULL default 0,
2810   `type` varchar(10) NOT NULL default '',
2811   `externalid` varchar(20) NOT NULL default '',
2812   PRIMARY KEY  (`alertid`),
2813   KEY `borrowernumber` (`borrowernumber`),
2814   KEY `type` (`type`,`externalid`)
2815 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;

Comment 1 Katrin Fischer 2015-01-08 14:41:34 UTC

*** Bug 10631 has been marked as a duplicate of this bug. ***

Comment 2 Katrin Fischer 2020-04-15 22:07:21 UTC

Created attachment 103039 [details] [review]
32# Attachment to Bug 13535 - Table alert is missing FK and not deleted with the patron

Bug 13535: Add FK constraint on borrowrnumber to alert table

When a patron is deleted, we should clean up the data connected
to the patron. This includes subscriptions to 'new issue arrived'
arrive messages from the serials module.

To test:

Part1:
- Apply patch
- Run updatedatabase - verify no error message
- Run updatedatabase again - verify still no error message
- Add a subscription and select a notice template for patron notification
- Log in as any patron (not yourself) and subscribe to the email alert
  in the OPAC
- Veriy on the subscription detail page, that the patron is subscribed
  in staff
- Note borrowernumber and delete patron
- SELECT * from alert; in a report
- There should no longer be an entry with the borrowernumber
Part 2:
- Start fresh with the web installer, verify there are no errors on
  creating the database tables

Comment 3 Jonathan Druart 2020-04-16 09:05:56 UTC

Katrin, we need to delete existing entries (those without valid borrowernumber), otherwise the FK won't be created.

Comment 4 Katrin Fischer 2020-04-16 09:20:50 UTC

(In reply to Jonathan Druart from comment #3)
> Katrin, we need to delete existing entries (those without valid
> borrowernumber), otherwise the FK won't be created.

Makes sense - I didn't think of that. 

Do you have a hint/recommendation on how to approach this?

Comment 5 Jonathan Druart 2020-04-16 09:58:02 UTC

See 18.12.00.014

Comment 6 Katrin Fischer 2020-08-14 12:54:09 UTC

Created attachment 108272 [details] [review]
Bug 13535: Add FK constraint on borrowrnumber to alert table

When a patron is deleted, we should clean up the data connected
to the patron. This includes subscriptions to 'new issue arrived'
arrive messages from the serials module.

To test:

Part1:
- Before applying the patch
- Add a subscription and select a notice template for patron notification
- Go to the OPAC and subscribe to the email alert
- Log in as any other patron (not yourself) and subscribe with that patron too
- Verfiy on the subscription detail page, that the patrons are subscribed
- Note the borrowernumber of your patrons
- Delete the second patron
- Note borrowernumber and delete patron
- Run: SELECT * from alert; in a report
- Despite the patron being deleted, you will see entries for both
- Apply patch
- Run updatedatabase - verify no error message
- Run updatedatabase again - verify still no error message
- There should no longer be an entry with the deleted borrowernumber,
  but only the entry for your own patron account
Part 2:
- Start fresh with the web installer, verify there are no errors on
  creating the database tables

Comment 7 Katrin Fischer 2020-08-14 13:05:32 UTC

Created attachment 108275 [details] [review]
Bug 13535: Add FK constraint on borrowrnumber to alert table

When a patron is deleted, we should clean up the data connected
to the patron. This includes subscriptions to 'new issue arrived'
arrive messages from the serials module.

To test:

Part1:
- Before applying the patch
- Add a subscription and select a notice template for patron notification
- Go to the OPAC and subscribe to the email alert
- Log in as any other patron (not yourself) and subscribe with that patron too
- Verfiy on the subscription detail page, that the patrons are subscribed
- Note the borrowernumber of your patrons
- Delete the second patron
- Note borrowernumber and delete patron
- Run: SELECT * from alert; in a report
- Despite the patron being deleted, you will see entries for both
- Apply patch
- Run updatedatabase - verify no error message
- Run updatedatabase again - verify still no error message
- There should no longer be an entry with the deleted borrowernumber,
  but only the entry for your own patron account
Part 2:
- Start fresh with the web installer, verify there are no errors on
  creating the database tables

Comment 8 Martin Renvoize (ashimema) 2020-09-16 13:20:15 UTC

Created attachment 110194 [details] [review]
Bug 13535: Add FK constraint on borrowrnumber to alert table

When a patron is deleted, we should clean up the data connected
to the patron. This includes subscriptions to 'new issue arrived'
arrive messages from the serials module.

To test:

Part1:
- Before applying the patch
- Add a subscription and select a notice template for patron notification
- Go to the OPAC and subscribe to the email alert
- Log in as any other patron (not yourself) and subscribe with that patron too
- Verfiy on the subscription detail page, that the patrons are subscribed
- Note the borrowernumber of your patrons
- Delete the second patron
- Note borrowernumber and delete patron
- Run: SELECT * from alert; in a report
- Despite the patron being deleted, you will see entries for both
- Apply patch
- Run updatedatabase - verify no error message
- Run updatedatabase again - verify still no error message
- There should no longer be an entry with the deleted borrowernumber,
  but only the entry for your own patron account
Part 2:
- Start fresh with the web installer, verify there are no errors on
  creating the database tables

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Comment 9 Martin Renvoize (ashimema) 2020-09-16 13:20:46 UTC

Yeay for proper constraints.

Thanks Katrin, looks good to me and works as expected. Signing off

Comment 10 Katrin Fischer 2020-09-16 14:52:59 UTC

Thank you, glad to see this finally move on :)

Comment 11 Katrin Fischer 2020-10-02 09:24:40 UTC

Hi Marcel, why BLOCKED?

Comment 12 Marcel de Rooy 2020-10-02 09:33:00 UTC

Created attachment 111076 [details] [review]
Bug 13535: Add FK constraint on borrowernumber to alert table

When a patron is deleted, we should clean up the data connected
to the patron. This includes subscriptions to 'new issue arrived'
arrive messages from the serials module.

To test:

Part1:
- Before applying the patch
- Add a subscription and select a notice template for patron notification
- Go to the OPAC and subscribe to the email alert
- Log in as any other patron (not yourself) and subscribe with that patron too
- Verfiy on the subscription detail page, that the patrons are subscribed
- Note the borrowernumber of your patrons
- Delete the second patron
- Note borrowernumber and delete patron
- Run: SELECT * from alert; in a report
- Despite the patron being deleted, you will see entries for both
- Apply patch
- Run updatedatabase - verify no error message
- Run updatedatabase again - verify still no error message
- There should no longer be an entry with the deleted borrowernumber,
  but only the entry for your own patron account
Part 2:
- Start fresh with the web installer, verify there are no errors on
  creating the database tables

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Comment 13 Marcel de Rooy 2020-10-02 09:34:15 UTC

(In reply to Katrin Fischer from comment #11)
> Hi Marcel, why BLOCKED?

To signal that something is going on ;) And keep other QAers away from doing the same thing twice

Comment 14 Jonathan Druart 2020-10-08 14:20:02 UTC

Pushed to master for 20.11, thanks to everybody involved!

Comment 15 Lucas Gass (lukeg) 2020-11-10 22:21:11 UTC

enhancement will not be backported to 20.05.x