Bug 38313

Summary: RESTOAuth2ClientCredentials pref description is confusing
Product: Koha Reporter: Jake Deery <jake.deery>
Component: Staff interfaceAssignee: Jake Deery <jake.deery>
Status: Passed QA --- QA Contact: Tomás Cohen Arazi (tcohen) <tomascohen>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, jonathan.druart, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: String patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 38313: Remove confusing string from RESTOAuth2ClientCredentials
Bug 38313: Remove confusing string from RESTOAuth2ClientCredentials
Bug 38313: Remove confusing string from RESTOAuth2ClientCredentials
Bug 38313: Remove confusing string from RESTOAuth2ClientCredentials

Description Jake Deery 2024-10-31 14:23:38 UTC
In Bug 20734, we added a string informing sysadmins that Net::OAuth2::AuthorizationServer is required for the RESTOAuth2ClientCredentials syspref to be operable.

I don't think we need this anymore, as for git installs this is in the cpanfile, and for package installs this is included as a dependency for koha-perldeps.

Could we remove this string to a) tidy things up, and b) avoid confusion of people installing the cpan module twice (once in Debian, and once in cpan-land)?

Jake.
Comment 1 Jake Deery 2024-10-31 14:28:21 UTC
Created attachment 173803 [details] [review]
Bug 38313: Remove confusing string from RESTOAuth2ClientCredentials

This patch removes a once necessary and now confusing warning
from the RESTOAuth2ClientCredentials syspref.

TO TEST:
a)  Notice how the RESTOAuth2ClientCredentials syspref references
    installing Net::OAuth2::AuthorizationServer, even though this
    module is in the cpanfile and koha-perldeps and has been since
    22.11
b)  APPLY PATCH
c)  Notice how the message is now gone
Comment 2 Jake Deery 2024-10-31 14:30:19 UTC
Created attachment 173804 [details] [review]
Bug 38313: Remove confusing string from RESTOAuth2ClientCredentials

This patch removes a once necessary and now confusing warning
from the RESTOAuth2ClientCredentials syspref.

TO TEST:
a)  Notice how the RESTOAuth2ClientCredentials syspref references
    installing Net::OAuth2::AuthorizationServer, even though this
    module is in the cpanfile and koha-perldeps and has been since
    22.11
b)  APPLY PATCH
c)  Run reset_all in k.t.d
d)  Notice how the message is now gone
Comment 3 William Lavoie 2025-01-13 21:23:50 UTC
Created attachment 176476 [details] [review]
Bug 38313: Remove confusing string from RESTOAuth2ClientCredentials

This patch removes a once necessary and now confusing warning
from the RESTOAuth2ClientCredentials syspref.

TO TEST:
a)  Notice how the RESTOAuth2ClientCredentials syspref references
    installing Net::OAuth2::AuthorizationServer, even though this
    module is in the cpanfile and koha-perldeps and has been since
    22.11
b)  APPLY PATCH
c)  Run reset_all in k.t.d
d)  Notice how the message is now gone

Signed-off-by: William Lavoie <william.lavoie@inLibro.com>
Comment 4 Jonathan Druart 2025-01-16 12:14:28 UTC
cpanfile:recommends 'Net::OAuth2::AuthorizationServer', '0.16';

It's recommends, not requires.

I guess that's why it's in the description of the syspref.
Comment 5 Jake Deery 2025-01-16 16:21:31 UTC
When you install a package, it is included as a dependency. This can be verified by looking at the metadata of the koha-perldeps package.

Perhaps we should update the cpanfile too?

Jake.
Comment 6 Tomás Cohen Arazi (tcohen) 2025-01-21 15:23:57 UTC
Created attachment 176867 [details] [review]
Bug 38313: Remove confusing string from RESTOAuth2ClientCredentials

This patch removes a once necessary and now confusing warning
from the RESTOAuth2ClientCredentials syspref.

TO TEST:
a)  Notice how the RESTOAuth2ClientCredentials syspref references
    installing Net::OAuth2::AuthorizationServer, even though this
    module is in the cpanfile and koha-perldeps and has been since
    22.11
b)  APPLY PATCH
c)  Run reset_all in k.t.d
d)  Notice how the message is now gone

Signed-off-by: William Lavoie <william.lavoie@inLibro.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>