https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42513

--- Comment #3 from Martin Renvoize (ashimema) 
<[email protected]> ---
Created attachment 203473
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203473&action=edit
Bug 42513: Stock rotation upgrades to handle cancelled advancements

If a 'StockrotationAdvance' branchtransfer for an item is cancelled (e.g.
because the item was marked lost) before it ever arrives, that transfer
never receives a 'datearrived'. Koha::StockRotationItem->needs_advancing
only ever looked at 'StockrotationAdvance' transfers, and dt_from_string()
falls back to "now" when passed an undef value, so the advancement clock
was effectively reset to zero every time the stockrotation cron ran. The
item would get repatriated back to its stage's branch, but would then sit
there forever, never advancing, unless manually advanced.

Koha::StockRotationItem->needs_advancing now measures elapsed time from a
stable arrival date for the item's *current* stage: normally that's simply
the 'StockrotationAdvance' transfer's arrival, exactly as before. Only when
that specific advance was cancelled before arriving do we fall back to the
first transfer - of any reason, not just 'StockrotationRepatriation' - that
subsequently, and successfully, brought the item back to the stage's
branch. Two things about that fallback matter:

* It has to be the *first* such recovery, not the most recent one: an item
  can drift off again later (e.g. checked out and returned to the wrong
  branch) and get transferred back a second time, and that must not keep
  pushing the advancement clock forward, or the item would never
  accumulate enough time at the stage to advance as long as this kept
  happening.

* It doesn't have to be a 'StockrotationRepatriation' transfer at all - any
  transfer reason that actually lands the item back at the stage branch
  counts (e.g. a plain 'Manual' transfer requested by staff).

There's a further wrinkle: if the item is found and simply checked in at
its own homebranch (which C4::Circulation::AddReturn already treats as
needing no transfer at all - see the 'homebranch' return-branch policy),
there will be *no* branchtransfers row recording that arrival. In that
case we know an advance was attempted (even though cancelled) and can see
via needs_repatriating that the item is genuinely home, so we treat it the
same as a fresh arrival rather than leaving it permanently stuck warning
about missing history.

This needed no changes to needs_repatriating, repatriate, or investigate()'s
priority order - once the item is genuinely back at its stage's branch, the
existing repatriation/advancement machinery already works correctly; the
bug was purely in how needs_advancing measured elapsed time.

Rewritten from Lisette Scheer's original patch to fix the root cause
directly (the stale/cancelled transfer date lookup) rather than layering
extra cancellation-handling branches on top of needs_advancing, repatriate
and investigate.

Test plan:
1. Apply the patch.
2. prove t/db_dependent/StockRotationItems.t
   => All tests pass, including the new subtest covering: a cancelled
      advance recovered with no transfer trail at all (advances rather
      than staying stuck), a cancelled advance not yet recovered (defers
      to needs_repatriating), recovery via a non-repatriation transfer
      reason (still anchors the clock), and a second, later recovery
      (does not roll the clock forward again).
3. To reproduce the original bug manually:
   a. Turn on the StockRotation system preference.
   b. Create a cyclical rota with 2+ stages at different libraries, each
      with a duration of 1 day.
   c. Add an item to the rota and activate it.
   d. Move the item to its next stage (creates a 'StockrotationAdvance'
      transfer).
   e. Mark the item lost (cancels the in-progress transfer), then remove
      the lost status again.
   f. Check the item in at the destination library, so it repatriates
      (creates and completes a 'StockrotationRepatriation' transfer).
   g. Run: perl misc/cronjobs/stockrotation.pl -x
      Before this patch: the item never reports 'advancement' again, no
      matter how much time passes - it is stuck reporting 'not-ready'.
      After this patch: once the stage's duration has elapsed since the
      repatriation's arrival, the item correctly reports 'advancement' and
      moves to the next stage.

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list -- [email protected]
To unsubscribe send an email to [email protected]
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to