The counter logs table has a column for "Imported by" but this is not currently stored
Created attachment 165029 [details] Example counter file
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
Created attachment 165031 [details] [review] Bug 36627: Embed patron record for display
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
Created attachment 165055 [details] [review] Bug 36627: Embed patron record for display
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
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
Created attachment 165064 [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>
Created attachment 165065 [details] [review] Bug 36627: Embed patron record for display Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 165066 [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>
Created attachment 165067 [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>
Created attachment 165068 [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>
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>
Created attachment 165071 [details] [review] Bug 36627: Embed patron record for display Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
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>
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>
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>
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>
Created attachment 165128 [details] [review] Bug 36627: Embed patron record for display Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
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>
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>
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>
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>
Created attachment 169218 [details] [review] Bug 36627: Embed patron record for display Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
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>
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>
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>
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>
Created attachment 174067 [details] [review] Bug 36627: Embed patron record for display Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
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>
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>
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>
For me tests are missing. You need low level tests: one for ->patron and one for the API route.
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
I am seeing "Borrowernumber 51", does not seem correct.
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>
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>
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>
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>
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>
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>
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>
(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.
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?
Pushed for 25.05! Well done everyone, thank you!
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
(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
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?
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
Nice work everyone! Pushed to 24.11.x for 24.11.03
Not backporting to 24.05.x unless requested