Bug 33537 - Move domain limits from koha-conf to staff SMTP configuration
Summary: Move domain limits from koha-conf to staff SMTP configuration
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Marcel de Rooy
QA Contact:
URL:
Keywords:
Depends on: 33360
Blocks:
  Show dependency treegraph
 
Reported: 2023-04-14 15:12 UTC by Marcel de Rooy
Modified: 2023-10-23 10:25 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This patch set allows you to define mail domain limits within Administration/SMTP servers. Just as you could add those in koha-conf.xml already (bug 33360), now you can enter them on a web page and they are saved in a new table. The koha-conf.xml entry is only used as a legacy fallback. The form uses REST API calls for all CRUD operations.
Version(s) released in:


Attachments
Bug 33537: Add better named table_exists alias in Installer module (3.52 KB, patch)
2023-07-12 12:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Fix some POD in Installer.pm (1.18 KB, patch)
2023-07-12 12:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Database revision, adding table mail_domain_limits (3.12 KB, patch)
2023-07-12 12:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: [DBIx] Schema changes for mail_domain_limits (3.38 KB, patch)
2023-07-12 12:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add undefined default value for belongs_to in TestBuilder (955 bytes, patch)
2023-07-12 12:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add Koha objects (11.56 KB, patch)
2023-07-12 12:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: REST API endpoints (19.75 KB, patch)
2023-07-12 12:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add staff template and script (17.03 KB, patch)
2023-07-12 12:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Connect the dots (13.13 KB, patch)
2023-07-12 12:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add Koha objects (11.68 KB, patch)
2023-07-12 13:33 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: REST API endpoints (19.75 KB, patch)
2023-07-12 13:33 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add staff template and script (17.21 KB, patch)
2023-07-12 13:33 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Connect the dots (13.13 KB, patch)
2023-07-12 13:33 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add better named table_exists alias in Installer module (3.52 KB, patch)
2023-08-25 08:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Fix some POD in Installer.pm (1.18 KB, patch)
2023-08-25 08:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Database revision, adding table mail_domain_limits (3.12 KB, patch)
2023-08-25 08:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: [DBIx] Schema changes for mail_domain_limits (3.38 KB, patch)
2023-08-25 08:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add undefined default value for belongs_to in TestBuilder (955 bytes, patch)
2023-08-25 08:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add Koha objects (11.68 KB, patch)
2023-08-25 08:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: REST API endpoints (19.73 KB, patch)
2023-08-25 08:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add staff template and script (17.21 KB, patch)
2023-08-25 08:20 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Connect the dots (13.13 KB, patch)
2023-08-25 08:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Tidy updated blocks in various files (34.54 KB, patch)
2023-08-25 08:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Resolve forbidden pattern in REST API (3.60 KB, patch)
2023-08-25 08:21 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add better named table_exists alias in Installer module (3.54 KB, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: Fix some POD in Installer.pm (1.20 KB, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: Database revision, adding table mail_domain_limits (3.14 KB, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: [DBIx] Schema changes for mail_domain_limits (3.39 KB, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: Add undefined default value for belongs_to in TestBuilder (974 bytes, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: Add Koha objects (11.69 KB, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: REST API endpoints (19.75 KB, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: Add staff template and script (17.23 KB, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: Connect the dots (13.15 KB, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Tidy updated blocks in various files (34.55 KB, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Resolve forbidden pattern in REST API (3.62 KB, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Modal UI fixes: (5.47 KB, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Use bootstrap modal native functions (2.41 KB, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Update kohastructure.sql to match atomic update script (1.01 KB, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Add 'SMTP Servers' button to domain limits page (1.03 KB, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Improve process_message_queue warning message (1.12 KB, patch)
2023-08-25 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Modal UI fixes: (5.57 KB, patch)
2023-08-28 07:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Use bootstrap modal native functions (2.50 KB, patch)
2023-08-28 07:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Update kohastructure.sql to match atomic update script (1.11 KB, patch)
2023-08-28 07:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Add 'SMTP Servers' button to domain limits page (1.12 KB, patch)
2023-08-28 07:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Improve process_message_queue warning message (1.22 KB, patch)
2023-08-28 07:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add better named table_exists alias in Installer module (3.58 KB, patch)
2023-10-13 07:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Fix some POD in Installer.pm (1.24 KB, patch)
2023-10-13 07:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Database revision, adding table mail_domain_limits (3.18 KB, patch)
2023-10-13 07:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: [DBIx] Schema changes for mail_domain_limits (3.43 KB, patch)
2023-10-13 07:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add undefined default value for belongs_to in TestBuilder (1014 bytes, patch)
2023-10-13 07:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add Koha objects (11.73 KB, patch)
2023-10-13 07:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: REST API endpoints (19.79 KB, patch)
2023-10-13 07:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add staff template and script (17.27 KB, patch)
2023-10-13 07:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Connect the dots (13.10 KB, patch)
2023-10-13 07:30 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Tidy updated blocks in various files (34.59 KB, patch)
2023-10-13 07:31 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Resolve forbidden pattern in REST API (3.66 KB, patch)
2023-10-13 07:31 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Modal UI fixes: (5.57 KB, patch)
2023-10-13 07:31 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Use bootstrap modal native functions (2.50 KB, patch)
2023-10-13 07:31 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Update kohastructure.sql to match atomic update script (1.11 KB, patch)
2023-10-13 07:31 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Add 'SMTP Servers' button to domain limits page (1.12 KB, patch)
2023-10-13 07:31 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Improve process_message_queue warning message (1.22 KB, patch)
2023-10-13 07:31 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add better named table_exists alias in Installer module (3.58 KB, patch)
2023-10-13 09:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Fix some POD in Installer.pm (1.24 KB, patch)
2023-10-13 09:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Database revision, adding table mail_domain_limits (3.18 KB, patch)
2023-10-13 09:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: [DBIx] Schema changes for mail_domain_limits (3.43 KB, patch)
2023-10-13 09:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add undefined default value for belongs_to in TestBuilder (1014 bytes, patch)
2023-10-13 09:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add Koha objects (11.73 KB, patch)
2023-10-13 09:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: REST API endpoints (19.80 KB, patch)
2023-10-13 09:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Add staff template and script (18.58 KB, patch)
2023-10-13 09:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: Connect the dots (14.26 KB, patch)
2023-10-13 09:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Update kohastructure.sql to match atomic update script (1.11 KB, patch)
2023-10-13 09:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33537: (QA follow-up) Add 'SMTP Servers' button to domain limits page (1.12 KB, patch)
2023-10-13 09:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2023-04-14 15:12:08 UTC
See 33360
Comment 1 Marcel de Rooy 2023-07-12 12:37:11 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2023-07-12 12:37:13 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2023-07-12 12:37:16 UTC Comment hidden (obsolete)
Comment 4 Marcel de Rooy 2023-07-12 12:37:19 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2023-07-12 12:37:21 UTC Comment hidden (obsolete)
Comment 6 Marcel de Rooy 2023-07-12 12:37:24 UTC Comment hidden (obsolete)
Comment 7 Marcel de Rooy 2023-07-12 12:37:27 UTC Comment hidden (obsolete)
Comment 8 Marcel de Rooy 2023-07-12 12:37:30 UTC Comment hidden (obsolete)
Comment 9 Marcel de Rooy 2023-07-12 12:37:33 UTC Comment hidden (obsolete)
Comment 10 Marcel de Rooy 2023-07-12 12:53:25 UTC
Note for testing and QA:
Tried to keep the patches clean and clear here.
Patch 3 and 4 are database oriented, introducing the new table mail_domain_limits.
Patch 6 is the Koha object update. It adds a search_with_group_domain that includes a self join on the new table following the 'foreign' key to itself for grouping domains.
Patch 7 is all regular REST API stuff. It uses the search_with_group_domain routine too. The render_helper routine was added to reduce the number of lines here. Note that I preferred smtp/ over config/smtp/ here.
Patch 8 is the main interface work. The perl script is short and simple. The template is small too; I moved all js to a separate file. The js includes REST API calls for all CRUD operations.
Patch 9 is the switch from koha-conf to table in Koha::Notice::Util in order to get all things working.

A test plan per patch is provided. But things can be tested along those lines also with all patches applied.
Comment 11 Marcel de Rooy 2023-07-12 13:33:27 UTC Comment hidden (obsolete)
Comment 12 Marcel de Rooy 2023-07-12 13:33:30 UTC Comment hidden (obsolete)
Comment 13 Marcel de Rooy 2023-07-12 13:33:32 UTC Comment hidden (obsolete)
Comment 14 Marcel de Rooy 2023-07-12 13:33:35 UTC Comment hidden (obsolete)
Comment 15 Marcel de Rooy 2023-07-14 06:25:12 UTC
Note for QA: I spent a lot of time on this one. It passed qa tools at the time. And now out of nowhere, qa tools has been changed quite a bit. And I am seeing warns about REST API validation and perl tidy. 

This was written before the cutoff date !
Comment 16 Marcel de Rooy 2023-08-25 07:08:55 UTC
Please wait, rebasing time
Comment 17 Marcel de Rooy 2023-08-25 07:17:15 UTC
Note for QA:

 FAIL   Koha/Schema/Result/MailDomainLimit.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 35)

Please ignore this qa tools warning. This file is generated automatically !
Comment 18 Marcel de Rooy 2023-08-25 07:18:38 UTC
Will still have a look at this, aaarggg :

 WARN   C4/Installer.pm
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 170, now: 171)

 FAIL   Koha/Exceptions/DomainLimit.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 4)

 WARN   Koha/MailDomainLimit.pm
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 1)

 WARN   Koha/Notice/Util.pm
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 26, now: 31)

 FAIL   Koha/REST/V1/Config/SMTP/DomainLimits.pm
   FAIL   forbidden patterns
                forbidden pattern: Use of $c->validation in API controllers should be avoided (bug 33556) (line 57)
                forbidden pattern: Use of $c->validation in API controllers should be avoided (bug 33556) (line 75)
                forbidden pattern: Use of $c->validation in API controllers should be avoided (bug 33556) (line 103)
                forbidden pattern: Use of $c->validation in API controllers should be avoided (bug 33556) (line 106)
                forbidden pattern: Use of $c->validation in API controllers should be avoided (bug 33556) (line 133)
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 32)

 WARN   admin/domain_limits.pl
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 1)

 WARN   installer/data/mysql/atomicupdate/bug33537.pl
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 5)

 WARN   t/db_dependent/Installer.t
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 18, now: 19)

 WARN   t/db_dependent/Koha/MailDomainLimits.t
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 3)

 WARN   t/db_dependent/Koha/Notice_Util.t
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 37, now: 38)

 WARN   t/db_dependent/api/v1/domain_limits.t
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 7)

 WARN   t/lib/TestBuilder.pm
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 211, now: 212)
Comment 19 Marcel de Rooy 2023-08-25 07:27:01 UTC
QA: Please ignore this one too. In the Exception modules we dont have subroutines or methods to cover. The message is misleading.

 FAIL   Koha/Exceptions/DomainLimit.pm
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666
Comment 20 Marcel de Rooy 2023-08-25 08:20:42 UTC Comment hidden (obsolete)
Comment 21 Marcel de Rooy 2023-08-25 08:20:44 UTC Comment hidden (obsolete)
Comment 22 Marcel de Rooy 2023-08-25 08:20:47 UTC Comment hidden (obsolete)
Comment 23 Marcel de Rooy 2023-08-25 08:20:49 UTC Comment hidden (obsolete)
Comment 24 Marcel de Rooy 2023-08-25 08:20:52 UTC Comment hidden (obsolete)
Comment 25 Marcel de Rooy 2023-08-25 08:20:54 UTC Comment hidden (obsolete)
Comment 26 Marcel de Rooy 2023-08-25 08:20:57 UTC Comment hidden (obsolete)
Comment 27 Marcel de Rooy 2023-08-25 08:20:59 UTC Comment hidden (obsolete)
Comment 28 Marcel de Rooy 2023-08-25 08:21:02 UTC Comment hidden (obsolete)
Comment 29 Marcel de Rooy 2023-08-25 08:21:04 UTC Comment hidden (obsolete)
Comment 30 Marcel de Rooy 2023-08-25 08:21:07 UTC Comment hidden (obsolete)
Comment 31 Marcel de Rooy 2023-08-25 08:23:01 UTC
Rebased (tiny change), perl tidied, applied changes for bug 33556.
Note the two important comments about a few qa tools warns that can be ignored.

Rest is fine now:

Processing files before patches
|========================>| 21 / 21 (100.00%)
Processing files after patches
|========================>| 21 / 21 (100.00%)

 OK     C4/Installer.pm
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   OK     pod coverage
   SKIP   spelling
   OK     tidiness
   OK     valid

 FAIL   Koha/Exceptions/DomainLimit.pm
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666
   SKIP   spelling
   OK     tidiness
   OK     valid

 OK     Koha/MailDomainLimit.pm
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   OK     pod coverage
   SKIP   spelling
   OK     tidiness
   OK     valid

 OK     Koha/MailDomainLimits.pm
   OK     critic
   OK     file permissions
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   OK     pod coverage
   SKIP   spelling
   OK     tidiness
   OK     valid

 OK     Koha/Notice/Util.pm
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   OK     pod coverage
   SKIP   spelling
   OK     tidiness
   OK     valid

 OK     Koha/REST/V1/Config/SMTP/DomainLimits.pm
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   OK     pod coverage
   SKIP   spelling
   OK     tidiness
   OK     valid

 FAIL   Koha/Schema/Result/MailDomainLimit.pm
   OK     critic
   OK     file permissions
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   FAIL   pod coverage
                POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666
   SKIP   spelling
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 35)
   OK     valid

 OK     admin/domain_limits.pl
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   SKIP   spelling
   OK     tidiness
   OK     valid

 OK     api/v1/swagger/definitions/smtp_domain_limits.yaml
   OK     git manipulation
   OK     rest_api_specs
   OK     yaml_valid

 OK     api/v1/swagger/paths/smtp_domain_limits.yaml
   OK     git manipulation
   OK     rest_api_specs
   OK     yaml_valid

 OK     api/v1/swagger/swagger.yaml
   OK     git manipulation
   OK     rest_api_specs
   OK     yaml_valid

 OK     installer/data/mysql/atomicupdate/bug33537.pl
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   SKIP   spelling
   OK     tidiness
   OK     valid

 OK     installer/data/mysql/kohastructure.sql
   OK     boolean_vs_tinyint
   OK     charset_collate
   OK     git manipulation
   OK     tinyint_has_boolean_flag

 OK     koha-tmpl/intranet-tmpl/prog/en/modules/admin/domain_limits.tt
   OK     filters
   OK     forbidden patterns
   OK     git manipulation
   OK     js_in_body
   SKIP   spelling
   OK     tt_valid
   OK     valid_template

 OK     koha-tmpl/intranet-tmpl/prog/en/modules/admin/smtp_servers.tt
   OK     filters
   OK     forbidden patterns
   OK     git manipulation
   OK     js_in_body
   SKIP   spelling
   OK     tt_valid
   OK     valid_template

 OK     koha-tmpl/intranet-tmpl/prog/js/domain_limits.js
   OK     git manipulation

 OK     t/db_dependent/Installer.t
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   SKIP   spelling
   OK     tidiness
   OK     valid

 OK     t/db_dependent/Koha/MailDomainLimits.t
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   SKIP   spelling
   OK     tidiness
   OK     valid

 OK     t/db_dependent/Koha/Notice_Util.t
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   SKIP   spelling
   OK     tidiness
   OK     valid

 OK     t/db_dependent/api/v1/domain_limits.t
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   SKIP   spelling
   OK     tidiness
   OK     valid

 OK     t/lib/TestBuilder.pm
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   OK     pod coverage
   SKIP   spelling
   OK     tidiness
   OK     valid
Comment 32 Pedro Amorim 2023-08-25 15:09:03 UTC Comment hidden (obsolete)
Comment 33 Pedro Amorim 2023-08-25 15:09:06 UTC Comment hidden (obsolete)
Comment 34 Pedro Amorim 2023-08-25 15:09:09 UTC Comment hidden (obsolete)
Comment 35 Pedro Amorim 2023-08-25 15:09:12 UTC Comment hidden (obsolete)
Comment 36 Pedro Amorim 2023-08-25 15:09:15 UTC Comment hidden (obsolete)
Comment 37 Pedro Amorim 2023-08-25 15:09:18 UTC Comment hidden (obsolete)
Comment 38 Pedro Amorim 2023-08-25 15:09:21 UTC Comment hidden (obsolete)
Comment 39 Pedro Amorim 2023-08-25 15:09:24 UTC Comment hidden (obsolete)
Comment 40 Pedro Amorim 2023-08-25 15:09:27 UTC Comment hidden (obsolete)
Comment 41 Pedro Amorim 2023-08-25 15:09:31 UTC Comment hidden (obsolete)
Comment 42 Pedro Amorim 2023-08-25 15:09:34 UTC Comment hidden (obsolete)
Comment 43 Pedro Amorim 2023-08-25 15:09:37 UTC Comment hidden (obsolete)
Comment 44 Pedro Amorim 2023-08-25 15:09:40 UTC Comment hidden (obsolete)
Comment 45 Pedro Amorim 2023-08-25 15:09:43 UTC Comment hidden (obsolete)
Comment 46 Pedro Amorim 2023-08-25 15:09:46 UTC Comment hidden (obsolete)
Comment 47 Pedro Amorim 2023-08-25 15:09:50 UTC Comment hidden (obsolete)
Comment 48 Pedro Amorim 2023-08-25 15:10:44 UTC
Marcel, please take a look at my patches for SO, or mark obsolete if you don't agree!

For reference, here is what I did to follow the test plan:
1) configured <message_domain_limits> in koha-conf.xml
2) configured an smtp server with my own google account app password
3) did an OPAC password reset which prompted an entry in the message_queue table
4) duplicated that message entry a couple times:
INSERT INTO message_queue (letter_id, borrowernumber, subject, content, letter_code, message_transport_type, status, to_address)
SELECT letter_id, borrowernumber, subject, content, letter_code, message_transport_type, 'pending', 'pedro.amorim@ptfs-europe.com'
FROM message_queue;
5) Run:
perl misc/cronjobs/process_message_queue.pl
Comment 49 Marcel de Rooy 2023-08-25 15:48:44 UTC
(In reply to Pedro Amorim from comment #48)
> Marcel, please take a look at my patches for SO, or mark obsolete if you
> don't agree!

Thx Pedro. I certainly will. But it sounds very promising!
Comment 50 Marcel de Rooy 2023-08-28 07:38:55 UTC Comment hidden (obsolete)
Comment 51 Marcel de Rooy 2023-08-28 07:38:58 UTC Comment hidden (obsolete)
Comment 52 Marcel de Rooy 2023-08-28 07:39:00 UTC Comment hidden (obsolete)
Comment 53 Marcel de Rooy 2023-08-28 07:39:03 UTC Comment hidden (obsolete)
Comment 54 Marcel de Rooy 2023-08-28 07:39:06 UTC Comment hidden (obsolete)
Comment 55 Marcel de Rooy 2023-08-28 07:41:24 UTC
(In reply to Marcel de Rooy from comment #49)
> (In reply to Pedro Amorim from comment #48)
> > Marcel, please take a look at my patches for SO, or mark obsolete if you
> > don't agree!
> 
> Thx Pedro. I certainly will. But it sounds very promising!

Looks good to me. Thanks!
Comment 56 Marcel de Rooy 2023-10-13 07:30:34 UTC Comment hidden (obsolete)
Comment 57 Marcel de Rooy 2023-10-13 07:30:37 UTC Comment hidden (obsolete)
Comment 58 Marcel de Rooy 2023-10-13 07:30:40 UTC Comment hidden (obsolete)
Comment 59 Marcel de Rooy 2023-10-13 07:30:43 UTC Comment hidden (obsolete)
Comment 60 Marcel de Rooy 2023-10-13 07:30:45 UTC Comment hidden (obsolete)
Comment 61 Marcel de Rooy 2023-10-13 07:30:48 UTC Comment hidden (obsolete)
Comment 62 Marcel de Rooy 2023-10-13 07:30:51 UTC Comment hidden (obsolete)
Comment 63 Marcel de Rooy 2023-10-13 07:30:54 UTC Comment hidden (obsolete)
Comment 64 Marcel de Rooy 2023-10-13 07:30:57 UTC Comment hidden (obsolete)
Comment 65 Marcel de Rooy 2023-10-13 07:31:00 UTC Comment hidden (obsolete)
Comment 66 Marcel de Rooy 2023-10-13 07:31:03 UTC Comment hidden (obsolete)
Comment 67 Marcel de Rooy 2023-10-13 07:31:06 UTC Comment hidden (obsolete)
Comment 68 Marcel de Rooy 2023-10-13 07:31:09 UTC Comment hidden (obsolete)
Comment 69 Marcel de Rooy 2023-10-13 07:31:12 UTC Comment hidden (obsolete)
Comment 70 Marcel de Rooy 2023-10-13 07:31:14 UTC Comment hidden (obsolete)
Comment 71 Marcel de Rooy 2023-10-13 07:31:17 UTC Comment hidden (obsolete)
Comment 72 Jonathan Druart 2023-10-13 07:51:07 UTC
I cannot add a new entry, because I am silly^Wa developer and just filled inputs with random data. I am expecting the form to tell it, but errors are ignored:

29         if ( !$('#domain_limits_modal form').get(0).checkValidity() ) return; // Do not return false here to allow validation.

I think it was working in a previous iteration.
Comment 73 Jonathan Druart 2023-10-13 07:51:38 UTC
If I remove the line I get the response's server (better than nothing!)

"Adding limit for 'eza' failed. (Status 400 - Bad request: expected columns were empty.)"
Comment 74 Marcel de Rooy 2023-10-13 09:03:32 UTC
Working here to restore things like they worked before.
Will squash a few patches now too.

Removing tidy from the middle.
Resolve forbidden patterns squashed with REST API.
Modal UI fixes squashed with Add staff template.

The Use bootstrap patch has my attention now. It removes some of the previously working checks.
Comment 75 Marcel de Rooy 2023-10-13 09:42:24 UTC
(In reply to Marcel de Rooy from comment #74)
> The Use bootstrap patch has my attention now. It removes some of the
> previously working checks.

Squashed it now too with Add staff template.
Added a event stop propagation to get validation back again.
Comment 76 Marcel de Rooy 2023-10-13 09:48:59 UTC
    #   Failed test 'Check warn for reaching limit A'
    #   at /usr/share/koha/t/db_dependent/Koha/Notice_Util.t line 134.
    # found warning: Sending messages: The domain a has reached the limit of 3 messages per 5 minutes at /usr/share/koha/Koha/Notice/Util.pm line 134.
    # expected to find warning: (?^u:Sending messages: domain a reached limit)
     # Looks like you failed 1 test of 12.

Last follow-up reworded the warn. Failing the test.
Corrected and squashed it too now with Connect the dots.
Comment 77 Marcel de Rooy 2023-10-13 09:53:34 UTC
Created attachment 157038 [details] [review]
Bug 33537: Add better named table_exists alias in Installer module

Test plan:
Run dbrev later. It is using table_exists.
Run t/db_dependent/Installer.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 78 Marcel de Rooy 2023-10-13 09:53:37 UTC
Created attachment 157039 [details] [review]
Bug 33537: Fix some POD in Installer.pm

Test plan:
Run qa tools.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 79 Marcel de Rooy 2023-10-13 09:53:40 UTC
Created attachment 157040 [details] [review]
Bug 33537: Database revision, adding table mail_domain_limits

Test plan:
Run dbrev or new install.
Verify that table is created as expected.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 80 Marcel de Rooy 2023-10-13 09:53:42 UTC
Created attachment 157041 [details] [review]
Bug 33537: [DBIx] Schema changes for mail_domain_limits

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 81 Marcel de Rooy 2023-10-13 09:53:45 UTC
Created attachment 157042 [details] [review]
Bug 33537: Add undefined default value for belongs_to in TestBuilder

Test plan:
See next patch. It calls build_object without specifying belongs_to.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 82 Marcel de Rooy 2023-10-13 09:53:48 UTC
Created attachment 157043 [details] [review]
Bug 33537: Add Koha objects

Test plan:
Run t/db_dependent/Koha/MailDomainLimits.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Run perltidy on MailDomainLimits.t
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 83 Marcel de Rooy 2023-10-13 09:53:51 UTC
Created attachment 157044 [details] [review]
Bug 33537: REST API endpoints

Test plan:
Run t/db_dependent/api/v1/domain_limits.t
List, add (post), get, update (put) and delete domain limit(s).
Using the paths:
- api/v1/smtp/domain_limits
- api/v1/smtp/domain_limits/{SOME_ID}

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

[EDIT] Applied the QA-follow-up-Resolve-forbidden-pattern-in-.patch here.
Comment 84 Marcel de Rooy 2023-10-13 09:53:54 UTC
Created attachment 157045 [details] [review]
Bug 33537: Add staff template and script

Test plan:
Go to Administration/SMTP Servers.
Click Domain limits.
Do some CRUD operations.
Bonus:
Add another tab with domain limits.
Edit or remove records on tab 1.
Try CRUD on changed or removed records on tab 2.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Added some preventDefault calls, as suggested by JD.
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

Bug 33537: (QA follow-up) Modal UI fixes:

Use ol instead of ul in modal fieldset inputs:
This ol->ul change makes the UI inputs more spaced
and overall more in line with other modals in Koha.

Use 'btn-primary' on 'Save' button to have it be shown
yellow as other confirmation buttons in Koha.

Add a11y attributes to modal form

Also prettied HTML code:
git show --ignore-all-space <commit_hash>

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Bug 33537: (QA follow-up) Use bootstrap modal native functions

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

[EDIT] To get validation working again, needed to add
ev.stopPropagation() in #domain_limits_modal button.save's onclick.
Comment 85 Marcel de Rooy 2023-10-13 09:53:57 UTC
Created attachment 157046 [details] [review]
Bug 33537: Connect the dots

Use Koha::MailDomainLimits in Koha::Notice::Util->load_domain_limits.
Only use koha-conf as fallback when table is still empty. (This is
just like SMTP servers.)

The test is adjusted accordingly and has moved more to the database
oriented side, first proving that both approaches return the same.

Test plan:
Run t/db_dependent/Koha/Notice_Util.t

NOTE: Replace A and B hereunder by two domains of your own choice.
Add a limit to KOHA-CONF: domain A, 1 per 1 hour.
Add another limit in WEB CONFIG: domain B, 1 per 1 hour
Send 2 notices to domain B. Is 1 message delayed?
Remove the table entry for B.
Run process_message_queue.pl. Is the delayed notice to B sent?
Send 2 notices to domain A. Is 1 delayed (using koha-conf fallback)?

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

Bug 33537: (QA follow-up) Improve process_message_queue warning message

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

[EDIT] Adjusted test for rewording of the warn.
Comment 86 Marcel de Rooy 2023-10-13 09:54:00 UTC
Created attachment 157047 [details] [review]
Bug 33537: (QA follow-up) Update kohastructure.sql to match atomic update script

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 87 Marcel de Rooy 2023-10-13 09:54:03 UTC
Created attachment 157048 [details] [review]
Bug 33537: (QA follow-up) Add 'SMTP Servers' button to domain limits page

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 88 Marcel de Rooy 2023-10-13 09:56:35 UTC
What I did not yet do, is reinsert some tidy stuff. The patch in the middle for that has been obsoleted.

Sure, I am willing to add it again ;) But if QA and RM are going to ask for doing it different again, this is just waste of time (again).

Shall i tidy after QA ?
Comment 89 Marcel de Rooy 2023-10-13 10:01:05 UTC
 WARN   C4/Installer.pm
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 170, now: 171)

 FAIL   Koha/Exceptions/DomainLimit.pm
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 4)

 WARN   Koha/MailDomainLimit.pm
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 1)

 WARN   Koha/Notice/Util.pm
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 19, now: 23)

 WARN   Koha/REST/V1/Config/SMTP/DomainLimits.pm
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 27)

 WARN   admin/domain_limits.pl
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 1)

 WARN   installer/data/mysql/atomicupdate/bug33537.pl
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 5)

 WARN   t/db_dependent/Installer.t
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 18, now: 19)

 WARN   t/db_dependent/Koha/MailDomainLimits.t
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 3)

 WARN   t/db_dependent/Koha/Notice_Util.t
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 37, now: 38)

 WARN   t/db_dependent/api/v1/domain_limits.t
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 0, now: 7)

 WARN   t/lib/TestBuilder.pm
   WARN   tidiness
                The file is less tidy than before (bad/messy lines before: 205, now: 206)
Comment 90 Martin Renvoize 2023-10-23 10:21:03 UTC
Whilst I'm slightly surprised by the way the JS was written here.. I can't honestly fail it.. but it does introduce a somewhat different pattern.

What I did find more apparent from an end user perspective however was the different navigation we introduce here. I find the 'Administration -> SMPT Servers -> Button in toolbar to get to Domain limits' flow a bit weird feeling.

I will admit, I've forgotten exactly how the actual feature fits together with the SMTP Servers stuff.. I thought it was actually effectively independent.. in which case I would have said it should match other admin pages and have its own independent menu option from the admin home and sidebars.
Comment 91 Marcel de Rooy 2023-10-23 10:24:28 UTC
Thx anyone for trying to get this further but it will not be ready in time for 23.11 anymore.
Will resume this later.

Changing to FQA for now. The "template/javascript does not meet Koha standards".
Some other points are not clear (link with SMTP servers); perhaps that is more a documentation issue btw.

If there is additional feedback to support the current stand, please add.