Description
Katrin Fischer
2022-03-06 12:35:53 UTC
I'd also take the opportunity to replace the ACCDETAILS notice with a new reworded default called 'WELCOME' or something along those lines Created attachment 131489 [details] [review] Bug 30237: Replace ACCDETAILS notice with WELCOME notice This patch drops the ACCDETAILS notice in preference to a new WELCOME notice. Created attachment 131490 [details] [review] Bug 30237: Reference new WELCOME notice This patch updates all references to the former ACCTDETAILS notice to use the new WELCOME email notice instead. Created attachment 131491 [details] [review] Bug 30237: Replace AutoEmailOpacUser with AutoEmailNewUser Created attachment 132785 [details] [review] Bug 30237: Replace ACCDETAILS notice with WELCOME notice This patch drops the ACCDETAILS notice in preference to a new WELCOME notice. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 132786 [details] [review] Bug 30237: Reference new WELCOME notice This patch updates all references to the former ACCTDETAILS notice to use the new WELCOME email notice instead. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 132787 [details] [review] Bug 30237: Replace AutoEmailOpacUser with AutoEmailNewUser Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Should there be a db update to rename the syspref AutoEmailOpacUser to AutoEmailNewUser? (In reply to Kyle M Hall from comment #8) > Should there be a db update to rename the syspref AutoEmailOpacUser to > AutoEmailNewUser? Definitely! It looks like it's in the second patch: + + # Update system preference + $dbh->do( q{ + UPDATE systempreferences SET variable = 'AutoEmailNewUser', explanation = 'Send an email to newly created patrons.' WHERE variable = 'AutoEmailOpacUser' + }); }, }; *** Bug 30442 has been marked as a duplicate of this bug. *** Hi all, almost there! 1) This includes a database update to insert a new letter. I am not sure about this approach. I think it would be better if we changed the letter code on the existing ACCTDETAILS instead, because it will cause less surprising behaviour. The existing notice will kept to be sent for libraries using the feature instead of it suddently changing text and turning English. (blocker) 2) We could add a note to the system preference about the letter code, so people will be able to see the change a little better. (suggestion) 3) QA test tools: FAIL installer/data/mysql/atomicupdate/bug_30237.pl FAIL file permissions File must have the exec flag 4) New letter Thank you for doing this, I already had filed a separate bug before stumbling onto this here! Some small stuff and hopefully good suggestions: a) Tiny typo: Thank you + - "Thankyou for joining [% Koha.Preference('LibraryName') %]" b) Linking 'here' Never link "here", it's a known accessibility issue with screen readers, but also because we are all used to scan for links now and it doesn't give an useful clue on where it leads. "The library's catalog can be found <a href='[% Koha.Preference('OPACBaseURL') %]'>here</a>." Better to say something like: You can search for all our materials in our <a href='[% Koha.Preference('OPACBaseURL') %]'>catalog</a>." And patron's won't know that this is Koha or who the administrator is, better to ask them to get in touch with the library staff :) If you have any problems or questions regarding your account, please contact your Koha Administrator. Better: If you have any problems or questions regarding your account, please contact your library. 5) French installer As we don't know yet if the French installer will be gone in time for release, we should update the installer with the new notice: installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql 6) While testing bug 9097 I noticed that the branch name is not resolved in the subject line: Welcome to We might want to remove the name there for now and fix the subject replacement for TT on a separate bug? Created attachment 133153 [details] [review] Bug 30237: Replace ACCDETAILS notice with WELCOME notice This patch drops the ACCDETAILS notice in preference to a new WELCOME notice. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 133154 [details] [review] Bug 30237: Reference new WELCOME notice This patch updates all references to the former ACCTDETAILS notice to use the new WELCOME email notice instead. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 133155 [details] [review] Bug 30237: Replace AutoEmailOpacUser with AutoEmailNewUser This patch replaces the AutoEmailOpacUser system preference with a new AutoEmailNewUser preference. This makes the functionof the preference clearer. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 133156 [details] [review] Bug 30237: (follow-up) Update Notice This patch updates the sample notice as suggested by Katrin. Created attachment 133157 [details] [review] Bug 30237: (follow-up) Add french sample notice Created attachment 133158 [details] [review] Bug 30237: Add a note about WELCOME notice to system preference (In reply to Katrin Fischer from comment #11) > Hi all, > > almost there! > > 1) This includes a database update to insert a new letter. I am not sure > about this approach. I think it would be better if we changed the letter > code on the existing ACCTDETAILS instead, because it will cause less > surprising behaviour. The existing notice will kept to be sent for libraries > using the feature instead of it suddenly changing text and turning English. > (blocker) This was actually a deliberate choice. The notice text is all changed, as you see below. By leaving the old notice there but no longer referencing it I felt users could use it as a reference when updating the new notice. With the password functionality having already been removed it makes a lot of sense to require users update this notice to reflect it's clarified use. I was intending to add such a comment in the release notes too. > > 2) We could add a note to the system preference about the letter code, so > people will be able to see the change a little better. (suggestion) Done in last follow-up. > 3) QA test tools: > > FAIL installer/data/mysql/atomicupdate/bug_30237.pl > FAIL file permissions > File must have the exec flag Done > 4) New letter > > Thank you for doing this, I already had filed a separate bug before > stumbling onto this here! Some small stuff and hopefully good suggestions: > > a) Tiny typo: Thank you > > + - "Thankyou for joining [% Koha.Preference('LibraryName') %]" > > b) Linking 'here' > > Never link "here", it's a known accessibility issue with screen readers, but > also because we are all used to scan for links now and it doesn't give an > useful clue on where it leads. > > "The library's catalog can be found <a href='[% > Koha.Preference('OPACBaseURL') %]'>here</a>." > > Better to say something like: > You can search for all our materials in our <a href='[% > Koha.Preference('OPACBaseURL') %]'>catalog</a>." > > And patron's won't know that this is Koha or who the administrator is, > better to ask them to get in touch with the library staff :) > > If you have any problems or questions regarding your account, please contact > your Koha Administrator. > > Better: > If you have any problems or questions regarding your account, please contact > your library. Done > 5) French installer > > As we don't know yet if the French installer will be gone in time for > release, we should update the installer with the new notice: > > installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql Done (In reply to Katrin Fischer from comment #12) > 6) While testing bug 9097 I noticed that the branch name is not resolved in > the subject line: > > Welcome to > > We might want to remove the name there for now and fix the subject > replacement for TT on a separate bug? This actually works for me.. are you sure you have the relevant syspref set? I did wonder where we should check the pref is filled.. would you like me to add that follow-up too? Created attachment 133159 [details] [review] Bug 30237: (follow-up) Make LibraryName conditional It's easy to forget to set the LibraryName preference.. as such we can make this a conditional and fall back to a default of 'the library'. (In reply to Martin Renvoize from comment #21) > Created attachment 133159 [details] [review] [review] > Bug 30237: (follow-up) Make LibraryName conditional > > It's easy to forget to set the LibraryName preference.. as such we can > make this a conditional and fall back to a default of 'the library'. Ah, I expected the name of the branch, should have read the notice template more carefully. It certainly was the missing pref then. Created attachment 133345 [details] [review] Bug 30237: Replace ACCDETAILS notice with WELCOME notice This patch drops the ACCDETAILS notice in preference to a new WELCOME notice. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 133346 [details] [review] Bug 30237: Reference new WELCOME notice This patch updates all references to the former ACCTDETAILS notice to use the new WELCOME email notice instead. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 133347 [details] [review] Bug 30237: Replace AutoEmailOpacUser with AutoEmailNewUser This patch replaces the AutoEmailOpacUser system preference with a new AutoEmailNewUser preference. This makes the functionof the preference clearer. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 133348 [details] [review] Bug 30237: (follow-up) Update Notice This patch updates the sample notice as suggested by Katrin. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 133349 [details] [review] Bug 30237: (follow-up) Add french sample notice Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 133350 [details] [review] Bug 30237: Add a note about WELCOME notice to system preference Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 133351 [details] [review] Bug 30237: (follow-up) Make LibraryName conditional It's easy to forget to set the LibraryName preference.. as such we can make this a conditional and fall back to a default of 'the library'. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 133352 [details] [review] Bug 30237: (QA follow-up) Spelling Thankyou => Thank you No thanks ;) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed to master for 22.05, thanks to everybody involved 🦄 |