Bug 26595

Summary: Add SMTP server column to libraries table
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: System AdministrationAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, jonathan.druart, kyle, lucas, martin.renvoize, tomascohen
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00
Bug Depends on: 22343    
Bug Blocks:    
Attachments: Bug 26595: Add Koha::Library->smtp_server_info
Bug 26595: Add smtp_server_info embed option to /libraries
Bug 26595: Add SMTP server column to libraries table
Bug 26595: Add the option to hide the SMTP server column
Bug 26595: Add Koha::Library->smtp_server_info
Bug 26595: Add smtp_server_info embed option to /libraries
Bug 26595: Add SMTP server column to libraries table
Bug 26595: Add the option to hide the SMTP server column
Bug 26595: Add Koha::Library->smtp_server_info
Bug 26595: Add smtp_server_info embed option to /libraries
Bug 26595: Add SMTP server column to libraries table
Bug 26595: Add the option to hide the SMTP server column
Bug 26595: Embed the whole smtp_server object
Bug 26595: Embed the whole smtp_server object
Bug 26595: Remove password from the smtp_server API object

Description Tomás Cohen Arazi 2020-10-01 15:04:44 UTC

    
Comment 1 Tomás Cohen Arazi 2020-10-02 11:19:02 UTC
Created attachment 111122 [details] [review]
Bug 26595: Add Koha::Library->smtp_server_info

This patch adds a method for retrieving brief information about the
linked (or not) SMTP server for embedding on API requests.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Library.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Comment 2 Tomás Cohen Arazi 2020-10-02 11:19:08 UTC
Created attachment 111123 [details] [review]
Bug 26595: Add smtp_server_info embed option to /libraries

This patch simply adds the option to embed the return value from
Koha::Library->smtp_server_info on the libraries GET actions.
Comment 3 Tomás Cohen Arazi 2020-10-02 11:19:13 UTC
Created attachment 111124 [details] [review]
Bug 26595: Add SMTP server column to libraries table

This patch makes the libraries datatable render the assigned SMTP server
on each row. 'Default' will be rendered when the system default is
detected. A link to the SMTP server config will be rendered when a real
SMTP server is assigned.

To test:
1. Apply this patches, reload al the things
2. Visit the libraries table
=> SUCCESS: There's an SMTP server column displaying the right thing
3. Assign an SMTP server to a library
=> SUCCESS: The table displays the server name as a link
=> SUCCESS: The link correctly points to the SMTP server config page
4. Sign off
Comment 4 Tomás Cohen Arazi 2020-10-02 11:19:19 UTC
Created attachment 111125 [details] [review]
Bug 26595: Add the option to hide the SMTP server column

This patch adds
Comment 5 Martin Renvoize 2020-10-02 13:42:43 UTC
Created attachment 111155 [details] [review]
Bug 26595: Add Koha::Library->smtp_server_info

This patch adds a method for retrieving brief information about the
linked (or not) SMTP server for embedding on API requests.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Library.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2020-10-02 13:42:51 UTC
Created attachment 111156 [details] [review]
Bug 26595: Add smtp_server_info embed option to /libraries

This patch simply adds the option to embed the return value from
Koha::Library->smtp_server_info on the libraries GET actions.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2020-10-02 13:43:01 UTC
Created attachment 111157 [details] [review]
Bug 26595: Add SMTP server column to libraries table

This patch makes the libraries datatable render the assigned SMTP server
on each row. 'Default' will be rendered when the system default is
detected. A link to the SMTP server config will be rendered when a real
SMTP server is assigned.

To test:
1. Apply this patches, reload al the things
2. Visit the libraries table
=> SUCCESS: There's an SMTP server column displaying the right thing
3. Assign an SMTP server to a library
=> SUCCESS: The table displays the server name as a link
=> SUCCESS: The link correctly points to the SMTP server config page
4. Sign off

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2020-10-02 13:43:08 UTC
Created attachment 111158 [details] [review]
Bug 26595: Add the option to hide the SMTP server column

