Bug 35942 - OPAC user can enroll several times to the same club
Summary: OPAC user can enroll several times to the same club
Status: Pushed to oldoldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 35941 12461
Blocks:
  Show dependency treegraph
 
Reported: 2024-01-30 14:00 UTC by Jonathan Druart
Modified: 2024-03-06 15:19 UTC (History)
15 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.03,23.05.09,22.11.15,22.05.19


Attachments
Bug 35942: OPAC user can enroll several times to the same club (7.83 KB, patch)
2024-01-30 16:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 35942: OPAC user can enroll several times to the same club (7.87 KB, patch)
2024-01-30 16:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 35942: OPAC user can enroll several times to the same club (7.94 KB, patch)
2024-01-31 09:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35942: OPAC user can enroll several times to the same club (8.00 KB, patch)
2024-01-31 13:51 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35942: OPAC user can enroll several times to the same club [23.05.x] (5.25 KB, patch)
2024-02-22 14:17 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 Jonathan Druart 2024-01-30 14:00:38 UTC
There is no restriction (certainly a unique key missing).

To recreate:
Create 3 clubs, 1 limited to library A, 1 limited to library B and one not limited

Use a patron with home library A.

Go to the opac-user page, "Clubs" tab show 0/2 (the one from library B is not listed)

Hit
http://localhost:8080/cgi-bin/koha/svc/club/enroll?id=1
http://localhost:8080/cgi-bin/koha/svc/club/enroll?id=2
http://localhost:8080/cgi-bin/koha/svc/club/enroll?id=3

Oops, you see 3/0

Hit again the same link

Oops, you see 6/0
Comment 1 Jonathan Druart 2024-01-30 14:01:43 UTC
There are 2 things here:
1. No unicity check on (patron, club, date_cancelled)
2. No check on the library limitation
Comment 2 Kyle M Hall 2024-01-30 16:03:18 UTC
Created attachment 161648 [details] [review]
Bug 35942: OPAC user can enroll several times to the same club

Test Plan:

1) Create 3 clubs, 1 limited to library A, 1 limited to library B and one not limited
2) Use a patron with home library A.
3) Go to the opac-user page, "Clubs" tab show 0/2 (the one from library B is not listed)
4) Browse to /cgi-bin/koha/svc/club/enroll?id=1
5) Reload that page a couple times
6) Note the patron is now enrolled in the same club multiple times
7) Delete those enrollments
8) Apply this patch
9) Restart all the things!
10) Repeat steps 2-7, note the lack of duplicate enrollments!
11) Repeat steps 2-10 for the staff interface
Comment 3 Kyle M Hall 2024-01-30 16:05:16 UTC
Created attachment 161649 [details] [review]
Bug 35942: OPAC user can enroll several times to the same club

Test Plan:

1) Create 3 clubs, 1 limited to library A, 1 limited to library B and one not limited
2) Use a patron with home library A.
3) Go to the opac-user page, "Clubs" tab show 0/2 (the one from library B is not listed)
4) Browse to /cgi-bin/koha/svc/club/enroll?id=1
5) Reload that page a couple times
6) Note the patron is now enrolled in the same club multiple times
7) Delete those enrollments
8) Apply this patch
9) Restart all the things!
10) Repeat steps 2-7, note the lack of duplicate enrollments!
11) Repeat steps 2-10 for the staff interface
Comment 4 Jonathan Druart 2024-01-31 09:02:06 UTC
Created attachment 161665 [details] [review]
Bug 35942: OPAC user can enroll several times to the same club

Test Plan:

1) Create 3 clubs, 1 limited to library A, 1 limited to library B and one not limited
2) Use a patron with home library A.
3) Go to the opac-user page, "Clubs" tab show 0/2 (the one from library B is not listed)
4) Browse to /cgi-bin/koha/svc/club/enroll?id=1
5) Reload that page a couple times
6) Note the patron is now enrolled in the same club multiple times
7) Delete those enrollments
8) Apply this patch
9) Restart all the things!
10) Repeat steps 2-7, note the lack of duplicate enrollments!
11) Repeat steps 2-10 for the staff interface

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 5 Jonathan Druart 2024-01-31 09:02:32 UTC
Not perfect but good enough for now. We should have the constraint in the DB.

