Bug 36627

Summary: Display importer for manually harvested SUSHI data
Product: Koha Reporter: Matt Blenkinsop <matt.blenkinsop>
Component: ERMAssignee: Pedro Amorim <pedro.amorim>
Status: Pushed to stable --- QA Contact: Jonathan Druart <jonathan.druart>
Severity: minor    
Priority: P5 - low CC: david, jonathan.druart, jonathan.field, kyle, martin.renvoize, matt.blenkinsop, pedro.amorim, wainuiwitikapark
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Trivial patch Documentation contact:
Documentation submission: Text to go in the release notes:
This fixes the ERM usage statistics import logs table to show who manually imported the SUSHI data. For eUSage > Data providers > [Name] > Import logs, the "Imported by" column now shows the staff patron, instead of just "Cronjob".
Version(s) released in:
25.05.00,24.11.03
Circulation function:
Bug Depends on: 36623    
Bug Blocks: 38782    
Attachments: Example counter file
Bug 36627: Record who is importing sushi data
Bug 36627: Embed patron record for display
Bug 36627: Record who is importing sushi data
Bug 36627: Embed patron record for display
Bug 36627: Rename embed method to patron
Bug 36627: Rename embed method to patron
Bug 36627: Record who is importing sushi data
Bug 36627: Embed patron record for display
Bug 36627: Rename embed method to patron
Bug 36627: Use patron_to_html in 'Imported by' column
Bug 36627: Add tests
Bug 36627: Record who is importing sushi data
Bug 36627: Embed patron record for display
Bug 36627: Rename embed method to patron
Bug 36627: Use patron_to_html in 'Imported by' column
Bug 36627: Add tests
Bug 36627: Record who is importing sushi data
Bug 36627: Embed patron record for display
Bug 36627: Rename embed method to patron
Bug 36627: Use patron_to_html in 'Imported by' column
Bug 36627: Add tests
Bug 36627: Record who is importing sushi data
Bug 36627: Embed patron record for display
Bug 36627: Rename embed method to patron
Bug 36627: Use patron_to_html in 'Imported by' column
Bug 36627: Add tests
Bug 36627: Record who is importing sushi data
Bug 36627: Embed patron record for display
Bug 36627: Rename embed method to patron
Bug 36627: Use patron_to_html in 'Imported by' column
Bug 36627: Add tests
Bug 36627: (QA follow-up): Add missing tests
Bug 36627: Record who is importing sushi data
Bug 36627: Embed patron record for display
Bug 36627: Rename embed method to patron
Bug 36627: Use patron_to_html in 'Imported by' column
Bug 36627: Add tests
Bug 36627: (QA follow-up): Add missing tests
Bug 36627: (follow-up) Use patron_to_html in 'Imported by' column
Bug 36627: (QA follow-up): Rename borrowernumber -> patron_id

Description Matt Blenkinsop 2024-04-17 16:22:46 UTC
The counter logs table has a column for "Imported by" but this is not currently stored
Comment 1 Matt Blenkinsop 2024-04-17 16:29:01 UTC
Created attachment 165029 [details]
Example counter file
Comment 2 Matt Blenkinsop 2024-04-17 16:29:50 UTC
Created attachment 165030 [details] [review]
Bug 36627: Record who is importing sushi data

This patch adds functionality for storing the patron (or cronjob) that triggers a harvest. Currently every counter log is stored as being imported by the cronjob

Test plan:
1) In the usage statistics module in ERM, create a data provider (the details in the provider are not important)
2) Click into the provider and click into manual upload
3) Upload the file attached to this bug
4) Once the background job is completed click into the counter logs tab in the provider. The imported by columns should show a user with a clickable link
Comment 3 Matt Blenkinsop 2024-04-17 16:29:52 UTC
Created attachment 165031 [details] [review]
Bug 36627: Embed patron record for display
Comment 4 Matt Blenkinsop 2024-04-18 08:50:03 UTC
Created attachment 165054 [details] [review]
Bug 36627: Record who is importing sushi data

This patch adds functionality for storing the patron (or cronjob) that triggers a harvest. Currently every counter log is stored as being imported by the cronjob

Test plan:
1) In the usage statistics module in ERM, create a data provider (the details in the provider are not important)
2) Click into the provider and click into manual upload
3) Upload the file attached to this bug
4) Once the background job is completed click into the counter logs tab in the provider. The imported by columns should show a user with a clickable link
Comment 5 Matt Blenkinsop 2024-04-18 08:50:05 UTC
Created attachment 165055 [details] [review]
Bug 36627: Embed patron record for display
Comment 6 Matt Blenkinsop 2024-04-18 09:23:29 UTC
Created attachment 165056 [details] [review]
Bug 36627: Rename embed method to patron

