Bug 23019 - Ability to create 'matching profiles' when importing records
Summary: Ability to create 'matching profiles' when importing records
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature with 11 votes (vote)
Assignee: Agustín Moyano
QA Contact: Testopia
URL:
Keywords: release-notes-needed
Depends on:
Blocks:
 
Reported: 2019-05-30 20:33 UTC by Joy Nelson
Modified: 2020-11-16 22:40 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.11.00


Attachments
Bug 23019: Update database structure and add atomic update (5.07 KB, patch)
2020-05-15 22:01 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Add koha_object_class and koha_objects_class methods to ImportBatch.pm schema (2.15 KB, patch)
2020-05-15 22:01 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Add ImportBatchesProfile schema and set is_boolean to parse_items column (3.73 KB, patch)
2020-05-15 22:01 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Add tests (6.19 KB, patch)
2020-05-15 22:01 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Add profiles to stage-import-batch and magnage-import-batch pages (45.80 KB, patch)
2020-05-15 22:01 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Update database structure and add atomic update (5.14 KB, patch)
2020-06-03 20:45 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Add koha_object_class and koha_objects_class methods to ImportBatch.pm schema (2.15 KB, patch)
2020-06-03 20:45 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Add ImportBatchesProfile schema and set is_boolean to parse_items column (3.70 KB, patch)
2020-06-03 20:45 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Add tests (6.17 KB, patch)
2020-06-03 20:45 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Add profiles to stage-import-batch and magnage-import-batch pages (45.68 KB, patch)
2020-06-03 20:45 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Add profiles to stage-import-batch and magnage-import-batch pages (45.70 KB, patch)
2020-06-04 01:04 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Add profiles to stage-import-batch and magnage-import-batch pages (45.84 KB, patch)
2020-06-04 14:07 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23019: Update database structure and add atomic update (5.17 KB, patch)
2020-06-05 19:27 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23019: Add koha_object_class and koha_objects_class methods to ImportBatch.pm schema (2.20 KB, patch)
2020-06-05 19:27 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23019: Add ImportBatchesProfile schema and set is_boolean to parse_items column (3.75 KB, patch)
2020-06-05 19:27 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23019: Add tests (6.23 KB, patch)
2020-06-05 19:27 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23019: Add profiles to stage-import-batch and magnage-import-batch pages (45.89 KB, patch)
2020-06-05 19:27 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23019: (QA follow-up) Fix typo Pre fill (1.03 KB, patch)
2020-07-05 16:20 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23019: (QA follow-up) Fix typo Pre fill (1.08 KB, patch)
2020-08-27 22:21 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: (follow-up) Fix usability issues (13.75 KB, patch)
2020-08-27 22:21 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: (follow-up) Fix usability issues (15.31 KB, patch)
2020-09-03 19:19 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Update database structure and add atomic update (5.20 KB, patch)
2020-10-26 18:57 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Add koha_object_class and koha_objects_class methods to ImportBatch.pm schema (2.20 KB, patch)
2020-10-26 18:57 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Add ImportBatchesProfile schema and set is_boolean to parse_items column (3.75 KB, patch)
2020-10-26 18:57 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Add tests (6.22 KB, patch)
2020-10-26 18:57 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Add profiles to stage-import-batch and magnage-import-batch pages (45.82 KB, patch)
2020-10-26 18:57 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: (QA follow-up) Fix typo Pre fill (1.08 KB, patch)
2020-10-26 18:58 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: (follow-up) Fix usability issues (15.31 KB, patch)
2020-10-26 18:58 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: Update database structure and add atomic update (5.23 KB, patch)
2020-10-30 15:39 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23019: Add koha_object_class and koha_objects_class methods to ImportBatch.pm schema (2.26 KB, patch)
2020-10-30 15:39 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23019: Add ImportBatchesProfile schema and set is_boolean to parse_items column (3.81 KB, patch)
2020-10-30 15:39 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23019: Add tests (6.29 KB, patch)
2020-10-30 15:39 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23019: Add profiles to stage-import-batch and magnage-import-batch pages (45.97 KB, patch)
2020-10-30 15:39 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23019: (QA follow-up) Fix typo Pre fill (1.13 KB, patch)
2020-10-30 15:39 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23019: (follow-up) Fix usability issues (15.42 KB, patch)
2020-10-30 15:39 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23019: (QA follow-up) fix test fiule permissions (613 bytes, patch)
2020-10-30 15:39 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23019: (follow-up) set table name to import_batch_profiles (4.40 KB, patch)
2020-11-06 18:42 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: (follow-up) Fix other things (8.25 KB, patch)
2020-11-06 18:42 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: (follow-up 2) set table name to import_batch_profiles (1.51 KB, patch)
2020-11-09 13:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23019: (follow-up) Fix test permissions and API definition bugs (3.83 KB, patch)
2020-11-10 14:57 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23019: (follow-up) Fix test permissions and API definition bugs (6.83 KB, patch)
2020-11-10 17:01 UTC, Agustín Moyano
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Joy Nelson 2019-05-30 20:33:56 UTC
This development will allow you to have the ability to create “matching profiles” for use on the “Stage MARC Records for Import” screen. This would make it possible to choose a profile rather than manually filling in each of the fields for record type, encoding, record and item matching rules and actions each time a batch of MARC records is imported.

For example, we might create “matching profiles” for different vendors or formats, and it would save time to select the appropriate profile (maybe from a drop-down list on the page) and have the fields populate automatically.

Specs:

Ability to set up profiles and save them while working in the Tools->StageMarcRecords module.

Ability to manage/edit profiles in the Admin area (similar to Record->Matching Rules).