Also I am wondering if the start/end dates should not be checked as well. That's for another day...

Thanks for the quick fix!
Comment 6 Martin Renvoize 2024-01-31 13:51:44 UTC
Created attachment 161689 [details] [review]
Bug 35942: OPAC user can enroll several times to the same club

Test Plan:

1) Create 3 clubs, 1 limited to library A, 1 limited to library B and one not limited
2) Use a patron with home library A.
3) Go to the opac-user page, "Clubs" tab show 0/2 (the one from library B is not listed)
4) Browse to /cgi-bin/koha/svc/club/enroll?id=1
5) Reload that page a couple times
6) Note the patron is now enrolled in the same club multiple times
7) Delete those enrollments
8) Apply this patch
9) Restart all the things!
10) Repeat steps 2-7, note the lack of duplicate enrollments!
11) Repeat steps 2-10 for the staff interface

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2024-01-31 13:52:03 UTC
All working, QA script happy, Passing QA
Comment 8 Kyle M Hall 2024-01-31 14:07:53 UTC
(In reply to Jonathan Druart from comment #5)
> Not perfect but good enough for now. We should have the constraint in the DB.
> 
> Also I am wondering if the start/end dates should not be checked as well.
> That's for another day...
> 
> Thanks for the quick fix!

I wasn't sure which columns should be part of a UNIQUE constraint for this table. It definitely needs club_id, borrowernumber, and date_cancelled. I'm not sure if date_cancelled being nullable causes any issues. If you think those are a good key set I'll add a followup to enforce those.
Comment 9 Jonathan Druart 2024-02-07 08:32:19 UTC
(In reply to Kyle M Hall from comment #8)
> (In reply to Jonathan Druart from comment #5)
> > Not perfect but good enough for now. We should have the constraint in the DB.
> > 
> > Also I am wondering if the start/end dates should not be checked as well.
> > That's for another day...
> > 
> > Thanks for the quick fix!
> 
> I wasn't sure which columns should be part of a UNIQUE constraint for this
> table. It definitely needs club_id, borrowernumber, and date_cancelled. I'm
> not sure if date_cancelled being nullable causes any issues. If you think
> those are a good key set I'll add a followup to enforce those.

Hum indeed, we certainly need a separate column (boolean) to tell that the patron is currently enrolled or not.
Comment 10 Martin Renvoize 2024-02-15 09:47:26 UTC
Pushed to master for 24.05.00
Comment 11 Lucas Gass 2024-02-21 21:52:34 UTC
This doesn't apply cleanly to 23.05.x. Kyle can we get a rebase?
Comment 12 Kyle M Hall 2024-02-22 14:17:19 UTC
Created attachment 162332 [details] [review]
Bug 35942: OPAC user can enroll several times to the same club [23.05.x]

Test Plan:

1) Create 3 clubs, 1 limited to library A, 1 limited to library B and one not limited
2) Use a patron with home library A.
3) Go to the opac-user page, "Clubs" tab show 0/2 (the one from library B is not listed)
4) Browse to /cgi-bin/koha/svc/club/enroll?id=1
5) Reload that page a couple times
6) Note the patron is now enrolled in the same club multiple times
7) Delete those enrollments
8) Apply this patch
9) Restart all the things!
10) Repeat steps 2-7, note the lack of duplicate enrollments!
11) Repeat steps 2-10 for the staff interface

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 wainuiwitikapark 2024-02-24 01:28:14 UTC
Hi there, I can't seem to reproduce the error on 22.05.x
Comment 14 Jonathan Druart 2024-02-26 10:55:21 UTC
(In reply to wainuiwitikapark from comment #13)
> Hi there, I can't seem to reproduce the error on 22.05.x

I do reproduce on 22.05.x
Comment 15 Fridolin Somers 2024-03-04 10:46:08 UTC
I see it is in 22.05.19

@Frederic

But I don't see it in 22.11.15
Comment 16 Frédéric Demians 2024-03-04 11:09:43 UTC
(In reply to Fridolin Somers from comment #15)

> But I don't see it in 22.11.15

Noooo! I don't know what happened. I can see myself testing this patch.
Comment 17 Fridolin Somers 2024-03-06 15:12:36 UTC
Patch pushed in v22.11.15-1