[Koha-bugs] [Bug 34032] Holds expirationdate left blank if waiting status is reverted

[bugzilla-daemon at bugs.koha-community.org]

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

--- Comment #7 from Emmi Takkinen <emmi.takkinen at koha-suomi.fi> ---
(In reply to Marcel de Rooy from comment #4)
> Did not finish this one completely now, but feel somehow that it needs more
> attention still (qa intuition). Hmm.
> 
> Reserves still contains:
>             expirationdate => $hold->patron_expiration_date,
> Are we not respecting that one?
If hold is not in storage, then we still use patron_expiration_date. It isn't
used even currently when we update hold. 

>          my %updated_columns = $self->_result->get_dirty_columns;
>          return $self->SUPER::store unless %updated_columns;
> This may be obvious. But should we add $hold_reverted here? Or just trust
> that we have dirty columns coming from RevertWaitingStatus?
It is probably save to assume that we receive dirty columns here, since
reverting waiting status changes holds priority as 1. And by my logic that
happens every time we revert.

> +                ( C4::Context->preference('DefaultHoldExpirationdate')
> +                && ! exists $updated_columns{expirationdate} )
> +                || ( C4::Context->preference('DefaultHoldExpirationdate')
> +                && exists $updated_columns{expirationdate}
> +                && $hold_reverted )
> Still breaking my head here.
Gosh, perltidy turns that block into a mess. But this is unnecessarily complex
and a I have no idea what I was thinking when writing this. We can probably
achieve same result with this:

> ( C4::Context->preference('DefaultHoldExpirationdate')
>                && ( ! exists $updated_columns{expirationdate} || $hold_reverted 
>) )

-- 
You are receiving this mail because:
You are watching all bug changes.