This patch renames the embed method from borrowernumber to patron to fit with the use of 'patron' in the API
Comment 7 Matt Blenkinsop 2024-04-18 09:24:58 UTC
Created attachment 165057 [details] [review]
Bug 36627: Rename embed method to patron

This patch renames the embed method from borrowernumber to patron to fit with the use of 'patron' in the API
Comment 8 Pedro Amorim 2024-04-18 10:21:11 UTC Comment hidden (obsolete)
Comment 9 Pedro Amorim 2024-04-18 10:21:14 UTC Comment hidden (obsolete)
Comment 10 Pedro Amorim 2024-04-18 10:21:16 UTC Comment hidden (obsolete)
Comment 11 Pedro Amorim 2024-04-18 10:21:19 UTC Comment hidden (obsolete)
Comment 12 Pedro Amorim 2024-04-18 10:21:22 UTC Comment hidden (obsolete)
Comment 13 Pedro Amorim 2024-04-18 10:26:28 UTC
Created attachment 165070 [details] [review]
Bug 36627: Record who is importing sushi data

This patch adds functionality for storing the patron (or cronjob) that triggers a harvest. Currently every counter log is stored as being imported by the cronjob

Test plan:
1) In the usage statistics module in ERM, create a data provider (the details in the provider are not important)
2) Click into the provider and click into manual upload
3) Upload the file attached to this bug
4) Once the background job is completed click into the counter logs tab in the provider. The imported by columns should show a user with a clickable link

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 14 Pedro Amorim 2024-04-18 10:26:35 UTC
Created attachment 165071 [details] [review]
Bug 36627: Embed patron record for display

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 15 Pedro Amorim 2024-04-18 10:26:38 UTC
Created attachment 165072 [details] [review]
Bug 36627: Rename embed method to patron

This patch renames the embed method from borrowernumber to patron to fit with the use of 'patron' in the API

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 16 Pedro Amorim 2024-04-18 10:26:40 UTC
Created attachment 165073 [details] [review]
Bug 36627: Use patron_to_html in 'Imported by' column

For consistency, and to avoid 'null koha' for a file imported by
the koha user (no first name).

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 17 Pedro Amorim 2024-04-18 10:26:43 UTC
Created attachment 165074 [details] [review]
Bug 36627: Add tests

k-t-d, from /kohadevbox/koha, run:
yarn run cypress run --spec "./t/cypress/integration/ERM/DataProviders_spec.ts"

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 18 Matt Blenkinsop 2024-04-18 14:52:28 UTC
Created attachment 165127 [details] [review]
Bug 36627: Record who is importing sushi data

This patch adds functionality for storing the patron (or cronjob) that triggers a harvest. Currently every counter log is stored as being imported by the cronjob

Test plan:
1) In the usage statistics module in ERM, create a data provider (the details in the provider are not important)
2) Click into the provider and click into manual upload
3) Upload the file attached to this bug
4) Once the background job is completed click into the counter logs tab in the provider. The imported by columns should show a user with a clickable link

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 19 Matt Blenkinsop 2024-04-18 14:52:31 UTC
Created attachment 165128 [details] [review]
Bug 36627: Embed patron record for display

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 20 Matt Blenkinsop 2024-04-18 14:52:33 UTC
Created attachment 165129 [details] [review]
Bug 36627: Rename embed method to patron

This patch renames the embed method from borrowernumber to patron to fit with the use of 'patron' in the API

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 21 Matt Blenkinsop 2024-04-18 14:52:36 UTC
Created attachment 165130 [details] [review]
Bug 36627: Use patron_to_html in 'Imported by' column

For consistency, and to avoid 'null koha' for a file imported by
the koha user (no first name).

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 22 Matt Blenkinsop 2024-04-18 14:52:38 UTC
Created attachment 165131 [details] [review]
Bug 36627: Add tests

k-t-d, from /kohadevbox/koha, run:
yarn run cypress run --spec "./t/cypress/integration/ERM/DataProviders_spec.ts"

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 23 Matt Blenkinsop 2024-07-19 13:01:34 UTC
Created attachment 169217 [details] [review]
Bug 36627: Record who is importing sushi data

This patch adds functionality for storing the patron (or cronjob) that triggers a harvest. Currently every counter log is stored as being imported by the cronjob

