Bug 33313

I wanted to have a look at this, but bug 31028 needs a rebase. Adding Martin.

(In reply to Katrin Fischer from comment #1)
> I wanted to have a look at this, but bug 31028 needs a rebase. Adding Martin.

It's because 31028 is pretty much all pushed.. you don't need to apply it.. it's in master.

Should I still put the 'depends on' when it's pushed? I just wanted to alert the people who worked on 31028 of related bugs that I found while documenting

Depends on is good.. it's just that in this case the bug was a bit messy and ended up going backwards in bugzilla states.. as such git bz ignore it as is should.

Created attachment 150130 [details] [review]
Bug 33313: Fix catalog links in TICKET_NOTIFY

This fixes the system preference name from IntranetBaseURL to
staffClientBaseURL.

Also makes some small typo fixes.

As this feature has just been pushed I chose to fix the
database updates that will be run when installing 23.05.

To test:
* Verify the wrong link in TICKET_NOTIFY
* Drop database, create database, run web installer
* Verify the link is now correct in the notice

Bonus: Test update from 22.11 to 23.05 and verify the
notices are correct there as well.

This fixes the link to the cataloging concerns management page, so happy to sign it off.

In addition, should the link to the record title be fixed as well, or should I log that as a separate bug? For example: I used Programming Perl - the link for this in the email is to http://cgi-bin/koha/catalogue/detail.pl?biblionumber=262 (in the notice it is <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=262" class="title">), which is an invalid link without the domain name.

Testing notes (using KTD):

1. After applying the patch, reset_all refreshes everything with the updated notice. (Need to set the required system preferences - enable CatalogConcerns, have a value for set CatalogerEmails, have a valid value for staffClientBaseURL, and have a valid email for the patron submitting the concern).

2. I'm not sure of the right steps using KTD to do the bonus test (test update from 22.11 to 23.05), so I haven't done that.

3. I set up email to test the actual emails received.[1]

[1] To test sending emails using a Google account, edit /etc/koha/sites/kohadev/koha-conf.xml file and add this configuration near the end (where <user_name> = your Google email address; <password> = your APP password, not your Google account password):

 <smtp_server>
    <host>smtp.gmail.com</host>
    <port>587</port>
    <timeout>5</timeout>
    <ssl_mode>STARTTLS</ssl_mode>
    <user_name>GOOGLEACCOUNTUSER</user_name>
    <password>GOOGLEAPPPASSWORD</password>
    <debug>1</debug>
 </smtp_server>

(In reply to David Nind from comment #6)
 
> In addition, should the link to the record title be fixed as well, or should
> I log that as a separate bug? For example: I used Programming Perl - the
> link for this in the email is to
> http://cgi-bin/koha/catalogue/detail.pl?biblionumber=262 (in the notice it
> is <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=262"
> class="title">), which is an invalid link without the domain name.

This is already logged as bug 33314.

Created attachment 150135 [details] [review]
Bug 33313: Fix catalog links in TICKET_NOTIFY

This fixes the system preference name from IntranetBaseURL to
staffClientBaseURL.

Also makes some small typo fixes.

As this feature has just been pushed I chose to fix the
database updates that will be run when installing 23.05.

To test:
* Verify the wrong link in TICKET_NOTIFY
* Drop database, create database, run web installer
* Verify the link is now correct in the notice

Bonus: Test update from 22.11 to 23.05 and verify the
notices are correct there as well.

Signed-off-by: David Nind <david@davidnind.com>

Additional testing note (using KTD):

1. When applying the patch, DON'T follow 31028 (the main patch is pushed to master, some follow-up patches have not yet been pushed).

(In reply to David Nind from comment #7)
> (In reply to David Nind from comment #6)
>  
> > In addition, should the link to the record title be fixed as well, or should
> > I log that as a separate bug? For example: I used Programming Perl - the
> > link for this in the email is to
> > http://cgi-bin/koha/catalogue/detail.pl?biblionumber=262 (in the notice it
> > is <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=262"
> > class="title">), which is an invalid link without the domain name.
> 
> This is already logged as bug 33314.

I was not aware of the other bug, I fixed the issue as a follow-up and made it dependent on this one.

(In reply to Katrin Fischer from comment #10)

> > This is already logged as bug 33314.
> 
> I was not aware of the other bug, I fixed the issue as a follow-up and made
> it dependent on this one.

Thanks for creating a patch for bug 33314! I've signed it off as well.

There are 2 "Thankyou" left in installer/data/mysql/db_revs/221200010.pl

I didn't get back to this in time, maybe consider a follow-up next time?

Created attachment 151384 [details] [review]
Bug 33313: Fix catalog links in TICKET_NOTIFY

This fixes the system preference name from IntranetBaseURL to
staffClientBaseURL.

As this feature has just been pushed I chose to fix the
database updates that will be run when installing 23.05.

To test:
* Verify the wrong link in TICKET_NOTIFY
* Drop database, create database, run web installer
* Verify the link is now correct in the notice

Bonus: Test update from 22.11 to 23.05 and verify the
notices are correct there as well.

Signed-off-by: David Nind <david@davidnind.com>

I am moving the typo fixes to a different bug. I believe we might get away with pushing this as the string might not change in pootle now.

It's currently:

You can mark this concern as resolved from the concern management %spage%s.

Created attachment 151417 [details] [review]
Bug 33313: Fix catalog links in TICKET_NOTIFY

This fixes the system preference name from IntranetBaseURL to
staffClientBaseURL.

As this feature has just been pushed I chose to fix the
database updates that will be run when installing 23.05.

To test:
* Verify the wrong link in TICKET_NOTIFY
* Drop database, create database, run web installer
* Verify the link is now correct in the notice

Bonus: Test update from 22.11 to 23.05 and verify the
notices are correct there as well.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

My mistake.. sure it was intranet at some point.. or I'm going mad.

Passing QA anyway

Pushed to master for 23.05.

Nice work everyone, thanks!

Missing dependencies - not backporting to 22.11.x