Bug 32500 - OPAC_REG_VERIFY has too many options for "Message body"
Summary: OPAC_REG_VERIFY has too many options for "Message body"
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-12-19 23:11 UTC by David Cook
Modified: 2023-01-22 23:51 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 32500: Show only relevant message body option for OPAC_REG_VERIFY (1.51 KB, patch)
2022-12-20 04:27 UTC, David Cook
Details | Diff | Splinter Review
Bug 32500: Show only relevant message body option for OPAC_REG_VERIFY (1.66 KB, patch)
2022-12-27 10:42 UTC, Frédéric Demians
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2022-12-19 23:11:59 UTC
When creating the "Message body" for the "OPAC_REG_VERIFY" email template, the only available options should be <<OPACBaseURL>> and <<borrower_modifications.verification_token>>
Comment 1 David Cook 2022-12-20 04:27:33 UTC
Created attachment 144731 [details] [review]
Bug 32500: Show only relevant message body option for OPAC_REG_VERIFY

This patch changes the "Message body" options for the
OPAC_REG_VERIFY notice so that only the relevant
"borrower_modifications.verification_token"
appears as an option.

Tables like branches, biblio, biblioitems, items, borrowers,
and issues aren't passed to OPAC_REG_VERIFY at processing time.
The only table passed is "borrower_modifications", and only the
token is the relevant field there.
Comment 2 Frédéric Demians 2022-12-27 10:42:16 UTC
Created attachment 144846 [details] [review]
Bug 32500: Show only relevant message body option for OPAC_REG_VERIFY

This patch changes the "Message body" options for the
OPAC_REG_VERIFY notice so that only the relevant
"borrower_modifications.verification_token"
appears as an option.

Tables like branches, biblio, biblioitems, items, borrowers,
and issues aren't passed to OPAC_REG_VERIFY at processing time.
The only table passed is "borrower_modifications", and only the
token is the relevant field there.

Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Comment 3 Marcel de Rooy 2023-01-13 10:36:39 UTC
Why not add_fields on borrower modifications? Maybe you want to change the notice and show some of the fields that have been entered? This seems too drastic?
Comment 4 David Cook 2023-01-15 23:00:04 UTC
(In reply to Marcel de Rooy from comment #3)
> Why not add_fields on borrower modifications? Maybe you want to change the
> notice and show some of the fields that have been entered? This seems too
> drastic?

Upon reviewing the borrower modifications table, I'm not sure why I thought verification_token should be the only field. It doesn't really make much sense.

My concern about add_fields() is that it would have access to the "password" field. It looks like it's the hashed password rather than the clear text password, but it should probably be excluded. But it looks like we could modify the "get_columns_for" function in letter.pl to exclude that field.

I'll have another think about this one another time!
Comment 5 David Cook 2023-01-22 23:51:10 UTC
I haven't reviewed bug 32221 in context but I think it might cover off the issue here...