Support for regular expressions in set selection operator in particular would be useful. It is common for free text fields to have slight variation depending when bibliographic record has been created, so exact match doesn't work very well for creating sets.

Created attachment 96041 [details] [review]
Bug 21520: More complex OAI sets mappings

Currently, the rules used to create OAI sets are processed with the 'or'
boolean operator between each rule.

This patch allows to use 'or' or 'and' between the rules.

The evaluation of the rules is done according to the boolean operators
precedence: AND has a higher precedence than OR.

For example:

A and B or C and D

will be evaluated as follow:

(A and B) or (C and D)

Test plan:

 - Apply the patch
 - Apply the atomicupdate
 - Prove t/db_dependant/OAI/AndSets.t
 - Check that existing mappings still work
 - Try modifying existing mappings
 - Try creating new mappings
 - Check that the boolean operators precedence is correctly taken into account

Hello Mathias,
thank you for patch. I tested it but if I add AND operator, whole updated set gone from OAI:

vagrant@kohadevbox:kohaclone(bug_21520)$ sudo koha-foreach --enabled /usr/share/koha/bin/migration_tools/build_oai_sets.pl -i -r
Summary:
Set 'test': 303 biblios
Set 'NKP': 303 biblios
Updating database... done.
vagrant@kohadevbox:kohaclone(bug_21520)$ sudo koha-foreach --enabled /usr/share/koha/bin/migration_tools/build_oai_sets.pl -i -r
Summary:
Set 'NKP': 303 biblios
Updating database... done.

Can you review code?

Thank you.

Sorry, my fault. Wrong boolean operators.

Created attachment 96052 [details] [review]
Bug 21520: More complex OAI sets mappings

Currently, the rules used to create OAI sets are processed with the 'or'
boolean operator between each rule.

This patch allows to use 'or' or 'and' between the rules.

The evaluation of the rules is done according to the boolean operators
precedence: AND has a higher precedence than OR.

For example:

A and B or C and D

will be evaluated as follow:

(A and B) or (C and D)

Test plan:

 - Apply the patch
 - Apply the atomicupdate
 - Prove t/db_dependant/OAI/AndSets.t
 - Check that existing mappings still work
 - Try modifying existing mappings
 - Try creating new mappings
 - Check that the boolean operators precedence is correctly taken into account

Great work!

 Signed-off-by: Michal Denar <black23@gmail.com>

Hi Matts,
A couple QA remarks:

1. FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/admin/oai_set_mappings.tt
   FAIL   tt_valid
                lines 49, 50, 77, 78

You are not allowed to build a HTML tag using TT conditional as it breaks the translator script.

2. update database:
 a. rule_order is always null, is that expected?
 b. first rule should have a null for "rule_operator", isn't it?

Before:
+--------+-----------+--------------+----------+-----------+
| set_id | marcfield | marcsubfield | operator | marcvalue |
+--------+-----------+--------------+----------+-----------+
|      3 | 100       | a            | equal    | pouet     |
|      3 | 200       | a            | equal    | pouet     |
|      3 | 300       | a            | equal    | pouet     |
+--------+-----------+--------------+----------+-----------+

After the update:
+--------+------------+---------------+-----------+--------------+----------+-----------+
| set_id | rule_order | rule_operator | marcfield | marcsubfield | operator | marcvalue |
+--------+------------+---------------+-----------+--------------+----------+-----------+
|      3 |       NULL | or            | 100       | a            | equal    | pouet     |
|      3 |       NULL | or            | 200       | a            | equal    | pouet     |
|      3 |       NULL | or            | 300       | a            | equal    | pouet     |
+--------+------------+---------------+-----------+--------------+----------+-----------+

Hi Joubu, thanks for the feedback.

1. I will attach a QA Follow-up

2.
 a. rule_order is always null, is that expected?

=> Kind of :) In fact, rule_order will be populated when a rule is modified/created in the GUI. The updatedatabase doesn't need to do it, because at this point, we only have rules with 'or' operator, and the order is not important.

b. first rule should have a null for "rule_operator", isn't it?

=> Same as 2.a. The first rule_operator could have been set to null by the updatedatabase, but it doesn't need to, since it will always be ignored in the code. However, when a rule is modified/created is the GUI, the first rule_operator will be set to null (just for better readability in the database).

Created attachment 96790 [details] [review]
Bug 21520: QA Follow-up: do not build html tags with TT conditions

(In reply to Matthias Meusburger from comment #7)
Why not fix the existing data then? That seems quite easy to do.

Well, I hesitated, but I chose not to, having those two things in mind:

 - Keep It Simple, Stupid.
 - If it's not broken, don't fix it.

That being said, I can come up with a follow-up for the updatedatabase if you want me to.

Created attachment 97319 [details] [review]
Bug 21520: (QA follow-up): Updatedatabase

rule_order is now set and the first rule_operator of each mapping is set to null.

Created attachment 97320 [details] [review]
Bug 21520: More complex OAI sets mappings

Currently, the rules used to create OAI sets are processed with the 'or'
boolean operator between each rule.

This patch allows to use 'or' or 'and' between the rules.

The evaluation of the rules is done according to the boolean operators
precedence: AND has a higher precedence than OR.

For example:

A and B or C and D

will be evaluated as follow:

(A and B) or (C and D)

Test plan:

 - Apply the patch
 - Apply the atomicupdate
 - Prove t/db_dependant/OAI/AndSets.t
 - Check that existing mappings still work
 - Try modifying existing mappings
 - Try creating new mappings
 - Check that the boolean operators precedence is correctly taken into account

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Created attachment 97321 [details] [review]
Bug 21520: QA Follow-up: do not build html tags with TT conditions

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Created attachment 97322 [details] [review]
Bug 21520: (QA follow-up): Updatedatabase

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Nice work everyone!

Pushed to master for 20.05

OK, I added the kohastructure update inline during push.. and made the DB update idempotent

Nice work everyone!

Pushed to master for 20.05

Enhancement not backported to 19.11.x