Bug 33739 - ModItemTransfer triggers indexing twice
Summary: ModItemTransfer triggers indexing twice
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on: 31212
Blocks:
  Show dependency treegraph
 
Reported: 2023-05-15 14:52 UTC by Tomás Cohen Arazi
Modified: 2023-06-07 09:38 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00


Attachments
Bug 33739: Only trigger indexing on last item modification at ModItemTransfer (2.46 KB, patch)
2023-05-15 15:15 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33739: Only trigger indexing on last item modification at ModItemTransfer (2.50 KB, patch)
2023-05-15 20:37 UTC, David Nind
Details | Diff | Splinter Review
Bug 33739: Only trigger indexing on last item modification at ModItemTransfer (2.74 KB, patch)
2023-05-16 06:47 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2023-05-15 14:52:31 UTC
With the changes from bug 31212, it is happening again, because the implicit call to `ModDateLastSeen` is not passed the skip_record_index flag.
Comment 1 Tomás Cohen Arazi 2023-05-15 14:53:25 UTC
Marking as blocker to get some attention.
Comment 2 Tomás Cohen Arazi 2023-05-15 15:15:46 UTC
Created attachment 151198 [details] [review]
Bug 33739: Only trigger indexing on last item modification at ModItemTransfer

This patch makes ModItemTransfer trigger record indexing only on the
last call (in $transfer->transit). And extra parameter is added to
->transit to effectively honour the ModItemTransfer parameter.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/SearchEngine/Indexer.t
=> FAIL: Indexing is requested twice, should only be once
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass! Indexing is requested only once!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi 2023-05-15 15:26:11 UTC
I wanted to mention that bug 31212 highlighted an existing issue, not catched by the current tests before its inclussion, because of the DT truncation taking place.
Comment 4 David Nind 2023-05-15 20:37:13 UTC
Created attachment 151219 [details] [review]
Bug 33739: Only trigger indexing on last item modification at ModItemTransfer

This patch makes ModItemTransfer trigger record indexing only on the
last call (in $transfer->transit). And extra parameter is added to
->transit to effectively honour the ModItemTransfer parameter.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/SearchEngine/Indexer.t
=> FAIL: Indexing is requested twice, should only be once
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass! Indexing is requested only once!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Jonathan Druart 2023-05-16 06:47:04 UTC
Created attachment 151244 [details] [review]
Bug 33739: Only trigger indexing on last item modification at ModItemTransfer

This patch makes ModItemTransfer trigger record indexing only on the
last call (in $transfer->transit). And extra parameter is added to
->transit to effectively honour the ModItemTransfer parameter.

I wanted to mention that bug 31212 highlighted an existing issue, not
catched by the current tests before its inclussion, because of the DT truncation
taking place.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/SearchEngine/Indexer.t
=> FAIL: Indexing is requested twice, should only be once
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass! Indexing is requested only once!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 6 Jonathan Druart 2023-05-16 06:47:51 UTC
I am wondering if we should not switch from "always index unless asked for" to "index on-demand only". This is getting messy.
Comment 7 Tomás Cohen Arazi 2023-05-16 07:56:24 UTC
(In reply to Jonathan Druart from comment #6)
> I am wondering if we should not switch from "always index unless asked for"
> to "index on-demand only". This is getting messy.

Agreed...
Comment 8 Jonathan Druart 2023-05-16 09:28:57 UTC
Pushed to master for 23.05.
Comment 9 Pedro Amorim 2023-06-07 09:38:40 UTC
Missing dependencies on 22.11.x, not pushing.