Bug 35352 - Cannot hide SMSalertnumber via BorrowerUnwantedField
Summary: Cannot hide SMSalertnumber via BorrowerUnwantedField
Status: Pushed to oldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Lucas Gass (lukeg)
QA Contact: Victor Grousset/tuxayo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-11-16 14:58 UTC by Lucas Gass (lukeg)
Modified: 2024-11-15 01:58 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:
24.05.00,23.11.01,23.05.07
Circulation function:


Attachments
Bug 35352: Allow SMSalertnumber to be hidden via BorrowerUnwantedField (2.71 KB, patch)
2023-11-16 15:00 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 35352: Allow SMSalertnumber to be hidden via BorrowerUnwantedField (2.77 KB, patch)
2023-11-16 22:04 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 35352: Allow SMSalertnumber to be hidden via BorrowerUnwantedField (2.83 KB, patch)
2023-11-17 04:10 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass (lukeg) 2023-11-16 14:58:12 UTC
To recreate:

1. Add a value to SMSSendDriver like 'Emial'.
2. Go to BorrowerUnwantedField and attempt to hide "Mobile phone number (smsalertnumber)"
3. Go to members/memberentry.pl and see the field is not hidden.
Comment 1 Lucas Gass (lukeg) 2023-11-16 15:00:31 UTC
Created attachment 159054 [details] [review]
Bug 35352: Allow SMSalertnumber to be hidden via BorrowerUnwantedField

To test:
1. Add a value to SMSSendDriver like 'Emial'.
2. Go to BorrowerUnwantedField and attempt to hide "Mobile phone number (smsalertnumber)"
3. Go to members/memberentry.pl and see the field is not hidden.
4. APPLY PATCH
5. Try step 3 again and this time the field should be properly hidden.

Note: This patch also includes some indentation changes.
Comment 2 ByWater Sandboxes 2023-11-16 22:04:35 UTC
Created attachment 159068 [details] [review]
Bug 35352: Allow SMSalertnumber to be hidden via BorrowerUnwantedField

To test:
1. Add a value to SMSSendDriver like 'Emial'.
2. Go to BorrowerUnwantedField and attempt to hide "Mobile phone number (smsalertnumber)"
3. Go to members/memberentry.pl and see the field is not hidden.
4. APPLY PATCH
5. Try step 3 again and this time the field should be properly hidden.

Note: This patch also includes some indentation changes.
Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 3 Andrew Fuerste-Henry 2023-11-16 22:17:18 UTC
This works as described. 
With the patch applied, the SMS Number field still shows up under the messaging pref box on moremember.pl. It kinda feels like setting it in BorrowerUnwanted should also hide it on moremember.pl.
Also, I just filed bug 35356 to suggest we not show SMS Number on moremember.pl at all if the patron doesn't have a value for that in their record.
Comment 4 Victor Grousset/tuxayo 2023-11-17 04:10:21 UTC
Created attachment 159074 [details] [review]
Bug 35352: Allow SMSalertnumber to be hidden via BorrowerUnwantedField

To test:
1. Add a value to SMSSendDriver like 'Emial'.
2. Go to BorrowerUnwantedField and attempt to hide "Mobile phone number (smsalertnumber)"
3. Go to members/memberentry.pl and see the field is not hidden.
4. APPLY PATCH
5. Try step 3 again and this time the field should be properly hidden.

Note: This patch also includes some indentation changes.
Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 5 Victor Grousset/tuxayo 2023-11-17 04:10:39 UTC
(In reply to Andrew Fuerste-Henry from comment #3)
> With the patch applied, the SMS Number field still shows up under the
> messaging pref box on moremember.pl. It kinda feels like setting it in
> BorrowerUnwanted should also hide it on moremember.pl.
> Also, I just filed bug 35356 to suggest we not show SMS Number on
> moremember.pl at all if the patron doesn't have a value for that in their
> record.

Good observation thanks for the new ticket :)
Comment 6 Victor Grousset/tuxayo 2023-11-17 04:11:18 UTC
Works, makes sense, QA script happy, code looks good, passing QA :)
Comment 7 Katrin Fischer 2023-12-11 18:46:59 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 8 Fridolin Somers 2023-12-12 07:28:02 UTC
Pushed to 23.11.x for 23.11.01
Comment 9 Lucas Gass (lukeg) 2023-12-14 21:04:34 UTC
Backported to 23.05.x for upcoming 23.05.07