Bug 26595 - Add SMTP server column to libraries table
Summary: Add SMTP server column to libraries table
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Testopia
URL:
Keywords:
Depends on: 22343
Blocks:
  Show dependency treegraph
 
Reported: 2020-10-01 15:04 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2021-06-14 21:33 UTC (History)
6 users (show)

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


Attachments
Bug 26595: Add Koha::Library->smtp_server_info (2.46 KB, patch)
2020-10-02 11:19 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 26595: Add smtp_server_info embed option to /libraries (2.06 KB, patch)
2020-10-02 11:19 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 26595: Add SMTP server column to libraries table (3.37 KB, patch)
2020-10-02 11:19 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 26595: Add the option to hide the SMTP server column (772 bytes, patch)
2020-10-02 11:19 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 26595: Add Koha::Library->smtp_server_info (2.53 KB, patch)
2020-10-02 13:42 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 26595: Add smtp_server_info embed option to /libraries (2.12 KB, patch)
2020-10-02 13:42 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 26595: Add SMTP server column to libraries table (3.44 KB, patch)
2020-10-02 13:43 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 26595: Add the option to hide the SMTP server column (838 bytes, patch)
2020-10-02 13:43 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 26595: Add Koha::Library->smtp_server_info (2.58 KB, patch)
2020-10-13 12:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26595: Add smtp_server_info embed option to /libraries (2.18 KB, patch)
2020-10-13 12:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26595: Add SMTP server column to libraries table (3.70 KB, patch)
2020-10-13 12:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26595: Add the option to hide the SMTP server column (895 bytes, patch)
2020-10-13 12:52 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26595: Embed the whole smtp_server object (6.21 KB, patch)
2020-10-14 12:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 26595: Embed the whole smtp_server object (6.27 KB, patch)
2020-10-14 13:10 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 26595: Remove password from the smtp_server API object (2.05 KB, patch)
2020-10-14 13:10 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2020-10-01 15:04:44 UTC

    
Comment 1 Tomás Cohen Arazi (tcohen) 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 (tcohen) 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 (tcohen) 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 (tcohen) 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 (ashimema) 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 (ashimema) 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 (ashimema) 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 (ashimema) 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 (ashimema) 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 (tcohen) 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 (tcohen) 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 (tcohen) 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 (tcohen) 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 (lukeg) 2020-11-13 14:59:24 UTC
enhancement will not be backported to 20.05.x