Create an API to pull the profile data into Import Tool when selected on the Import Screen.
Comment 1 Agustín Moyano 2020-05-15 22:01:31 UTC
Created attachment 104972 [details] [review]
Bug 23019: Update database structure and add atomic update
Comment 2 Agustín Moyano 2020-05-15 22:01:34 UTC
Created attachment 104973 [details] [review]
Bug 23019: Add koha_object_class and koha_objects_class methods to ImportBatch.pm schema
Comment 3 Agustín Moyano 2020-05-15 22:01:38 UTC
Created attachment 104974 [details] [review]
Bug 23019: Add ImportBatchesProfile schema and set is_boolean to parse_items column
Comment 4 Agustín Moyano 2020-05-15 22:01:41 UTC
Created attachment 104975 [details] [review]
Bug 23019: Add tests
Comment 5 Agustín Moyano 2020-05-15 22:01:46 UTC
Created attachment 104976 [details] [review]
Bug 23019: Add profiles to stage-import-batch and magnage-import-batch pages

This patch adds the logic and the needed UI elements to be able to pre-load an import profile. It also displays which profile was used to stage an import in staged import manager.

To test:
1. Apply all patches
2. Updatedatabase
3. Go to Stage MARC records for Import tool in admin, and upload a file with MARC records.
CHECK => after uploading, there is a fieldset with the legend “Profile settings”
              => inside the fieldset there is a select labeled “Pre fill values with profile”. The only value it has is “Do not use profile”.

4. Change some settings, and set “profile 1” as profile name and click on “Add profile”
SUCCESS => The select now has the new profile selected

5. Change profile select to “Do not use profile”
SUCCESS => Default values are now displayed in the form

6. Reload the page and upload the file again
SUCCESS => the select still has the profile recently added

7. Select the profile, change some parameter in the form and set the profile name to “profile 2”, and click add profile
SUCCESS => there are two profiles now, and if you toggle between them, the parameter changes

8. Select profile 1, change one parameter and click on update profile
SUCCESS => if you toggle that profile with the other, the new parameter of the value is shown when you select profile 1

9. Select profile 2, change some parameter and click Add profile (leaving the name as profile 2)
SUCCESS => the page complains there is another profile with the same name, and asks if you want to replace it.

10. Click on accept
SUCCESS => profile 2 now has the new value in the parameter

11. Select profile 2 and change the name to profile 1
SUCCESS => the page complains there is another profile with that name, and asks if you want to replace it

12. Click on accept
SUCCESS => in profile select there is only one profile called profile 1 that has the values of profile 2

13. Select profile 1 and click remove profile
SUCCESS => there is no profile in profile select.

14. prove t/db_dependent/ImportBatch.t t/db_dependent/api/v1/import_batch_profiles.t
15. Sign off
Comment 6 David Cook 2020-05-21 23:30:37 UTC
Yes! I've been meaning to do something like this for years. 

I've even been thinking about implementing it for https://github.com/minusdavid/koha-plugin-oaipmh-import, so it would be great to have this in the core codebase!

I'll make a note to come back to this, but very very interested.
Comment 7 David Cook 2020-05-26 09:46:37 UTC
Comment on attachment 104976 [details] [review]
Bug 23019: Add profiles to stage-import-batch and magnage-import-batch pages

Review of attachment 104976 [details] [review]:
-----------------------------------------------------------------