Test plan:
1) In the usage statistics module in ERM, create a data provider (the details in the provider are not important)
2) Click into the provider and click into manual upload
3) Upload the file attached to this bug
4) Once the background job is completed click into the counter logs tab in the provider. The imported by columns should show a user with a clickable link

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 24 Matt Blenkinsop 2024-07-19 13:01:36 UTC
Created attachment 169218 [details] [review]
Bug 36627: Embed patron record for display

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 25 Matt Blenkinsop 2024-07-19 13:01:39 UTC
Created attachment 169219 [details] [review]
Bug 36627: Rename embed method to patron

This patch renames the embed method from borrowernumber to patron to fit with the use of 'patron' in the API

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 26 Matt Blenkinsop 2024-07-19 13:01:41 UTC
Created attachment 169220 [details] [review]
Bug 36627: Use patron_to_html in 'Imported by' column

For consistency, and to avoid 'null koha' for a file imported by
the koha user (no first name).

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 27 Matt Blenkinsop 2024-07-19 13:01:44 UTC
Created attachment 169221 [details] [review]
Bug 36627: Add tests

k-t-d, from /kohadevbox/koha, run:
yarn run cypress run --spec "./t/cypress/integration/ERM/DataProviders_spec.ts"

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 28 Matt Blenkinsop 2024-11-06 09:49:08 UTC
Created attachment 174066 [details] [review]
Bug 36627: Record who is importing sushi data

This patch adds functionality for storing the patron (or cronjob) that triggers a harvest. Currently every counter log is stored as being imported by the cronjob

Test plan:
1) In the usage statistics module in ERM, create a data provider (the details in the provider are not important)
2) Click into the provider and click into manual upload
3) Upload the file attached to this bug
4) Once the background job is completed click into the counter logs tab in the provider. The imported by columns should show a user with a clickable link

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 29 Matt Blenkinsop 2024-11-06 09:49:11 UTC
Created attachment 174067 [details] [review]
Bug 36627: Embed patron record for display

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 30 Matt Blenkinsop 2024-11-06 09:49:14 UTC
Created attachment 174068 [details] [review]
Bug 36627: Rename embed method to patron

This patch renames the embed method from borrowernumber to patron to fit with the use of 'patron' in the API

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 31 Matt Blenkinsop 2024-11-06 09:49:16 UTC
Created attachment 174069 [details] [review]
Bug 36627: Use patron_to_html in 'Imported by' column

For consistency, and to avoid 'null koha' for a file imported by
the koha user (no first name).

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 32 Matt Blenkinsop 2024-11-06 09:49:19 UTC
Created attachment 174070 [details] [review]
Bug 36627: Add tests

k-t-d, from /kohadevbox/koha, run:
yarn run cypress run --spec "./t/cypress/integration/ERM/DataProviders_spec.ts"

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 33 Jonathan Druart 2024-11-06 11:08:27 UTC
For me tests are missing. You need low level tests: one for ->patron and one for the API route.
Comment 34 Pedro Amorim 2024-11-12 11:47:01 UTC
Created attachment 174402 [details] [review]
Bug 36627: (QA follow-up): Add missing tests

1) Apply patch
2) $ yarn api:bundle
3) $ prove t/db_dependent/Koha/ERM/EUsage/CounterLog.t
4) $ prove t/db_dependent/api/v1/erm_counter_logs.t
Comment 35 Jonathan Druart 2024-11-18 14:45:53 UTC
I am seeing "Borrowernumber 51", does not seem correct.
Comment 36 Jonathan Druart 2024-11-18 14:48:35 UTC
Created attachment 174706 [details] [review]
Bug 36627: Record who is importing sushi data

This patch adds functionality for storing the patron (or cronjob) that triggers a harvest. Currently every counter log is stored as being imported by the cronjob

Test plan:
1) In the usage statistics module in ERM, create a data provider (the details in the provider are not important)
2) Click into the provider and click into manual upload
3) Upload the file attached to this bug
4) Once the background job is completed click into the counter logs tab in the provider. The imported by columns should show a user with a clickable link

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 37 Jonathan Druart 2024-11-18 14:48:38 UTC
Created attachment 174707 [details] [review]
Bug 36627: Embed patron record for display

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 38 Jonathan Druart 2024-11-18 14:48:42 UTC
Created attachment 174708 [details] [review]
Bug 36627: Rename embed method to patron

This patch renames the embed method from borrowernumber to patron to fit with the use of 'patron' in the API

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 39 Jonathan Druart 2024-11-18 14:48:45 UTC
Created attachment 174709 [details] [review]
Bug 36627: Use patron_to_html in 'Imported by' column

For consistency, and to avoid 'null koha' for a file imported by
the koha user (no first name).

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 40 Jonathan Druart 2024-11-18 14:48:48 UTC
Created attachment 174710 [details] [review]
Bug 36627: Add tests

