Bug 13535 - Table alert is missing FK and not deleted with the patron
Summary: Table alert is missing FK and not deleted with the patron
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Katrin Fischer
QA Contact: Marcel de Rooy
URL:
Keywords:
: 10631 (view as bug list)
Depends on:
Blocks: 5334
  Show dependency treegraph
 
Reported: 2015-01-07 17:37 UTC by Katrin Fischer
Modified: 2020-11-10 22:21 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
20.11.00


Attachments
32# Attachment to Bug 13535 - Table alert is missing FK and not deleted with the patron (2.71 KB, patch)
2020-04-15 22:07 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 13535: Add FK constraint on borrowrnumber to alert table (2.96 KB, patch)
2020-08-14 12:54 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 13535: Add FK constraint on borrowrnumber to alert table (3.14 KB, patch)
2020-08-14 13:05 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 13535: Add FK constraint on borrowrnumber to alert table (3.19 KB, patch)
2020-09-16 13:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 13535: Add FK constraint on borrowernumber to alert table (3.30 KB, patch)
2020-10-02 09:33 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 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 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 2020-11-10 22:21:11 UTC
enhancement will not be backported to 20.05.x