::: api/v1/swagger/definitions/import-batch-profile.json
@@ +9,5 @@
> +      "description": "name of this profile",
> +      "type": "string"
> +    },
> +    "matcher_id": {
> +      "description": "the id of the match rule used (matchpoints.matcher_id)",

Wouldn't marc_matchers.matcher_id  be a more accurate table description here?
Comment 8 David Cook 2020-05-26 09:48:47 UTC
(In reply to David Cook from comment #6)
> Yes! I've been meaning to do something like this for years. 
> 
> I've even been thinking about implementing it for
> https://github.com/minusdavid/koha-plugin-oaipmh-import, so it would be
> great to have this in the core codebase!
> 

Looks like this won't be a good fit for the plugin but interesting nonetheless!
Comment 9 Agustín Moyano 2020-05-26 12:44:48 UTC
Change back to Assigned because just discovered issues to fix
Comment 10 Abbey Holt 2020-05-28 15:42:38 UTC
This sounds really slick. I'm looking forward to testing it! I tried creating a sandbox with this bug number to see what the development looks like in action, but the staff client won't load for me right now.
Comment 11 Agustín Moyano 2020-06-03 20:45:07 UTC
Created attachment 105536 [details] [review]
Bug 23019: Update database structure and add atomic update
Comment 12 Agustín Moyano 2020-06-03 20:45:11 UTC
Created attachment 105537 [details] [review]
Bug 23019: Add koha_object_class and koha_objects_class methods to ImportBatch.pm schema
Comment 13 Agustín Moyano 2020-06-03 20:45:15 UTC
Created attachment 105538 [details] [review]
Bug 23019: Add ImportBatchesProfile schema and set is_boolean to parse_items column
Comment 14 Agustín Moyano 2020-06-03 20:45:19 UTC
Created attachment 105539 [details] [review]
Bug 23019: Add tests
Comment 15 Agustín Moyano 2020-06-03 20:45:23 UTC
Created attachment 105540 [details] [review]
Bug 23019: Add profiles to stage-import-batch and magnage-import-batch pages

This patch adds the logic and the needed UI elements to be able to pre-load an import profile. It also displays which profile was used to stage an import in staged import manager.

To test:
1. Apply all patches
2. Updatedatabase
3. Go to Stage MARC records for Import tool in admin, and upload a file with MARC records.
CHECK => after uploading, there is a fieldset with the legend “Profile settings”
              => inside the fieldset there is a select labeled “Pre fill values with profile”. The only value it has is “Do not use profile”.

4. Change some settings, and set “profile 1” as profile name and click on “Add profile”
SUCCESS => The select now has the new profile selected

5. Change profile select to “Do not use profile”
SUCCESS => Default values are now displayed in the form

6. Reload the page and upload the file again
SUCCESS => the select still has the profile recently added

7. Select the profile, change some parameter in the form and set the profile name to “profile 2”, and click add profile
SUCCESS => there are two profiles now, and if you toggle between them, the parameter changes

8. Select profile 1, change one parameter and click on update profile
SUCCESS => if you toggle that profile with the other, the new parameter of the value is shown when you select profile 1

9. Select profile 2, change some parameter and click Add profile (leaving the name as profile 2)
SUCCESS => the page complains there is another profile with the same name, and asks if you want to replace it.

10. Click on accept
SUCCESS => profile 2 now has the new value in the parameter

11. Select profile 2 and change the name to profile 1
SUCCESS => the page complains there is another profile with that name, and asks if you want to replace it

12. Click on accept
SUCCESS => in profile select there is only one profile called profile 1 that has the values of profile 2

13. Select profile 1 and click remove profile
SUCCESS => there is no profile in profile select.

14. Create a profile and click on “Stage for import”
15. Go to Staged MARC management page
SUCCESS => Improt should have the name of the profile in profile column, and when you click on the file name, there should be the name of the profile in the details.

16. prove t/db_dependent/ImportBatch.t t/db_dependent/api/v1/import_batch_profiles.t
17. Sign off
Comment 16 Abbey Holt 2020-06-03 21:43:50 UTC
This is great! There is one area where my matching preferences would not stick when I created a profile. If I select "No" under "Check for embedded item record data" and click "Add profile", No switches back to Yes. My selection for "how to process items" is remembered. It is just the yes-no button that isn't being saved.
Comment 17 Agustín Moyano 2020-06-04 01:04:19 UTC
Created attachment 105541 [details] [review]
Bug 23019: Add profiles to stage-import-batch and magnage-import-batch pages

This patch adds the logic and the needed UI elements to be able to pre-load an import profile. It also displays which profile was used to stage an import in staged import manager.

To test:
1. Apply all patches
2. Updatedatabase
3. Go to Stage MARC records for Import tool in admin, and upload a file with MARC records.
CHECK => after uploading, there is a fieldset with the legend “Profile settings”
              => inside the fieldset there is a select labeled “Pre fill values with profile”. The only value it has is “Do not use profile”.

4. Change some settings, and set “profile 1” as profile name and click on “Add profile”
SUCCESS => The select now has the new profile selected

5. Change profile select to “Do not use profile”
SUCCESS => Default values are now displayed in the form

6. Reload the page and upload the file again
SUCCESS => the select still has the profile recently added

7. Select the profile, change some parameter in the form and set the profile name to “profile 2”, and click add profile
SUCCESS => there are two profiles now, and if you toggle between them, the parameter changes

8. Select profile 1, change one parameter and click on update profile
SUCCESS => if you toggle that profile with the other, the new parameter of the value is shown when you select profile 1

9. Select profile 2, change some parameter and click Add profile (leaving the name as profile 2)
SUCCESS => the page complains there is another profile with the same name, and asks if you want to replace it.

10. Click on accept
SUCCESS => profile 2 now has the new value in the parameter

11. Select profile 2 and change the name to profile 1
SUCCESS => the page complains there is another profile with that name, and asks if you want to replace it

12. Click on accept
SUCCESS => in profile select there is only one profile called profile 1 that has the values of profile 2

13. Select profile 1 and click remove profile
SUCCESS => there is no profile in profile select.

14. Create a profile and click on “Stage for import”
15. Go to Staged MARC management page
SUCCESS => Improt should have the name of the profile in profile column, and when you click on the file name, there should be the name of the profile in the details.

16. prove t/db_dependent/ImportBatch.t t/db_dependent/api/v1/import_batch_profiles.t
17. Sign off
Comment 18 Agustín Moyano 2020-06-04 01:05:37 UTC
(In reply to Abbey Holt from comment #16)
> This is great! There is one area where my matching preferences would not
> stick when I created a profile. If I select "No" under "Check for embedded
> item record data" and click "Add profile", No switches back to Yes. My
> selection for "how to process items" is remembered. It is just the yes-no
> button that isn't being saved.

Hi Abbey, thanks so much for pointing that out.. could you please try again?

Thanks!
Comment 19 Abbey Holt 2020-06-04 14:06:41 UTC
(In reply to Agustín Moyano from comment #18)
> (In reply to Abbey Holt from comment #16)
> > This is great! There is one area where my matching preferences would not
> > stick when I created a profile. If I select "No" under "Check for embedded
> > item record data" and click "Add profile", No switches back to Yes. My
> > selection for "how to process items" is remembered. It is just the yes-no
> > button that isn't being saved.
> 
> Hi Abbey, thanks so much for pointing that out.. could you please try again?
> 
> Thanks!

It worked! I'm signing off.
Comment 20 ByWater Sandboxes 2020-06-04 14:07:34 UTC
Created attachment 105555 [details] [review]
Bug 23019: Add profiles to stage-import-batch and magnage-import-batch pages

This patch adds the logic and the needed UI elements to be able to pre-load an import profile. It also displays which profile was used to stage an import in staged import manager.

To test:
1. Apply all patches
2. Updatedatabase
3. Go to Stage MARC records for Import tool in admin, and upload a file with MARC records.
CHECK => after uploading, there is a fieldset with the legend “Profile settings”
              => inside the fieldset there is a select labeled “Pre fill values with profile”. The only value it has is “Do not use profile”.

4. Change some settings, and set “profile 1” as profile name and click on “Add profile”
SUCCESS => The select now has the new profile selected

5. Change profile select to “Do not use profile”
SUCCESS => Default values are now displayed in the form

6. Reload the page and upload the file again
SUCCESS => the select still has the profile recently added

7. Select the profile, change some parameter in the form and set the profile name to “profile 2”, and click add profile
SUCCESS => there are two profiles now, and if you toggle between them, the parameter changes

8. Select profile 1, change one parameter and click on update profile
SUCCESS => if you toggle that profile with the other, the new parameter of the value is shown when you select profile 1

9. Select profile 2, change some parameter and click Add profile (leaving the name as profile 2)
SUCCESS => the page complains there is another profile with the same name, and asks if you want to replace it.

10. Click on accept
SUCCESS => profile 2 now has the new value in the parameter

11. Select profile 2 and change the name to profile 1
SUCCESS => the page complains there is another profile with that name, and asks if you want to replace it

12. Click on accept
SUCCESS => in profile select there is only one profile called profile 1 that has the values of profile 2

13. Select profile 1 and click remove profile
SUCCESS => there is no profile in profile select.

14. Create a profile and click on “Stage for import”
15. Go to Staged MARC management page
SUCCESS => Improt should have the name of the profile in profile column, and when you click on the file name, there should be the name of the profile in the details.

16. prove t/db_dependent/ImportBatch.t t/db_dependent/api/v1/import_batch_profiles.t
17. Sign off

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>
Comment 21 Tomás Cohen Arazi 2020-06-05 19:11:07 UTC
(In reply to Abbey Holt from comment #16)
> This is great! There is one area where my matching preferences would not
> stick when I created a profile. If I select "No" under "Check for embedded
> item record data" and click "Add profile", No switches back to Yes. My
> selection for "how to process items" is remembered. It is just the yes-no
> button that isn't being saved.

Abbey, I think you need to acknowledge the attachments count on the bug, and tell the sandboxes that you want to SO 5 patches.
Comment 22 ByWater Sandboxes 2020-06-05 19:27:39 UTC
Created attachment 105600 [details] [review]
Bug 23019: Update database structure and add atomic update

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>
Comment 23 ByWater Sandboxes 2020-06-05 19:27:42 UTC
Created attachment 105601 [details] [review]
Bug 23019: Add koha_object_class and koha_objects_class methods to ImportBatch.pm schema

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>
Comment 24 ByWater Sandboxes 2020-06-05 19:27:46 UTC
Created attachment 105602 [details] [review]
Bug 23019: Add ImportBatchesProfile schema and set is_boolean to parse_items column

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>
Comment 25 ByWater Sandboxes 2020-06-05 19:27:49 UTC
Created attachment 105603 [details] [review]
Bug 23019: Add tests

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>
Comment 26 ByWater Sandboxes 2020-06-05 19:27:52 UTC
Created attachment 105604 [details] [review]
Bug 23019: Add profiles to stage-import-batch and magnage-import-batch pages

This patch adds the logic and the needed UI elements to be able to pre-load an import profile. It also displays which profile was used to stage an import in staged import manager.

To test:
1. Apply all patches
2. Updatedatabase
3. Go to Stage MARC records for Import tool in admin, and upload a file with MARC records.
CHECK => after uploading, there is a fieldset with the legend “Profile settings”
              => inside the fieldset there is a select labeled “Pre fill values with profile”. The only value it has is “Do not use profile”.

4. Change some settings, and set “profile 1” as profile name and click on “Add profile”
SUCCESS => The select now has the new profile selected

5. Change profile select to “Do not use profile”
SUCCESS => Default values are now displayed in the form

6. Reload the page and upload the file again
SUCCESS => the select still has the profile recently added

7. Select the profile, change some parameter in the form and set the profile name to “profile 2”, and click add profile
SUCCESS => there are two profiles now, and if you toggle between them, the parameter changes

8. Select profile 1, change one parameter and click on update profile
SUCCESS => if you toggle that profile with the other, the new parameter of the value is shown when you select profile 1

9. Select profile 2, change some parameter and click Add profile (leaving the name as profile 2)
SUCCESS => the page complains there is another profile with the same name, and asks if you want to replace it.

10. Click on accept
SUCCESS => profile 2 now has the new value in the parameter

11. Select profile 2 and change the name to profile 1
SUCCESS => the page complains there is another profile with that name, and asks if you want to replace it

12. Click on accept
SUCCESS => in profile select there is only one profile called profile 1 that has the values of profile 2

13. Select profile 1 and click remove profile
SUCCESS => there is no profile in profile select.

14. Create a profile and click on “Stage for import”
15. Go to Staged MARC management page
SUCCESS => Improt should have the name of the profile in profile column, and when you click on the file name, there should be the name of the profile in the details.

16. prove t/db_dependent/ImportBatch.t t/db_dependent/api/v1/import_batch_profiles.t
17. Sign off

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>
Comment 27 Abbey Holt 2020-06-05 19:43:13 UTC
(In reply to Tomás Cohen Arazi from comment #21)
> (In reply to Abbey Holt from comment #16)
> > This is great! There is one area where my matching preferences would not
> > stick when I created a profile. If I select "No" under "Check for embedded
> > item record data" and click "Add profile", No switches back to Yes. My
> > selection for "how to process items" is remembered. It is just the yes-no
> > button that isn't being saved.
> 
> Abbey, I think you need to acknowledge the attachments count on the bug, and
> tell the sandboxes that you want to SO 5 patches.


Thank you. I think I've signed off on everything now. Let me know if I need to do anything else!
Comment 28 Tomás Cohen Arazi 2020-06-06 14:41:08 UTC
(In reply to Abbey Holt from comment #27)
> (In reply to Tomás Cohen Arazi from comment #21)
> > (In reply to Abbey Holt from comment #16)
> > > This is great! There is one area where my matching preferences would not
> > > stick when I created a profile. If I select "No" under "Check for embedded
> > > item record data" and click "Add profile", No switches back to Yes. My
> > > selection for "how to process items" is remembered. It is just the yes-no
> > > button that isn't being saved.
> > 
> > Abbey, I think you need to acknowledge the attachments count on the bug, and
> > tell the sandboxes that you want to SO 5 patches.
> 
> 
> Thank you. I think I've signed off on everything now. Let me know if I need
> to do anything else!

That worked great!
Comment 29 Katrin Fischer 2020-07-05 16:19:31 UTC
This works, but I feel the GUI is not very intuitive. Actually, it took me a while to figure it out. 

The profile is "started" at the top of the page, but then after making your settings, you have to make sure to hit "Update profile" for the settings to be saved. So the workflow is like this to add a profile for it to work:

1) Enter a name for a profile, click on add
2) Make the settings you want
3) DONT CLICK IMPORT - or your profile will save with the wrong values
4) Go back up, click "Update profile"
5) Import

I got it wrong the first time and was very confused why the "wrong" values showed up on managing the staged import. 

If a profile was selected, values can be changed. After importing it's not clear what was used for the import - the values from the profile or those set (and not saved in the profile) of the form? The profile values will show.

I really believe we need to improve this a bit before including it.

Ideas:

- Remove the buttons from the profile on top of the form and only show the list of existing profiles there.
- If a profile is selected, change the values in the form below.
- When one of the settings is changed, show a hint behind the field.
- When importing:
  - when profile is used and no field was changed: import
  - when no profile was used: import
  - when profile was used and values changed: ask if the profile or the changed values should be used (and possibly if the existing profile should be updated)
- At the end of the form offer to save the settings as a new profile or update the existing one. This way, you can start at the top and work your way down.


Alternatively: 
- Allow creating, updating and deleting profiles from a different configuration screen. 
- When a profile is selected, disable the other form fields so it's an "either or" and it's always clear what will be applied for the import.
Comment 30 Katrin Fischer 2020-07-05 16:20:05 UTC
Created attachment 106555 [details] [review]
Bug 23019: (QA follow-up) Fix typo Pre fill
Comment 31 Katrin Fischer 2020-07-05 16:21:47 UTC
Oh also: Please write up the addition to the REST API:
https://wiki.koha-community.org/wiki/REST_api_RFCs
Comment 32 Agustín Moyano 2020-08-26 17:02:34 UTC
Hi Katrin, I'll respond between lines

(In reply to Katrin Fischer from comment #29)
> This works, but I feel the GUI is not very intuitive. Actually, it took me a
> while to figure it out. 
> 
> The profile is "started" at the top of the page, but then after making your
> settings, you have to make sure to hit "Update profile" for the settings to
> be saved. So the workflow is like this to add a profile for it to work:
> 
> 1) Enter a name for a profile, click on add
> 2) Make the settings you want
> 3) DONT CLICK IMPORT - or your profile will save with the wrong values
> 4) Go back up, click "Update profile"
> 5) Import
> 
> I got it wrong the first time and was very confused why the "wrong" values
> showed up on managing the staged import. 
> 
> If a profile was selected, values can be changed. After importing it's not
> clear what was used for the import - the values from the profile or those
> set (and not saved in the profile) of the form? The profile values will show.
> 

Selecting a profile just pre-fills the form, if you change a value in the form, that's what counts for importing.

I thought it good to keep that flexibility, thinking that maybe someone trying to import, might want to use almost all values from a profile, but change just one or two. 

> I really believe we need to improve this a bit before including it.
> 
> Ideas:
> 
> - Remove the buttons from the profile on top of the form and only show the
> list of existing profiles there.
> - If a profile is selected, change the values in the form below.
> - When one of the settings is changed, show a hint behind the field.
> - When importing:
>   - when profile is used and no field was changed: import
>   - when no profile was used: import
>   - when profile was used and values changed: ask if the profile or the
> changed values should be used (and possibly if the existing profile should
> be updated)
> - At the end of the form offer to save the settings as a new profile or
> update the existing one. This way, you can start at the top and work your
> way down.
> 

I'll remove the profile from the top and place them below. I'll also add a text to clarify that selecting a profile just pre-fill's your form, but values can be changed.

> 
> Alternatively: 
> - Allow creating, updating and deleting profiles from a different
> configuration screen. 
> - When a profile is selected, disable the other form fields so it's an
> "either or" and it's always clear what will be applied for the import.

I thought of this, but it would be a configuration screen exactly the same as this one.. it seemed kind of repetitive and less maintainable.
Comment 33 Agustín Moyano 2020-08-26 17:03:47 UTC
> I'll remove the profile from the top and place them below. I'll also add a
> text to clarify that selecting a profile just pre-fill's your form, but
> values can be changed.
> 

I ment, I'll remove the profile buttons from the top
Comment 34 Agustín Moyano 2020-08-27 21:07:55 UTC
(In reply to Katrin Fischer from comment #31)
> Oh also: Please write up the addition to the REST API:
> https://wiki.koha-community.org/wiki/REST_api_RFCs

Hi Katrin, here is the RFC 
https://wiki.koha-community.org/wiki/Import_batch_profiles_endpoint_RFC

Is it ok?.. it's my first RFC..
Comment 35 Agustín Moyano 2020-08-27 22:21:41 UTC
Created attachment 109265 [details] [review]
Bug 23019: (QA follow-up) Fix typo Pre fill

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Comment 36 Agustín Moyano 2020-08-27 22:21:47 UTC
Created attachment 109266 [details] [review]
Bug 23019: (follow-up) Fix usability issues

This patch place profile buttons in the bottom of the page, next to "Stage for import"

It removes the "Update profile" button, and changes "Add profile" to "Save profile". Save profile can be used to add and to update a profile.

It also adds feedback when a profile was saved or deleted.

Last, it also replaces input-batch-profile endpoint name for input_batch_profile, which is more aligned with other endpoint names.
Comment 37 Agustín Moyano 2020-08-27 22:32:18 UTC
Ok, when I run qa I get this two errors:

 FAIL	Koha/ImportBatch.pm
   FAIL	  pod coverage
		POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666

which is weird, since Koha/ImportBatch.pm is created in this patchset

 FAIL	api/v1/swagger/definitions/import-batch-profiles.json
   FAIL	  git manipulation
		The file has been added and deleted in the same patchset

I get this error because I created api/v1/swagger/definitions/import-batch-profiles.json in a patch that is now signed off, and I'm trying to rename it to import_batch_profiles.json (Since previous patch is signed off, I don't think it wise to squash with a not signed patch).. because the file has less than 10 lines and file content also changes, git just marks it for delete, and creates the new file.
Comment 38 Nick Clemens 2020-08-28 15:00:37 UTC
This did not work in my testing, whenever I click 'Save profile' I get an error:
An error occurred

[object Object]

in console:
Uncaught (in promise) 
{…}
​
abort: function abort(a)
​
always: function always()
​
complete: function add()
​
done: function add()
​
error: function add()
​
fail: function add()
​
getAllResponseHeaders: function getAllResponseHeaders()
​
getResponseHeader: function getResponseHeader(a)
​
overrideMimeType: function overrideMimeType(a)
​
pipe: function pipe()
​
progress: function add()
​
promise: function promise(a)
​
readyState: 4
​
responseText: "<!DOCTYPE html>\n<!-- Request ID: e249b4ad -->\n<html>\n  <head>\n    <title>Page not found (development mode)</title>\n    <meta http-equiv=\"Pragma\" content=\"no-cache\">\n    <meta http-equiv=\"Expires\" content=\"-1\">\n    <script src=\"/mojo/jquery/jquery.js\"></script>\n    <script src=\"/mojo/prettify/run_prettify.js\"></script>\n    <link href=\"/mojo/prettify/prettify-mojo-dark.css\" rel=\"stylesheet\">\n    <style>\n      a img {\n        border: 0;\n      }\n      body {\n        background: url(/mojo/pinstripe-light.png);\n        color: #445555;\n        font: 0.9em 'Helvetica Neue', Helvetica, sans-serif;\n        font-weight: normal;\n        line-height: 1.5em;\n        margin: 0;\n      }\n      :not(pre) > code {\n        background-color: #eef9ff;\n        border: solid #cce4ff 1px;\n        border-radius: 5px;\n        color: #333;\n        font: 0.9em Consolas, Menlo, Monaco, Courier, monospace;\n        padding: 0.4em;\n      }\n      h1 {\n        color: #2a2a2a;\n        font-size: 1.5em;\n        margin: 0;…"
​
setRequestHeader: function setRequestHeader(a, b)
​
state: function state()
​
status: 404
​
statusCode: function statusCode(a)
​
statusText: "Not Found"
​
success: function add()
​
then: function then()
​
<prototype>: Object { … }
Comment 39 Agustín Moyano 2020-09-03 19:19:44 UTC
Created attachment 109637 [details] [review]
Bug 23019: (follow-up) Fix usability issues

This patch place profile buttons in the bottom of the page, next to "Stage for import"

It removes the "Update profile" button, and changes "Add profile" to "Save profile". Save profile can be used to add and to update a profile.

It also adds feedback when a profile was saved or deleted.

Last, it also replaces input-batch-profile endpoint name for input_batch_profile, which is more aligned with other endpoint names.
Comment 40 Agustín Moyano 2020-09-03 19:21:06 UTC
(In reply to Nick Clemens from comment #38)
> This did not work in my testing, whenever I click 'Save profile' I get an
> error:
> An error occurred
> 
> [object Object]
> 
> in console:
> Uncaught (in promise) 
> {…}
> ​
> abort: function abort(a)
> ​
> always: function always()
> ​
> complete: function add()
> ​
> done: function add()
> ​
> error: function add()
> ​
> fail: function add()
> ​
> getAllResponseHeaders: function getAllResponseHeaders()
> ​
> getResponseHeader: function getResponseHeader(a)
> ​
> overrideMimeType: function overrideMimeType(a)
> ​
> pipe: function pipe()
> ​
> progress: function add()
> ​
> promise: function promise(a)
> ​
> readyState: 4
> ​
> responseText: "<!DOCTYPE html>\n<!-- Request ID: e249b4ad -->\n<html>\n 
> <head>\n    <title>Page not found (development mode)</title>\n    <meta
> http-equiv=\"Pragma\" content=\"no-cache\">\n    <meta
> http-equiv=\"Expires\" content=\"-1\">\n    <script
> src=\"/mojo/jquery/jquery.js\"></script>\n    <script
> src=\"/mojo/prettify/run_prettify.js\"></script>\n    <link
> href=\"/mojo/prettify/prettify-mojo-dark.css\" rel=\"stylesheet\">\n   
> <style>\n      a img {\n        border: 0;\n      }\n      body {\n       
> background: url(/mojo/pinstripe-light.png);\n        color: #445555;\n      
> font: 0.9em 'Helvetica Neue', Helvetica, sans-serif;\n        font-weight:
> normal;\n        line-height: 1.5em;\n        margin: 0;\n      }\n     
> :not(pre) > code {\n        background-color: #eef9ff;\n        border:
> solid #cce4ff 1px;\n        border-radius: 5px;\n        color: #333;\n     
> font: 0.9em Consolas, Menlo, Monaco, Courier, monospace;\n        padding:
> 0.4em;\n      }\n      h1 {\n        color: #2a2a2a;\n        font-size:
> 1.5em;\n        margin: 0;…"
> ​
> setRequestHeader: function setRequestHeader(a, b)
> ​
> state: function state()
> ​
> status: 404
> ​
> statusCode: function statusCode(a)
> ​
> statusText: "Not Found"
> ​
> success: function add()
> ​
> then: function then()
> ​
> <prototype>: Object { … }

Hi Nick, sorry about that.. forgot to change where endpoint was called.

Please try again

Thanks!
Comment 41 Katrin Fischer 2020-09-13 09:10:02 UTC
Several tests are failing:

    #   Failed test 'exact match for JSON Pointer "/name"'
    #   at t/db_dependent/api/v1/import_batch_profiles.t line 134.
    #          got: undef
    #     expected: 'theProfile'

    #   Failed test 'profile name should be the updated one'
    #   at t/db_dependent/api/v1/import_batch_profiles.t line 138.
    #          got: 'someProfile'
    #     expected: 'theProfile'
    # Looks like you failed 4 tests of 5.
t/db_dependent/api/v1/import_batch_profiles.t .. 3/4 
#   Failed test 'edit profile'
#   at t/db_dependent/api/v1/import_batch_profiles.t line 143.

    #   Failed test '204 No Content'
    #   at t/db_dependent/api/v1/import_batch_profiles.t line 168.
    #          got: '404'
    #     expected: '204'

    #   Failed test 'profile should be erased'
    #   at t/db_dependent/api/v1/import_batch_profiles.t line 173.
    #          got: 'Koha::ImportBatchProfile=HASH(0x55d394fd0cb8)'
    #     expected: undef
    # Looks like you failed 2 tests of 3.
Comment 42 Agustín Moyano 2020-10-26 18:57:25 UTC
Created attachment 112531 [details] [review]
Bug 23019: Update database structure and add atomic update

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>
Comment 43 Agustín Moyano 2020-10-26 18:57:32 UTC
Created attachment 112532 [details] [review]
Bug 23019: Add koha_object_class and koha_objects_class methods to ImportBatch.pm schema

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>
Comment 44 Agustín Moyano 2020-10-26 18:57:37 UTC
Created attachment 112533 [details] [review]
Bug 23019: Add ImportBatchesProfile schema and set is_boolean to parse_items column

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>
Comment 45 Agustín Moyano 2020-10-26 18:57:47 UTC
Created attachment 112534 [details] [review]
Bug 23019: Add tests

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>
Comment 46 Agustín Moyano 2020-10-26 18:57:55 UTC
Created attachment 112535 [details] [review]
Bug 23019: Add profiles to stage-import-batch and magnage-import-batch pages

This patch adds the logic and the needed UI elements to be able to pre-load an import profile. It also displays which profile was used to stage an import in staged import manager.

To test:
1. Apply all patches
2. Updatedatabase
3. Go to Stage MARC records for Import tool in admin, and upload a file with MARC records.
CHECK => after uploading, there is a fieldset with the legend “Profile settings”
              => inside the fieldset there is a select labeled “Pre fill values with profile”. The only value it has is “Do not use profile”.

4. Change some settings, and set “profile 1” as profile name and click on “Add profile”
SUCCESS => The select now has the new profile selected

5. Change profile select to “Do not use profile”
SUCCESS => Default values are now displayed in the form

6. Reload the page and upload the file again
SUCCESS => the select still has the profile recently added

7. Select the profile, change some parameter in the form and set the profile name to “profile 2”, and click add profile
SUCCESS => there are two profiles now, and if you toggle between them, the parameter changes

8. Select profile 1, change one parameter and click on update profile
SUCCESS => if you toggle that profile with the other, the new parameter of the value is shown when you select profile 1

9. Select profile 2, change some parameter and click Add profile (leaving the name as profile 2)
SUCCESS => the page complains there is another profile with the same name, and asks if you want to replace it.

10. Click on accept
SUCCESS => profile 2 now has the new value in the parameter

11. Select profile 2 and change the name to profile 1
SUCCESS => the page complains there is another profile with that name, and asks if you want to replace it

12. Click on accept
SUCCESS => in profile select there is only one profile called profile 1 that has the values of profile 2

13. Select profile 1 and click remove profile
SUCCESS => there is no profile in profile select.

14. Create a profile and click on “Stage for import”
15. Go to Staged MARC management page
SUCCESS => Improt should have the name of the profile in profile column, and when you click on the file name, there should be the name of the profile in the details.

16. prove t/db_dependent/ImportBatch.t t/db_dependent/api/v1/import_batch_profiles.t
17. Sign off

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>
Comment 47 Agustín Moyano 2020-10-26 18:58:08 UTC
Created attachment 112536 [details] [review]
Bug 23019: (QA follow-up) Fix typo Pre fill

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Comment 48 Agustín Moyano 2020-10-26 18:58:32 UTC
Created attachment 112537 [details] [review]
Bug 23019: (follow-up) Fix usability issues

This patch place profile buttons in the bottom of the page, next to "Stage for import"

It removes the "Update profile" button, and changes "Add profile" to "Save profile". Save profile can be used to add and to update a profile.

It also adds feedback when a profile was saved or deleted.

Last, it also replaces input-batch-profile endpoint name for input_batch_profile, which is more aligned with other endpoint names.
Comment 49 Agustín Moyano 2020-10-26 18:59:40 UTC
(In reply to Katrin Fischer from comment #41)
> Several tests are failing:
> 
>     #   Failed test 'exact match for JSON Pointer "/name"'
>     #   at t/db_dependent/api/v1/import_batch_profiles.t line 134.
>     #          got: undef
>     #     expected: 'theProfile'
> 
>     #   Failed test 'profile name should be the updated one'
>     #   at t/db_dependent/api/v1/import_batch_profiles.t line 138.
>     #          got: 'someProfile'
>     #     expected: 'theProfile'
>     # Looks like you failed 4 tests of 5.
> t/db_dependent/api/v1/import_batch_profiles.t .. 3/4 
> #   Failed test 'edit profile'
> #   at t/db_dependent/api/v1/import_batch_profiles.t line 143.
> 
>     #   Failed test '204 No Content'
>     #   at t/db_dependent/api/v1/import_batch_profiles.t line 168.
>     #          got: '404'
>     #     expected: '204'
> 
>     #   Failed test 'profile should be erased'
>     #   at t/db_dependent/api/v1/import_batch_profiles.t line 173.
>     #          got: 'Koha::ImportBatchProfile=HASH(0x55d394fd0cb8)'
>     #     expected: undef
>     # Looks like you failed 2 tests of 3.

Hi Katrin, could you please try again?

Thanks
Comment 50 Nick Clemens 2020-10-30 15:39:22 UTC
Created attachment 112715 [details] [review]
Bug 23019: Update database structure and add atomic update

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 51 Nick Clemens 2020-10-30 15:39:26 UTC
Created attachment 112716 [details] [review]
Bug 23019: Add koha_object_class and koha_objects_class methods to ImportBatch.pm schema

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 52 Nick Clemens 2020-10-30 15:39:30 UTC
Created attachment 112717 [details] [review]
Bug 23019: Add ImportBatchesProfile schema and set is_boolean to parse_items column

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 53 Nick Clemens 2020-10-30 15:39:34 UTC
Created attachment 112718 [details] [review]
Bug 23019: Add tests

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 54 Nick Clemens 2020-10-30 15:39:38 UTC
Created attachment 112719 [details] [review]
Bug 23019: Add profiles to stage-import-batch and magnage-import-batch pages

This patch adds the logic and the needed UI elements to be able to pre-load an import profile. It also displays which profile was used to stage an import in staged import manager.

To test:
1. Apply all patches
2. Updatedatabase
3. Go to Stage MARC records for Import tool in admin, and upload a file with MARC records.
CHECK => after uploading, there is a fieldset with the legend “Profile settings”
              => inside the fieldset there is a select labeled “Pre fill values with profile”. The only value it has is “Do not use profile”.

4. Change some settings, and set “profile 1” as profile name and click on “Add profile”
SUCCESS => The select now has the new profile selected

5. Change profile select to “Do not use profile”
SUCCESS => Default values are now displayed in the form

6. Reload the page and upload the file again
SUCCESS => the select still has the profile recently added

7. Select the profile, change some parameter in the form and set the profile name to “profile 2”, and click add profile
SUCCESS => there are two profiles now, and if you toggle between them, the parameter changes

8. Select profile 1, change one parameter and click on update profile
SUCCESS => if you toggle that profile with the other, the new parameter of the value is shown when you select profile 1

9. Select profile 2, change some parameter and click Add profile (leaving the name as profile 2)
SUCCESS => the page complains there is another profile with the same name, and asks if you want to replace it.

10. Click on accept
SUCCESS => profile 2 now has the new value in the parameter

11. Select profile 2 and change the name to profile 1
SUCCESS => the page complains there is another profile with that name, and asks if you want to replace it

12. Click on accept
SUCCESS => in profile select there is only one profile called profile 1 that has the values of profile 2

13. Select profile 1 and click remove profile
SUCCESS => there is no profile in profile select.

14. Create a profile and click on “Stage for import”
15. Go to Staged MARC management page
SUCCESS => Improt should have the name of the profile in profile column, and when you click on the file name, there should be the name of the profile in the details.

16. prove t/db_dependent/ImportBatch.t t/db_dependent/api/v1/import_batch_profiles.t
17. Sign off

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>

Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 55 Nick Clemens 2020-10-30 15:39:42 UTC
Created attachment 112720 [details] [review]
Bug 23019: (QA follow-up) Fix typo Pre fill

Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 56 Nick Clemens 2020-10-30 15:39:47 UTC
Created attachment 112721 [details] [review]
Bug 23019: (follow-up) Fix usability issues

This patch place profile buttons in the bottom of the page, next to "Stage for import"

It removes the "Update profile" button, and changes "Add profile" to "Save profile". Save profile can be used to add and to update a profile.

It also adds feedback when a profile was saved or deleted.

Last, it also replaces input-batch-profile endpoint name for input_batch_profile, which is more aligned with other endpoint names.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 57 Nick Clemens 2020-10-30 15:39:52 UTC
Created attachment 112722 [details] [review]
Bug 23019: (QA follow-up) fix test fiule permissions

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 58 Jonathan Druart 2020-11-06 11:11:44 UTC Comment hidden (obsolete)
Comment 59 Jonathan Druart 2020-11-06 11:25:11 UTC
1. Shouldn't the table be named "import_batch_profiles" instead of "import_batches_profile"?

2. Double check string escaping (save profile with "<alert>script("❤");</alert>" for instance)

3. 
+      "x-koha-authorization": {
+        "permissions": {
+          "catalogue": "1"
+        }
+      }

Is this correct? Should not it be { tools => 'stage_marc_import' } like tools/stage-marc-import.pl instead?

4. In api/v1/swagger/parameters.json
+  "profile_id_pp": {
should not it be more specific, "import_batch_profile_id"?
Comment 60 Jonathan Druart 2020-11-06 11:26:23 UTC
5. Error handling:
Insert a long name for the profile:
"""
An error occurred

[object Object]
"""
Comment 61 Agustín Moyano 2020-11-06 18:42:12 UTC
Created attachment 113276 [details] [review]
Bug 23019: (follow-up) set table name to import_batch_profiles
Comment 62 Agustín Moyano 2020-11-06 18:42:17 UTC
Created attachment 113277 [details] [review]
Bug 23019: (follow-up) Fix other things

This patch:
* Fixes table name change
* Adds schema file
* Changes api parameter name
* Fixes string escaping
* Fixes error handling
Comment 63 Jonathan Druart 2020-11-09 13:47:22 UTC
Created attachment 113311 [details] [review]
Bug 23019: (follow-up 2) set table name to import_batch_profiles
Comment 64 Jonathan Druart 2020-11-09 13:47:54 UTC
t/db_dependent/api/v1/import_batch_profiles.t is failing.
Comment 65 Agustín Moyano 2020-11-10 14:57:47 UTC
Created attachment 113416 [details] [review]
Bug 23019: (follow-up) Fix test permissions and API definition bugs
Comment 66 Jonathan Druart 2020-11-10 15:17:15 UTC
(In reply to Agustín Moyano from comment #65)
> Created attachment 113416 [details] [review] [review]
> Bug 23019: (follow-up) Fix test permissions and API definition bugs

I don't understand why you need to adjust the permissions in tests, flags => 1 means superlibrarian.
Comment 67 Jonathan Druart 2020-11-10 15:24:34 UTC
(In reply to Jonathan Druart from comment #66)
> (In reply to Agustín Moyano from comment #65)
> > Created attachment 113416 [details] [review] [review] [review]
> > Bug 23019: (follow-up) Fix test permissions and API definition bugs
> 
> I don't understand why you need to adjust the permissions in tests, flags =>
> 1 means superlibrarian.

Ok got it, the tests pass without it.
Comment 68 Agustín Moyano 2020-11-10 16:58:34 UTC
(In reply to Jonathan Druart from comment #67)
> (In reply to Jonathan Druart from comment #66)
> > (In reply to Agustín Moyano from comment #65)
> > > Created attachment 113416 [details] [review] [review] [review] [review]
> > > Bug 23019: (follow-up) Fix test permissions and API definition bugs
> > 
> > I don't understand why you need to adjust the permissions in tests, flags =>
> > 1 means superlibrarian.
> 
> Ok got it, the tests pass without it.

No, I really got it all wrong about permissions, hehe.. I'll attach an improved test.
Comment 69 Agustín Moyano 2020-11-10 17:01:13 UTC
Created attachment 113421 [details] [review]
Bug 23019: (follow-up) Fix test permissions and API definition bugs
Comment 70 Jonathan Druart 2020-11-11 15:09:13 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 71 Lucas Gass 2020-11-16 22:40:16 UTC
new feature will not be backported to 20.05.x