k-t-d, from /kohadevbox/koha, run:
yarn run cypress run --spec "./t/cypress/integration/ERM/DataProviders_spec.ts"

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 41 Jonathan Druart 2024-11-18 14:48:51 UTC
Created attachment 174711 [details] [review]
Bug 36627: (QA follow-up): Add missing tests

1) Apply patch
2) $ yarn api:bundle
3) $ prove t/db_dependent/Koha/ERM/EUsage/CounterLog.t
4) $ prove t/db_dependent/api/v1/erm_counter_logs.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 42 Jonathan Druart 2024-11-18 14:48:56 UTC
Created attachment 174712 [details] [review]
Bug 36627: (follow-up) Use patron_to_html in 'Imported by' column

We don't need those 2 lines

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 43 Jonathan Druart 2024-11-18 14:49:11 UTC
(In reply to Jonathan Druart from comment #35)
> I am seeing "Borrowernumber 51", does not seem correct.

It displays the correct value when the job has been processed.
Comment 44 Katrin Fischer 2024-12-23 11:43:38 UTC
If we have a column that is now empty, but should not be, I believe this is a bug.

1) FAIL	t/db_dependent/api/v1/erm_counter_logs.t
   FAIL	  spelling
		 embeded ==> embedded

Fixed this and other occurences in a follow-up patch.

Are you using qa-test-tools inside ktd? I sometimes wonder why these spelling errors don't seem to show for other QAers.


2) For another bug:

--- a/api/v1/swagger/definitions/erm_counter_log.yaml
+++ b/api/v1/swagger/definitions/erm_counter_log.yaml
@@ -11,7 +11,7 @@ properties:
     - integer
     - "null"
   borrowernumber:
-    description: borrowernumber of the counter_log
+    description: borrowernumber of the counter log


Shouldn't this be patron_id to stick with the common terminology we aimed for in the API?
Comment 45 Katrin Fischer 2024-12-23 12:52:27 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 46 Katrin Fischer 2024-12-23 13:56:29 UTC
I believe this broke 3 tests, it's always the same error:

14:20:25 koha_1       | #   at t/db_dependent/api/v1/erm_custom_reports.t line 144.
14:20:25 koha_1       | Can't use an undefined value as a HASH reference at /kohadevbox/koha/Koha/ERM/EUsage/CounterFile.pm line 563.
14:20:25 koha_1       | # Looks like your test exited with 255 just after 1.

t_db_dependent_Koha_BackgroundJobs_ErmSushiHarvester_t
t_db_dependent_Koha_ERM_EUsage_CounterFile_t
Comment 47 Pedro Amorim 2024-12-24 10:45:40 UTC
(In reply to Katrin Fischer from comment #46)
> I believe this broke 3 tests, it's always the same error:
> 
> 14:20:25 koha_1       | #   at t/db_dependent/api/v1/erm_custom_reports.t
> line 144.
> 14:20:25 koha_1       | Can't use an undefined value as a HASH reference at
> /kohadevbox/koha/Koha/ERM/EUsage/CounterFile.pm line 563.
> 14:20:25 koha_1       | # Looks like your test exited with 255 just after 1.
> 
> t_db_dependent_Koha_BackgroundJobs_ErmSushiHarvester_t
> t_db_dependent_Koha_ERM_EUsage_CounterFile_t

Added bug 38782
Comment 48 Katrin Fischer 2024-12-27 14:03:04 UTC
Keeping "additional_work_needed" for a reply to:

> 2) For another bug:
> 
> --- a/api/v1/swagger/definitions/erm_counter_log.yaml
> +++ b/api/v1/swagger/definitions/erm_counter_log.yaml
> @@ -11,7 +11,7 @@ properties:
>      - integer
>      - "null"
>    borrowernumber:
> -    description: borrowernumber of the counter_log
> +    description: borrowernumber of the counter log
> 
> 
> Shouldn't this be patron_id to stick with the common terminology we aimed
> for in the API?
Comment 49 Pedro Amorim 2024-12-30 11:30:35 UTC
Created attachment 175990 [details] [review]
Bug 36627: (QA follow-up): Rename borrowernumber -> patron_id

To test:
$ prove t/db_dependent/api/v1/erm_counter_logs.t
Comment 50 Paul Derscheid 2025-03-19 23:00:32 UTC
Nice work everyone!

Pushed to 24.11.x for 24.11.03
Comment 51 Wainui Witika-Park 2025-04-17 06:32:01 UTC
Not backporting to 24.05.x unless requested