This patch adds

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2020-10-02 13:44:35 UTC
All works well, Signing off.

Minor feedback.. I wonder if when clicking through to edit smtp server details from the libraries table whether we should redirect back to the libraries table on completion as opposed to being taken to the SMPT servers management page?
Comment 10 Tomás Cohen Arazi 2020-10-02 14:07:51 UTC
(In reply to Martin Renvoize from comment #9)
> All works well, Signing off.
> 
> Minor feedback.. I wonder if when clicking through to edit smtp server
> details from the libraries table whether we should redirect back to the
> libraries table on completion as opposed to being taken to the SMPT servers
> management page?

That's a good point
Comment 11 Katrin Fischer 2020-10-13 12:51:58 UTC
Created attachment 111550 [details] [review]
Bug 26595: Add Koha::Library->smtp_server_info

This patch adds a method for retrieving brief information about the
linked (or not) SMTP server for embedding on API requests.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Library.t
=> SUCCESS: Tests pass!
3. Sign off :-D

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Katrin Fischer 2020-10-13 12:52:06 UTC
Created attachment 111551 [details] [review]
Bug 26595: Add smtp_server_info embed option to /libraries

This patch simply adds the option to embed the return value from
Koha::Library->smtp_server_info on the libraries GET actions.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2020-10-13 12:52:11 UTC
Created attachment 111552 [details] [review]
Bug 26595: Add SMTP server column to libraries table

This patch makes the libraries datatable render the assigned SMTP server
on each row. 'Default' will be rendered when the system default is
detected. A link to the SMTP server config will be rendered when a real
SMTP server is assigned.

To test:
1. Apply this patches, reload al the things
2. Visit the libraries table
=> SUCCESS: There's an SMTP server column displaying the right thing
3. Assign an SMTP server to a library
=> SUCCESS: The table displays the server name as a link
=> SUCCESS: The link correctly points to the SMTP server config page
4. Sign off

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2020-10-13 12:52:15 UTC
Created attachment 111553 [details] [review]
Bug 26595: Add the option to hide the SMTP server column

This patch adds

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Jonathan Druart 2020-10-14 12:13:25 UTC
Created attachment 111606 [details] [review]
Bug 26595: Embed the whole smtp_server object
Comment 16 Jonathan Druart 2020-10-14 12:13:55 UTC
(In reply to Jonathan Druart from comment #15)
> Created attachment 111606 [details] [review] [review]
> Bug 26595: Embed the whole smtp_server object

Tomas, why did not you embed the whole whole like in this patch?
Comment 17 Tomás Cohen Arazi 2020-10-14 12:37:36 UTC
(In reply to Jonathan Druart from comment #16)
> (In reply to Jonathan Druart from comment #15)
> > Created attachment 111606 [details] [review] [review] [review]
> > Bug 26595: Embed the whole smtp_server object
> 
> Tomas, why did not you embed the whole whole like in this patch?

Primarily, because of permissions/security. Someone without the right permissions shouldn't be able to see the user/password settings of an SMTP server.

I thought of a workaround to this, making the API not send the password along in the object i.e. make $smtp_server->to_api delete the attribute. But I didn't see a real use case.
Comment 18 Tomás Cohen Arazi 2020-10-14 13:10:37 UTC
Created attachment 111621 [details] [review]
Bug 26595: Embed the whole smtp_server object

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 19 Tomás Cohen Arazi 2020-10-14 13:10:44 UTC
Created attachment 111622 [details] [review]
Bug 26595: Remove password from the smtp_server API object

This patch makes the SMTP server objects do not include the password for
API representation. If we need the password itself, we can add a route
to retrieve it with specific permissions.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 20 Jonathan Druart 2020-10-14 13:55:24 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 21 Lucas Gass 2020-11-13 14:59:24 UTC
enhancement will not be backported to 20.05.x